feat(review): Add atom feed for review team actions.#9846
feat(review): Add atom feed for review team actions.#9846kivinen wants to merge 1 commit intoietf-tools:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9846 +/- ##
=======================================
Coverage 88.54% 88.55%
=======================================
Files 316 316
Lines 42265 42317 +52
=======================================
+ Hits 37423 37472 +49
- Misses 4842 4845 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I wonder if the parameterization part of this can be processed at the edge, letting us leave a relatively static computation of the feed (updated only when there are directorate action changes) at the edge? |
|
Robert Sparks writes:
●rjsparks left a comment (ietf-tools/datatracker#9846)
I wonder if the parameterization part of this can be processed at the edge,
letting us leave a relatively static computation of the feed (updated only
when there are directorate action changes) at the edge?
Edge can do that but it needs to parse the title or the body to do
that, meaning it is error prone, and will break if the format is
changed.
Also it is MUCH MUCH faster to do the filtering in the datatracker.
Fetching atom feed for me from
***@***.***
takes 0.23 seconds real time, and results feed file that is 48802
bytes long, and transfering that data takes 0.005 seconds.
Fetching the full secdir feed from
http://localhost:8000/feed/review-requests/secdir/
takes 16.36 seconds real time, and results feed file that is 3571770
bytes long (3.4 MB), and transfering that data takes 0.4 seconds.
So the processing time for full feed it is more than 16 seconds,
compared to the 0.2 seconds for limited feed.
If I limit the feed for changes in last 100 days (since=100) then the
http://localhost:8000/feed/review-requests/secdir/?since=100
takes only 0.74 seconds real time and data transfer takes less 0.06
seconds (140 kB).
Actually looking at the full feed for secdir will take that long to
generate in my machine, we might want to set the default date limit to
30 days for full feed, and 365 days for personal feeds...
--
***@***.***
|
|
And to change the default date limit just changing the line to in the feeds.py will change the default from unlimited to 30 days. |
jennifer-richards
left a comment
There was a problem hiding this comment.
Given the times you're seeing we should at least put a pretty tight limit on the since parameter.
Suggestion inline to see whether optimization via select_related() on the queries can help. I suspect there's a very large win there.
| reviewer=self.reviewer_email) | ||
| else: | ||
| history = ReviewAssignment.history.model.objects.filter( | ||
| review_request__team__acronym=obj.acronym) |
There was a problem hiding this comment.
| review_request__team__acronym=obj.acronym) | |
| review_request__team__acronym=obj.acronym | |
| ).select_related("type__name", "team__acronym", "doc", "person", "state") |
(also above - and will need some profiling / inspection to get the details right)
|
Jennifer Richards writes:
@jennifer-richards commented on this pull request.
Given the times you're seeing we should at least put a pretty tight limit on
the since parameter.
Suggestion inline to see whether optimization via select_related() on the
queries can help. I suspect there's a very large win there.
I think the thing that will take time is this:
description_template = "group/feed_request_description.html"
I.e., it will use that template to format each item in the feed, and
it will need to run that template quite a few times.
But yes, setting default limit for 30 days, and maximum of 365 would
most likely be good.
--
***@***.***
|
Right, but if it has the data it needs that is relatively fast. As it stands, it will have an N+1 query pattern - 1 query to fetch the list of results, and then N (really N times a few) queries to look up the details for each one. The I think your profiling shows that n=365 will likely take a few seconds to process. If so, I think that's problematic - we'll at least need to rate limit that pretty strictly. |
Add atom feed for review team actions. The url is /feed/review-requests//, and it can have two parameters, since and reviewer_email that can limit the feed to specific reviewer (?reviewer_email) and for last n days (?since=). (Fixes #8652)