Skip to content

fix: reduce agenda data calculation time - #4622

Merged
rjsparks merged 2 commits into
ietf-tools:mainfrom
rjsparks:agendaperformance2
Oct 21, 2022
Merged

fix: reduce agenda data calculation time#4622
rjsparks merged 2 commits into
ietf-tools:mainfrom
rjsparks:agendaperformance2

Conversation

@rjsparks

Copy link
Copy Markdown
Member

This intentionally leaves a commented out debug statement. Against main without #4621, it will report differences with the secretariat group. It will report equal once #4621 is in place.

I had done the work to precalculate the order in the view, but while investigating what led to #4621, discovered that the calculations had already been made and attached to the objects.

It's not clear why it wasn't being used.

@codecov

codecov Bot commented Oct 21, 2022

Copy link
Copy Markdown

Codecov Report

Merging #4622 (438bf29) into main (cbf3431) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 438bf29 differs from pull request most recent head 63f9b17. Consider uploading reports for the commit 63f9b17 to get more accurate results

@@            Coverage Diff             @@
##             main    #4622      +/-   ##
==========================================
- Coverage   88.39%   88.39%   -0.01%     
==========================================
  Files         296      296              
  Lines       39741    39707      -34     
==========================================
- Hits        35130    35098      -32     
+ Misses       4611     4609       -2     
Impacted Files Coverage Δ
ietf/help/urls.py 100.00% <ø> (ø)
ietf/ietfauth/views.py 91.04% <ø> (+0.25%) ⬆️
ietf/meeting/views.py 90.52% <ø> (ø)
ietf/doc/views_draft.py 91.94% <100.00%> (+0.04%) ⬆️
ietf/ietfauth/forms.py 95.10% <100.00%> (+0.26%) ⬆️
ietf/person/models.py 91.91% <100.00%> (+0.75%) ⬆️
ietf/person/utils.py 86.98% <100.00%> (ø)
ietf/utils/pipe.py 82.60% <0.00%> (-4.35%) ⬇️
ietf/utils/mail.py 79.62% <0.00%> (-0.47%) ⬇️
ietf/doc/views_search.py 88.81% <0.00%> (-0.42%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks fine.

It is possible for a session to have order_number = None if session.group is None. I don't think those should be showing up in the schedules, but it might cause bad links if it does. Could fall back to 0 if it comes up instead of propagating the None.

Other thought - it'd be good to update the comment on preprocess_assignments_for_agenda() to document that it's setting the order_number.

@rjsparks

Copy link
Copy Markdown
Member Author

It is possible for a session to have order_number = None if session.group is None. I don't think those should be showing up in the schedules, but it might cause bad links if it does. Could fall back to 0 if it comes up instead of propagating the None.

Session is defined such that Django will not allow session.group to be None.

@rjsparks
rjsparks merged commit 26e2e23 into ietf-tools:main Oct 21, 2022
@rjsparks
rjsparks deleted the agendaperformance2 branch October 21, 2022 16:03
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants