Skip to content

Cherry-picks #28066 into koa.master - #28869

Merged
1 commit merged into
openedx:open-release/koa.masterfrom
open-craft:koa/feanil/arch-1821-alternative
Oct 13, 2021
Merged

Cherry-picks #28066 into koa.master #28869
1 commit merged into
openedx:open-release/koa.masterfrom
open-craft:koa/feanil/arch-1821-alternative

Conversation

@pomegranited

@pomegranited pomegranited commented Sep 29, 2021

Copy link
Copy Markdown
Contributor

Description

Cherry-picks https://github.com/edx/edx-platform/pull/28066 into open-release/koa.master.

Testing instructions

(Ocim Admins only)

  1. Visit https://periodic-build-koa.opencraft.hosting/heartbeat?extended

  2. Ensure that the celery check is passing.

  3. Shell into the latest deployed instance and run:

    # LMS
    $ /edx/bin/edxapp-shell-lms 
    >>> from django.conf import settings
    >>> settings.HEARTBEAT_CELERY_ROUTING_KEY
    'edx.lms.core.high'
    >>> settings.HIGH_PRIORITY_QUEUE
    'edx.lms.core.high'   
    >>> settings.CELERY_QUEUES
    {'edx.lms.core.default': {}, 'edx.lms.core.high': {}, 'edx.lms.core.high_mem': {}, 'edx.cms.core.default': {}}
    >>> assert settings.HEARTBEAT_CELERY_ROUTING_KEY in settings.CELERY_QUEUES
    >>>
    
    # Studio
    $ /edx/bin/edxapp-shell-cms 
    >>> from django.conf import settings
    >>> settings.HEARTBEAT_CELERY_ROUTING_KEY
    'edx.cms.core.high'
    >>> settings.HIGH_PRIORITY_QUEUE
    'edx.cms.core.high'
    >>> settings.CELERY_QUEUES
    {'edx.cms.core.default': {}, 'edx.cms.core.high': {}, 'edx.lms.core.default': {}}
    >>> assert settings.HEARTBEAT_CELERY_ROUTING_KEY in settings.CELERY_QUEUES
    >>>
    

Deadline

None

Review

fix!: Make default queue names service variant specific.

(cherry picked from commit 4b29872)
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @pomegranited! I've created OSPR-6081 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 the open-source-contribution PR author is not from Axim or 2U label Sep 29, 2021
@natabene

Copy link
Copy Markdown
Contributor

@pomegranited Thank you for the contribution. Will you find a reviewer among the CCs?

@Agrendalath Agrendalath 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.

👍

  • I tested this: verified that the celery check passes and the queues are set up correctly, checked that this cherry-picks #28066 (it has just one extra empty line removal)
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

@Agrendalath

Copy link
Copy Markdown
Member

@natabene, I've reviewed this.

@sarina, @nedbat, this is a clean backport. Can we merge this after the CC review?

@sarina

sarina commented Sep 29, 2021

Copy link
Copy Markdown
Contributor

@Agrendalath - for any PRs into any open-release branch, I defer to the BTR group. @BbrSofiane is currently the head of that group.

@ChrisChV

Copy link
Copy Markdown
Contributor

Second Internal review:

@pomegranited All looks good 👍

  • I tested this: I verified that all checks passes and I ran the commands in the description to verify the queues.
  • I read through the code
  • [N/A] I checked for accessibility issues
  • [N/A] Includes documentation
  • [N/A] I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository

@nedbat

nedbat commented Sep 30, 2021

Copy link
Copy Markdown
Contributor

for any PRs into any open-release branch, I defer to the BTR group. @BbrSofiane is currently the head of that group.

... and @arbrandes I think is still the release manager for Koa.

@BbrSofiane

Copy link
Copy Markdown
Contributor

@edx-community-bot merge

@ghost ghost added the automerge label Oct 13, 2021
@ghost
ghost merged commit 17c7c9e into openedx:open-release/koa.master Oct 13, 2021
@openedx-webhooks

Copy link
Copy Markdown

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

@pomegranited
pomegranited deleted the koa/feanil/arch-1821-alternative branch October 18, 2021 01:29
@pomegranited

Copy link
Copy Markdown
Contributor Author

Thank you for merging this @BbrSofiane !

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

9 participants