-
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
LocalModule Storage Option #3335
Comments
Hi @jacksund, the above design makes sense to me. Is this something you'd be interested in working on? |
Sure thing! I'll use the Local storage class as a starting point and give this an attempt. |
I wrote a mock version of the LocalModule class and I've attached my script below (it's without version checking and only supports stored_as_script=True for the moment). I could use some feedback on the best way to test this though. Will I be able to register a flow with this custom storage on the Prefect Cloud? My best guess is to simply try it out with the following code...
When I try this, I get an Error, so I'm not sure where I'm going wrong. |
I'm trying to dig further into the error. The issue is coming from when I try to serialize the LocalModule storage class. When I run storage.serialize(build=True) and look at the result, I can see the storage serialization failed and gave: ('storage', (None, {'_schema': 'Unsupported object type: LocalModule'})). I had my LocalModule class inherit from 'prefect.environments.storage.Storage' so I'm not sure why this is the case. |
A few comments:
|
Sorry about that. I hadn't made a gist before. Here's the code as a gist: link. And I'm using Cloud, so that's the problem... I haven't tried Prefect Server yet, but I'll give it a go. Thank you! |
Thanks for the link! I'll definitely need some help with the Cloud when we get there too. For now, I have a bare-bones version working with Prefect Server in development mode along with a LocalAgent!! I could use some feedback too. I've uploaded the forked repo on my profile. Sorry but I'm new to this and not sure if I'm sharing correctly. Should I being giving more than just a link here? If that's all you need, here you go: localmod-storage When you compare this code to the master branch, it worth noting that I haven't finished the test_**.py files so there are a number of commented out lines there. |
If you want to open a draft PR, that might be easier for providing commentary on your implementation line-by-line |
I opened up a PR request for it. Thanks for taking a look 👍 |
I won't be able to continue this until next week (specifically until the 28th), but feel free to add comments to my changed files! If you'd like me to just add more comments to the code or clear up meaning in the doc strings, I can do that as well. |
Closed by #3351 |
Hi all,
This is a cross posting of discussion #3304. This implementation will allow for easier sharing of python workflow libraries that are installed via pip/conda in any type of environment, including LocalEnvironment.
Proposed behavior
LocalModule will serve just like Local with stored_as_script=True, where instead of a path='/path/to/my/flows/exampleflow.py', you could use module_path='from myrepo.flows import exampleflow' or 'myrepo.flows.exampleflow'. Thus the flow is accessible in any environment where that module import works, not just the environments that share the same path structure.
In an example workflow library, workflows are separated into scripts and can thus be imported and used. One can easily do the following psuedo code...
or alternatively... While I import the entire script above, it may be easier to simply import the single instance (the flow variable).
where the exampleflow.py looks like...
The LocalModule Class
Just to start with implementation, LocalModule should be pretty simple to implement in terms of serialization and dynamic loading.
The serialization of a module is simply saving the path and version:
And dynamic loading can be done with...
The text was updated successfully, but these errors were encountered: