Atom feed #845
Atom feed #845
Conversation
|
Erm are we going somewhere with this or it'll just fail time to time because of the formatting settings? |
|
|
The two remaining issues are pycodestyle bugs, fixed in later versions, see: PyCQA/flake8#1845 (comment) This is taken care of in #847 (and should be fixed once you merge upstream |
thp
left a comment
thp
left a comment
There was a problem hiding this comment.
See comments. I think it's feature-complete, but making the _e() thing a bit less smart would make maintaining this easier.
|
|
||
| logger.warning("%s: invalid atom feed", self.config['path']) | ||
| except etree.LxmlError as e: | ||
| logger.warning("failed to parse %s: %s", self.config['path'], e) |
There was a problem hiding this comment.
Maybe mention here that a new file will be created. Should the old file be copied somewhere to not cause data loss?
| # fix the namespaces | ||
| for elem in tree.iter(): | ||
| if hasattr(elem, 'tag') and elem.tag.startswith(nspfx): | ||
| elem.tag = elem.tag[len(nspfx):] |
There was a problem hiding this comment.
Could this be done instead with using namespace features in etree?
.iter() doesn't seem to support namespaces, but .iterfind() seems to do..
| def _attrs_equal(self, a, b, exist): | ||
| for k in a.keys() | b.keys(): | ||
| if ( | ||
| k not in exist and a.get(k) != b.get(k) | ||
| or k in exist and k not in a | ||
| ): | ||
| return False | ||
|
|
||
| return True |
There was a problem hiding this comment.
Can you add a comment here, explaining in words what this is supposed to do in terms of "a", "b" and "exist"? It seems to compare "a" and "b" for equality, somehow taking "exist" into account.
| def _entry_updated(self, entry): | ||
| """Tries to fetch the updated timestamp from the entry""" | ||
| updated = entry.find('./updated') | ||
| return updated is not None and updated.text or '2099-01-01T00:00:00Z' |
There was a problem hiding this comment.
Why the magical value of 2099? Can we just not have a value here, or use the current time/date?
|
|
||
| maxitems = self.config.get('maxitems', 0) | ||
| if maxitems < 0: | ||
| logger.warning("atom: maxitems can't be negative") |
There was a problem hiding this comment.
Also, what is the effect here? Ignoring and using all items? Maybe the warning should say that, something like "maxitems can't be negative, not limiting amount of items" or something.
| 'enabled': False, | ||
| 'maxitems': 50, | ||
| 'path': '/path/to/feed.xml', | ||
| 'title': 'URLWatch Updates', |
There was a problem hiding this comment.
Branding.
| 'title': 'URLWatch Updates', | |
| 'title': 'urlwatch Updates', |
| .\" indent \\n[an-margin] | ||
| .\" old: \\n[rst2man-indent\\n[rst2man-indent-level]] | ||
| .nr rst2man-indent-level -1 | ||
| .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]] |
There was a problem hiding this comment.
Possibly revert this file change for now. It's updated mechanically on release, and it seems to have more than just the added changes, so we shouldn't do the file update as part of this PR.
| e = functools.partial(self._e, entry) | ||
|
|
||
| e("id", self._mkuuid()) | ||
| e("title", f'{job_state.verb}: {job.pretty_name()}') | ||
|
|
||
| if job.location_is_url(): | ||
| e("link", job.get_location(), target='href') | ||
| else: | ||
| e("summary", job.get_location()) | ||
|
|
||
| content = self._format_content(job_state, cfg['diff']) | ||
| e("content", str(content), target='cdata', type='html') | ||
| e("updated", self._tsfmt(timestamp)) |
There was a problem hiding this comment.
I'm not yet sure if I like the whole _e thing. It's probably clever and stuff, but I don't fully get it, and it's probably hard to maintain(?) if I don't understand it. Would it be possible to split it up into multiple functions? If not, why not?
| # Optional: Unique feed ID (automatically generated if omitted) | ||
| id: "urn:uuid:ffa6dc6e-7436-48f6-bc99-020ab1e7d429" | ||
| # Optional: Title of the feed | ||
| title: "URLWatch" |
There was a problem hiding this comment.
Branding.
| title: "URLWatch" | |
| title: "urlwatch changes" |
2 participants
An implementation of the reporter, storing the updates in the Atom feed at the specified location.