Skip to content
This repository was archived by the owner on May 6, 2024. It is now read-only.

feat: add nginx server names hash override [BB-5512] - #6685

Merged
github-actions[bot] merged 1 commit into
openedx-unsupported:open-release/lilac.masterfrom
open-craft:kaustav/cherry_pick_nginx_names_hash_size
Apr 2, 2022
Merged

feat: add nginx server names hash override [BB-5512]#6685
github-actions[bot] merged 1 commit into
openedx-unsupported:open-release/lilac.masterfrom
open-craft:kaustav/cherry_pick_nginx_names_hash_size

Conversation

@kaustavb12

@kaustavb12 kaustavb12 commented Feb 19, 2022

Copy link
Copy Markdown
Contributor

Configuration Pull Request

Description

cherry picked from commit 59cbc50 (PR#6682)

This PR adds support for optional configuration of nginx server_names_hash_bucket_size in case length of the hostname exceeds the default hash bucket size.

Supporting information

BB-5512

Make sure that the following steps are done before merging:

  • A SRE team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  • Are you adding any new default values that need to be overridden when this change goes live? If so:
    • Update the appropriate internal repo (be sure to update for all our environments)
    • If you are updating a secure value rather than an internal one, file a SRE ticket with details.
    • Add an entry to the CHANGELOG.
  • If you are making a complicated change, have you performed the proper testing specified on the Ops Ansible Testing Checklist? Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  • Think about how this change will affect Open edX operators. Have you updated the wiki page for the next Open edX release?

@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @kaustavb12! I've created OSPR-6478 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Feb 19, 2022

@farhaanbukhsh farhaanbukhsh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

  • ✅ I tested this, we have deployed this on various platforms
  • ✅ I read through the code
  • ❌ I checked for accessibility issues
  • ❌ Includes documentation
  • ✅ I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

@natabene

natabene commented Mar 2, 2022

Copy link
Copy Markdown
Contributor

@kaustavb12 Thank you for your contribution. Is this ready for our review?

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Mar 2, 2022
@kaustavb12

Copy link
Copy Markdown
Contributor Author

@natabene Yes it is :)

@openedx-webhooks openedx-webhooks added awaiting prioritization and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 2, 2022
@natabene

natabene commented Mar 2, 2022

Copy link
Copy Markdown
Contributor

@BbrSofiane Could you please look into having this reviewed and merged? This is a cherry pick for Lilac.

@BbrSofiane BbrSofiane left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me

@BbrSofiane

Copy link
Copy Markdown

@openedx/edx-community-bot merge

@BbrSofiane

Copy link
Copy Markdown

@edx-community-bot merge

@BbrSofiane

Copy link
Copy Markdown

@openedx-community-bot merge

@github-actions
github-actions Bot merged commit 3e3f69c into openedx-unsupported:open-release/lilac.master Apr 2, 2022
@openedx-webhooks

Copy link
Copy Markdown

@kaustavb12 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@kaustavb12
kaustavb12 deleted the kaustav/cherry_pick_nginx_names_hash_size branch April 2, 2022 14:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

automerge merged open-source-contribution PR author is not from Axim or 2U

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants