cephfs-top, qa: Remove unnecessary global statements in tests by tchaikov · Pull Request #62567 · ceph/ceph

tchaikov · 2025-03-30T03:54:35Z

Removes unused global statements to fix flake8 F824 errors.

Recent flake8 runs were failing with:

./tasks/radosgw_admin.py:330:5: F824 `global log` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:99:5: F824 `global incompat_paths` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:164:5: F824 `global backward_compat` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:165:5: F824 `global fast_shouldnt_skip` is unused: name is never assigned in scope

Since these variables are only being referenced and not assigned within their scopes, the global declarations are unnecessary and can be safely removed. This change:

Removes all flagged global statements
Fixes the failing flake8 checks in the CI pipeline
Maintains the original code behavior as variable references still work without the global keyword

The global keyword is only needed when assigning to global variables within a function scope, not when simply referencing them.

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins test classic perf Jenkins Job | Jenkins Job Definition
jenkins test crimson perf Jenkins Job | Jenkins Job Definition
jenkins test signed Jenkins Job | Jenkins Job Definition
jenkins test make check Jenkins Job | Jenkins Job Definition
jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
jenkins test submodules Jenkins Job | Jenkins Job Definition
jenkins test dashboard Jenkins Job | Jenkins Job Definition
jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
jenkins test api Jenkins Job | Jenkins Job Definition
jenkins test docs ReadTheDocs | Github Workflow Definition
jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
jenkins test windows Jenkins Job | Jenkins Job Definition
jenkins test rook e2e Jenkins Job | Jenkins Job Definition

Removes unused `global` statements from Python test files to fix flake8 F824 errors. Recent flake8 runs were failing with: ``` ./tasks/radosgw_admin.py:330:5: F824 `global log` is unused: name is never assigned in scope ./workunits/dencoder/test_readable.py:99:5: F824 `global incompat_paths` is unused: name is never assigned in scope ./workunits/dencoder/test_readable.py:164:5: F824 `global backward_compat` is unused: name is never assigned in scope ./workunits/dencoder/test_readable.py:165:5: F824 `global fast_shouldnt_skip` is unused: name is never assigned in scope ``` Since these variables are only being referenced and not assigned within their scopes, the `global` declarations are unnecessary and can be safely removed. This change: - Removes all flagged `global` statements - Fixes the failing flake8 checks in the CI pipeline - Maintains the original code behavior as variable references still work without the `global` keyword The `global` keyword is only needed when assigning to global variables within a function scope, not when simply referencing them. Signed-off-by: Kefu Chai <tchaikov@gmail.com>

tchaikov · 2025-03-30T03:57:02Z

for an instance of the test failure addressed by this change, see https://jenkins.ceph.com/job/ceph-pull-requests/154956/consoleFull#868021901e840cee4-f4a4-4183-81dd-42855615f2c1:

