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.

Sign up for GitHub

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

Jump to bottom

Augment xcom docs #20755

Merged
merged 15 commits into from Feb 3, 2022
Merged

Augment xcom docs #20755

merged 15 commits into from Feb 3, 2022

Conversation

lewismc
Copy link
Member

@lewismc lewismc commented Jan 7, 2022

This PR addresses some investigation and subsequent assistance on user@ regarding the development and deployment of custom XCom backends.
This is only a documentation patch.
Thanks for any review.

uranusjr
uranusjr reviewed Jan 8, 2022
View reviewed changes
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpickings.

docs/apache-airflow/concepts/xcoms.rst Outdated Show resolved Hide resolved
docs/apache-airflow/concepts/xcoms.rst Outdated Show resolved Hide resolved
@lewismc
Copy link
Member Author

lewismc commented Jan 8, 2022

@uranusjr I also didn't know I could merge your suggestion. I've never used that GH feature before. That's pretty cool. Thanks for taking the time to make the suggestion.

@lewismc
Copy link
Member Author

lewismc commented Jan 8, 2022

... and the "nitpickings" are what make good codebases much better. Thanks for the review.

@github-actions
Copy link

github-actions bot commented Jan 8, 2022

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.

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Jan 8, 2022
@potiuk
Copy link
Member

potiuk commented Jan 10, 2022

some build docs and static checks are failing

@lewismc
Copy link
Member Author

lewismc commented Jan 10, 2022

@potiuk thanks for the nudge :)
I see the following

blacken-docs.............................................................................Failed

  • hook id: blacken-docs
  • exit code: 1
    docs/apache-airflow/concepts/xcoms.rst:91: code block parse error Cannot parse: 1:16: Traceback (most recent call last):

However I have no idea what to fix in line 91 of xcoms.rst. Any ideas? Thanks

@potiuk
Copy link
Member

potiuk commented Jan 10, 2022

Sure. The code starting this line needs to be valid python code.

@potiuk
Copy link
Member

potiuk commented Jan 22, 2022

Would you rebase and fix static/docs errors?

@lewismc
Copy link
Member Author

lewismc commented Jan 23, 2022

Hi @potiuk I made some changes lets see if CI likes them or not. Thanks.

@potiuk
Copy link
Member

potiuk commented Jan 24, 2022

Still some errors, I am afraid. I recommend to install pre-commit locally and run it for static checks and then run docs build (for example using ./breeze build-docs .

@lewismc
Copy link
Member Author

lewismc commented Jan 27, 2022

@potiuk finally :) thanks

@eladkal eladkal added this to the Airflow 2.2.4 milestone Feb 1, 2022
@eladkal eladkal added the type:doc-only Changelog: Doc Only label Feb 1, 2022
jedcunningham
jedcunningham reviewed Feb 1, 2022
View reviewed changes
docs/apache-airflow/concepts/xcoms.rst Outdated Show resolved Hide resolved
docs/apache-airflow/concepts/xcoms.rst Outdated Show resolved Hide resolved
lewismc and others added 2 commits February 2, 2022 09:26
@lewismc @uranusjr
Update docs/apache-airflow/concepts/xcoms.rst
2f13ed9 
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@lewismc @jedcunningham
Update docs/apache-airflow/concepts/xcoms.rst
59028b5 
Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
@lewismc
Copy link
Member Author

lewismc commented Feb 2, 2022

Thanks for the review @jedcunningham

jedcunningham
jedcunningham reviewed Feb 3, 2022
View reviewed changes
docs/apache-airflow/concepts/xcoms.rst Outdated Show resolved Hide resolved
docs/apache-airflow/concepts/xcoms.rst Outdated Show resolved Hide resolved
docs/apache-airflow/concepts/xcoms.rst Outdated Show resolved Hide resolved
@jedcunningham
Copy link
Member

Noticed a few more things, overall LGTM

lewismc and others added 3 commits February 2, 2022 17:42
@lewismc @jedcunningham
Update docs/apache-airflow/concepts/xcoms.rst
ec01936 
Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
@lewismc @jedcunningham
Update docs/apache-airflow/concepts/xcoms.rst
0bc737f 
Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
@lewismc @jedcunningham
Update docs/apache-airflow/concepts/xcoms.rst
053d12b 
Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
@lewismc
Copy link
Member Author

