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

configurable tmp dir #30

Merged
merged 6 commits into from
Jun 28, 2024
Merged

configurable tmp dir #30

merged 6 commits into from
Jun 28, 2024

Conversation

chrstnbwnkl
Copy link
Collaborator

@chrstnbwnkl chrstnbwnkl commented Jun 7, 2024

Proposed Changes

This PR separates the directories for created packages and all other files (i.e. planet PBF, graph, etc.). DATA_DIR refers to the location where all created packages will be stored whereas TMP_DIR stores the planet PBF, the graph tiles, elevation data, and so on.

Migration

Migration should be easy, just move all the files that are not packaged extracts to the new TMP_DATA_DIR location before starting the services.

@chrstnbwnkl
Copy link
Collaborator Author

we can run the test once the update only PR is merged and we change the target branch to main :)

o: bind
tmp_packages: # do not change any detail of this volume
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just copied the volume configuration from the other one for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's call it just tmp_data, there's no packages

@nilsnolde nilsnolde marked this pull request as ready for review June 17, 2024 06:32
@nilsnolde
Copy link
Collaborator

hm, I just realize CI is still running on ubuntu 20.04. might wanna change that to ubuntu-latest (either here or at some point).

driver: local
driver_opts:
type: none
device: $TMP_DATA_DIR # DATA_DIR is the host directory for the data. It has to be in the environment, e.g. in .env file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
device: $TMP_DATA_DIR # DATA_DIR is the host directory for the data. It has to be in the environment, e.g. in .env file
device: $TMP_DATA_DIR # TMP_DATA_DIR is the host directory for the data. It has to be in the environment, e.g. in .env file

o: bind
tmp_packages: # do not change any detail of this volume
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's call it just tmp_data, there's no packages

@nilsnolde
Copy link
Collaborator

It might also be good to have a test in place. Current tests are checking whether a package was created and has the right graph packaged, but all from a request's response. I guess it'd be enough to check the path set by the env var?

@nilsnolde
Copy link
Collaborator

hm, I just realize CI is still running on ubuntu 20.04. might wanna change that to ubuntu-latest (either here or at some point).

was thinking about the other PR 😅

@chrstnbwnkl
Copy link
Collaborator Author

It might also be good to have a test in place. Current tests are checking whether a package was created and has the right graph packaged, but all from a request's response. I guess it'd be enough to check the path set by the env var?

End to end testing is not trivial, we don't actually push the jobs in the Arq queue when testing. This would require us to have a distinct test setup with only a small PBF instead of the planet and so on... maybe something for the future.

@ltbam ltbam merged commit f0632d1 into cb-update-only Jun 28, 2024
@ltbam ltbam deleted the cb-temp-dir branch June 28, 2024 12:53
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.

3 participants