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

Add tags to jobs and a command line arg to select them #789

Merged
merged 5 commits into from
Apr 24, 2024
Merged

Conversation

Jamstah
Copy link
Contributor

@Jamstah Jamstah commented Jan 25, 2024

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 ;)

lib/urlwatch/config.py Outdated Show resolved Hide resolved
Copy link
Owner

@thp thp left a comment

Choose a reason for hiding this comment

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

Needs rebasing.

Wonder if this tagging mechanism could be used for other things and have a bit broader scope? (e.g. reporters could also use tags, e.g for #507, #781 and #790)

lib/urlwatch/config.py Outdated Show resolved Hide resolved
lib/urlwatch/main.py Outdated Show resolved Hide resolved
Copy link
Owner

@thp thp left a 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 <james.hewitt@uk.ibm.com>
Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
@Jamstah
Copy link
Contributor Author

Jamstah commented Feb 17, 2024

Thanks, appreciate the thorough review, I still have python reflexes from before 3.x - don't use it often!

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
Copy link
Owner

@thp thp left a 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.

Comment on lines 26 to 27
indexes or tags of job(s) to run, depending on --tags.
If using indexes, they are as numbered according to the --list command.
Copy link
Owner

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:

Suggested change
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.

Copy link
Contributor Author

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/config.py Show resolved Hide resolved
@@ -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')
Copy link
Owner

Choose a reason for hiding this comment

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

Nitpicky, but:

Suggested change
__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')

Copy link
Contributor Author

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.

lib/urlwatch/jobs.py Outdated Show resolved Hide resolved
lib/urlwatch/jobs.py Show resolved Hide resolved
lib/urlwatch/worker.py Show resolved Hide resolved
lib/urlwatch/main.py Show resolved Hide resolved
Copy link
Owner

@thp thp left a comment

Choose a reason for hiding this comment

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

Thanks!

@thp thp merged commit 1b045b6 into thp:master Apr 24, 2024
5 checks passed
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