-
Notifications
You must be signed in to change notification settings - Fork 300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --raw-output-data-prefix to the pyflyte-execute command #167
Conversation
@@ -447,6 +447,8 @@ def _get_container_definition( | |||
"{{.input}}", | |||
"--output-prefix", | |||
"{{.outputPrefix}}", | |||
"--raw-data-output-prefix", | |||
"{{.outputPrefix}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be raw output prefix, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this is when i realized that flyteplugins wasn't done
Hurrah! One step closer to portable Workflows. Definitely all the examples will be portable |
@@ -83,7 +83,8 @@ def _execute_task(task_module, task_name, inputs, output_prefix, test): | |||
_data_proxy.Data.get_data(inputs, local_inputs_file) | |||
input_proto = _utils.load_proto_from_file(_literals_pb2.LiteralMap, local_inputs_file) | |||
_engine_loader.get_engine().get_task(task_def).execute( | |||
_literal_models.LiteralMap.from_flyte_idl(input_proto), context={"output_prefix": output_prefix}, | |||
_literal_models.LiteralMap.from_flyte_idl(input_proto), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow this seems some hairy logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
TL;DR
Please see the linked issue for more information. This adds a
--raw-output-data-prefix
switch to the pyflyte-execute command and container args will be changed so that they're registered with--raw-data-output-prefix {{.rawOutputDataPrefix}}
. Propeller will then fill them in.Release this change last, after testing propeller/plugins
Type
Are all requirements met?
Complete description
get_random_path
is called. If it is missing it will fall back to the current behavior of looking up either the S3 shard formatter config or the GCS prefix config.Tracking Issue
flyteorg/flyte#211
Follow-up issue
NA