flake8: install_deps /home/jenkins-build/build/workspace/ceph-pull-requests/qa> python -I -m pip install flake8
flake8: freeze /home/jenkins-build/build/workspace/ceph-pull-requests/qa> python -m pip freeze --all
flake8: flake8==7.2.0,mccabe==0.7.0,pip==25.0.1,pycodestyle==2.13.0,pyflakes==3.3.0,setuptools==75.8.0,wheel==0.45.1
flake8: commands[0] /home/jenkins-build/build/workspace/ceph-pull-requests/qa> flake8 --select=F,E9 --exclude=venv,.tox
./tasks/radosgw_admin.py:330:5: F824 `global log` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:99:5: F824 `global incompat_paths` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:164:5: F824 `global backward_compat` is unused: name is never assigned in scope
./workunits/dencoder/test_readable.py:165:5: F824 `global fast_shouldnt_skip` is unused: name is never assigned in scope
flake8: exit 1 (3.86 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/qa> flake8 --select=F,E9 --exclude=venv,.tox pid=2309377
flake8: FAIL ✖ in 9.14 seconds
...
  flake8: FAIL code 1 (9.14=setup[5.28]+cmd[3.86] seconds)
  mypy: OK (32.21=setup[14.89]+cmd[17.31] seconds)
  deadsymlinks: OK (2.19=setup[2.04]+cmd[0.14] seconds)
  evaluation failed :( (43.85 seconds)
...
The following tests FAILED:
	 28 - run-tox-cephfs-top (Failed)
	289 - run-tox-qa (Failed)

Recent flake8 runs were failing with: ``` py3: flake8==7.2.0,mccabe==0.7.0,pip==25.0.1,pycodestyle==2.13.0,pyflakes==3.3.0,setuptools==75.8.0,wheel==0.45.1 py3: commands[0] /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/cephfs/top> flake8 --ignore=W503 --max-line-length=100 cephfs-top cephfs-top:344:9: F824 `global fs_list` is unused: name is never assigned in scope cephfs-top:466:13: F824 `global current_states` is unused: name is never assigned in scope cephfs-top:872:9: F824 `global metrics_dict` is unused: name is never assigned in scope cephfs-top:872:9: F824 `global current_states` is unused: name is never assigned in scope cephfs-top:911:9: F824 `global fs_list` is unused: name is never assigned in scope cephfs-top:981:9: F824 `global current_states` is unused: name is never assigned in scope cephfs-top:1126:13: F824 `global current_states` is unused: name is never assigned in scope py3: exit 1 (0.77 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/cephfs/top> flake8 --ignore=W503 --max-line-length=100 cephfs-top pid=2309605 py3: FAIL code 1 (8.15=setup[7.38]+cmd[0.77] seconds) evaluation failed :( (8.24 seconds) ``` Since these variables are only being referenced and not assigned within their scopes, the `global` declarations are unnecessary and can be safely removed. This change: - Removes all flagged `global` statements - Fixes the failing flake8 checks in the CI pipeline - Maintains the original code behavior as variable references still work without the `global` keyword The `global` keyword is only needed when assigning to global variables within a function scope, not when simply referencing them. Signed-off-by: Kefu Chai <tchaikov@gmail.com>

ronen-fr · 2025-03-30T09:59:28Z

LGTM (but I cannot approve a CephFS PR...)

tchaikov · 2025-03-31T00:38:52Z

LGTM (but I cannot approve a CephFS PR...)

@ronen-fr hi Ronen, thank you for your review and approval. i will wait for 2 more days before merging this change. it's urgent, because the CI tests in all PRs are now failing.

phlogistonjohn · 2025-03-31T13:57:34Z

I think you should feel free to merge as soon as you are ready as tests continue to fail without these changes.

cbodley

ack for rgw, thanks

cbodley · 2025-04-01T15:06:04Z

i believe this needs backports for reef:

The following tests FAILED:
28 - run-tox-cephfs-top (Failed)
34 - run-rbd-unit-tests-127.sh (Failed)
263 - run-tox-qa (Failed)

and squid:

The following tests FAILED:
28 - run-tox-cephfs-top (Failed)
273 - run-tox-qa (Failed)

edit: i'll prepare them

joscollin

Approved.

tchaikov added the tests label Mar 30, 2025

github-actions bot added the cephfs Ceph File System label Mar 30, 2025

tchaikov requested review from NitzanMordhai and joscollin March 30, 2025 04:05

tchaikov changed the title ~~qa: Remove unnecessary global statements in tests~~ cephfs-top, qa: Remove unnecessary global statements in tests Mar 30, 2025

This was referenced Mar 30, 2025

cmake: Allow building with fmt <= 11.1.4 #62564

Merged

fmt: bump up version to 11.1.4 #62568

Merged

ronen-fr approved these changes Mar 30, 2025

View reviewed changes

tchaikov mentioned this pull request Mar 31, 2025

neorados/test, googletest: adapt to googletest and bump up googletest submodule #62566

Merged

14 tasks

phlogistonjohn approved these changes Mar 31, 2025

View reviewed changes

cbodley approved these changes Mar 31, 2025

View reviewed changes

tchaikov merged commit 72f4cc5 into ceph:main Mar 31, 2025
15 of 16 checks passed

tchaikov deleted the qa-remove-unused-global branch March 31, 2025 15:04

This was referenced Apr 1, 2025

squid: cephfs-top, qa: Remove unnecessary global statements in tests #62605

Merged

reef: cephfs-top, qa: Remove unnecessary global statements in tests #62606

Merged

joscollin reviewed Apr 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cephfs-top, qa: Remove unnecessary global statements in tests#62567

cephfs-top, qa: Remove unnecessary global statements in tests#62567
tchaikov merged 2 commits intoceph:mainfrom
tchaikov:qa-remove-unused-global

tchaikov commented Mar 30, 2025 •

edited

Loading

Uh oh!

tchaikov commented Mar 30, 2025 •

edited

Loading

Uh oh!

ronen-fr commented Mar 30, 2025

Uh oh!

tchaikov commented Mar 31, 2025 •

edited

Loading

Uh oh!

phlogistonjohn commented Mar 31, 2025

Uh oh!

cbodley left a comment

Uh oh!

Uh oh!

cbodley commented Apr 1, 2025 •

edited

Loading

Uh oh!

joscollin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

tchaikov commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

tchaikov commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ronen-fr commented Mar 30, 2025

Uh oh!

tchaikov commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phlogistonjohn commented Mar 31, 2025

Uh oh!

cbodley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cbodley commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joscollin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tchaikov commented Mar 30, 2025 •

edited

Loading

tchaikov commented Mar 30, 2025 •

edited

Loading

tchaikov commented Mar 31, 2025 •

edited

Loading

cbodley commented Apr 1, 2025 •

edited

Loading