-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Add Python dependencies example workflow. #1576
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
Conversation
Updates sample to use relative imports. Adds license headers. Uses 4 spaces for indentation.
tswast
left a comment
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.
Thanks!
I had some nits, but I fixed them in 99b2401
As we discussed, I've moved to using relative imports since I think that will be more future-proof for Python 3.
| @@ -0,0 +1,11 @@ | |||
| # Python dependencies example | |||
|
|
|||
| In this example DAG (`dag.py`), we need to use two Python packages: | |||
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.
Let's add a requirements.txt file for these dependencies. The test runner uses the presence of requirements.txt to find samples.
| @@ -0,0 +1,18 @@ | |||
| # This DAG consists of a single BashOperator that prints the result of a coin flip. | |||
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 required Apache 2 license headers per http://go/releasing/preparing#headers
|
|
||
| from airflow import DAG | ||
| from airflow.operators.bash_operator import BashOperator | ||
| from datetime import datetime, timedelta |
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.
Nit: Import modules not classes/functions.
Also, the order of imports should be: (1) built-ins (2) third-party (3) local packages
| from airflow import DAG | ||
| from airflow.operators.bash_operator import BashOperator | ||
| from datetime import datetime, timedelta | ||
| from dependencies import coin_package |
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.
Should this be a relative import instead?
| # scipy PyPI package installed. | ||
|
|
||
| import numpy as np # numpy is installed by default in Composer. | ||
| from scipy import special # scipy is not. |
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.
Nit: Two spaces before comments.
| @@ -0,0 +1,16 @@ | |||
| # This custom PyPI package requires your environment to have the | |||
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.
Missing license header.
|
|
||
| # Returns "Heads" or "Tails" depending on a calculation | ||
| def flip_coin(): | ||
| # Returns a 2x2 randomly sampled array of values in the range [-5, 5] |
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.
Nit: Indent 4 spaces in public Python samples. (Follows PEP-8 style guide.)
Add an example workflow using a PyPI dependency and a custom dependency, and corresponding custom dependency.