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

Allows setting a mask for updatedb script. #70

Merged
merged 3 commits into from
Jul 17, 2018

Conversation

neutvd
Copy link
Contributor

@neutvd neutvd commented Jul 13, 2018

Pass along an environment variable in the docker exec command to update
certain datasets.

For example:

MASK='^eprofile_L0_[0-9]{5}-[A-Z]\.xml$'
docker exec --env ADAGUC_DATASET_MASK=${MASK} -i -t adagucserver
/adaguc/adaguc-server-updatedatasets.sh

where adagucserver is the name of the docker container you wish to run
the command in.

Pass along an environment variable in the docker exec command to update
certain datasets.

For example:
```
MASK='^eprofile_L0_[0-9]{5}-[A-Z]\.xml$'
docker exec --env ADAGUC_DATASET_MASK=${MASK} -i -t adagucserver
/adaguc/adaguc-server-updatedatasets.sh
```
where adagucserver is the name of the docker container you wish to run
the command in.
If the ADAGUC_DATASET_MASK environment variable is set in
adaguc-server-updatedatasets.sh the servicehealth directory
should not be cleaned or we lose previously updated datasets.
@saskiawagenaar
Copy link
Collaborator

Zoals besproken moeten we eigenlijk #71 oplossen, omdat dit scriptje nu niet onderhoudbaar is. In het kader van backwards compatibility is dit nu de enige oplossing, totdat we met Maarten kunnen overleggen of we dit kunnen breken.

@saskiawagenaar
Copy link
Collaborator

Wat wel nog een oplossing is, is als we een lost scriptje maken wat bestaat naast adaguc-server-updatedatasets.sh en wat wel de correcte commandline parsing heeft. Nadeel daarvan is dat er tijdelijk twee scripts onderhouden moeten worden.

Should be refactored at some point to support more flexible command
line option parsin.
@saskiawagenaar saskiawagenaar merged commit 36bc45a into KNMI:master Jul 17, 2018
@maartenplieger
Copy link
Member

Het script moet getopts gaan gebruiken. Backwards compatibility is nog niet zo'n issue. Ik denk dat het het beste is om een nieuwe release te maken met een vermelding dat dit veranderd is. Ook moet de readme dan worden aangepast.

Ik zou liever geen environment variabelen gebruiken tenzij het niet anders kan.

@neutvd
Copy link
Contributor Author

neutvd commented Aug 8, 2018

De environment variabele is inderdaad een hack, maar was de enige mogelijkheid die ik zag om dit te laten werken zonder, in jou afwezigheid, backwards compatibility te breken. We kunnen een jira story aanmaken om dit met getopts te refactoren.

maartenplieger pushed a commit that referenced this pull request May 22, 2019
Allows setting a mask for updatedb script via environment variable.
maartenplieger pushed a commit that referenced this pull request May 22, 2019
Allows setting a mask for updatedb script via environment variable.
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.

None yet

3 participants