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
Addressed some issues in the tutorial mentioned in discussion #22233 #22236
Addressed some issues in the tutorial mentioned in discussion #22233 #22236
Conversation
…addressed some concerns in duscussion # 22233
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
Co-authored-by: Ephraim Anierobi <splendidzigy24@gmail.com>
I spent a bit more time refactoring (after noticing a thought I left incomplete). I assume most tutorial readers are either trying to get things running on their machine or are evaluating whether it's worth taking the time to try, so I tried to make the example terse but still be batteries-included. Main changes:
|
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.
Love the attention to details !
Thanks @MattTriano -> let's see how the CI likes it :) |
The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
These errors caused the For Error 2, I think the issue was the lack of whitespace between the text and the bracketed-URI, but I'm not too familiar with the reStructuredText spec, so it's possible that I'm wrong and wasting more compute. |
You can run it locally (see the error output - it explains what commands you can run to build the docs locally) |
…ker doesn't recognize the word 'preprocessing'.
What a |
Awesome work, congrats on your first merged pull request! |
Thanks! Really cool! |
While working through the tutorial, some parts seemed a bit unclear or incorrect, which took me to issue #18950 ( converted into discussion #22233 about 15 minutes ago). I had already resolved some of the open concerns and saw potiuk encouraging people to improve the documentation, so I took it upon myself.
Main Changes:
get_data()
task.merge_data()
task so that its behavior matches the stated description.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.