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.

Sign up for GitHub

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

Jump to bottom

Update morango and stop locking sync when db backend is postgres #9556

Merged
merged 3 commits into from Jul 8, 2022
Merged

Update morango and stop locking sync when db backend is postgres #9556

merged 3 commits into from Jul 8, 2022

Conversation

bjester
Copy link
Member

@bjester bjester commented Jul 8, 2022 •
edited

Summary

References

#9524

Reviewer guidance

  • Initiate a sync with the client instance using a postgres backend
  • Run sync integration tests

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@bjester bjester added TODO: needs review Waiting for review TAG: dependencies Pull requests that update a dependency file python Pull requests that update Python code labels Jul 8, 2022
@bjester bjester requested review from jamalex and rtibbles July 8, 2022 13:58
@bjester
Copy link
Member Author

bjester commented Jul 8, 2022

I'm not sure what's run with the linting check

@rtibbles
Copy link
Member

rtibbles commented Jul 8, 2022

It's the git:// protocol issue again - we can go ahead and merge without it.

jredrejo
jredrejo approved these changes Jul 8, 2022
View reviewed changes
Copy link
Member

@jredrejo jredrejo left a comment

Choose a reason for hiding this comment

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

I left a small nitpick comment. Also I see this is targeted for v0.14.x. I guess this is going to be ported to v0.15.x and future releases too, isn't it?

docker-compose.yml

services:
postgres:
image: postgres:12
Copy link
Member

Choose a reason for hiding this comment

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

Taking into account that GCP uses v13 as default, and thinking in the future, wouldn it be best to use postgres:13?

Copy link
Member

Choose a reason for hiding this comment

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

Our CI tests currently use 12, so I think good for it to be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Our github actions (in newer release branches) use version 12, so this is consistent with that. Hopefully this shouldn't make much of a difference

@rtibbles rtibbles merged commit 1ae395e into learningequality:release-v0.14.x Jul 8, 2022
@marcellamaki marcellamaki mentioned this pull request Jul 12, 2022
Closed
@rtibbles rtibbles mentioned this pull request Aug 29, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jredrejo jredrejo jredrejo approved these changes

@jamalex jamalex Awaiting requested review from jamalex

@rtibbles rtibbles Awaiting requested review from rtibbles

Assignees
No one assigned
Labels
python Pull requests that update Python code TAG: dependencies Pull requests that update a dependency file TODO: needs review Waiting for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bjester @rtibbles @jredrejo