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

added explaining concept of logical date in DAG run #21433

Merged
merged 13 commits into from Feb 17, 2022
Merged

added explaining concept of logical date in DAG run #21433

merged 13 commits into from Feb 17, 2022

Conversation

howardyoo
Copy link
Contributor

Added two paragraphs under the DAG run to describe the difference between logical date and dag start and end date, and some of the reason behind its naming 'logical,' as well as providing an example. The intention for this additional content is to clarify any confusions about what logical date would mean compared to the DAG run's start and end date.


^ 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.

Added two paragraphs under the DAG run to describe the difference between logical date and dag start and end date, and some of the reason behind its naming 'logical,' as well as providing an example.
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 8, 2022

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)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@uranusjr
Copy link
Member

There are sections in DAG run and FAQ pages on this as well, it may be a good idea to link them together with seealso.

changed  a bit of formatting
@howardyoo
Copy link
Contributor Author

@uranusjr , I'm not sure which locations are you referring to in sections in DAG run and FAQ pages. Can you let me know so that I can perhaps create the seealso to it? Thanks!

@potiuk
Copy link
Member

potiuk commented Feb 15, 2022

@potiuk
Copy link
Member

potiuk commented Feb 15, 2022

And yeah I think we should aim to bring all those inter-connected. I think our docuemenation has too lttle interlinking and some information is repeated in places (and often some of the variants of the descriptions are out-dated).

I tend to simply run search/grep on similar terms when I update some documentation. Our documentation has pretty fast and good search (I know people aren't used to search in the documentation but I actually learned that the search here is actually very decent, fast and accurate :) https://airflow.apache.org/docs/apache-airflow/stable/index.html

@howardyoo
Copy link
Contributor Author

Ok, added those references on each of the files mentioned above.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

LGTM. @uranusjr ?

@github-actions
Copy link

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 Feb 16, 2022
@howardyoo
Copy link
Contributor Author

done - I believe all the tests have succeeded, and the PR is ready to be merged.

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 thoughts on wording. Generally looks nice, thanks for taking the time to put this in words! It’s inheritantly a difficult thing to explain!

docs/apache-airflow/concepts/dags.rst Outdated Show resolved Hide resolved
docs/apache-airflow/concepts/dags.rst Outdated Show resolved Hide resolved
docs/apache-airflow/concepts/dags.rst Outdated Show resolved Hide resolved
howardyoo and others added 3 commits February 16, 2022 23:05
sounds good! I'll commit the changes.

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
dag run does not have end date when it starts, but when it ends. Modified the description correctly.
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@potiuk potiuk added the type:doc-only Changelog: Doc Only label Feb 17, 2022
@potiuk potiuk added this to the Airflow 2.2.4 milestone Feb 17, 2022
@jedcunningham jedcunningham merged commit 752d538 into apache:main Feb 17, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 17, 2022

Awesome work, congrats on your first merged pull request!

@jedcunningham
Copy link
Member

@howardyoo thanks! Congrats on your first commit 🎉

jedcunningham pushed a commit that referenced this pull request Feb 17, 2022
@howardyoo
Copy link
Contributor Author

@howardyoo thanks! Congrats on your first commit 🎉

Thank you!! I hope for more to come! :-)

@howardyoo howardyoo deleted the patch-1 branch February 18, 2022 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants