Skip to content

improvement(treewide): Use ruff format instead of autopep8 #11291

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pehala
Copy link
Contributor

@pehala pehala commented Jun 27, 2025

Ruff is a superior formatting tool, in my opinion, and we already use it. We also use it in dtest.
If we want to cut down on number of changes we can ignore quotes (or extract them into followup PR), but I still think unifying them make sense. Given how many lines were changed, we can also delay this PR for later, as it will probably make conflicts in existing PRs.

  • All changes were generated automatically
  • Remove autopep8

Backporting to limit the future problems with backports

Testing

  • None

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

@fruch
Copy link
Contributor

fruch commented Jun 27, 2025

I suggested it when we introduced ruff

Since there were too many changes to the code we didn't roll it out, cause it's gonna break every single backport to branches that don't have it.

2024.1 and v15, we don't have ruff yet...

I think we'll need to wait a bit more

@pehala
Copy link
Contributor Author

pehala commented Jun 27, 2025

I suggested it when we introduced ruff

Since there were too many changes to the code we didn't roll it out, cause it's gonna break every single backport to branches that don't have it.

2024.1 and v15, we don't have ruff yet...

I think we'll need to wait a bit more

I will leave the call up to you, but 2024.1 should be not supported anymore, since 2025.1 and
regarding V15, I do not know the deprecation date but I am willing to introduce ruff there as well, via custom backport, if we wanted to give it a try

Copy link
Contributor

@vponomaryov vponomaryov left a comment

Choose a reason for hiding this comment

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

I like how fast is the ruff formatter.

But this is not acceptable:

from

        self.reconfigure_keyspaces_to_use_network_topology_strategy(
            keyspaces=system_keyspaces + ["keyspace1"],
            replication_factors={dc: len(status[dc].keys()) for dc in status}
        )

to

        self.reconfigure_keyspaces_to_use_network_topology_strategy(keyspaces=system_keyspaces + ["keyspace1"], replication_factors={dc: len(status[dc].keys()) for dc in status})

178 symbols in one line ^

Need to update the pyproject.toml config to match the value that is set for autopep8 config - 120 symbols per line limit.
Currently it is set to be 240 >.<

Reason:
I belong to group of people who use split screen in IDE and GitHub.
Using 15.6" display on laptop and keeping readable font size in (my) best case one line fits 112 symbols.
Small count of lines to be <119 is kind of ok.

@pehala
Copy link
Contributor Author

pehala commented Jun 27, 2025

Need to update the pyproject.toml config to match the value that is set for autopep8 config - 120 symbols per line limit.
Currently it is set to be 240 >.<

I am all for changing the line limit :)

Ruff is a superior formatting tool and we already have it
Lower the line length to 120
Same as we do in dtest
@pehala
Copy link
Contributor Author

pehala commented Jun 27, 2025

Need to update the pyproject.toml config to match the value that is set for autopep8 config - 120 symbols per line limit.
Currently it is set to be 240 >.<

changed

@fruch
Copy link
Contributor

fruch commented Jun 28, 2025

I suggested it when we introduced ruff
Since there were too many changes to the code we didn't roll it out, cause it's gonna break every single backport to branches that don't have it.
2024.1 and v15, we don't have ruff yet...
I think we'll need to wait a bit more

I will leave the call up to you, but 2024.1 should be not supported anymore, since 2025.1 and regarding V15, I do not know the deprecation date but I am willing to introduce ruff there as well, via custom backport, if we wanted to give it a try

there still 6 months for 2024.1 to be supported, and we don't have a due date deprecating V15.
if you want to create PRs for each with introducing ruff in, then I'm o.k. with it.

otherwise it would break every single back port we'll need to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants