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.

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

Allow disabling addons on 404 pages #373

Merged
merged 4 commits into from
Sep 19, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Sep 18, 2024

It reads a META tag readthedocs-http-status to get the HTTP status code for the original request to the documentation URL.

It uses this HTTP status code to decide if each of the addons should be enabled or not.

It reads a META tag `readthedocs-http-status` to get the HTTP status code for
the original request to the documentation URL.

It uses this HTTP status code to decide if each of the addons should be enabled
or not.

Closes #371
Requires readthedocs/common#222
@humitos humitos requested a review from a team as a code owner September 18, 2024 12:09
@humitos humitos requested a review from agjohnson September 18, 2024 12:09
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Looks great!

Perhaps eventually we'll want to pass around a request object instead of just the HTTP status, but let's just keep this in mind for now.

@humitos
Copy link
Member Author

humitos commented Sep 18, 2024

Hmm. Interesting... How could we inject the whole response/request object from CF? Could we pickle it somehow? If you have an idea, please open an issue explaining this so we can discuss/track it there

Having these objects around would be useful for other stuff for sure.

@agjohnson
Copy link
Contributor

Not sure actually. I suppose if we find other users looking for other response attributes we can figure that out. I might just be thinking of a simple object we maintain to consolidate arguments we're passing around now, but I also don't have any other strong recommendations for other data we might need to pass around.

@humitos
Copy link
Member Author

humitos commented Sep 19, 2024

I might just be thinking of a simple object we maintain to consolidate arguments we're passing around now

Good point. I opened #376 to discuss/track this work.

@humitos humitos merged commit b5b1ffe into main Sep 19, 2024
4 checks passed
@humitos humitos deleted the humitos/enable-on-specific-http-status branch September 19, 2024 07:14
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.

Integrate with error pages
2 participants