lewismc commented Feb 3, 2022

Thanks again @jedcunningham

@jedcunningham jedcunningham merged commit 40d3a76 into apache:main Feb 3, 2022
jedcunningham pushed a commit that referenced this pull request Feb 3, 2022
@lewismc @jedcunningham
Augment xcom docs (#20755)
74ff37c 
(cherry picked from commit 40d3a76)
jhtimmins added a commit that referenced this pull request Feb 4, 2022
@jhtimmins @ashb
@lewismc @luciakasman @fj-sanchez @mik-laj
Simplify fab has access lookup (#19294)
3e98280 
* Use FAB models.

* Remove incorrect conversions to new permission naming scheme.

* Fix missing FAB renames.

* Remove unused FAB compatibility fixes in models.py.

* Set perms directly on user objects.

* Set perms properties on User model.

* Rename missed old naming scheme conversion.

* Remove unused imports.

* Remove unused imports.

* Remeve get_user_roles() method.

* Make permissions eagerload.

* Remove unused imports.

* Clarify query params.

* Modify sort logic so MSSQL passes.

* Add text modifier to order_by values.

* Remove calls to get_*_dags.

* Add back execution_date

* Add back comma to match rest of file.

* Remove unused permission functions.

* Fix failing tests.

* Pass user object to current_app.appbuilder.sm.has_all_dags_access.

* Remove attempts to fix query.

* Update the api_connexion query builders.

* Add typing.

* Apply sorts directly to model objects.

* Apply sorts directly to model objects.

* Standardize custom sort code.

* Code review

* Augment xcom docs (#20755)

* Fix relationship join bug in FAB/SecurityManager with SQLA 1.4 (#21296)

This is fixed in SQLA 1.4.19, but the fix makes the intent clearer here
anyway.

* Docs: Fix task order in overview example (#21282)

* Update stat_name_handler documentation (#21298)

Previously stat_name_handler was under the scheduler section of the
configuration but it was moved to the metrics section since 2.0.0.

* Update recipe for Google Cloud SDK (#21268)

* Use FAB models.

* Remove incorrect conversions to new permission naming scheme.

* Fix missing FAB renames.

* Remove unused FAB compatibility fixes in models.py.

* Set perms directly on user objects.

* Set perms properties on User model.

* Rename missed old naming scheme conversion.

* Remove unused imports.

* Remove unused imports.

* Remeve get_user_roles() method.

* Make permissions eagerload.

* Remove unused imports.

* Clarify query params.

* Modify sort logic so MSSQL passes.

* Add text modifier to order_by values.

* Remove calls to get_*_dags.

* Add back execution_date

* Add back comma to match rest of file.

* Remove unused permission functions.

* Fix failing tests.

* Pass user object to current_app.appbuilder.sm.has_all_dags_access.

* Remove attempts to fix query.

* Update the api_connexion query builders.

* Add typing.

* Apply sorts directly to model objects.

* Apply sorts directly to model objects.

* Standardize custom sort code.

* Make sure joined fields prefetch.

* Dont use cached_property, since its only on > 3.8.

Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Lewis John McGibbney <lewis.mcgibbney@gmail.com>
Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
Co-authored-by: Lucia Kasman <38845383+luciakasman@users.noreply.github.com>
Co-authored-by: Fran Sánchez <fj-sanchez@users.noreply.github.com>
Co-authored-by: Kamil Breguła <mik-laj@users.noreply.github.com>
jedcunningham pushed a commit that referenced this pull request Feb 10, 2022
@lewismc @jedcunningham
Augment xcom docs (#20755)
e304067 
(cherry picked from commit 40d3a76)
jedcunningham pushed a commit that referenced this pull request Feb 17, 2022
@lewismc @jedcunningham
Augment xcom docs (#20755)
8890087 
(cherry picked from commit 40d3a76)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potiuk potiuk potiuk left review comments

@uranusjr uranusjr uranusjr approved these changes

@jedcunningham jedcunningham jedcunningham approved these changes

@kaxil kaxil Awaiting requested review from kaxil

Assignees
No one assigned
Labels
kind:documentation okay to merge It's ok to merge this PR as it does not require more tests type:doc-only Changelog: Doc Only
Projects
None yet
Milestone
Airflow 2.2.4
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lewismc @potiuk @jedcunningham @uranusjr @eladkal