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

WIP: Hotfix/file_resource_script_generation #46

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

tbkr
Copy link

@tbkr tbkr commented May 21, 2024

When working with marsflow I encountered problems with the pf.FileResource class which was mainly due to the reason that the internals of the resource class have changed.

I enabled moved the assignment of the server_filename to the constructor of the pf.FileResource class and adjusted other occurrences.

In case my changes broke the intended architecture, feel free to comment.

@FussyDuck
Copy link

FussyDuck commented May 21, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@tbkr tbkr force-pushed the hotfix/resource-script-generation branch 13 times, most recently from 2e98378 to ef8fd98 Compare May 28, 2024 09:01
@tbkr tbkr force-pushed the hotfix/resource-script-generation branch 16 times, most recently from c35578a to d872fe9 Compare June 4, 2024 09:32
@tbkr tbkr force-pushed the hotfix/resource-script-generation branch 2 times, most recently from 2f67bdd to cfd3d11 Compare June 11, 2024 07:05
@tbkr tbkr force-pushed the hotfix/resource-script-generation branch from cfd3d11 to d62c139 Compare June 11, 2024 07:29
@tbkr tbkr requested a review from corentincarton June 11, 2024 07:32
@corentincarton
Copy link
Collaborator

@tbkr, thanks for this contribution! I never really used the resource feature of pyflow. Could you just add a test for the Resource class?

@tbkr tbkr force-pushed the hotfix/resource-script-generation branch from 169f10d to f7caf93 Compare June 11, 2024 08:54
@tbkr tbkr changed the title WIP: Hotfix/resource script generation WIP: Hotfix/file_resource_script_generation Jun 11, 2024
@tbkr tbkr force-pushed the hotfix/resource-script-generation branch 3 times, most recently from 57f5559 to dab0d83 Compare June 11, 2024 10:02
@tbkr tbkr force-pushed the hotfix/resource-script-generation branch from dab0d83 to 85217b3 Compare June 11, 2024 10:07
@tbkr
Copy link
Author

tbkr commented Jun 11, 2024

@corentincarton I added a test case for the resource file creation and also for the use case of deploying it to several hosts.

Could you have a look?

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.

3 participants