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

require admins & tz #4469

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

require admins & tz #4469

wants to merge 6 commits into from

Conversation

nilsnolde
Copy link
Member

fixes #4467

this is just a start, unfortunately these changes let all gurka tests fail currently with "no edges near location", very cryptic..

in any case, I wouldn't merge this until just before the next release, it's kinda major for anyone running valhalla.

@nilsnolde
Copy link
Member Author

Do I remember right that we came to the conclusion to host the DBs on Github artifacts instead of baking them into a release? I know there's the risk of broken admin data, but that's what it is then IMO. If someone has a strong need for better coverage, they can do the work to come up with a sane process to achieve that.

Also this move makes admin & tz db's necessary for all tests. While we could technically generate the tz db for windows too, it'd be easier to just download it in CI.

@nilsnolde
Copy link
Member Author

maybe a better route would be: we add a boolean arg to the build process to control whether we require the dbs, which valhalla_build_tiles can set to true. then it's hardly any changes. I don't really like that tests now require them as well.

wdyt?

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.

make admins & timezones required
1 participant