Open
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds support for persisting observation and geocoder cache files to a configurable directory in Docker.
- Introduces the
PERSISTENCE_LOCATIONenvironment variable to override default storage paths. - Updates
LocalWeatherProviderandGeocoderclasses to usePERSISTENCE_LOCATIONfor their JSON files. - Adjusts the
Dockerfileto create, own, and mount/data, and setPERSISTENCE_LOCATIONby default.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| routes/weatherProviders/local.ts | Added observationsFile property and switched file operations to use the env var. |
| routes/geocoders/Geocoder.ts | Updated cacheFile path to respect PERSISTENCE_LOCATION or fall back to default. |
| Dockerfile | Defines PERSISTENCE_LOCATION, prepares /data directory with correct ownership, and declares it as a volume. |
Comments suppressed due to low confidence (2)
routes/geocoders/Geocoder.ts:7
- Use path.join or path.resolve for building the cacheFile path to avoid hardcoded separators and improve cross-platform compatibility.
private static cacheFile: string = (process.env.PERSISTENCE_LOCATION || __dirname + "/../../..") + "/geocoderCache.json";
Dockerfile:2
- Update the project’s README or other documentation to explain the new PERSISTENCE_LOCATION environment variable and its effect on file storage.
ENV PERSISTENCE_LOCATION=/data
|
|
||
| export default class LocalWeatherProvider extends WeatherProvider { | ||
|
|
||
| public static observationsFile: string = ( process.env.PERSISTENCE_LOCATION ? process.env.PERSISTENCE_LOCATION + "/observations.json" : "observations.json"); |
There was a problem hiding this comment.
Construct file paths using Node’s path.join or path.resolve instead of manual string concatenation to ensure correct cross-platform behavior.
|
|
||
| export default class LocalWeatherProvider extends WeatherProvider { | ||
|
|
||
| public static observationsFile: string = ( process.env.PERSISTENCE_LOCATION ? process.env.PERSISTENCE_LOCATION + "/observations.json" : "observations.json"); |
There was a problem hiding this comment.
The fallback path "observations.json" is relative to the current working directory and may not resolve as expected; consider using __dirname or a configured base path for consistency.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey,
every time I recreated my docker container I had to wait 24 hours until enough data has been collected to return some zimmermann calculation.
Persisting the observations in the containers working directory has been a good start, but didn't help in my case.
I added another environment Variable which is making the user able to define a different directory storing observations.json and geocoderCache.json.
Furthermore I extended the Dockerfile. It is preset to use an anonymous volume storing those files. Its mounted into /data.
Regards