Updated implementations and removed container volume mounting. Added python_callable extension. #23
Conversation
huntcsg
force-pushed
the
non-volume-mount-airflow-docker
branch
from
3ab9069 to
9018c40
Compare
huntcsg
force-pushed
the
non-volume-mount-airflow-docker
branch
from
9dfd391 to
516e733
Compare
|
|
||
| PYTHON_ENTRYPOINT = textwrap.dedent( | ||
| """\ | ||
| #!/usr/bin/env python |
There was a problem hiding this comment.
should this be a separate file that you open instead?
There was a problem hiding this comment.
🤷♂ I kind of like it here since it's a python string template (and it's completely tied to the implementation in this module). But if you felt strongly I'd move it out.
There was a problem hiding this comment.
I was mostly thinking for like linting/highlighting/etc. otherwise i dont really feel strongly.
| else: | ||
| kwargs = op_kwargs | ||
|
|
||
| if operator.command is None and python_callable is not None: |
There was a problem hiding this comment.
i think this would be better reversed, e.g.
if operator.command and python_callable:
raise ValueError('It appears...')
# You have a path here that i can't quite grasp the purpose of which
# no-ops, and i feel like it should be explicit.
if ...:
return
operator.log.info('Preparing...')
...There was a problem hiding this comment.
I think you're right. I'll rejigger that
- rejigger that
| operator.log.info( | ||
| "Preparing entry point script to call: {}".format(python_callable) | ||
| ) | ||
| module, callable = python_callable.split(":") |
There was a problem hiding this comment.
it should be okay for the callable to be implicit and for us to just "execute" the module, no? Maybe args and kwargs don't make sense in that case, but 🤷
There was a problem hiding this comment.
I'd say, not, because in that event you should be just using the Operator as:
task = Operator(
command="python -m module.to.execute"
)
There was a problem hiding this comment.
I think both ought to work. this mode should work as a first class citizen
| entrypoint = PYTHON_ENTRYPOINT.format( | ||
| python_import_path=module, | ||
| callable=callable, | ||
| args=base64.b64encode(json.dumps(args).encode("utf-8")).decode("ascii"), |
There was a problem hiding this comment.
this b64/dumps/encode/decode bit should probably be de-inlined and maybe set as an attribute of the class, to enable it to be plugged?
| for filename in t.getnames(): | ||
| file = t.extractfile(filename) | ||
| if file: | ||
| key = filename.split(base, 1)[-1] # Gets the path after the first root |
There was a problem hiding this comment.
*_, key = filename.split(base, 1)?
No description provided.