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.

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

CI using GitHub Actions #64

Merged
merged 3 commits into from
Jul 20, 2021
Merged

CI using GitHub Actions #64

merged 3 commits into from
Jul 20, 2021

Conversation

ddohler
Copy link
Contributor

@ddohler ddohler commented Jul 8, 2021

Overview

travis-ci.org shut down, and migrating to GitHub Actions seemed easy and is our preferred solution anyway, so this makes the switch.

Notes

This is draft while I test (I suspect CI may fail initially because I had problems with credentials locally).

  • The Esri credentials we had for omgeo testing seemed to have disappeared, so I created some new ones and updated the ones we have
  • This is heavily cribbed from the setup for django-ecsmanage.

Testing instructions

  • Push a dummy commit to this branch and confirm that it triggers a build (and that the build succeeds).
  • I'm not sure whether we can fully test the release flow until this is merged to develop, but I believe the path to test should be as follows:
    • Register on TestPyPi and get account credentials (this is probably best done by someone with easy access to the email we use for company-wide accounts).
    • Put the account credentials into the appropriate secrets for this repository
    • Push a dummy tag and confirm that a release is made to TestPyPi
    • Confirm you can install the new release from TestPyPi

Assuming the above is completed, the following cleanup tasks also need to be done before merging:

  • Update the repository secrets to point to production PyPi
  • Remove the Delete me! commit from this branch / develop
  • Delete the test tag

@ddohler ddohler marked this pull request as draft July 8, 2021 15:45
@ddohler ddohler marked this pull request as ready for review July 8, 2021 18:21
@ddohler
Copy link
Contributor Author

ddohler commented Jul 12, 2021

@azavea/operations -- I attempted to assign this to you, but it doesn't appear that GitHub supports assigning to or requesting review from teams, so I'm leaving this comment here; hopefully that's the appropriate path. Please review when you have a chance, thanks!

@colekettler colekettler self-requested a review July 13, 2021 13:45
Comment on lines +32 to +41
- name: Install packages
run: pip install flake8

- name: Lint
run: flake8

Choose a reason for hiding this comment

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

We may want to move these dependencies into setup.cfg from a packaging semantics standpoint, like we do here. But I think that's an unrelated task and should be part of a separate PR if we decide to do so. This works fine as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! I opened #65

Copy link

@colekettler colekettler left a comment

Choose a reason for hiding this comment

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

Excellent work! I was able to test the release workflow successfully using TestPyPI. Results are in this workflow run. I was able to use our existing PyPI credentials to log in, since I assume it's based off of a snapshot of their production database. The package also installs successfully.

I went ahead and deleted the dummy tag I used to trigger the workflow. Thank you so much for taking care of this. This is good to go 🚢

@colekettler
Copy link

Also, I think I figured out why you couldn't tag the operations team. It seems like you can only assign teams as reviewers in private repositories for the organization. I'm not totally clear on if that's due to team secrecy settings somewhere or if that's the intended default behavior.

@ddohler ddohler force-pushed the feature/dpd/ci-github-actions branch 2 times, most recently from d86194c to 4c0c250 Compare July 20, 2021 13:01
@ddohler ddohler force-pushed the feature/dpd/ci-github-actions branch from 4c0c250 to b293546 Compare July 20, 2021 13:19
The default is 10 seconds and sometimes it's slower than that, which
causes test failures.
@ddohler ddohler force-pushed the feature/dpd/ci-github-actions branch from 85e266c to 474e653 Compare July 20, 2021 14:44
@ddohler
Copy link
Contributor Author

ddohler commented Jul 20, 2021

As I was rebasing to finish this, I discovered that matrix testing seems to cause the US Census geocoder to become unstable in some way that makes it unlikely that all three version tests succeed on any given run. A single Python version has a much higher chance of success, so rather than disable the US Census tests, I opted to reduce the testing matrix down to just Python 3.8. My reasoning is that this is an old library that doesn't do anything fancy syntax-wise, so the primary thing we need to be concerned about with testing different versions is probably the possibility of older constructs being deprecated in newer Python versions, so it made sense to focus on the newest version available. I also opened an issue to see if we can figure out a way to still run a multiple-version test successfully. #66

@ddohler ddohler merged commit cc944d4 into develop Jul 20, 2021
@ddohler ddohler deleted the feature/dpd/ci-github-actions branch July 20, 2021 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants