Skip to content

bug: fix not overrideing Server-Timing if it exists in header already#2303

Open
JohananOppongAmoateng wants to merge 4 commits intodjango-commons:mainfrom
JohananOppongAmoateng:fix-server-timing-header-overriding-in-middleware
Open

bug: fix not overrideing Server-Timing if it exists in header already#2303
JohananOppongAmoateng wants to merge 4 commits intodjango-commons:mainfrom
JohananOppongAmoateng:fix-server-timing-header-overriding-in-middleware

Conversation

@JohananOppongAmoateng
Copy link
Contributor

@JohananOppongAmoateng JohananOppongAmoateng commented Jan 31, 2026

Description

Fix middleware overriding server timing header

Fixes #2300

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

AI/LLM Usage

  • This PR includes code generated with the help of an AI/LLM

@github-actions
Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  debug_toolbar
  middleware.py
Project Total  

This report was generated by python-coverage-comment-action

@JohananOppongAmoateng JohananOppongAmoateng marked this pull request as ready for review January 31, 2026 21:13
Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

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

Ah, actually @JohananOppongAmoateng we should note this in the change log as per the PR template. I think this is valuable to mention.

@JohananOppongAmoateng
Copy link
Contributor Author

@tim-schilling added it

Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

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

Looking at this again, it looks like we're always going to append to the header with this structure. I'm not sure we want to do that in all cases. For example, in HistoryPanel.get_headers() it's trying to set the request id. If it's already been set, we wouldn't want to append. We'd want to either leave it alone or overwrite it entirely.

Do you have any ideas on how to make this more reusable?

@JohananOppongAmoateng
Copy link
Contributor Author

the problem is how do you determine what to override or what to leave

@tim-schilling
Copy link
Member

I can try to come up with something, but it may be a bit. If you'd like to propose something, that may be faster. We can iterate on things too, it doesn't need to be perfect in the first attempt.

@tim-schilling
Copy link
Member

tim-schilling commented Feb 10, 2026

@JohananOppongAmoateng so I think one way is to have two different ways for a panel to set a header in the response. An update collection (overrides the value) and an append collection (adds to the values). See 3d40ff7 for the actual code. This was a first attempt, but the tests pass. We probably need to clean it up and rework our documentation to use it, if we want to.

@JohananOppongAmoateng
Copy link
Contributor Author

@JohananOppongAmoateng so I think one way is to have two different ways for a panel to set a header in the response. An update collection (overrides the value) and an append collection (adds to the values). See 3d40ff7 for the actual code. This was a first attempt, but the tests pass. We probably need to clean it up and rework our documentation to use it, if we want to.

i think that would work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 do not override Server-Timing if it exists

2 participants