Skip to content

Allow job reference by name#846

Open
Jamstah wants to merge 6 commits into
thp:masterfrom
Jamstah:ref-by-name
Open

Allow job reference by name#846
Jamstah wants to merge 6 commits into
thp:masterfrom
Jamstah:ref-by-name

Conversation

@Jamstah

@Jamstah Jamstah commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@thp thp left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Should this also be mentioned in the documentation somewhere?

Seems like "by location or index" should be changed to "location, index or name":

% ag 'location or index'
docs/source/manpage.rst
77:          delete job by location or index
80:          enable job by location or index
83:          delete job by location or index
86:          change the location of an existing job by location or index
89:          test filter output of job by location or index
92:          test diff filter output of job by location or index (needs at least 2 snapshots)

lib/urlwatch/config.py
93:        group.add_argument('--delete', metavar='JOB', help='delete job by location or index')
94:        group.add_argument('--enable', metavar='JOB', help='enable job by location or index')
95:        group.add_argument('--disable', metavar='JOB', help='disable job by location or index')
97:        group.add_argument('--change-location', metavar=('JOB', 'NEW_LOCATION'), nargs=2, help='change the location of an existing job by location or index')
98:        group.add_argument('--test-filter', metavar='JOB', help='test filter output of job by location or index')
100:                           help='test diff filter output of job by location or index (needs at least 2 snapshots)')

What in corner cases such as a name being a valid index (right now now we check int(query) and index <= 0), or a name matching a URL?

Would it make more sense to formalize this, so we can easily extend it in the future? Less magic and less chance for ambiguity (does the user intend to match on name, location or index?).

For example:

  • <integer> ... A valid integer is always an index (like now)
  • location=<url> ... location: prefix matches the URL (preferred new way, documented)
  • name=<name> ... name: prefix matches the name
  • <anthing else> ... Interpreted as location match (for legacy compatibility, not documented)

Comment thread lib/urlwatch/command.py Outdated
@Jamstah Jamstah requested a review from thp June 25, 2026 14:02

@thp thp left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks! It might be worth noting that an index will always win over a name (good!) in case someone names a job with a number that's also an index. Also, if the name is non-unique, or the name matches one of the job locations, the first item in the list "wins", but that's something that the user must take care of when specifying the list of jobs.

An additional safety measure would be to instead of finding the first match, find all of them:

all_matches = [job for job in self.urlwatcher.jobs if query in (job.get_location(), job.name)]

Then, if len(all_matches) == 0, nothing was found. If len(all_matches) == 1, we have found a unique match and can continue with all_matches[0], if len(all_matches) > 1, then we can print an error (and abort), with all matched items and their index (because the user can then re-run the command with the index, which is always unique).

Something like (made-up output, might not be correct):

$ urlwatch --enable foo
Error: Multiple jobs match "foo", use the index instead:

Index: 3
Name: foo
Location: http://example.com/abc

Index: 8
Name: foo
Location: http://example.org/xyz

After which the user can do:

$ urlwatch --enable 8
...
(works successfully)

I'm not sure if this was an issue before already (are URLs unique?), but it's worth adding, what do you think?

@Jamstah

Jamstah commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Now stuck behind #870 but will rebase after that's sorted.

I had a play and switched the logic around a little and added a method that gets one job or rage quits. I think it simplifies some of the later logic too.

I looked at listing the matching jbos with indexes, but the indexes are dynamically generated when used, so you can't get the index for a specific job after they've been filtered. To get consistent indexes we could add an idx attribute to the jobs and populate it when loading, but I didn't fancy doing the work.

Comment thread lib/urlwatch/command.py
raise SystemExit(1)
return job.with_defaults(self.urlwatcher.config_storage.config)
if len(jobs) > 1:
print('Matched multiple jobs: {!r}'.format(id))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can/should this possibly also pretty-print the value of all matching jobs? Especially with many jobs and a long list, printing what actually matched is a good idea, as it might be tedious/complicated for the user to manually figure out why there are multiple matches.

I ran into such an issue last year with a different project, and patched it then.

Matched multiple jobs: foo

vs

Matched multiple jobs: foo
Job:

Job:

...

@Jamstah

Jamstah commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Without indexes?

With this change:

(venv) jammy@mac007470 urlwatch % ./urlwatch --config lib/urlwatch/tests/data/urlwatch.yaml --urls lib/urlwatch/tests/data/jobs-with-tags.yaml --list              
1: UTC ( date -u )
2: RFC ( date -R )
3: Local ( date )
4: Static ( echo hi )
5: Static ( echo world )
6: Static-that-would-fail ( Static )
(venv) jammy@mac007470 urlwatch % ./urlwatch --config lib/urlwatch/tests/data/urlwatch.yaml --urls lib/urlwatch/tests/data/jobs-with-tags.yaml --test-filter Static
Matched multiple jobs: 'Static'
Static ( echo hi )
Static ( echo world )
Static-that-would-fail ( Static )

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.

2 participants