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

Get rabbit ready before we start workers #2875

Merged
merged 1 commit into from
Mar 17, 2016
Merged

Conversation

nedbat
Copy link
Contributor

@nedbat nedbat commented Mar 17, 2016

This is the same change as in #2871, but also applies it to the devstack
and fullstack vagrant plays.

@openedx-webhooks
Copy link

PR Cover Letter

  • A detailed description of the changes in the body of the PR
  • If available, a link to the relevant JIRA ticket in the body of the PR
    • [PROJ-XXXX](https://openedx.atlassian.net/browse/PROJ-XXXX)

Sandbox

  • Build a sandbox for your branch and add a link here

Testing

  • i18n
  • RTL
  • Unit, integration, acceptance tests as appropriate
  • Analytics
  • Performance
  • Database migrations are backwards-compatible

Reviewers

If you've been tagged for review, please check your corresponding box once you've given the 👍.

Documentation considerations

  • Are there user visible changes? If so, list who is affected, and include a pointer to the relevant code.
  • Should this be mentioned in the release notes? If so, make sure that you included a description.

DevOps assistance

If it's needed, or even if you suspect it might be needed, do any/all these things as early as possible. Don't surprise devops right before merging! For example, touching envs/aws.py or or envs/common.py are dead giveaways that you'll need assistance when pushing your changes out. Things that require devops attention include, but are not limited to:

  • configuration changes
  • feature flags
  • migrations
    Ways to get devops attention, pick-and-choose whichever is most appropriate for your specific PR:
  • Tag devops on your PR (@edx/devops), either for review (more specific to devops = better) or just a general FYI
  • File a support ticket

FYI: Tag anyone who might be interested in this PR here.

Post-review

  • Squash commits

@nedbat
Copy link
Contributor Author

nedbat commented Mar 17, 2016

@feanil @jibsheet This is the same as #2871, with the change made in more places. Do I need rabbit in devstack if edxapp doesn't have worker=True?

Also, bonus points for anyone who can explain why this is happening now. It feels like we are leaking the future into the Dogwood past, and we don't know where.

I'll be cherry-picking this commit onto dogwood.rc when approved.

@feanil
Copy link
Contributor

feanil commented Mar 17, 2016

The error looks to be un-related to this change. 👍

@jibsheet
Copy link
Contributor

It looks like in devstacks, you can leave the order alone, since we only install rabbit for ecom_worker
caebe2d
Otherwise 👍

@feanil
Copy link
Contributor

feanil commented Mar 17, 2016

@jibsheet there is also an error with the edxapp workers that is being reported on the mailing list and in slack that should be adressed by moving the rabbit role up. It seems like there is a chance for occassional deadlock when the workers come up and can't connect to rabbit.

@jibsheet
Copy link
Contributor

@feanil on devstack? Devstack workers are in process I thought.

@nedbat
Copy link
Contributor Author

nedbat commented Mar 17, 2016

@jibsheet I wondered about the need for the re-ordering on devstack, but it still seems like a good idea to install lower-level services (like rabbit) before upper-level, and to have devstack and fullstack be similar where possible.

nedbat added a commit that referenced this pull request Mar 17, 2016
Get rabbit ready before we start workers
@nedbat nedbat merged commit 58a95c5 into master Mar 17, 2016
@nedbat nedbat deleted the ned/start-rabbit-sooner branch March 17, 2016 17:55
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.

4 participants