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

Cloud workflow does not work with some yaml args #1213

Open
tahorst opened this issue Oct 29, 2021 · 2 comments
Open

Cloud workflow does not work with some yaml args #1213

tahorst opened this issue Oct 29, 2021 · 2 comments
Assignees

Comments

@tahorst
Copy link
Member

tahorst commented Oct 29, 2021

It looks like borealis should be able to accept idle_for_waiters and some other values from my_launchpad.yaml based on the comment here but including any of these additional values in the yaml file will result in an error when trying to run wcm.py because of fireworks not expecting the arg:

Traceback (most recent call last):                                                                                                           
  File "runscripts/cloud/wcm.py", line 482, in <module>                                                                                      
    script.cli()                                                                                                                             
  File "/home/travis/wcEcoli/wholecell/utils/scriptBase.py", line 642, in cli                                                                
    self.run(args)                                                                                                                           
  File "runscripts/cloud/wcm.py", line 474, in run                                                                                           
    wf.send_to_lpad(                                                                                                                         
  File "/home/travis/wcEcoli/runscripts/cloud/util/workflow.py", line 521, in send_to_lpad                                                   
    lpad = LaunchPad(**config)                                                                                                               
TypeError: __init__() got an unexpected keyword argument 'idle_for_waiters'

I think we would also need to pull those options out of the config here:

metadata = {'db': db_name}
copy_key(config, 'host', metadata)
copy_key(config, 'uri_mode', metadata)
copy_key(config, 'username', metadata)
copy_key(config, 'password', metadata)

@1fish2 1fish2 self-assigned this Oct 29, 2021
@1fish2
Copy link
Contributor

1fish2 commented Oct 29, 2021

Indeed, this is confusing.

The Borealis workers read those parameters from the my_launchpad.yaml file that's on the worker machines in GCE, not the one on the dev computer running wcm.py (unless you run workers locally).

The worker code pops those two parameters from the dict before constructing LaunchPad(**lpad_config) so it doesn't get the unexpected keyword argument TypeError.

What to do?

  • It'd be useful to put idle_for_waiters, idle_for_rockets, and machine-type in a config file on our dev machines. Then runscripts/cloud/wcm.py, runscripts/cloud/util/workflow.py, et al could read those parameters and use them as defaults (overridable by CLI options) for values to send to the workers.
  • They should go in a separate config file than my_launchpad.yaml. Although the lpad CLI ignores the extra fields, any code that loads the file and calls LaunchPad(**lpad_config) would have to know all the custom fields to remove from the dict.
  • Environment variables are an alternative but they're messier to manage for configuring projects.

@tahorst
Copy link
Member Author

tahorst commented Oct 30, 2021

Oh that makes more sense that it's not the local yaml file where the options can exist.

It would be nice to be able to pop them locally as well but might be some redundant code with what's in borealis. I agree that we'd probably want it to be a different yaml file but we can pass that in as with the -l arg or even let users decide if they want to keep it as the default filename of my_launchpad.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants