-
Notifications
You must be signed in to change notification settings - Fork 24
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
Allow separate config for worker Part 1 #376
Conversation
…ate-worker-config
): | ||
executable = "/usr/bin/du" | ||
args = ["-sb", self.user_download_cache_path] | ||
if not os.path.exists(self.user_download_cache_path): |
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.
os.path.exists
checks only if the path exists, but not if it is a directory or not.
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.
True. If there is no directory and no file it will create the folder. Do you think I should check if it exists if it is a file and then raise an error?
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.
yes. Otherwise the next step would fail, or not?
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.
No it would be fine:
/usr/bin/du -sb download_cache_management.py
2838 download_cache_management.py
Although it would indicate a config error. But an error that the download cache is not configured correctly wouldn't help a user much at this point. If it is important to check it would make more sense somewhere else, e.g. on actinia startup where the person installing actinia would be made aware of it. Do you think that should be added here in this PR? Or new feature request?
As for this PR all tests pass and also processing for actinia worker with same config is successful, I will merge now. Follow-Up PR is coming. |
Currently, the processing part in actinia reads the config from the process queue in redis (when configured) via the
resource data container
object. This means that the config is used which is meant for the actinia which receives the job and writes it into the queue.The worker config can only be specified for
But many other config is not read from the worker config, e.g. additional redis config (for users etc), data mounts, ... which obviously can differ between the actinia instance receiving the job and the actinia worker.
This PR starts to fix this and to read in config from config file for the worker. Additionally the tests needed to be adjusted. Follow up PR will change tests to not have so many overwrites for different test cases and more adjustments for a different config between job receiving actinia and worker.