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

Switch to Github Actions and remove constraints on the cryptography and mysqlclient packages. - #124

Merged
symbolist merged 2 commits into
masterfrom
symbolist/setup-github-actions
Sep 29, 2021
Merged

Switch to Github Actions and remove constraints on the cryptography and mysqlclient packages.#124
symbolist merged 2 commits into
masterfrom
symbolist/setup-github-actions

Conversation

@symbolist

@symbolist symbolist commented Sep 7, 2021

Copy link
Copy Markdown
Contributor

Description

We have a constraint on the cryptography package because the CircleCI tests use alpine docker image on which a Rust dependency is unavailable.

To be able to install blockstore as a package in edx-platform, this constraint needs to be removed (edx-platform is on the latest version of the cryptography package).

CircleCI provides custom docker images but to be able to use Docker Hub images, to get around the rate-limiting, tokens will need to be set up (I am not clear why we haven't hit that for the alpine image). Instead it is simpler to switch to Github actions which is already being used for two workflows in this repo and which a number of other repos in the edx org have switched to.

Ticket

Author Comments, Concerns, and Open Questions

Test Instructions

  • Verify the Github Actions job run the relevant tests and quality checks.
  • Checkout the branch locally.
  • Run
make destroy
make build
make dev.up
make dev.provision
make blockstore-shell

# In the shell
make test

TODOs

If anything isn't yet done, list it here

  • Get Github Actions CI job green.
  • Remove constraints on the cryptography and mysqlclient packages.

@openedx-webhooks

openedx-webhooks commented Sep 7, 2021

Copy link
Copy Markdown

Thanks for the pull request, @symbolist! I've created OSPR-6028 to keep track of it in JIRA.

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

@openedx-webhooks openedx-webhooks added core committer open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Sep 7, 2021
@symbolist
symbolist marked this pull request as draft September 7, 2021 15:29
@symbolist
symbolist force-pushed the symbolist/setup-github-actions branch 13 times, most recently from 9706d1a to 7f36971 Compare September 8, 2021 09:39
@natabene

natabene commented Sep 8, 2021

Copy link
Copy Markdown

@symbolist Thank you for your contribution. @bradenmacdonald Would you like to review this?

@bradenmacdonald

Copy link
Copy Markdown
Contributor

@natabene Yes, I'd be happy to review this.

@natabene

natabene commented Sep 9, 2021

Copy link
Copy Markdown

@bradenmacdonald Thank you!

@bradenmacdonald

Copy link
Copy Markdown
Contributor

@symbolist I see this is green but still marked as a draft; let me know when it's ready for review.

@symbolist
symbolist force-pushed the symbolist/setup-github-actions branch from 7f36971 to 766f38f Compare September 13, 2021 17:34
@symbolist
symbolist force-pushed the symbolist/setup-github-actions branch from 766f38f to 3fd8022 Compare September 13, 2021 17:43
@symbolist
symbolist marked this pull request as ready for review September 13, 2021 17:48
@symbolist

Copy link
Copy Markdown
Contributor Author

@bradenmacdonald It is ready for review now!

Comment thread Dockerfile-3.8
ENV VIRTUAL_ENV=/blockstore/venv

RUN apt-get update
RUN apt-get install libmysqlclient-dev libjpeg-dev libssl-dev libffi-dev python3 python3-venv python3-pip git -y

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

libssl-dev and libffi-dev were required by the cryptography package but according to the docs, with the latest version of pip, a binary wheel is installed so those are no longer needed.

@bradenmacdonald bradenmacdonald left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 please merge at your convenience.

  • I tested this: Ran make dev.build and make easyserver, confirmed GitHub build is green
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a

@symbolist symbolist reopened this Sep 20, 2021
@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed rejected labels Sep 20, 2021
@natabene

Copy link
Copy Markdown

@ormsbee Any chance you would help us with this CircleCI request?

@kdmccormick

Copy link
Copy Markdown
Contributor

Just disabled the build in CircleCI settings. Going to close and re-open this. fyi @natabene @symbolist

@kdmccormick kdmccormick reopened this Sep 28, 2021
@openedx-webhooks

Copy link
Copy Markdown

Hello @symbolist: We are unable to continue with review of your submission at this time. Please see the associated JIRA ticket for more explanation.

@openedx-webhooks openedx-webhooks added rejected and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Sep 28, 2021
@kdmccormick kdmccormick reopened this Sep 28, 2021
@natabene

Copy link
Copy Markdown

@pomegranited Is this helpful?

@kdmccormick

kdmccormick commented Sep 28, 2021

Copy link
Copy Markdown
Contributor

Hm, that may not have worked. I'm on a phone right now -- I can take another look in an hour when I'm on a laptop.

@natabene

Copy link
Copy Markdown

@kdmccormick Thanks for much for looking into this.

@kdmccormick
kdmccormick force-pushed the symbolist/setup-github-actions branch from 3fd8022 to 4229a01 Compare September 28, 2021 20:53
@kdmccormick

kdmccormick commented Sep 28, 2021

Copy link
Copy Markdown
Contributor

@symbolist I amended your most recent commit (with no changes) and force-pushed. Let's see if that works.

@kdmccormick

Copy link
Copy Markdown
Contributor

Great, there we go. I think the branch's head commit hash needed to be changed in order to get rid of the CircleCI status.

@natabene @symbolist @pomegranited @bradenmacdonald This PR should be good to move forward now.

@symbolist
symbolist force-pushed the symbolist/setup-github-actions branch 2 times, most recently from 3fd8022 to 4229a01 Compare September 29, 2021 13:26
@symbolist
symbolist merged commit 9fa872f into master Sep 29, 2021
@symbolist
symbolist deleted the symbolist/setup-github-actions branch September 29, 2021 13:27
@openedx-webhooks

Copy link
Copy Markdown

@ormsbee, @kdmccormick: thought you might like to know that symbolist merged this pull request.

@symbolist

Copy link
Copy Markdown
Contributor Author

@kdmccormick Thanks, that worked!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants