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

[Ready for Review] add attempt_ok flag so that UI will not show up unknown node #1830

Merged
merged 2 commits into from
May 16, 2024

Conversation

darinyu
Copy link
Collaborator

@darinyu darinyu commented May 9, 2024

This attempt_ok attribute is not used within metaflow runtime but is needed for UI to correctly display the status of a node. For example, in metaflow-service repo here, if attempt_ok is not set, then the status become unknown.

For example, like this:
metaflow-unknown

@darinyu darinyu changed the title add attempt_ok flag so that UI will not show up unknown node [Ready for Review] add attempt_ok flag so that UI will not show up unknown node May 9, 2024
Copy link
Contributor

@romain-intel romain-intel left a comment

Choose a reason for hiding this comment

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

Not sure what's up with the tests. I'll try to retrigger

@@ -66,6 +66,12 @@ def clone_task_helper(
type="attempt",
tags=metadata_tags,
),
MetaDatum(
field="attempt_ok",
value=True, # During clone, the task is always considered successful.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be str(True)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right

@savingoyal savingoyal self-requested a review May 9, 2024 17:40
@savingoyal savingoyal merged commit 7cb3a36 into master May 16, 2024
25 of 26 checks passed
@savingoyal savingoyal deleted the fix_resume_metaflow_ui branch May 16, 2024 17:35
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.

None yet

3 participants