Skip to content

Comments

#10472: Add D200 pydocstyle check - part1#11587

Closed
Samira-g-js wants to merge 59 commits intoapache:masterfrom
Samira-g-js:py-docstyle-check-D200
Closed

#10472: Add D200 pydocstyle check - part1#11587
Samira-g-js wants to merge 59 commits intoapache:masterfrom
Samira-g-js:py-docstyle-check-D200

Conversation

@Samira-g-js
Copy link

Related to #10472 - Pydocstyle check

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:CLI area:dev-tools area:Scheduler including HA (high availability) scheduler area:secrets area:webserver Webserver related Issues provider:amazon AWS/Amazon - related issues labels Oct 16, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 16, 2020

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/master/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, pylint 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

mik-laj and others added 7 commits October 16, 2020 18:28
Co-authored-by: Tomek Urbaszek <turbaszek@gmail.com>
Co-authored-by: Tomek Urbaszek <turbaszek@gmail.com>
- The dag_run argument is only there for test mocks, and only to access a static method. Removing this simplifies the function, reduces confusion.
- Give optional arguments a default value, reduce indentation of arg list to PEP / Black standard.
- Clean up tests for readability
Although these lists are short, there's no need to re-create them each
time, and also no need for them to be a method.

I have made them lowercase (`finished`, `running`) instead of uppercase
(`FINISHED`, `RUNNING`) to distinguish them from the actual states.
@potiuk
Copy link
Member

potiuk commented Oct 16, 2020

Can you please rebase @Samira-g-js ? there are some conflicts :(

kaxil and others added 13 commits October 18, 2020 21:12
`cllient` -> `client`
`environement` -> `environment`
`naamespace` -> `namespace`
`alllow` -> `allow`
* Improves stability of K8S tests by caching binaries and repeats

The K8S tests on CI are controlled from the host, not from
inside of the CI container image. Therefore it needs virtualenv
to run the tests as well as some tools such as helm, kubectl
and kind. While those tools can bee downloaded and installed
on demand, from time to time the download fails intermittently.

This change introduces the following improvements:

* the commands to download and setup kind, helm, kubectl are
  repeated up to 4 times in case they fail

* the "bin" directory where those binaries are downloaded is
  cached between runs. Only the same combination of
  versions of the tools are sharing the same cache.

This way both cases - regular re-runs of the same jobs and
upgrade of tools will be much more stable.
Seems like the trap with several steps and || true does not really
work the way I wanted and when kill is run but the process is
already gone, we had error in the script.

Looks like this approach with sub-process kill will do it.
A few remnants of earlier version of the script caused occasional
errors. Error introduced in #11541
In case of very simple changes, there might be no merge commits
generated by GitHub. In such cases we should take the commit SHA
instead as the base of change calculation for selective tests.
We do not dump airflow logs on success any more, but we dump them
and all the container logs in case of failure, so that we can
better investigate cases like #11543 - that includes enabling
full deadlock information dumping in our mysql database.
Since we support Py 3.6, there is no need of six library
* fix: 🐛 Float to Int columns conversion

The `_fix_int_dytpes` method is applying the `astype` transformation to
the return of a `np.where` call. I added an extra step to the method in
order to apply this to the whole pd.Series. Note that Int64Dtype must be
used as an instance, since Pandas will raise an Exception if a class is
used.

* test: Add dtype test for integers

* style: Change line length
@Samira-g-js Samira-g-js changed the title #10472: Add D200 pydocstyle check #10472: Add D200 pydocstyle check - part1 Oct 19, 2020
alxdembo and others added 8 commits October 19, 2020 10:57
Closed #11388

Co-authored-by: Ryan Hamilton <ryan@ryanahamilton.com>
When we prepare pre-release versions, they are not intended to be
converted to final release versions, so there is no need to replace
version number for them artificially,

For release candidates on the other hand, we should internally use the
"final" version because those packages might be simply renamed to the
final "production" versions.

Fixes #11585
@Samira-g-js
Copy link
Author

@potiuk Hey I've rebased with my changes on top, can we just double check that no one else's changes since have been overwritten. I'm a bit of a newbie :(

@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.

@mik-laj mik-laj removed the area:API Airflow's REST/HTTP API label Oct 19, 2020
@Samira-g-js Samira-g-js marked this pull request as ready for review October 20, 2020 10:56
@kaxil
Copy link
Member

kaxil commented Nov 4, 2020

Can you please rebase your PR on latest Master since we have applied Black and PyUpgrade on Master.

It will help if your squash your commits into single commit first so that there are less conflicts.

@Samira-g-js Samira-g-js deleted the py-docstyle-check-D200 branch November 17, 2020 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI area:dev-tools area:Scheduler including HA (high availability) scheduler area:secrets area:webserver Webserver related Issues provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.