Skip to content
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

Annotations - workflow metadata #308

Merged
merged 4 commits into from
Jan 5, 2021

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Dec 31, 2020

  • Add a WorkflowMetadata dataclass
  • Add a WorkflowMetadataDefaults dataclass
  • Hook up data classes to constuctors and expose those settings in the workflow decorator
  • Remove unnecessary Workflow input to construct_input_promises
  • Add a docstring to the workflow decorator.

@wild-endeavor wild-endeavor changed the title wip Annotations - workflow metadata Dec 31, 2020
def workflow(_workflow_function=None, *args, reference: Optional[WorkflowReference] = None, **kwargs):
def workflow(
_workflow_function=None,
reference: Optional[WorkflowReference] = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the workflow has both - reference and metadata. I guess we will keep it like task. That was one though I had, should we just expose the WorkflowMetadata TaskMetadata objects to the user?



@dataclass
class WorkflowMetadataDefaults(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we call this something else like WorkflowDefaults much better than WorkflowMetadataDefaults
And should this be part of WorkflowMetadata?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this as it stands but up to you. They are two separate objects in the IDL so I just kept that pattern, it makes the to_flyte_model function easier to reason about. Also, these aren't exposed, only the settings themselves are in the decorator.

@wild-endeavor wild-endeavor merged commit aa2694f into annotations Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants