Skip to content

[Backfill corrections] Fetch user-provided paths only at setup + increase pip timeout #1754

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

Merged
merged 2 commits into from
Jan 11, 2023

Conversation

nmdefries
Copy link
Contributor

Description

Make sure that user-provided paths are saved permanently (evaluated only once) by using make's := assignment operator.

Previously, these variables were assigned with = which re-evaluates the expression every time the variable is used. Since we change the contents of params.json, at some points in the Makefile, the supposedly user-provided path variables actually contain the fixed dir names intended for use inside of the docker container.

Bonus: increase pip install timeout from the default of 15 s. When building the docker container, installing delphi-utils and dependencies exceeds this, causing an error.

Changelog

  • Makefile

Fixes

Pipeline complained about not finding any relevant files in input dir, despite dir and files existing.

@nmdefries nmdefries requested a review from krivard January 10, 2023 19:17
@krivard
Copy link
Contributor

krivard commented Jan 11, 2023

Is the backfill corrections check expected to fail?

@nmdefries
Copy link
Contributor Author

Whoops, didn't notice. The build was getting a 503 error from one of the package sites. Rerunning resolved it.

Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

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

solid fix

@nmdefries
Copy link
Contributor Author

This can be merged @krivard

@krivard krivard merged commit 3d2581f into main Jan 11, 2023
@krivard krivard deleted the ndefries/backcorr-eval-vars-once-timeout branch January 11, 2023 22:22
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.

2 participants