-
-
Notifications
You must be signed in to change notification settings - Fork 351
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
Add tags to jobs and a command line arg to select them #789
Conversation
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.
2d5b32a
to
8b6b90e
Compare
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.
Added some comments. And as you mentioned, needs rebasing.
This allows the user to selectively run jobs based on a tag for use cases where you might want to run urlwatch on different schedules for different jobs and other use cases. Signed-off-by: James Hewitt <[email protected]>
Signed-off-by: James Hewitt <[email protected]>
Thanks, appreciate the thorough review, I still have python reflexes from before 3.x - don't use it often! |
Signed-off-by: James Hewitt <[email protected]>
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.
Thanks for working on this! See comments.
docs/source/manpage.rst
Outdated
indexes or tags of job(s) to run, depending on --tags. | ||
If using indexes, they are as numbered according to the --list command. |
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.
Something along the lines of:
indexes or tags of job(s) to run, depending on --tags. | |
If using indexes, they are as numbered according to the --list command. | |
index(es) or tag(s) of job(s) to run. | |
If --tags is set, JOB will be interpreted as tags, | |
if not, as index(es) numbered according to the --list command. |
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.
Done something along those lines :)
lib/urlwatch/jobs.py
Outdated
@@ -196,7 +197,10 @@ def ignore_error(self, exception): | |||
|
|||
class Job(JobBase): | |||
__required__ = () | |||
__optional__ = ('name', 'filter', 'max_tries', 'diff_tool', 'compared_versions', 'diff_filter', 'enabled', 'treat_new_as_changed', 'user_visible_url') | |||
__optional__ = ('name', 'tags', 'filter', 'max_tries', 'diff_tool', 'compared_versions', 'diff_filter', 'enabled', 'treat_new_as_changed', 'user_visible_url') |
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.
Nitpicky, but:
__optional__ = ('name', 'tags', 'filter', 'max_tries', 'diff_tool', 'compared_versions', 'diff_filter', 'enabled', 'treat_new_as_changed', 'user_visible_url') | |
__optional__ = ('name', 'filter', 'max_tries', 'diff_tool', 'compared_versions', 'diff_filter', 'enabled', 'treat_new_as_changed', 'user_visible_url', 'tags') |
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.
I've moved it, but why at the end? Its not alphanum sorted, and I thought putting it with name
made sense as they're both identifying metadata.
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.
Thanks!
This allows the user to selectively run jobs based on a tag for use cases where you might want to run urlwatch on different schedules for different jobs and other use cases.
This is also part 1 of addressing #507 - next up would be allowing the report config to be an array (retaining existing behaviour for maps) and allowing users to assign tags to each reporter.
I am aware this will conflict with #785, and will rebase as required ;)