Add environment vars expansion and pre defined git hashes in the yaml

ploomber / soopervisor

☁️ Export Ploomber pipelines to Kubernetes (Argo), Airflow, AWS Batch, SLURM, and Kubeflow.

https://soopervisor.readthedocs.io

Apache License 2.0

45 stars 18 forks source link

Add environment vars expansion and pre defined git hashes in the yaml #120

Closed feribg closed 1 year ago

feribg commented 1 year ago

Describe your changes

Issue ticket number and link

Closes #116

Checklist before requesting a review

[ x ] I have performed a self-review of my code
[ x ] I have added thorough tests (when necessary).
[ ] I have added the right documentation (when needed). Product update? If yes, write one line about this update.

edublancas commented 1 year ago

hey @feribg, I fixed and merged the PR. Please take the version in master for a spin and let me know if it works for you. If so, I'll make a release.

Note: I saw you had a build_time placeholder (apart from the git ones) in your logic, I deleted it because I believe that will never get executed as it is not defined as a default placeholder in ploomber. is that right?

feribg commented 1 year ago

@edublancas thanks i will take a look and try to use the master one and report back. Yes I just added that one but didnt end up implementing it the idea being is to act as a git hash if it's a git repo but I dont really need it, figured it might be useful.

edublancas commented 1 year ago

great, I also released a new ploomber version that contains your changes.

feribg commented 1 year ago

@edublancas doesn't look like its working correctly.

env_default = EnvDict({}, path_to_here=Path(path_to_env).parent)

* {{git_hash}}: Ensure git is installed and git repository exists

>>> soopervisor.__version__
'0.9.1'

Seems like it keeps looking for .git for whatever reason and its a bit tricky to debug it in AWS batch.

the .env file in the container isn't the merged one it doesn't contain git and git_hash only the original values. Also any thoughts on having a env_name.env.yaml in that gets moved to the container instead of the env.yaml seems safer for prod vs local dev

edublancas commented 1 year ago

Since ploomber implements the logic for expanding these values, it's possible to replicate this exclusively with ploomber, right? No AWS Batch, no docker.

Based on the error message, looks like there is a bug that's erroring because there is no repository, so probably we need to add some extra logic to ploomber that checks if those placeholders are defined already and skip checking for a git repository. Do you have time to submit a PR?