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

Allow user to expand pool if block device is expanded #906

Merged
merged 4 commits into from
Oct 13, 2022

Conversation

mulkieran
Copy link
Member

@mulkieran mulkieran commented Aug 29, 2022

Related stratis-storage/project#50

Implements a new command: stratis pool extend-data and extends the output of blockdev list and pool list. blockdev list shows the new size of the pool if different from the current in use-size. pool list displays an Alert on the pool, if different.

Supercedes #901

@mulkieran mulkieran self-assigned this Aug 29, 2022
@mulkieran
Copy link
Member Author

will fail stratisd tests.

@mulkieran mulkieran marked this pull request as ready for review September 13, 2022 16:41
@mulkieran
Copy link
Member Author

rebased.

@mulkieran
Copy link
Member Author

rebased.

@mulkieran mulkieran requested a review from drckeefe September 19, 2022 17:28
Copy link
Member

@bmr-cymru bmr-cymru left a comment

Choose a reason for hiding this comment

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

I was a bit puzzled by the new class name in _error_codes but I think my initial expectation is probably looking for a foolish consistency :)

It would be nice to expand the class docstring comment though!

@@ -120,12 +120,76 @@ def from_str(code_str):
)


class PoolDeviceSizeChangeCode(IntEnum):
Copy link
Member

Choose a reason for hiding this comment

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

This is the only class in _error_codes that doesn't follow the Pool.*ErrorCode naming pattern (and also the only class to define an I(nfo) as well as W/E). PoolDeviceSizeChangeErrorCode is a bit of a mouthful at 29 chars. It stuck out a bit and I wondered if it might be confusing to come across the class name in isolation but I think I'm probably being nit-picky here :)

Copy link
Member Author

Choose a reason for hiding this comment

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

it's the first code that includes an info possibility, so the name was chosen with that in mind. It might have been better to leave out "Error" from all the class names at the start, just to allow for future variability.

Copy link
Member

Choose a reason for hiding this comment

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

Yep that makes sense, thanks! I Think that was the conclusion I was slowly coming to myself :-)

@@ -120,12 +120,76 @@ def from_str(code_str):
)


class PoolDeviceSizeChangeCode(IntEnum):
"""
Codes to extend if a device
Copy link
Member

Choose a reason for hiding this comment

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

Comment trails off?

@mulkieran
Copy link
Member Author

Filed an issue for the naming question with error codes: #920

@mulkieran mulkieran requested a review from bmr-cymru October 13, 2022 17:07
@mulkieran mulkieran force-pushed the expand-block-size-2 branch from d7d2e21 to 7e09a9f Compare October 13, 2022 17:10
@mulkieran mulkieran merged commit 9a6278e into stratis-storage:master Oct 13, 2022
@mulkieran mulkieran deleted the expand-block-size-2 branch October 13, 2022 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants