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

Expand file-based storage to all other storage types #2944

Merged
merged 22 commits into from
Jul 16, 2020

Conversation

joshmeek
Copy link

@joshmeek joshmeek commented Jul 10, 2020

Thanks for contributing to Prefect!

Please describe your work and make sure your PR:

  • adds new tests (if appropriate)
  • add a changelog entry in the changes/ directory (if appropriate)
  • updates docstrings for any new functions or function arguments, including docs/outline.toml for API reference docs (if appropriate)

Note that your PR will not be reviewed unless all three boxes are checked.

What does this PR change?

This PR closes #2887 by expanding file-based storage (and hot reloading flows) to all other current storage types. This adds a stored_as_file: bool kwarg to the storage types that also support pickle storage.

Example Workflow
# flow.py
from prefect import task, Flow
from prefect.environments.storage import S3

@task
def imi():
    print("I am, I am, I am, I am")

f = Flow("s3-file", tasks=[imi])
f.storage = S3(bucket="my-flows", key="flow.py", stored_as_file=True)

Upload to S3:

aws s3 cp flow.py s3://my-flows/flow.py

Register with backend:

prefect register flow -f flow.py -p "Demo"

Opening this PR to get some feedback on the implementation. I plan on adding some more documentation around this pattern and making the storage documentation better. I also need to revamp the healthcheck.py for Docker storage because it is pickle-based right now (current implementation skips over healthcheck if using a file but will re-add it once working 👍)

@joshmeek joshmeek requested review from jcrist and cicdw July 10, 2020 16:29
@codecov
Copy link

codecov bot commented Jul 10, 2020

Codecov Report

Merging #2944 into master will decrease coverage by 0.00%.
The diff coverage is 90.78%.

@jcrist
Copy link

jcrist commented Jul 10, 2020

Woot, this is great to see!

This adds a stored_as_file: bool kwarg to the storage types that also support pickle storage.

Since pickles are also stored as files (it's just the type of the contents of the file that is different), I'm not sure if that's the best kwarg. Perhaps something with py or python or script in the name?

@joshmeek joshmeek changed the title [WIP] Expand file-based storage to all other storage types Expand file-based storage to all other storage types Jul 15, 2020
docs/core/idioms/file-based.md Outdated Show resolved Hide resolved
docs/core/idioms/file-based.md Show resolved Hide resolved
src/prefect/environments/storage/_healthcheck.py Outdated Show resolved Hide resolved
docs/core/idioms/file-based.md Outdated Show resolved Hide resolved
tests/environments/storage/test_azure_storage.py Outdated Show resolved Hide resolved
docs/core/idioms/file-based.md Outdated Show resolved Hide resolved
joshmeek and others added 8 commits July 15, 2020 12:08
Copy link

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

LGTM!

@jcrist jcrist merged commit 6e18531 into master Jul 16, 2020
@jcrist jcrist deleted the storage_enhancements branch July 16, 2020 21:36
@jameslamb jameslamb mentioned this pull request Jul 24, 2020
3 tasks
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.

Expand file-based storage option to other storage types
2 participants