CI: format isort output for detection by azure pipelines #27334

leeyspaul · 2019-07-11T04:56:33Z

closes CI: Format isort output to be detected as Azure pipelines errors #27179
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

datapythonista · 2019-07-11T10:03:28Z

ci/code_checks.sh

@@ -109,7 +109,7 @@ if [[ -z "$CHECK" || "$CHECK" == "lint" ]]; then

    # Imports - Check formatting using isort see setup.cfg for settings
    MSG='Check import format using isort ' ; echo $MSG
-    isort --recursive --check-only pandas asv_bench
+    isort --recursive --check-only pandas asv_bench | awk -F ":" '##vso[task.logissue type=error;sourcepath=%(path)s]%(text)s'


Looks good, but we just want to change the format when the environment variable AZURE is set. You can see how this is done for flake8. Sorry it wasn't clear in the issue.

Can you also change the order of an import to see this failing in the CI please? So we can see it working before we merge. Thanks!

yes, thank you for being so helpful!

Does this still grab the return code from isort?

leeyspaul · 2019-07-12T04:43:53Z

@datapythonista sorry I'm a bit new to bash scripting. It's definitely catching the import error but it is ignoring the exception as shown here:

BrokenPipeError: [Errno 32] Broken pipe
Check import format using isort DONE
##[error]Bash exited with code '120'.```

fix isort formatting for awk fix isort format output test awk change fix isort output for azure remove comment fix deliberate import errors fix whitespace whitespace fix

leeyspaul · 2019-07-29T05:13:14Z

pinging for review! Spent some time on it to understand awk. @datapythonista thank you!

datapythonista

Thanks @leeyspaul, getting closer, still some comments.

ci/code_checks.sh

datapythonista · 2019-07-29T10:33:41Z

ci/code_checks.sh

@@ -109,9 +109,12 @@ if [[ -z "$CHECK" || "$CHECK" == "lint" ]]; then

    # Imports - Check formatting using isort see setup.cfg for settings
    MSG='Check import format using isort ' ; echo $MSG
-    isort --recursive --check-only pandas asv_bench
+    if [[ "$AZURE" == "true" ]]; then
+        isort --recursive --check-only pandas asv_bench | awk -F ':' '{print $0"\n" "##vso[task.logissue type=error;sourcepath=%(path)s]%(text)s"}'


I think the return code $? we are checking later to see if isort was successful or not, will actually be the one of awk with this, which will always be 0.

This means that if the imports are not correctly sorted, we won't detect them in the CI.

Can you start by swapping the order of two imports in pandas in this PR, so we know if this is working or not? We'll remove them once the PR is approved.

Yes, I will push up the badly sorted imports

here is the output! Check import format using isort ERROR: /home/vsts/work/1/s/pandas/plotting/_matplotlib/core.py Imports are incorrectly sorted. ##[error]%(path)s(,): error : %(text)s Check import format using isort DONE ##[error]Bash exited with code '1'. ##[section]Finishing: Linting

ci/code_checks.sh

datapythonista · 2019-07-29T13:39:47Z

Seems like awk is keeping the exit status of isort, I guess it's because of the set -o pipefail in the function at the top of the function, we'll have to fix that.

But the first things would probably be fixing the output, from:

ERROR: /home/vsts/work/1/s/pandas/plotting/_matplotlib/core.py 
##[error]%(path)s(,): error : %(text)s

to

##vso[task.logissue type=error;sourcepath=/home/vsts/work/1/s/pandas/plotting/_matplotlib/core.py] Imports are incorrectly sorted.

Thanks!

fix output test azuredev output fix awk output to show path reformat output to incorporate error msg format output to catch path and error name

leeyspaul · 2019-08-03T16:37:45Z

ok! I think I'm getting closer now. @datapythonista could you check out the failure? I've pasted below what it's returning for ease.

Check import format using isort
##[error]pandas/plotting/_matplotlib/core.py(,): error :   Imports are incorrectly sorted.Check import format using isort DONE
##[error]Bash exited with code '1'.
##[section]Finishing: Linting

I added the ORS as the error message was moving to a new line with print. If there is a better way to do this do let me know, thank you!

datapythonista · 2019-08-03T16:51:04Z

is there a reason to not use the ##vso code I suggested? I think is what we use in the other cases, if there is no reason better to keep consistent.

leeyspaul · 2019-08-03T17:00:57Z

is there a reason to not use the ##vso code I suggested? I think is what we use in the other cases, if there is no reason better to keep consistent.

