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

Workflows cleanup: use secrets directly and dedupe test runs #145

Merged
merged 4 commits into from
May 21, 2024

Conversation

cisaacstern
Copy link
Contributor

@cisaacstern cisaacstern commented May 8, 2024

Closes #143

This PR cleans up our CI testing / workflows developer experience in two ways:

  1. Removes use of a namespaced Deployment "environment" for holding our GitHub Secrets in favor of using the global GitHub Secrets namespace directly. This aligns us with the most common usage pattern for testing CI wherein the test checks are nested under a collapsible status icon associated with a commit, and there is not (as we currently have) a cluttering of the (non-collapsible) PR history with Deployments notifications (as shown in the screenshot in What is the Testing deployment environment for? #143).
  2. Also I've changed the triggering rules to de-duplicate test runs for PRs: currently we are getting a run for the commit to the PR and a duplicate run for the push to the branch from which the PR originates. By changing the rule to only trigger test runs on pushes to the default branch name, this duplication will be avoided.

Note: To get CI to work with this new secrets configuration, we will need to recreate the Testing Deployment Secrets as Repository Secrets, on https://github.com/wildlife-dynamics/ecoscope/settings/secrets/actions, here:
Screenshot 2024-05-08 at 11 58 36 AM
For probably good security reasons, we can't move existing secrets, so they will need to be re-added by someone who has access to the original secrets.

@cisaacstern
Copy link
Contributor Author

and there is not (as we currently have) a cluttering of the (non-collapsible) PR history with Deployments notifications

Now that CI has run here, we can see this in action:

This PR #138
Screenshot 2024-05-08 at 12 05 51 PM Screenshot 2024-05-08 at 12 06 07 PM

So if I may be so bold ...

image

@cisaacstern
Copy link
Contributor Author

I assume windows tests are failing because, as @atmorling mentioned at standup today, windows throws an error if EarthEngine credentials are not available, and we haven't yet recreated the test secrets as Repository Secrets. Once that's done, we can re-run tests to confirm.

@cisaacstern cisaacstern mentioned this pull request May 8, 2024
4 tasks
Copy link
Contributor

@atmorling atmorling left a comment

Choose a reason for hiding this comment

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

Sounds good - looks good.
@ericgitonga Would you have the original credentials set up in the Testing environment?

@ericgitonga
Copy link
Contributor

Sounds good - looks good. @ericgitonga Would you have the original credentials set up in the Testing environment?

I have the creds I use on my local... For original creds, let me check on those.

@cisaacstern
Copy link
Contributor Author

Thanks @ericgitonga !

@atmorling
Copy link
Contributor

@ericgitonga since #129 exists I guess the original creds were linked to someone's personal account.

@ericgitonga
Copy link
Contributor

@ericgitonga since #129 exists I guess the original creds were linked to someone's personal account.

Yes indeed. Peter Kulits was the one who first came up with this, so we'd have to check with him for those. I've asked him and await his response.

@ericgitonga
Copy link
Contributor

@ericgitonga since #129 exists I guess the original creds were linked to someone's personal account.

Yes indeed. Peter Kulits was the one who first came up with this, so we'd have to check with him for those. I've asked him and await his response.

I have emailed to you both (on your earthranger accounts) the info from Peter.

@atmorling
Copy link
Contributor

I've gone ahead and added the credentials to the repo secrets

@atmorling
Copy link
Contributor

I've also updated the branch protection rules as they required a successful 'deployment' which is no longer relevant with this change

@atmorling atmorling merged commit 0cad9c7 into master May 21, 2024
3 checks passed
@atmorling atmorling deleted the cs/drop-deployment-env branch May 21, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What is the Testing deployment environment for?
3 participants