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

[Epic][CI] Upgrade flake8 to version 7.1.1 #47991

Open
4 of 25 tasks
MortalHappiness opened this issue Oct 11, 2024 · 4 comments
Open
4 of 25 tasks

[Epic][CI] Upgrade flake8 to version 7.1.1 #47991

MortalHappiness opened this issue Oct 11, 2024 · 4 comments
Assignees
Labels
bug Something that is supposed to be working; but isn't ci-test enhancement Request for new feature and/or capability

Comments

@MortalHappiness
Copy link
Member

MortalHappiness commented Oct 11, 2024

What happened + What you expected to happen

While running the pre-commit hook of flake8, the following error occurs if Python version is 3.12. It's because the version of flake8 is too old.

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

Traceback (most recent call last):
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/bin/flake8", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/main/cli.py", line 22, in main
    app.run(argv)
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 363, in run
    self._run(argv)
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 350, in _run
    self.initialize(argv)
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 330, in initialize
    self.find_plugins(config_finder)
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 153, in find_plugins
    self.check_plugins = plugin_manager.Checkers(local_plugins.extension)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/plugins/manager.py", line 356, in __init__
    self.manager = PluginManager(
                   ^^^^^^^^^^^^^^
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/plugins/manager.py", line 238, in __init__
    self._load_entrypoint_plugins()
  File "/home/mortalhappiness/.cache/pre-commit/repo785be_6i/py_env-python3.12/lib/python3.12/site-packages/flake8/plugins/manager.py", line 254, in _load_entrypoint_plugins
    eps = importlib_metadata.entry_points().get(self.namespace, ())
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'EntryPoints' object has no attribute 'get'

Versions / Dependencies

Python 3.12

Reproduction script

If python version is 3.9 -> no error

If python version is 3.12 -> has error

Upgrade the following packages to the newest version solves this issue:

  • flake8==7.1.1
  • flake8-comprehensions==3.15.0
  • flake8-quotes==3.4.0
  • flake8-bugbear==24.8.19

Issue Severity

Low: It annoys or frustrates me.

flake8 upgrade subtasks tracking:

Thanks for @CheyuWu 's comment here #48022 (comment)

pycodestyle

pyflakes

flake8-bugbear

https://github.com/PyCQA/flake8-bugbear

flake8-comprehensions

https://github.com/adamchainz/flake8-comprehensions

CI upgrade

Cleanup

Notes for contributors

  • Use the following command to filter files that need to be fixed for single rule:
    flake8 --select E721 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
    
    Replace E721 with the rule that you want to fix.
  • Please note that the pre-commit hook still need to be run, but flake8 will fail, preventing you from committing. The temporary workaround is as follows:
    1. Perform a normal git add and commit.
    2. At this point, the pre-commit hook will be executed. Ideally, only flake8 should fail, and everything else should pass.
    3. Use git commit -n to bypass the pre-commit hook and commit.
    4. Use git commit --amend --no-edit --signoff to sign off the commit from the previous step.
@MortalHappiness MortalHappiness added bug Something that is supposed to be working; but isn't ci-test labels Oct 11, 2024
@CheyuWu
Copy link

CheyuWu commented Oct 11, 2024

@MortalHappiness I'd like to help with this!

@MortalHappiness MortalHappiness self-assigned this Oct 15, 2024
@MortalHappiness MortalHappiness changed the title [CI] flake8 version is too old to be compatible with Python 3.12 [Epic][CI] Upgrade flake8 to version 7.1.1 Oct 15, 2024
@MortalHappiness MortalHappiness added the enhancement Request for new feature and/or capability label Oct 15, 2024
This was referenced Oct 17, 2024
pcmoritz pushed a commit that referenced this issue Oct 23, 2024
<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See #47991 
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs : 

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)



## Related issue number
Closes #48059 
<!-- For example: "Closes #1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: LeoLiao123 <leoyeepaa@gmail.com>
@jjyao
Copy link
Collaborator

jjyao commented Oct 30, 2024

@MortalHappiness we should explore replacing flake8 with https://github.com/astral-sh/ruff if we want to do the upgrade.

@MortalHappiness
Copy link
Member Author

@jjyao Do you mean to replace flake8 completely with ruff?

@jjyao
Copy link
Collaborator

jjyao commented Oct 30, 2024

Yes. Reasoning is that ruff is drop-in replacement for flake8 so upgrading flake8 and switching to ruff should require the similar amount of efforts.

Jay-ju pushed a commit to Jay-ju/ray that referenced this issue Nov 5, 2024
…oject#48188)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See ray-project#47991 
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs : 

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)



## Related issue number
Closes ray-project#48059 
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: LeoLiao123 <leoyeepaa@gmail.com>
JP-sDEV pushed a commit to JP-sDEV/ray that referenced this issue Nov 14, 2024
…oject#48188)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See ray-project#47991 
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs : 

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)



## Related issue number
Closes ray-project#48059 
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: LeoLiao123 <leoyeepaa@gmail.com>
mohitjain2504 pushed a commit to mohitjain2504/ray that referenced this issue Nov 15, 2024
…oject#48188)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See ray-project#47991
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs :

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)

## Related issue number
Closes ray-project#48059
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: LeoLiao123 <leoyeepaa@gmail.com>
Signed-off-by: mohitjain2504 <mohit.jain@dream11.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to be working; but isn't ci-test enhancement Request for new feature and/or capability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants