search

go-spatial / tegola

Tegola is a Mapbox Vector Tile server written in Go
http://tegola.io/
MIT License
1.3k stars 198 forks source link

ci: build docker only when secrets are present #996

Closed iwpnd closed 4 months ago

iwpnd commented 4 months ago

as discussed in #995 docker build should only run if secrets.DOCKERHUB_USERNAME and secrets.DOCKERHUB_TOKEN are present.

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 5117e2b44-PR-996

Details

Totals Coverage Status
Change from base Build 86cf1919d-PR-992: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls
ARolek commented 4 months ago

@iwpnd thanks for jumping on this! I think you updated the wrong action though. I believe you meant to update https://github.com/go-spatial/tegola/blob/master/.github/workflows/on_pr_push.yml but instead updated the release action.

iwpnd commented 4 months ago

The failing docker build is part of on_release. If you do not want to build at all in PRs we can also remove the 

on:
  pull_request:

from the release workflow. In that case we can also skip the check on whether the secrets are defined, because it only runs after the PR is merged. @ARolek

ARolek commented 4 months ago

@iwpnd ah yes, you're right! I remember when this was added years ago. It had to do with errors popping up during release that didn't get caught during the PR. So let's leave the check. Your changes LGTM! 🙏

ARolek commented 4 months ago

@iwpnd it appears we have a syntax error in the workflow file:

image

Edit: Workflow run: https://github.com/go-spatial/tegola/actions/runs/9751755854

iwpnd commented 4 months ago

@iwpnd it appears we have a syntax error in the workflow file:

image

Edit: Workflow run: https://github.com/go-spatial/tegola/actions/runs/9751755854

I’ll take a look in the morning. Seems I don’t have access to secrets in the if statement after all. 🤔

sorry for the noise 🙏🏻

ARolek commented 4 months ago

I’ll take a look in the morning. Seems I don’t have access to secrets in the if statement after all. 🤔 sorry for the noise 🙏🏻

No worries! I was hoping this worked as it was a clean solution.

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 45fcda619-PR-996

Details

Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls
coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 2caf5013a-PR-996

Details

Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls
iwpnd commented 4 months ago

okay TIL i dont have access to secrets, but I can export secrets to the environment variables and use that instead. 😄

@ARolek now when we merge the secrets should be populated in those environment variables and the docker build should execute successfully and not be skipped.

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 098825527-PR-996

Details

Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls
ARolek commented 4 months ago

okay TIL i dont have access to secrets, but I can export secrets to the environment variables and use that instead.

Interesting! Still looks clean to me ;-)

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 6bb4ab97e-PR-996

Details

Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls
coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 26ed63dc0-PR-996

Details

Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls