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

Replace Ruff's known-third-party setting with namespace-packages #32873

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Replace Ruff's known-third-party setting with namespace-packages #32873

merged 1 commit into from
Jul 27, 2023

Conversation

charliermarsh
Copy link
Contributor

👋 This PR removes the known-third-party and known-first-party settings from Airflow's Ruff configuration with a single namespace-packages entry.

Previously, all subdirectories of airflow/providers were enumerated in the pyproject.toml, but it looks like a few were added to the codebase over time but not to the list in pyproject.toml, leading to some import-categorization drift (as visible in the below changes).

Ruff allows for namespace packages to be enumerated directly, which will ensure that future packages are handled consistently without any need to update the pyproject.toml.

@boring-cyborg
Copy link

boring-cyborg bot commented Jul 27, 2023

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 (ruff, 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: [email protected]
    Slack: https://s.apache.org/airflow-slack

@@ -21,9 +21,10 @@
from typing import Any, Iterable

import apprise
from airflow.hooks.base import BaseHook
from apprise import AppriseConfig, NotifyFormat, NotifyType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As an example: these apprise imports were incorrectly marked as first-party, since apprise was missing from the known-third-party.

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.

This is awesome

@uranusjr uranusjr merged commit 9194144 into apache:main Jul 27, 2023
61 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 27, 2023

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

potiuk added a commit to potiuk/airflow that referenced this pull request Jul 27, 2023
The apache#32873 has been merged without merging and commiting breeze's hash
accounting for changed dependencies, breaking unit tests of breeze.

This PR brings hash of breeze back adter running `breeze setup version`
manually and commiting the changed hash.
potiuk added a commit that referenced this pull request Jul 27, 2023
The #32873 has been merged without merging and commiting breeze's hash
accounting for changed dependencies, breaking unit tests of breeze.

This PR brings hash of breeze back adter running `breeze setup version`
manually and commiting the changed hash.
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:openlineage AIP-53
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants