[SPARK-31153][BUILD] Cleanup several failures in lint-python #27910

nchammas · 2020-03-14T06:49:38Z

What changes were proposed in this pull request?

This PR cleans up several failures -- most of them silent -- in dev/lint-python. I don't understand how we haven't been bitten by these yet. Perhaps we've been lucky?

Fixes include:

Fix how we compare versions. All the version checks currently in master silently fail with:
```
  File "<string>", line 2
    print(LooseVersion("""2.3.1""") >= LooseVersion("""2.4.0"""))
    ^
IndentationError: unexpected indent
```
Another problem is that distutils.version is undocumented and unsupported.
Fix some basic bugs. e.g. We have an incorrect reference to $PYDOCSTYLEBUILD, which doesn't exist, which was causing the doc style test to silently fail with:
```
./dev/lint-python: line 193: --version: command not found
```
Stop suppressing error output! It's hiding problems and serves no purpose here.

Why are the changes needed?

lint-python is part of our CI build and is currently doing any combination of the following: silently failing; incorrectly skipping tests; incorrectly downloading libraries when a suitable library is already available.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Lots of manual testing with set -x enabled.

SparkQA · 2020-03-14T07:05:02Z

Test build #119787 has finished for PR 27910 at commit 51488cf.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

nchammas · 2020-03-14T08:20:10Z

Both Jenkins and GitHub are reporting:

The pydocstyle command was not found. Skipping pydocstyle checks for now.

So we are not running doc style tests anywhere...

I will address this in a separate PR.

SparkQA · 2020-03-14T09:56:57Z

Test build #119793 has finished for PR 27910 at commit 5ad9396.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-03-14T10:41:58Z

Test build #119792 has finished for PR 27910 at commit 02184d3.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2020-03-14T14:04:32Z

I think it's OK if it fixes the linter, and doesn't uncover old failures that would currently fail.

nchammas · 2020-03-14T16:50:07Z

Jenkins, retest this please.

SparkQA · 2020-03-14T19:09:59Z

Test build #119801 has finished for PR 27910 at commit 5ad9396.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2020-03-15T04:00:52Z

dev/lint-python

    RUN_LOCAL_PYCODESTYLE="False"
    if hash "$PYCODESTYLE_BUILD" 2> /dev/null; then
-        VERSION=$( $PYCODESTYLE_BUILD --version 2> /dev/null)
-        EXPECTED_PYCODESTYLE=$( ("$PYTHON_EXECUTABLE" -c 'from distutils.version import LooseVersion;


I think it was mistakenly re-indented at 42c4838#diff-ccd847a0316575dde31bd89786bbe1f2. cc @shaneknapp FYI

HyukjinKwon · 2020-03-15T04:07:27Z

It fixes a bunch of mistakes we should avoid in general ...

HyukjinKwon · 2020-03-15T04:09:20Z

Merged to master.

HyukjinKwon · 2020-03-15T04:11:11Z

@nchammas, it has conflicts with branch-3.0. Do you mind if I ask to make a backporting PR? We should port it back to branch-3.0 as 42c4838 laded to to branch-3.0 too.

This PR cleans up several failures -- most of them silent -- in `dev/lint-python`. I don't understand how we haven't been bitten by these yet. Perhaps we've been lucky? Fixes include: * Fix how we compare versions. All the version checks currently in `master` silently fail with: ``` File "<string>", line 2 print(LooseVersion("""2.3.1""") >= LooseVersion("""2.4.0""")) ^ IndentationError: unexpected indent ``` Another problem is that `distutils.version` is undocumented and unsupported. * Fix some basic bugs. e.g. We have an incorrect reference to `$PYDOCSTYLEBUILD`, which doesn't exist, which was causing the doc style test to silently fail with: ``` ./dev/lint-python: line 193: --version: command not found ``` * Stop suppressing error output! It's hiding problems and serves no purpose here. `lint-python` is part of our CI build and is currently doing any combination of the following: silently failing; incorrectly skipping tests; incorrectly downloading libraries when a suitable library is already available. No. Lots of manual testing with `set -x` enabled. Closes apache#27910 from nchammas/SPARK-31153-lint-python. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

Backport of #27910 / 0ce5519. --- This PR cleans up several failures -- most of them silent -- in `dev/lint-python`. I don't understand how we haven't been bitten by these yet. Perhaps we've been lucky? Fixes include: * Fix how we compare versions. All the version checks currently in `master` silently fail with: ``` File "<string>", line 2 print(LooseVersion("""2.3.1""") >= LooseVersion("""2.4.0""")) ^ IndentationError: unexpected indent ``` Another problem is that `distutils.version` is undocumented and unsupported. * Fix some basic bugs. e.g. We have an incorrect reference to `$PYDOCSTYLEBUILD`, which doesn't exist, which was causing the doc style test to silently fail with: ``` ./dev/lint-python: line 193: --version: command not found ``` * Stop suppressing error output! It's hiding problems and serves no purpose here. `lint-python` is part of our CI build and is currently doing any combination of the following: silently failing; incorrectly skipping tests; incorrectly downloading libraries when a suitable library is already available. Closes #27917 from nchammas/SPARK-31153-lint-python-branch-3.0. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

### What changes were proposed in this pull request? As discovered here #27910 (comment), pydocstyle tests were not running anywhere (not on Jenkins; not on GitHub). ~This PR enables those tests.~ It also seems like a [large hill to climb](#27912 (comment)) to enable any meaningful checks, so we're going to just rip pydocstyle out for now. ### Why are the changes needed? Presumably, we defined those doc style tests because we care about whatever it is they enforce. Since we're not actually testing anything, though, it's better to clear the cruft. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Will check the GitHub workflow logs on this PR. Closes #27912 from nchammas/SPARK-31155-pydocstyle. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

### What changes were proposed in this pull request? As discovered here #27910 (comment), pydocstyle tests were not running anywhere (not on Jenkins; not on GitHub). ~This PR enables those tests.~ It also seems like a [large hill to climb](#27912 (comment)) to enable any meaningful checks, so we're going to just rip pydocstyle out for now. ### Why are the changes needed? Presumably, we defined those doc style tests because we care about whatever it is they enforce. Since we're not actually testing anything, though, it's better to clear the cruft. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Will check the GitHub workflow logs on this PR. Closes #27912 from nchammas/SPARK-31155-pydocstyle. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit b4748ca) Signed-off-by: HyukjinKwon <[email protected]>

shaneknapp · 2020-03-17T19:52:26Z

late "lgtm" from me. catching up now that i'm locked in my house for the foreseeable future. :)

…

On Sat, Mar 14, 2020 at 9:11 PM Hyukjin Kwon ***@***.***> wrote: @nchammas <https://github.com/nchammas>, it has conflicts with branch-3.0. Do you mind if I ask to make a backporting PR? We should port it back to branch-3.0 as 42c4838 <42c4838> laded to to branch-3.0 too. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27910 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMIHLEGY7DHF5HINOUFXMTRHRIOZANCNFSM4LINRG5Q> .

### What changes were proposed in this pull request? This PR cleans up several failures -- most of them silent -- in `dev/lint-python`. I don't understand how we haven't been bitten by these yet. Perhaps we've been lucky? Fixes include: * Fix how we compare versions. All the version checks currently in `master` silently fail with: ``` File "<string>", line 2 print(LooseVersion("""2.3.1""") >= LooseVersion("""2.4.0""")) ^ IndentationError: unexpected indent ``` Another problem is that `distutils.version` is undocumented and unsupported. * Fix some basic bugs. e.g. We have an incorrect reference to `$PYDOCSTYLEBUILD`, which doesn't exist, which was causing the doc style test to silently fail with: ``` ./dev/lint-python: line 193: --version: command not found ``` * Stop suppressing error output! It's hiding problems and serves no purpose here. ### Why are the changes needed? `lint-python` is part of our CI build and is currently doing any combination of the following: silently failing; incorrectly skipping tests; incorrectly downloading libraries when a suitable library is already available. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Lots of manual testing with `set -x` enabled. Closes apache#27910 from nchammas/SPARK-31153-lint-python. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

### What changes were proposed in this pull request? As discovered here apache#27910 (comment), pydocstyle tests were not running anywhere (not on Jenkins; not on GitHub). ~This PR enables those tests.~ It also seems like a [large hill to climb](apache#27912 (comment)) to enable any meaningful checks, so we're going to just rip pydocstyle out for now. ### Why are the changes needed? Presumably, we defined those doc style tests because we care about whatever it is they enforce. Since we're not actually testing anything, though, it's better to clear the cruft. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Will check the GitHub workflow logs on this PR. Closes apache#27912 from nchammas/SPARK-31155-pydocstyle. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

nchammas added 3 commits March 14, 2020 02:24

add function for doing minimum version check

26b01bf

use consistent VERSION naming + don't suppress errors!

a150f27

check expected versions using new function

51488cf

nchammas added 4 commits March 14, 2020 04:03

use vendored packaging lib

abe157e

restore flake8 version extraction

9b543b0

typo -> EXPECTED_PYDOCSTYLE

02184d3

quote consistently

5ad9396

nchammas mentioned this pull request Mar 14, 2020

[SPARK-31155] Remove pydocstyle tests #27912

Closed

HyukjinKwon reviewed Mar 15, 2020

View reviewed changes

HyukjinKwon approved these changes Mar 15, 2020

View reviewed changes

HyukjinKwon closed this in 0ce5519 Mar 15, 2020

nchammas deleted the SPARK-31153-lint-python branch March 15, 2020 04:16

nchammas mentioned this pull request Mar 15, 2020

[SPARK-31153][BUILD][3.0] Cleanup several failures in lint-python #27917

Closed

[SPARK-31153][BUILD] Cleanup several failures in lint-python #27910

[SPARK-31153][BUILD] Cleanup several failures in lint-python #27910

Uh oh!

Conversation

nchammas commented Mar 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Mar 14, 2020

Uh oh!

nchammas commented Mar 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Mar 14, 2020

Uh oh!

SparkQA commented Mar 14, 2020

Uh oh!

srowen commented Mar 14, 2020

Uh oh!

nchammas commented Mar 14, 2020

Uh oh!

SparkQA commented Mar 14, 2020

Uh oh!

HyukjinKwon Mar 15, 2020

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HyukjinKwon commented Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HyukjinKwon commented Mar 15, 2020

Uh oh!

shaneknapp commented Mar 17, 2020 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nchammas commented Mar 14, 2020 •

edited

Loading

nchammas commented Mar 14, 2020 •

edited

Loading

HyukjinKwon commented Mar 15, 2020 •

edited

Loading

HyukjinKwon commented Mar 15, 2020 •

edited

Loading