mgr/volume: Python flake warning fixes. #52208

manishym · 2023-06-27T09:47:26Z

Each commit fixes one tracker issue.

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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

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 retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows

* Two lines between class definitions. * Fixes: https://tracker.ceph.com/issues/51379 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51386 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51388 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51389 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51394 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51395 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51397 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51399 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51404 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

Fixes: https://tracker.ceph.com/issues/51405 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

dparmar18 · 2023-07-03T13:16:40Z

I had went through the PR last week and the changes look good to me

vshankar · 2023-07-13T12:29:25Z

@rishabh-d-dave PTAL.

rishabh-d-dave

The changes look fine.

But I am confused how did you find this issues? I mean what command did you run to get these? I am asking because I see many more such issues when I run flake8 command on top the PR branch -

flake8 --exclude=venv,.tox src/pybind/mgr/volumes/fs | wc -l
786

For Python code in qa/ we run flake8 this way - flake8 --select=F,E9 --exclude=venv,.tox qa/. Running this command suggests 21 issues -

flake8 --select=F,E9 --exclude=venv,.tox src/pybind/mgr/volumes/fs | wc -l
21

Ah, I think you fixed only the issues mentioned in the tickets. Sadly these tickets don't mention what parameters were pased to flake8/pep. I would recommend running the above command and fixing the rest of issues as well.

rishabh-d-dave · 2023-07-19T16:04:47Z

src/pybind/mgr/volumes/fs/volume.py

-    ### volume operations -- create, rm, ls
+    # volume operations -- create, rm, ls


Multiple #s are perhaps deliberately added so that this comment is more easily apparent to readers.

The reason single # with a space is recommended in pep8 is because someone might think

################################################################ # volume operations -- create, rm, ls # ################################################################

is more emphasizing than

### volume operations -- create, rm, ls.

Again style guides are just that, guides. If we don't agree, I can close the PR.

If former is accepted by pep8/flake, let's switch to it instead?

rishabh-d-dave · 2023-07-19T16:05:02Z

src/pybind/mgr/volumes/fs/volume.py

-    ### subvolume operations
+    # subvolume operations


rishabh-d-dave · 2023-07-19T16:06:20Z

src/pybind/mgr/volumes/fs/volume.py

-    ### subvolume snapshot
+    # subvolume snapshot


rishabh-d-dave · 2023-07-19T16:07:49Z

src/pybind/mgr/volumes/fs/volume.py

-    ### group snapshot
+    # group snapshot


rishabh-d-dave · 2023-07-19T16:14:11Z

src/pybind/mgr/volumes/fs/operations/template.py

-    CREATE                = 'create'
-    REMOVE                = 'rm'
-    REMOVE_FORCE          = 'rm-force'
-    PIN                   = 'pin'
-    LIST                  = 'ls'
-    GETPATH               = 'getpath'
-    INFO                  = 'info'
-    RESIZE                = 'resize'
-    SNAP_CREATE           = 'snap-create'
-    SNAP_REMOVE           = 'snap-rm'
-    SNAP_LIST             = 'snap-ls'
-    SNAP_INFO             = 'snap-info'
-    SNAP_PROTECT          = 'snap-protect'
-    SNAP_UNPROTECT        = 'snap-unprotect'
-    CLONE_SOURCE          = 'clone-source'
-    CLONE_CREATE          = 'clone-create'
-    CLONE_STATUS          = 'clone-status'
-    CLONE_CANCEL          = 'clone-cancel'
-    CLONE_INTERNAL        = 'clone_internal'
-    ALLOW_ACCESS          = 'allow-access'
-    DENY_ACCESS           = 'deny-access'
-    AUTH_LIST             = 'auth-list'
-    EVICT                 = 'evict'
-    USER_METADATA_SET     = 'user-metadata-set'
-    USER_METADATA_GET     = 'user-metadata-get'
-    USER_METADATA_LIST    = 'user-metadata-ls'
-    USER_METADATA_REMOVE  = 'user-metadata-rm'
-    SNAP_METADATA_SET     = 'snap-metadata-set'
-    SNAP_METADATA_GET     = 'snap-metadata-get'
-    SNAP_METADATA_LIST    = 'snap-metadata-ls'
-    SNAP_METADATA_REMOVE  = 'snap-metadata-rm'
+    CREATE = 'create'
+    REMOVE = 'rm'
+    REMOVE_FORCE = 'rm-force'


This formatting was deliberately this way for sake of readability.

The reason pep8 style guide does not recommend aligning = signs is that sooner or later, you will get a longer variable, which causes you to re-align all those = symbols. It is subjective if aligned = is more readable or not. Given that we have a tracker issue and we want it to be fixed, which means we want to adhere to pep8, I have fixed it. If not we can close the PR.

rishabh-d-dave · 2023-07-21T08:22:54Z

@manishym Let me know when this PR is ready for review again.

manishym · 2023-08-14T12:15:12Z

@rishabh-d-dave Can you take a look. I have fixed other formatting errors as well.

github-actions · 2023-11-01T15:01:46Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

gregsfortytwo · 2023-11-21T16:11:55Z

ping @rishabh-d-dave

github-actions · 2023-11-29T19:28:39Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

rishabh-d-dave · 2023-11-29T19:41:30Z

@manishym Please ping me when this PR is ready.

(If possible, please ping me on Slack/IRC/G-chat. Since GH notifications are always too many, usually most of us use this approach)

