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

Parse arguments outside of main so we can have a custom entrypoint #140

Closed
wants to merge 2 commits into from
Closed

Parse arguments outside of main so we can have a custom entrypoint #140

wants to merge 2 commits into from

Conversation

maerteijn
Copy link
Contributor

With this pull request it becomes possible to write a custom entrypoint with a different argument parser or another source of arguments (for example a config file). Now this is almost impossible without monkey patching or duplicating a lot of code.

A custom entrypoint could looks like this:

import sys

from dramatiq.cli import argument_parser, main


def my_entrypoint():
    args = ['my.module:broker', 'my.module.actors', '-Q', 'myqueue']
    parsed_args = argument_parser().parse_args(args)
    return main(parsed_args)

if __name__ == "__main__":
    sys.exit(my_entrypoint())

Changing the existing arguments or adding extra ones will be much simpler then as well. Could also simplify the dramatiq-django integration by calling the python module directly instead of os.execvp.

Now it becomes very  easy to write a custom entrypoint with a different argument parser, or another source of arguments (a configfile for example)
@Bogdanp
Copy link
Owner

Bogdanp commented Nov 24, 2018

Thanks! Please make a commit adding yourself to the contributors file.

@@ -118,7 +118,7 @@ def folder_path(value):
return os.path.abspath(value)


def parse_arguments():
def argument_parser():
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a particular reason to change this function? Do you plan to augment the argument parser?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wlel, the function first parsed the arguments and returned them. Now it returns the argument parser itself.

It think it should be clear what the function is doing, that's why I suggest to change the name.

Copy link
Owner

Choose a reason for hiding this comment

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

Right, I wasn't asking about the name specifically. I was asking about the reasoning behind changing it to return the parser. :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, sorry for the misunderstanding 😃 I have two existing scenarios in our project where we would like to pass the arguments to the main function instead of parsing the arguments inside it.

  1. A custom entrypoint (console_script) with predefined arguments, see the example in the description of this pull request
  2. A custom entrypoint (console_script) which does not parse the arguments from the commandline but from a configuration file deployed with our configuration manager

Now I have to mokeypatch the main function in cli.py to do so because there the parsing happens.

@Bogdanp
Copy link
Owner

Bogdanp commented Nov 25, 2018

Thanks! I made a couple small changes and merged this with rebase. I'll cut a release later today.

@Bogdanp Bogdanp closed this Nov 25, 2018
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.

None yet

2 participants