er, I didn't mean to remove anything. I'm not too familiar on vso codes but I read up on logging here: https://github.com/microsoft/azure-pipelines-tasks/blob/master/docs/authoring/commands.md

Could you clarify what you mean by the vso code and I will change it back, or point me to the docs. Thank you!

datapythonista · 2019-08-03T17:04:50Z

I refer to what is suggested in #27334 (comment) instead of using ##[error]

leeyspaul · 2019-08-03T17:10:19Z

ah right. So oddly it seems to be getting converted from ##vso to ##[error] though the awk line I've used is isort --recursive --check-only pandas asv_bench | awk '{ORS=""} {print "##vso[task.logissue type=error;sourcepath="$2";]"} {$1=$2=""; print $0}'.

I might be missing something obvious here

datapythonista · 2019-08-06T12:58:32Z

ah, sorry, didn't see in the code you were already using the ##vso. I guess that's some azure magic then, but will be applied to all the other commands too, so it's ok.

I'd prefer to remove the awk and leave the original call when not using azure. Will be misleading if the errors are different when people call isort directly than with our script I think.

And the thing that concerns me is the set -o pipefail inside the if at the beginning. If eventually we change that code, your code will fail. And that doesn't seem right, since both things are unrelated, and shouldn't be using side-effects of each other. I'd start by adding set +o pipefail after the grep in line 39. That should already be there, will avoid the side effects, and will make your code fail. After that, you can wrap your code with the same set -o pipefail and set +o pipefail, and that should fix it again.

Does this make sense?

leeyspaul · 2019-08-06T16:43:10Z

ah, sorry, didn't see in the code you were already using the ##vso. I guess that's some azure magic then, but will be applied to all the other commands too, so it's ok.

I'd prefer to remove the awk and leave the original call when not using azure. Will be misleading if the errors are different when people call isort directly than with our script I think.

And the thing that concerns me is the set -o pipefail inside the if at the beginning. If eventually we change that code, your code will fail. And that doesn't seem right, since both things are unrelated, and shouldn't be using side-effects of each other. I'd start by adding set +o pipefail after the grep in line 39. That should already be there, will avoid the side effects, and will make your code fail. After that, you can wrap your code with the same set -o pipefail and set +o pipefail, and that should fix it again.

Does this make sense?

I think so, I'll take a look at this tonight! Thanks for the info. : ) (will ping if I can't get it!)

WillAyd · 2019-09-05T17:30:45Z

@leeyspaul can you merge master again? Old builds are no longer available

WillAyd · 2019-09-13T01:51:04Z

@leeyspaul is this still active?

leeyspaul · 2019-09-13T01:53:15Z

@WillAyd hey yeah, sorry. Had gotten caught up with work. I will work on this this weekend.

WillAyd · 2019-09-13T20:14:52Z

Great! No worries on timing - not in any rush. Just wanted to make sure this was something that you were still looking at, so thanks for confirming

jreback · 2019-10-06T23:52:06Z

@leeyspaul can you merge master and update

jbrockmendel · 2019-10-28T18:16:42Z

@leeyspaul can you rebase

alimcmaster1 · 2019-11-02T17:27:26Z

If we dont hear from @leeyspaul I can take this on

leeyspaul · 2019-11-04T03:37:12Z

@alimcmaster1 feel free to take over. I don't think I'll be able to complete this any time soon. Thanks!

alimcmaster1 · 2019-11-16T03:00:10Z

Restarted this over here: #29654

Closing this for now

datapythonista · 2019-11-16T03:03:08Z

I'm moving the Checks build to actions, and I think it only supports the ##[error] formatting, not the one with file, line number...

alimcmaster1 · 2019-11-16T04:10:54Z

Ahh so there probably isn’t much value in this at all? I can close my PR if so

datapythonista · 2019-11-16T04:17:02Z

There is surely value on letting the CI know what's an error. Just the format will probably be different.

format isort output

d8628d5

leeyspaul changed the title ~~format isort output~~ CI: Format isort output to be detected as Azure pipelines errors Jul 11, 2019

leeyspaul changed the title ~~CI: Format isort output to be detected as Azure pipelines errors~~ CI: format isort output for detection by azure pipeliens Jul 11, 2019

leeyspaul changed the title ~~CI: format isort output for detection by azure pipeliens~~ CI: format isort output for detection by azure pipelines Jul 11, 2019

datapythonista reviewed Jul 11, 2019

View reviewed changes

datapythonista added CI Continuous Integration Error Reporting Incorrect or improved errors from pandas labels Jul 11, 2019

leeyspaul added 2 commits July 11, 2019 20:37

refactor isort formatting

c7a64c3

intentional misordering of imports to test ci failure

7762fc1

change isort formatting

d2f9874

fix isort formatting for awk fix isort format output test awk change fix isort output for azure remove comment fix deliberate import errors fix whitespace whitespace fix

datapythonista requested changes Jul 29, 2019

View reviewed changes

leeyspaul added 3 commits August 2, 2019 22:03

address pr comments and deliberate bad imports ordering

35861cf

fix output test azuredev output fix awk output to show path reformat output to incorporate error msg format output to catch path and error name

fix error output substr

0316923

formatting output to only have error msg

1368f87

alimcmaster1 mentioned this pull request Nov 16, 2019

CI: Format isort output for azure #29654

Merged

4 tasks

alimcmaster1 closed this Nov 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: format isort output for detection by azure pipelines #27334

CI: format isort output for detection by azure pipelines #27334

leeyspaul commented Jul 11, 2019 •

edited

Loading

datapythonista Jul 11, 2019

leeyspaul Jul 11, 2019

WillAyd Jul 11, 2019

leeyspaul commented Jul 12, 2019

leeyspaul commented Jul 29, 2019

datapythonista left a comment

datapythonista Jul 29, 2019

leeyspaul Jul 29, 2019

leeyspaul Jul 29, 2019

datapythonista commented Jul 29, 2019

leeyspaul commented Aug 3, 2019

datapythonista commented Aug 3, 2019

leeyspaul commented Aug 3, 2019

datapythonista commented Aug 3, 2019

leeyspaul commented Aug 3, 2019

datapythonista commented Aug 6, 2019

leeyspaul commented Aug 6, 2019 •

edited

Loading

WillAyd commented Sep 5, 2019

WillAyd commented Sep 13, 2019

leeyspaul commented Sep 13, 2019

WillAyd commented Sep 13, 2019

jreback commented Oct 6, 2019

jbrockmendel commented Oct 28, 2019

alimcmaster1 commented Nov 2, 2019 •

edited

Loading

leeyspaul commented Nov 4, 2019 •

edited

Loading

alimcmaster1 commented Nov 16, 2019

datapythonista commented Nov 16, 2019

alimcmaster1 commented Nov 16, 2019

datapythonista commented Nov 16, 2019

CI: format isort output for detection by azure pipelines #27334

CI: format isort output for detection by azure pipelines #27334

Conversation

leeyspaul commented Jul 11, 2019 • edited Loading

datapythonista Jul 11, 2019

Choose a reason for hiding this comment

leeyspaul Jul 11, 2019

Choose a reason for hiding this comment

WillAyd Jul 11, 2019

Choose a reason for hiding this comment

leeyspaul commented Jul 12, 2019

leeyspaul commented Jul 29, 2019

datapythonista left a comment

Choose a reason for hiding this comment

datapythonista Jul 29, 2019

Choose a reason for hiding this comment

leeyspaul Jul 29, 2019

Choose a reason for hiding this comment

leeyspaul Jul 29, 2019

Choose a reason for hiding this comment

datapythonista commented Jul 29, 2019

leeyspaul commented Aug 3, 2019

datapythonista commented Aug 3, 2019

leeyspaul commented Aug 3, 2019

datapythonista commented Aug 3, 2019

leeyspaul commented Aug 3, 2019

datapythonista commented Aug 6, 2019

leeyspaul commented Aug 6, 2019 • edited Loading

WillAyd commented Sep 5, 2019

WillAyd commented Sep 13, 2019

leeyspaul commented Sep 13, 2019

WillAyd commented Sep 13, 2019

jreback commented Oct 6, 2019

jbrockmendel commented Oct 28, 2019

alimcmaster1 commented Nov 2, 2019 • edited Loading

leeyspaul commented Nov 4, 2019 • edited Loading

alimcmaster1 commented Nov 16, 2019

datapythonista commented Nov 16, 2019

alimcmaster1 commented Nov 16, 2019

datapythonista commented Nov 16, 2019

leeyspaul commented Jul 11, 2019 •

edited

Loading

leeyspaul commented Aug 6, 2019 •

edited

Loading

alimcmaster1 commented Nov 2, 2019 •

edited

Loading

leeyspaul commented Nov 4, 2019 •

edited

Loading