github-actions · 2024-01-28T21:01:16Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2024-02-27T22:01:22Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

tchaikov · 2024-02-28T01:42:34Z

the last commit does not have "Signed-off-by" tag. and please resolve the merge conflicts.

github-actions · 2024-04-28T02:12:47Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

manishym added 10 commits June 27, 2023 15:08

mgr/volumes/exeption: Fix pep8 style warning

1bd03ec

* Two lines between class definitions. * Fixes: https://tracker.ceph.com/issues/51379 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mgr/fs/volumes: Fix python style warning

badc974

Fixes: https://tracker.ceph.com/issues/51386 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

fs/index: fix python style warnings

504ad23

Fixes: https://tracker.ceph.com/issues/51388 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mgr/volumes: Fix python style warnings

d2983f8

Fixes: https://tracker.ceph.com/issues/51389 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mgr/volume: Fix python style warnings

ab14566

Fixes: https://tracker.ceph.com/issues/51394 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mrg/volume: Fix python style warnings

4cd48f2

Fixes: https://tracker.ceph.com/issues/51395 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mgr/volume: Fix python style warnings

030ab9b

Fixes: https://tracker.ceph.com/issues/51397 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mgr/volume: Fix python style warnings

612ad33

Fixes: https://tracker.ceph.com/issues/51399 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mgr/volume/operations: python style warning fix

d79d13b

Fixes: https://tracker.ceph.com/issues/51404 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

mgr/volumes/operations: Python style warning fixes

d977e14

Fixes: https://tracker.ceph.com/issues/51405 Signed-off-by: Manish M Yathnalli <myathnal@redhat.com>

github-actions bot added cephfs Ceph File System pybind labels Jun 27, 2023

manishym requested review from kotreshhr, vshankar and rishabh-d-dave June 27, 2023 09:49

dparmar18 requested a review from a team June 28, 2023 11:57

vshankar assigned rishabh-d-dave Jul 13, 2023

rishabh-d-dave reviewed Jul 19, 2023

View reviewed changes

vshankar requested a review from rishabh-d-dave August 1, 2023 06:26

Python formatting fixes

201b14d

github-actions bot added the stale label Nov 1, 2023

github-actions bot removed the stale label Nov 21, 2023

github-actions bot added the needs-rebase label Nov 29, 2023

github-actions bot added the stale label Jan 28, 2024

github-actions bot closed this Feb 27, 2024

tchaikov reopened this Feb 28, 2024

github-actions bot removed the stale label Feb 28, 2024

github-actions bot added the stale label Apr 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mgr/volume: Python flake warning fixes. #52208

mgr/volume: Python flake warning fixes. #52208

manishym commented Jun 27, 2023 •

edited

dparmar18 commented Jul 3, 2023

vshankar commented Jul 13, 2023

rishabh-d-dave left a comment

rishabh-d-dave Jul 19, 2023

manishym Jul 20, 2023 •

edited

rishabh-d-dave Jul 20, 2023

rishabh-d-dave Jul 19, 2023

rishabh-d-dave Jul 19, 2023

rishabh-d-dave Jul 19, 2023

rishabh-d-dave Jul 19, 2023

manishym Jul 20, 2023

rishabh-d-dave commented Jul 21, 2023

manishym commented Aug 14, 2023

github-actions bot commented Nov 1, 2023

gregsfortytwo commented Nov 21, 2023

github-actions bot commented Nov 29, 2023

rishabh-d-dave commented Nov 29, 2023 •

edited

github-actions bot commented Jan 28, 2024

github-actions bot commented Feb 27, 2024

tchaikov commented Feb 28, 2024

github-actions bot commented Apr 28, 2024

		### volume operations -- create, rm, ls
		# volume operations -- create, rm, ls

mgr/volume: Python flake warning fixes. #52208

Are you sure you want to change the base?

mgr/volume: Python flake warning fixes. #52208

Conversation

manishym commented Jun 27, 2023 • edited

Contribution Guidelines

Checklist

dparmar18 commented Jul 3, 2023

vshankar commented Jul 13, 2023

rishabh-d-dave left a comment

Choose a reason for hiding this comment

rishabh-d-dave Jul 19, 2023

Choose a reason for hiding this comment

manishym Jul 20, 2023 • edited

Choose a reason for hiding this comment

rishabh-d-dave Jul 20, 2023

Choose a reason for hiding this comment

rishabh-d-dave Jul 19, 2023

Choose a reason for hiding this comment

rishabh-d-dave Jul 19, 2023

Choose a reason for hiding this comment

rishabh-d-dave Jul 19, 2023

Choose a reason for hiding this comment

rishabh-d-dave Jul 19, 2023

Choose a reason for hiding this comment

manishym Jul 20, 2023

Choose a reason for hiding this comment

rishabh-d-dave commented Jul 21, 2023

manishym commented Aug 14, 2023

github-actions bot commented Nov 1, 2023

gregsfortytwo commented Nov 21, 2023

github-actions bot commented Nov 29, 2023

rishabh-d-dave commented Nov 29, 2023 • edited

github-actions bot commented Jan 28, 2024

github-actions bot commented Feb 27, 2024

tchaikov commented Feb 28, 2024

github-actions bot commented Apr 28, 2024

manishym commented Jun 27, 2023 •

edited

manishym Jul 20, 2023 •

edited

rishabh-d-dave commented Nov 29, 2023 •

edited