search

earthlab / hub-ops

Infrastructure and operations for the Earth Lab JupyterHub
https://earthlab-hub-ops.readthedocs.io/en/latest/
4 stars 8 forks source link

PR only build? #327

Closed lwasser closed 3 years ago

lwasser commented 3 years ago

hey @kcranston i think we previously only built the docker image on pull requests vs deploy

https://github.com/earthlab/hub-ops/blob/main/.github/workflows/build-only.yml

are you open to (i don't know how deploy works) only building the image in this action here in the future - no rush on this just wanting builds to pass. it can be a future enhancement now that things are working!!!

kcranston commented 3 years ago

I am not sure I understand this request. That action only runs on pull requests. The build-deploy action runs after merge. (This image does not get re-built if it exists already).

lwasser commented 3 years ago

oh - well i saw that all of nathan's pr's were failing and you mentioned that was because they don't have access to secrets. in my mind, i thought maybe pull requests didn't need to push to dockerhub and thus wouldn't need authentication. somehow on travis when you submitted a PR it would build but not push. Maybe however that was because travis stored the secrets instead of github?

essentially it would be nice to merge passing PR's is the end goal and know the containers build ok before doing so. short term we can totally deal as things seem to be working beautifully now!! thank you!

kcranston commented 3 years ago

Ah, I get it now. Because the tests involve authentication and reading secrets stored in Settings -> Secrets, PR builds will only pass if they originate from branches of this repo (not from forks). So, we should make sure that contributors have the permission to push branches and create pull requests, and we should update the docs to reflect this.

kcranston commented 3 years ago

In the previous travis-based CI, the deploy.py script used files in the secrets dir to authenticate to dockerhub and gcloud. In the move to Actions, we've followed the GitHub guides for moving secrets to repository settings. Also, the deploy script can now be run locally without fear of messing up local authentication to services.

kcranston commented 3 years ago

It looks like it might be possible to run the existing Action on PRs from forks, based on this blog post by changing the event type to pull_request_target.

lwasser commented 3 years ago

oh - that's worth ha shot @kcranston we do get outside contributors every once in a while.

lwasser commented 3 years ago

this was fixed. closing