aws / amazon-mwaa-docker-images

Apache License 2.0
24 stars 11 forks source link

No longer possible to pass PyPI password as envar defined in config options #126

Closed jaklan closed 1 week ago

jaklan commented 1 month ago

Hi, already for a year, on various Airflow 2.x versions, we were using the below method to pass the password to the requirements.txt file:

image

https://docs.aws.amazon.com/mwaa/latest/userguide/best-practices-dependencies.html#best-practices-dependencies-custom-auth-url

--constraint /usr/local/airflow/dags/constraints.txt

--extra-index-url https://__token__:${AIRFLOW__GITLAB__TOKEN}@<gitlab-domain>.com/api/v4/projects/6423/packages/pypi/simple

It worked correctly until we upgraded to 2.9 - since then we get the following error:

WARNING: 401 Error, Credentials not correct for https://<gitlab-domain>.com/api/v4/projects/6423/packages/pypi/simple/<package-name>/

There were no changes in configuration options, the option name and password value are still the same. When we hardcode the password directly into requirements.txt - it works correctly, so the password itself is not a problem. It seems the given config is simply not accessible anymore as envar and the related placeholder is evaluated as empty.

rafidka commented 1 month ago

Hmm.. this is strange. We are not doing any preprocessing on the requirements.txt file that modifies its content, and literally just pass it to safe-pip-install (our version of pip install). Are you able to produce it using the Docker Compose setup (in the README instructions)?

rafidka commented 1 month ago

Ohh.. I see, you are passing in the username and password via environment variables, sorry didn't notice that. Then yeah, I see why that wouldn't work, as we are not passing the environment variables down to the safe-pip-install call. This should be an easy fix.

rafidka commented 1 month ago

In fact, the issue you are seeing started to happen after we merged PR #122, as we removed passing down the environment variables to the pip in that PR, because I mistakenly thought it is not useful and might cause issues. We can just revert that line and we should be good.

jaklan commented 1 month ago

@rafidka thanks for quick analysis and answer!

We can just revert that line and we should be good.

That would be really great as that's currently the only option not to store a password directly in requirements.txt afaik