-
Notifications
You must be signed in to change notification settings - Fork 963
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
Package update feed #2165
Package update feed #2165
Conversation
I hope it solves pypi#1683 issue. Url like `/rss/{package_name}/updates.xml` with last 10 releases.
Hi @AyumuKasuga - thanks for this PR. |
@@ -0,0 +1,14 @@ | |||
{% extends "base.xml" %} | |||
{% block title %}Recent Updates of {{ project_name }} package{% endblock %} | |||
{% block description %}Recent Updates of {{ project_name }} to the Python Package Index{% endblock %} |
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.
It might be better if this title and description said something like "Latest releases of the {{ project_name }}
package".
At the very least, this should say "updates to the {{ project_name }}
package" instead of "updates of {{ project_name }}
package"
@@ -56,6 +56,10 @@ <h3 class="package-snippet__title"> | |||
|
|||
{% block description %}{{ release.summary }}{% endblock %} | |||
|
|||
{% block extra_rss %} | |||
<link rel="alternate" type="application/rss+xml" title="RSS: updates of {{ release.project.normalized_name }}" href="{{ request.route_path('rss.project_updates', name=release.project.normalized_name) }}"> |
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.
Same comment here about "updates of".
warehouse/rss/views.py
Outdated
) | ||
|
||
if not latest_releases: | ||
return HTTPNotFound() |
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 think there's a difference here between the case when project hasn't been registered yet, and the case when the project exists, but doesn't have any releases.
For the former, I would expect a 404, but for the latter, I would expect an RSS feed with no <item>
s.
This would be handled by using traverse
as mentioned in my other comment.
tests/unit/test_routes.py
Outdated
@@ -141,6 +141,8 @@ def add_policy(name, filename): | |||
"https://files.example.com/packages/{path}", | |||
), | |||
pretend.call("rss.updates", "/rss/updates.xml", domain=warehouse), | |||
pretend.call("rss.project_updates", | |||
"/rss/{name}/updates.xml", domain=warehouse), |
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.
If you set the following:
traverse="/{name}",
factory="warehouse.packaging.models:ProjectFactory",
you can expect a project
argument to the rss_project_updates
function, and a query will be unnecessary. This will also handle 404-ing if the project doesn't exist.
warehouse/rss/views.py
Outdated
return { | ||
"project_name": project_name, | ||
"latest_releases": latest_releases | ||
} |
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.
If we made the necessary changes to get a project
via the ProjectFactory
, we could just pass the project
variable to the template here, and use project.name
and project.releases
(which is ordered by creation date) directly in the template.
Hello @di |
@@ -0,0 +1,14 @@ | |||
{% extends "base.xml" %} | |||
{% block title %}Latest releases of the {{ project.name }} package{% endblock %} |
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.
Nitpick: this should probably say "project", not package. It would be good to put the project name in quotes too.
@@ -0,0 +1,14 @@ | |||
{% extends "base.xml" %} | |||
{% block title %}Latest releases of the {{ project.name }} package{% endblock %} | |||
{% block description %}Latest releases of the {{ project.name }} to the Python Package Index{% endblock %} |
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.
Same here, let's add quotes and "project".
@di thanks, fixed! |
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.
LGTM 👍
Anything else needed for this PR? Seems very useful (though I'm biased 😄). |
Hello @di! Is there something preventing merge and can I do something to make it happen? |
Yes, sorry for not following up. I had some reservations about this after approving, but haven't had a chance to address them (my time has mostly been spent in favor of development which helps fully us transition from pypi-legacy to Warehouse), so this PR has been on the backburner. One thing: I wanted to check/ensure that this view is either not cached, or purged when a new release is made. Another thing is that right now it's possible to delete releases (via pypi-legacy, and soon via Warehouse as well) and it would be ideal to have an entry in this feed for when a release is deleted as well. As it stands right now, the feed introduced by this PR just shows all the existing releases for a project. It might make more sense to look at the Journal entries for a project instead, which would include new releases and deletions. I don't really want to start @AyumuKasuga off on revising this PR until I've really had a chance to think about it again (and other PyPI maintainers have weighed in) but perhaps they could do a little investigation of those ideas and report back. |
I'm also sorry for the slow response! To second Dustin: The folks working on Warehouse have gotten funding to concentrate on improving and deploying Warehouse, and have kicked off work towards our development roadmap -- the most urgent task is, as Dustin said, to improve Warehouse to the point where we can redirect pypi.python.org to pypi.org so the site is more sustainable and reliable. Since this feature isn't something that the legacy site has, I've moved it to a future milestone. But Dustin, Ernest, and other developers are interested in giving @AyumuKasuga feedback as they proceed, here or in IRC. @di mentioned that the right approach might be to provide a feed of Journal updates -- @AyumuKasuga, you should check out #2871 to see that work progress, and maybe build on that! Thanks and sorry again for the wait. |
@AyumuKasuga Now that #2871 is finished, would you like to return to this PR and talk with us about how to proceed? Perhaps using the Journal entries? As always, if you have questions along the way as you work on this, please feel free to ask them here, in |
@AyumuKasuga Ping :) |
@brainwane sorry, currently don't have time to work on this. |
No worries @AyumuKasuga. Given that, I'm going to close this PR in favor of a different, yet-to-be-implemented API (see #284) which will ideally have the features I mentioned in #2165 (comment). Anyone that was hoping for this to be merged: Feel free to file a new issue with your specific use-case and we can try to help you make do with the existing APIs (this will also help us consider your use-case when creating the new API). |
Fixes #2551, fixes #1683.
Url like
/rss/{package_name}/updates.xml
with last 10 releases.