-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add ability to update lockfile without updating poprox-concepts #58
Conversation
@@ -1,19 +1,61 @@ | |||
#!/bin/sh | |||
#!/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated because the new logic uses Bash arrays to deal (more) sanely with lists of arguments, etc.
# make sure we're in the right directory | ||
if [ ! -f pyproject.toml ]; then | ||
if [[ ! -f pyproject.toml ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're /bin/bash
now, use the more robust conditional construct.
if [[ ${#concept_urls[@]} -eq 1 ]]; then | ||
echo "found locked poprox-concepts at $concept_urls" | ||
elif [[ ${#concept_urls[@]} -eq 0 ]]; then | ||
echo "ERROR: no poprox-concepts not found in lockfile" >&2 | ||
exit 3 | ||
else | ||
echo "WARNING: lockfile has multiple poprox-concepts versions" >&2 | ||
for ver in "${concept_urls[@]}"; do | ||
echo "- $ver" >&2 | ||
done | ||
echo "using: ${concept_urls[0]}" >&2 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should always have exactly 1 distinct poprox-concepts
URL (including hash) in the lockfile — I do not know of a way that would be violated in current setup. However, to make this robust in the unlikely scenario that we do somehow have such an invalid lock file, this is some error checking to yell about the siutation if parsing conda-lock
does not yield exactly one source URL.
cat >dev/poprox-concepts-dynamic-constraint.yml <<EOF | ||
dependencies: | ||
- pip: | ||
- ${concept_urls[0]} | ||
EOF | ||
lock_args=("${lock_args[@]}" -f dev/poprox-concepts-dynamic-constraint.yml) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We implement the constraint by writing an additional constraint file and including it in the conda-lock
inputs.
echo "+ conda-lock lock ${lock_args[*]}" >&2 | ||
exec conda-lock lock "${lock_args[@]}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bit of quick debugging support. The [*]
vs [@]
distinction is deliberate — for the debug message, we don't want the shell to do word-splitting.
@@ -9,6 +9,7 @@ dependencies: | |||
- dvc >=3.51,<4 | |||
- dvc-s3 | |||
- conda-lock >=2.5,<3 | |||
- yq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script updates use yq
to parse the conda-lock.yml
file.
There is a good argument to be made that we should be specifying versions in I'm happy to help build such a workflow, but doing so is outside the scope of "make it so we can add/change external deps while others are working on updates for concept changes". |
This PR adds a
--freeze-concepts
option toupdate-dep-lock.sh
to allow us to update the lock file (e.g. to add a new dependency) without also updating the version ofpoprox-concepts
in use. This is useful to allow development of things that need new dependencies while someone else is working on updating the recommender to a newpoprox-concepts
version.Adding the reverse — updating only
poprox-concepts
without other dependencies — is still blocking on conda/conda-lock#652.