-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Refactor environment build() method #398
Conversation
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.
A few minor comments; for the comment concerning the use of the term "key", just a docstring update is all I was thinking. I see that the word "key" is used all over the place.
Also your 3.4 and 3.5 tests appear to be failing due to an ordering issue.
src/prefect/environments.py
Outdated
- registry_url (str, optional): The registry to push the image to | ||
- python_dependencies (list, optional): The list of pip installable python packages | ||
that will be installed on build of the Docker container | ||
- secrets (list, optional): A list of secret value names to be loaded into the environment | ||
- flow_id (str, optional): A consistent flow ID (generally set on build and not passed in) | ||
""" | ||
|
||
def __init__( | ||
self, | ||
image: str, |
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.
Can this change to base_image
?
Marking as |
This PR makes two principal changes:
Previously, environments were a little complex and mutated themselves when they were built, which had the potential to lead to statefulness bugs (if an environment were built twice, for example). Now, they are purely functional classes that are instantiated with any known values and produce dynamic values as a
key
when theirbuild()
method is called. In order to run the environment, the dynamickey
is passed to therun()
method, where it is combined with static attributes to manipulate the flow.Note to @joshmeek this will slightly affect the GQL call that the deploy agent needs to make to collect the data it requires