Add D400 pydocstyle check - core Airflow only#31297
Merged
eladkal merged 13 commits intoapache:mainfrom May 18, 2023
Merged
Conversation
vincbeck
requested review from
XD-DENG,
ashb,
bbovenzi,
bolkedebruin,
dstandish,
ephraimbuddy,
jedcunningham,
jhtimmins,
kaxil,
o-nikolas,
potiuk,
ryanahamilton and
uranusjr
as code owners
boring-cyborg
bot
added
area:API
Contributor
Author
vincbeck
changed the title
vincbeck
force-pushed
the
vincbeck/d400-core
branch
from
3f1f7d7 to
62dfa88
Compare
uranusjr
reviewed
May 16, 2023
uranusjr
reviewed
May 16, 2023
Member
uranusjr
left a comment
uranusjr
left a comment
There was a problem hiding this comment.
Nice! Just a few things I think would be best addressed in this PR. I have some other quasi-related improvements locally that can be submitted separately after this one is merged.
airflow/utils/sqlalchemy.py
Outdated
Comment on lines
52
to
62
| Almost equivalent to :class:`~sqlalchemy.types.TIMESTAMP` with | ||
| ``timezone=True`` option, but it differs from that by: | ||
|
|
||
| - Never silently take naive :class:`~datetime.datetime`, instead it | ||
| always raise :exc:`ValueError` unless time zone aware value. | ||
| - :class:`~datetime.datetime` value's :attr:`~datetime.datetime.tzinfo` | ||
| is always converted to UTC. | ||
| - Unlike SQLAlchemy's built-in :class:`~sqlalchemy.types.TIMESTAMP`, | ||
| it never return naive :class:`~datetime.datetime`, but time zone | ||
| aware value, even with SQLite or MySQL. | ||
| - Always returns TIMESTAMP in UTC | ||
| - Always returns TIMESTAMP in UTC. | ||
|
|
Member
There was a problem hiding this comment.
This should probably be reworded a bit, the new format is even more wrong.
Contributor
Author
There was a problem hiding this comment.
To be honest I dont understand the previous 2 level lists. To me the new format makes more sense but I must miss something
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
Contributor
Author
Thanks for the review! I applied all your suggestion but one :) See my comment |
uranusjr
reviewed
May 17, 2023
uranusjr
approved these changes
May 18, 2023
eladkal
approved these changes
May 18, 2023
ephraimbuddy
added
type:misc/internal
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a follow up of #31135. It had been decided to split #31135 in multiple PRs to make it easier to review. This PR's scope is core Airflow only.
Part of #10742.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.