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

allow using requirments.txt in PythonVirtualEnvOperator #16037

Closed
alexInhert opened this issue May 25, 2021 · 14 comments · Fixed by #17349
Closed

allow using requirments.txt in PythonVirtualEnvOperator #16037

alexInhert opened this issue May 25, 2021 · 14 comments · Fixed by #17349
Assignees
Labels
area:core-operators Operators, Sensors and hooks within Core Airflow contributors-workshop Issues that are good for first-time contributor's workshop good first issue kind:feature Feature Requests

Comments

@alexInhert
Copy link

Currently the operator allows to set requirement as list that needs to be hard coded.
It would be nice if airflow can support reading from file directly (something similar to how operators read sql file)

@alexInhert alexInhert added the kind:feature Feature Requests label May 25, 2021
@mik-laj
Copy link
Member

mik-laj commented May 25, 2021

Can you see if you can pass ["-r", "requireements.txt] as. requirements?

@potiuk
Copy link
Member

potiuk commented May 25, 2021

How about this (I know it's not perfect, but I think it should do the job):

requirements = (lambda: open("requirements.txt").readlines())()

@uranusjr
Copy link
Member

The requirements file format supports some things that can’t be passed into the command line, so the open-readline approach can only partially solve the issue. Making things worse, the file format is internal to pip, so there’s no way for the user to parse it on their own, the file must be passed into pip. So some way to support this would be nice. (I think I read another issue on this before…?)

I believe requirements=["-r", "requireements.txt] will work. Pretty hacky though.

@potiuk
Copy link
Member

potiuk commented May 25, 2021

Agree. Having a good, templated parameter could work. Now when I think obout it might actually be as simple as adding this to the operator and handling the case where requirements is also a string and properly using it with -r/temp file by the operator in this case:

    template_fields = ("requirements",)
    template_ext = (".txt",)

The template_ext is one of the lesser-known features of Airflow operators

@uranusjr
Copy link
Member

Or maybe we can do something similar to bash_command? If a requirements argument looks like a file (os.path.isfile() or contains os.sep), automatically insert a -r in from of it.

@potiuk
Copy link
Member

potiuk commented May 25, 2021

I thinkl it the same feature we are talking about :).

The "template_ext" works in the way that it will read the file as string when the parameter has the extension registered there and it will automatically process the file trough the template engine, so you can add jinja-variables from context /xcom variables in the file. It's built-in feature in BaseOperator

All that you need is in this case to have the right extension (".txt") in the value of the parameter.

requirements="[<path>/]reqiurements.txt"

@potiuk
Copy link
Member

potiuk commented May 25, 2021

If the parameter is not string or does not have the extension it is processed as usual (i.e. passing list of strings will continue to work - but it will additionally process the strings through template engine (since "requirements" will be in "template_fields" tuple) so you could also add jinja variables in the strings in the array and they will be interpolated.

@alexInhert
Copy link
Author

But in this case you want the functionality of template_ext without the template_fields
There is no need to go through jinja engine

@potiuk
Copy link
Member

potiuk commented May 25, 2021

But in this case you want the functionality of template_ext without the template_fields
There is no need to go through jinja engine

But this might be very useful. For example you can use jinja variable for version of a dependency and get it from another task. or have conditional requirements.txt content based on dev/prod settings etc.

This is pretty useful feature IMHO.

@alexInhert
Copy link
Author

I think in your use case the whole requirement file is in fact not a requirement.txt but a value pushed to xcom by previous task.

@potiuk
Copy link
Member

potiuk commented May 25, 2021

I think in your use case the whole requirement file is in fact not a requirement.txt but a value pushed to xcom by previous task.

No. I am talking about "requirement.txt" file in which some values might be provided by XCom.

I can imagine this kind of requirement.txt file for example:

dependency1==1.0.0
dependency2==2.0.0
dependency3=={{ code to retrieve depedency version from xcom}}
dependency4==1.1.0
{{- % if retrireve deployment == 'production'  % }}
# only needed in production deployment
dependency5==5.0.0
{{- % endif % }}

@eladkal eladkal added the area:core-operators Operators, Sensors and hooks within Core Airflow label May 31, 2021
@alexInhert
Copy link
Author

ah cool!!
This is even better than what I had in mind

@turbaszek turbaszek added the contributors-workshop Issues that are good for first-time contributor's workshop label Jul 4, 2021
@vijaykiran
Copy link

I'd like to pick this (as part of the contributors-workshop)

@rounakdatta
Copy link
Contributor

Hello @potiuk @alexInhert, I have tried an implementation of the above discussions, can you review once? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core-operators Operators, Sensors and hooks within Core Airflow contributors-workshop Issues that are good for first-time contributor's workshop good first issue kind:feature Feature Requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants