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.

Sign up for GitHub

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

Jump to bottom

Print log when formatter ecosystem checks fail #6187

Merged
merged 2 commits into from
Jul 31, 2023
Merged

Print log when formatter ecosystem checks fail #6187

merged 2 commits into from
Jul 31, 2023

Conversation

konstin
Copy link
Member

@konstin konstin commented Jul 31, 2023 •
edited
Loading

Summary Print the errors when the formatter ecosystem checks failed. Im not happy that we current collect the log in the first place, but this is the less invasive change and we need it to unblock reviewing #6152.

Test Plan https://github.com/astral-sh/ruff/actions/runs/5713112075/job/15477879403?pr=6188

@konstin konstin marked this pull request as ready for review July 31, 2023 09:23
@konstin konstin added internal An internal refactor or improvement formatter Related to the formatter labels Jul 31, 2023
@github-actions
Copy link
Contributor

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.2±0.27ms     4.0 MB/sec    1.08     11.0±0.45ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.0±0.10ms     8.3 MB/sec    1.05      2.1±0.09ms     7.8 MB/sec
formatter/numpy/globals.py                 1.00    224.9±7.63µs    13.1 MB/sec    1.06   237.5±13.93µs    12.4 MB/sec
formatter/pydantic/types.py                1.00      4.4±0.23ms     5.7 MB/sec    1.05      4.7±0.20ms     5.5 MB/sec
linter/all-rules/large/dataset.py          1.03     14.4±0.73ms     2.8 MB/sec    1.00     14.0±0.55ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.14ms     4.8 MB/sec    1.01      3.5±0.15ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.00   477.9±19.07µs     6.2 MB/sec    1.06   506.3±79.82µs     5.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.1±0.18ms     4.2 MB/sec    1.04      6.3±0.22ms     4.0 MB/sec
linter/default-rules/large/dataset.py      1.03      7.7±0.29ms     5.3 MB/sec    1.00      7.4±0.20ms     5.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1562.9±68.20µs    10.7 MB/sec    1.00  1558.5±56.06µs    10.7 MB/sec
linter/default-rules/numpy/globals.py      1.03    177.9±6.06µs    16.6 MB/sec    1.00    173.2±6.41µs    17.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.3±0.09ms     7.6 MB/sec    1.00      3.4±0.16ms     7.6 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     12.5±0.46ms     3.2 MB/sec    1.01     12.7±0.46ms     3.2 MB/sec
formatter/numpy/ctypeslib.py               1.03      2.4±0.08ms     6.8 MB/sec    1.00      2.4±0.10ms     7.0 MB/sec
formatter/numpy/globals.py                 1.00   267.3±20.64µs    11.0 MB/sec    1.03   275.1±19.53µs    10.7 MB/sec
formatter/pydantic/types.py                1.00      5.3±0.23ms     4.8 MB/sec    1.00      5.3±0.19ms     4.8 MB/sec
linter/all-rules/large/dataset.py          1.03     17.2±0.46ms     2.4 MB/sec    1.00     16.7±0.44ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.4±0.22ms     3.7 MB/sec    1.00      4.4±0.19ms     3.8 MB/sec
linter/all-rules/numpy/globals.py          1.04   561.3±40.88µs     5.3 MB/sec    1.00   540.0±24.42µs     5.5 MB/sec
linter/all-rules/pydantic/types.py         1.01      7.7±0.23ms     3.3 MB/sec    1.00      7.6±0.24ms     3.4 MB/sec
linter/default-rules/large/dataset.py      1.04      9.4±0.32ms     4.3 MB/sec    1.00      9.1±0.33ms     4.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1895.6±61.52µs     8.8 MB/sec    1.00  1880.3±64.39µs     8.9 MB/sec
linter/default-rules/numpy/globals.py      1.00   212.1±14.42µs    13.9 MB/sec    1.00    211.8±8.89µs    13.9 MB/sec
linter/default-rules/pydantic/types.py     1.03      4.1±0.38ms     6.1 MB/sec    1.00      4.0±0.21ms     6.3 MB/sec

MichaReiser
MichaReiser approved these changes Jul 31, 2023
View reviewed changes
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Extending the summary with some details would be helpful to ease reviewing.

It took me a bit of time to understand what I'm reviewing

  • How did you test the change?
  • we need it to unblock my reviews. What is it and what reviews?

@konstin
Copy link
Member Author

konstin commented Jul 31, 2023

I expected this to fail because it fails for me locally and now i'm confused. See #6188 for a proper failure

konstin added a commit that referenced this pull request Jul 31, 2023
@konstin
Log configuration in ruff_dev
418d8c4 
**Summary** This includes two changes:
 * Allow settings `-v` in `ruff_dev`, using the `ruff_cli` code
 * `debug!` which configuration strategy was used

This is a byproduct of debugging #6187.

**Test Plan** n/a
@konstin konstin mentioned this pull request Jul 31, 2023
Merged
@konstin
Copy link
Member Author

konstin commented Jul 31, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@konstin
Copy link
Member Author

konstin commented Jul 31, 2023

This failed for me locally because i had target/progress_projects symlinked and apparently ignore::WalkBuilder resolves symlinks in the path, so it only reads ruff/.gitignore if the directory isn't symlinked.

Debugging this spawned #6193.

@konstin
Copy link
Member Author

konstin commented Jul 31, 2023

Updated the summary

@konstin konstin merged commit a540933 into main Jul 31, 2023
@konstin konstin deleted the formatter_ecosystem_ci_debugging branch July 31, 2023 12:45
This was referenced Jul 31, 2023
Merged
Merged
@davidszotten
Copy link
Contributor

this is cool. do you think we could get it to print the line containing the syntax error (ideally source + ruff output)? atm i still need to re-run locally to find the actual issue (unless i'm missing a trick here?)

@konstin
Copy link
Member Author

konstin commented Jul 31, 2023

Unfortunately this is hard with our parser (it's not really meant for error recovery). I normally take the filename and let ruff_shrinking (documented in the ruff_python_formatter readme) give me a minimized example and then work on that

konstin added a commit that referenced this pull request Jul 31, 2023
@konstin
Log configuration in ruff_dev
e0a9a06 
**Summary** This includes two changes:
 * Allow settings `-v` in `ruff_dev`, using the `ruff_cli` code
 * `debug!` which configuration strategy was used

This is a byproduct of debugging #6187.

**Test Plan** n/a
konstin added a commit that referenced this pull request Jul 31, 2023
@konstin
Log configuration in ruff_dev (#6193)
e52b636 
**Summary** This includes two changes:
 * Allow setting `-v` in `ruff_dev`, using the `ruff_cli` implementation
 * `debug!` which ruff configuration strategy was used

This is a byproduct of debugging #6187.

**Test Plan** n/a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees
No one assigned
Labels
formatter Related to the formatter internal An internal refactor or improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@konstin @davidszotten @MichaReiser