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

Layer names that are substrings cause atlas to serve all layers that match #995

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

KoduIsGreat
Copy link
Contributor

refers to #869

use direct comparison instead of strings.Contains to filter layernames

happy to discuss if we need to support both approaches.

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build be6ef56dc-PR-995

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%

Totals Coverage Status
Change from base Build 86cf1919d-PR-992: 0.0%
Covered Lines: 6551
Relevant Lines: 14024

💛 - Coveralls

Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for hunting down that bug. That has been there for a very long time.

We will need to detail this change in the release notes just in case people see some changes in the data returned after upgrading. This is nuanced though so I don't suspect it has hit too many people.

Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

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

LGTM

@gdey
Copy link
Member

gdey commented Jul 1, 2024

@ARolek seems our dockerhub login key has expired.

@iwpnd
Copy link
Member

iwpnd commented Jul 1, 2024

Nah, this is because the PR does not have access to the credentials I believe. If you were to merge this now, it will have them @gdey.
Maybe we can split the docker build step and make the publish step including login only happen on push to master.

@ARolek
Copy link
Member

ARolek commented Jul 1, 2024

seems our dockerhub login key has expired.

This has to do with external PRs not being able to access the repo secrets. I have been trying to figure out how we could make that job conditional based on if you have access to the secrets or not. Here's a related discussion. Seems like a few work arounds also reference this iss.

@ARolek
Copy link
Member

ARolek commented Jul 1, 2024

@KoduIsGreat can you please squash all the commits into a single one? I can help with this if you would like me to do it.

@KoduIsGreat
Copy link
Contributor Author

I can do it this afternoon if you haven't got to it already

@KoduIsGreat
Copy link
Contributor Author

@ARolek commits squashed

@coveralls
Copy link

coveralls commented Jul 1, 2024

Pull Request Test Coverage Report for Build 7613ca265-PR-995

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%

Totals Coverage Status
Change from base Build 86cf1919d-PR-992: 0.0%
Covered Lines: 6551
Relevant Lines: 14024

💛 - Coveralls

@ARolek ARolek merged commit c7296fd into go-spatial:master Jul 1, 2024
11 of 12 checks passed
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.

5 participants