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.

Sign up for GitHub

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

Jump to bottom

don't create run ids if dependencies for data store are missing i.e. fail fast #1666

Merged
merged 5 commits into from Jan 11, 2024
Merged

don't create run ids if dependencies for data store are missing i.e. fail fast #1666

merged 5 commits into from Jan 11, 2024

Conversation

madhur-ob
Copy link
Collaborator

Fixes #1058

@madhur-ob madhur-ob changed the title add decorator for checking s3 deps don't create run ids if dependencies for data store are missing i.e. fail fast Jan 7, 2024
@madhur-ob madhur-ob changed the title don't create run ids if dependencies for data store are missing i.e. fail fast [WIP]: don't create run ids if dependencies for data store are missing i.e. fail fast Jan 8, 2024
@madhur-ob madhur-ob changed the title [WIP]: don't create run ids if dependencies for data store are missing i.e. fail fast don't create run ids if dependencies for data store are missing i.e. fail fast Jan 10, 2024
@madhur-ob
Copy link
Collaborator Author

TLDR: the following approach is wrong, which was already present in various places of the codebase:

# cannot decorate __init__... invoke it with dummy decoratee
check_gs_deps(lambda: 0)

and it turns out that we CAN actually decorate the __init__ method of a class...
(also confirmed locally via a small snippet and verified via ChatGPT too!)

@nflx-mf-bot
Copy link
Collaborator

Testing[592] @ dba534c

@nflx-mf-bot
Copy link
Collaborator

Testing[592] @ dba534c had 15 FAILUREs.

@romain-intel romain-intel merged commit fcd8fe1 into Netflix:master Jan 11, 2024
22 of 23 checks passed
madhur-ob added a commit to madhur-ob/metaflow that referenced this pull request Jan 18, 2024
@madhur-ob
don't create run ids if dependencies for data store are missing i.e. …
9dc7b17 
…fail fast (Netflix#1666)

* add decorator for s3 deps

* add comments

* check before creation of run id

* decorate __init__ correctly

* linting
emattia pushed a commit to emattia/metaflow that referenced this pull request Jan 23, 2024
@madhur-ob @emattia
don't create run ids if dependencies for data store are missing i.e. …
aed0f55 
…fail fast (Netflix#1666)

* add decorator for s3 deps

* add comments

* check before creation of run id

* decorate __init__ correctly

* linting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romain-intel romain-intel romain-intel approved these changes

@saikonen saikonen saikonen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

run id is assigned even if boto3 doesn't exist for flow execution
4 participants
@madhur-ob @nflx-mf-bot @romain-intel @saikonen