Skip to content

Pass through --extra-settings to worker processes#80

Merged
PeterJCLaw merged 3 commits into
masterfrom
worker-passthrough-extra-settings
Nov 14, 2022
Merged

Pass through --extra-settings to worker processes#80
PeterJCLaw merged 3 commits into
masterfrom
worker-passthrough-extra-settings

Conversation

@PeterJCLaw

@PeterJCLaw PeterJCLaw commented Nov 14, 2022

Copy link
Copy Markdown
Contributor

This introduces a base command class which handles the extra settings since they're now in quite a few places, then uses that as part of passing through the extra settings to the worker processes.

Tested manually using an extra settings file containing import sys; print(sys.argv) and checking that it gets run for the worker process.

This changes the signature of the runner function, however as that's not meant to be a public API I'm not considering it a breaking change.

Fixes #78

@PeterJCLaw PeterJCLaw marked this pull request as ready for review November 14, 2022 11:13
@PeterJCLaw PeterJCLaw requested a review from lirsacc November 14, 2022 11:14
extra_settings = options['extra_settings']
if extra_settings is not None:
load_extra_settings(extra_settings)
super().handle_extra_settings(**options)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered just making this part of handle on the super class and then having subclasses call super().handle(...) instead of a custom method?

I can't see any weird side effects and it's probably a bit more expected when going through inheritance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, unfortunately Django's BaseCommand.handle raises NotImplementedError so we'd need where to stop the super() calls to avoid errors and also means that calling super().handle(...) in an implementation could easily look like it's a bug.
I also considered an approach of adding this functionality as a decorator (which would make it harder to get wrong) however that felt more complicated (class decorators being more complicated than function ones) and likely wouldn't have a great way to support the return value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right gotcha.

@PeterJCLaw PeterJCLaw merged commit da734c1 into master Nov 14, 2022
@PeterJCLaw PeterJCLaw deleted the worker-passthrough-extra-settings branch November 14, 2022 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Worker processes ignore exact-configurations

2 participants