Allow reporters to be specified multiple times #822
Conversation
Jamstah
force-pushed
the
multiple-reporters
branch
from
bb8fa04
to
8564853
Compare
Jamstah
force-pushed
the
multiple-reporters
branch
2 times, most recently
from
aafd0a6
to
7cf98cc
Compare
lib/urlwatch/reporters.py
Outdated
| 'reported {count} of {total} watched URLs in {duration} seconds'.format( | ||
| count=len(self.job_states), | ||
| total=self.job_count_total, | ||
| duration=self.duration.seconds), |
There was a problem hiding this comment.
The "reported in X seconds" is kind of wrong, as most of the time will be spent watching (hopefully).
Maybe just leave this as-is, as I'm not sure how useful this is (apart from debugging, which is already in the logs?).
There was a problem hiding this comment.
Have removed the count, it was hard to describe what the count meant. Instead, we can just say what the process did.
I think that's clearer, or, we could just remove the sentence altogether.
Jamstah
force-pushed
the
multiple-reporters
branch
4 times, most recently
from
a02231b
to
4c82895
Compare
This allows different jobs to be selected for reporting in different ways, for example, allowing different changes to be emailed to different addresses. Closes thp#790 Signed-off-by: James Hewitt <[email protected]>
Jamstah
force-pushed
the
multiple-reporters
branch
from
4c82895
to
956cd2f
Compare
| reported = report.finish_one(name) | ||
|
|
||
| if not reported: | ||
| raise ValueError(f'Reporter not enabled: {name}') |
There was a problem hiding this comment.
| reported = report.finish_one(name) | |
| if not reported: | |
| raise ValueError(f'Reporter not enabled: {name}') | |
| if not report.finish_one(name): | |
| raise ValueError(f'Reporter not enabled: {name}') |
..but is that changing behaviour from before?
| reported = self.report.finish() | ||
|
|
||
| if not reported: | ||
| logger.warning('No reporters enabled.') |
There was a problem hiding this comment.
I like to do it like this mostly so that the "reported" local variable doesn't leak into the scope (this way, we can be sure that it isn't used below, since we don't give the expression result a name to begin with):
| reported = self.report.finish() | |
| if not reported: | |
| logger.warning('No reporters enabled.') | |
| if not self.report.finish(): | |
| logger.warning('No reporters enabled.') |
| if cfg.get('enabled', False): | ||
| any_enabled = True | ||
| logger.info('Submitting with %s (%r)', name, subclass) | ||
| base_config = subclass.get_base_config(report) | ||
| matching_job_states = filter_by_tags(job_states, cfg.get("tags", [])) |
There was a problem hiding this comment.
I wonder if we can get away with this, and then not having to check for the empty tags in filter_by_tags():
| matching_job_states = filter_by_tags(job_states, cfg.get("tags", [])) | |
| if tags := cfg.get("tags"): | |
| job_states = filter_by_tags(job_states, tags) |
| subclass(report, cfg, [job_state], len(job_states), duration).submit() | ||
| job_state.reported_count = job_state.reported_count + 1 | ||
| else: | ||
| subclass(report, cfg, job_states, duration).submit() | ||
| subclass(report, cfg, matching_job_states, len(job_states), duration).submit() |
There was a problem hiding this comment.
I haven't checked what's the right way, but curiously it's always len(job_states), not len(matching_job_states), is that correct/intended or a bug?
| subclass(report, cfg, job_states, duration).submit() | ||
| subclass(report, cfg, matching_job_states, len(job_states), duration).submit() | ||
| for job_state in matching_job_states: | ||
| job_state.reported_count = job_state.reported_count + 1 |
There was a problem hiding this comment.
| job_state.reported_count = job_state.reported_count + 1 | |
| job_state.reported_count += 1 |
| @classmethod | ||
| def submit_all(cls, report, job_states, duration): | ||
| any_enabled = False | ||
| for name in cls.__subclasses__.keys(): |
There was a problem hiding this comment.
This might also work:
| for name in cls.__subclasses__.keys(): | |
| for name in cls.__subclasses__: |
| def submit_all(cls, report, job_states, duration): | ||
| any_enabled = False | ||
| for name in cls.__subclasses__.keys(): | ||
| any_enabled = any_enabled | ReporterBase.submit_one(name, report, job_states, duration) |
There was a problem hiding this comment.
| any_enabled = any_enabled | ReporterBase.submit_one(name, report, job_states, duration) | |
| any_enabled |= ReporterBase.submit_one(name, report, job_states, duration) |
| @@ -96,8 +104,10 @@ def get_signature(self): | |||
| copyright=urlwatch.__copyright__), | |||
| 'Website: {url}'.format(url=urlwatch.__url__), | |||
| 'Support urlwatch development: https://github.com/sponsors/thp', | |||
| 'watched {count} URLs in {duration} seconds'.format(count=len(self.job_states), | |||
| duration=self.duration.seconds), | |||
| 'watched {total} URLs in {duration} seconds'.format( | |||
There was a problem hiding this comment.
Does it make sense to show here that something is filtered down? e.g. watched 5 or 9 URLs in 12.34 seconds?
This allows different jobs to be selected for reporting in different ways, for example, allowing different changes to be emailed to different addresses.
Closes #790