Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Update group dependencies for new rfc model #6773

Merged
merged 10 commits into from
Dec 14, 2023
Merged

fix: Update group dependencies for new rfc model #6773

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6b5230e
fix: Avoid accessing slug when state is None
jennifer-richards Dec 13, 2023
05f701c
fix: ignore pre-rfc drafts as rfc-to-rfc references
jennifer-richards Dec 13, 2023
a3d09a8
fix: also ignore pre-rfc to pre-rfc refs
jennifer-richards Dec 13, 2023
a2008c1
chore: remove breakpoint
jennifer-richards Dec 13, 2023
368d525
fix: ignore subseries non-downrefs
jennifer-richards Dec 13, 2023
7d83576
fix: label nodes as RFCS when possible
jennifer-richards Dec 13, 2023
79b5f3b
fix: get "rfc" flag right
jennifer-richards Dec 13, 2023
2a20e34
chore: Remove comment, answer seems to be "no"
jennifer-richards Dec 13, 2023
db13860
refactor: Specify state type; drop redundant source doc type check
jennifer-richards Dec 14, 2023
fdaa6d3
refactor: Specify state type in old code
jennifer-richards Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 additions & 15 deletions ietf/group/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,23 +740,40 @@ def dependencies(request, acronym, group_type=None):
source__type="draft",
relationship__slug__startswith="ref",
)

both_rfcs = Q(source__type_id="rfc", target__type_id="rfc")
inactive = Q(source__states__slug__in=["expired", "repl"])
rfc_or_subseries = {"rfc", "bcp", "fyi", "std"}
both_rfcs = Q(source__type_id="rfc", target__type_id__in=rfc_or_subseries)
pre_rfc_draft_to_rfc = Q(
source__states__type="draft",
source__states__slug="rfc",
target__type_id__in=rfc_or_subseries,
)
both_pre_rfcs = Q(
source__states__type="draft",
source__states__slug="rfc",
target__type_id="draft",
target__states__type="draft",
target__states__slug="rfc",
)
inactive = Q(
source__states__type="draft",
source__states__slug__in=["expired", "repl"],
)
jennifer-richards marked this conversation as resolved.
Show resolved Hide resolved
attractor = Q(target__name__in=["rfc5000", "rfc5741"])
removed = Q(source__states__slug__in=["auth-rm", "ietf-rm"])
removed = Q(source__states__type="draft", source__states__slug__in=["auth-rm", "ietf-rm"])
relations = (
RelatedDocument.objects.filter(references)
.exclude(both_rfcs)
.exclude(pre_rfc_draft_to_rfc)
.exclude(both_pre_rfcs)
.exclude(inactive)
.exclude(attractor)
.exclude(removed)
)

links = set()
for x in relations:
target_state = x.target.get_state_slug("draft")
if target_state != "rfc" or x.is_downref():
always_include = x.target.type_id not in rfc_or_subseries and x.target.get_state_slug("draft") != "rfc"
if always_include or x.is_downref():
links.add(x)

replacements = RelatedDocument.objects.filter(
Expand All @@ -771,13 +788,12 @@ def dependencies(request, acronym, group_type=None):
graph = {
"nodes": [
{
"id": x.name,
"rfc": x.get_state("draft").slug == "rfc",
"post-wg": not x.get_state("draft-iesg").slug
in ["idexists", "watching", "dead"],
"expired": x.get_state("draft").slug == "expired",
"replaced": x.get_state("draft").slug == "repl",
"group": x.group.acronym if x.group.acronym != "none" else "",
"id": x.became_rfc().name if x.became_rfc() else x.name,
Copy link
Member

Choose a reason for hiding this comment

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

Check this on a busy group and if the time is noticeable, I think we can get the db to grab this with the primary query instead of n+1 -ing it here.

Copy link
Member

Choose a reason for hiding this comment

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

We might do that in a way the puts the conditional into the query as well, so that we don't see this pattern repeating.

Copy link
Member Author

@jennifer-richards jennifer-richards Dec 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

On the idr group (see below to gauge whether this is "busy", but it looks it to me), this brute force approach takes the request from 2-2.4 seconds up to 2.2-2.5 seconds on my dev machine. The code here is taking advantage of the caching in became_rfc(), so I believe it's at least only paying the N+1 penalty and not hammering the db any worse than that.

(edit to add: the faster time is what I get if I replace all the became_rfc() calls with simple references to name or remove them entirely in the nodes.rfc logic)

image

"rfc": x.type_id == "rfc" or x.became_rfc() is not None,
"post-wg": x.get_state_slug("draft-iesg") not in ["idexists", "watching", "dead"],
"expired": x.get_state_slug("draft") == "expired",
"replaced": x.get_state_slug("draft") == "repl",
"group": x.group.acronym if x.group and x.group.acronym != "none" else "",
"url": x.get_absolute_url(),
"level": x.intended_std_level.name
if x.intended_std_level
Expand All @@ -789,8 +805,8 @@ def dependencies(request, acronym, group_type=None):
],
"links": [
{
"source": x.source.name,
"target": x.target.name,
"source": x.source.became_rfc().name if x.source.became_rfc() else x.source.name,
"target": x.target.became_rfc().name if x.target.became_rfc() else x.target.name,
"rel": "downref" if x.is_downref() else x.relationship.slug,
}
for x in links
Expand Down
Loading