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

empty compression setting on imported pools blocks extra mount options. Fixes #1896 #1899

Conversation

phillxnet
Copy link
Member

Newly created pools have Pool.compression set to 'no' however imported pools, and the system pool, defaulted to Null / None for this field. Consequently "Empty" was indicated by x-editable inline compression edit on pool overview and detail pages. This entry and it's consequent value failed the dual submission of compression and mount options.

Summary:

  • Set imported pools to 'no' compression by default.
  • Handle "Empty" output from inline editable of compression setting.
  • Put "Empty" inline editable colour in line with links as not an error condition (to allow for legacy imported pools).
  • Change "Empty" legacy pool imports compression text to "Unset".
  • Improve and normalise compression tooltip text.
  • Add 'compression' and 'extra mount options' tooltips to Pool overview.
  • Remove inconsistent root pool mount edit options see issue inconsistent root pool mount edit options #1861 where compression and mount options were offered on system drive details page.
  • Display system Pool.compression and Pool.mnt_options as per other pools but without edit options: i.e. 'no', if set, rather than 'OFF' as a hard wired value.

Fixes #1896
and additionally:
Fixes #1861
Please see both issues texts for context.

@schakrava Ready for review.

Test for 'proof of fix':

Imported pools on fresh build and ensured that their 'extra mount options' could be inline edited prior to their 'Empty' (post pr 'Unset') compression entries having a selection made. Also edited system pool's compression and mnt_options entries in storageadmin_pool db to ensure future configuration on these elements will display as intended. Tested a completely fresh build to ensure system pool had it's Pool.compression set as intended.

Outstanding caveat: When editing both compression and mount options in Pool details page without a page refresh between them, stale settings can be re-applied. This is due to the Pool info held by that page not being refreshed and the js using this outdated info on the second settings submission. This bug can be addressed separately against an issue opened for this purpose.

…rockstor#1896

Newly created pools have Pool.compression set to 'no' however imported
pools, and the system pool, defaulted to Null / None for this field.
Consequently "Empty" was indicated by x-editable inline compression edit
on pool overview and detail pages. This entry and it's consequent value
failed the dual submission of compression and mount options.

Summary:

- Set imported pools to 'no' compression by default.
- Handle "Empty" output from inline editable of compression setting.
- Put "Empty" inline editable colour in line with links as not an error
contition (to allow of legacy imported pools).
- Change "Empty" legacy pool imports compression text to "Unset".
- Improve and normalise compression tooltip text.
- Add 'compression' and 'extra mount options' tooltips to Pool overview.
- Remove inconsistent root pool mount edit options see issue rockstor#1861 where
compression and mount options were offered on system drive details page.
- Display system Pool.compression and Pool.mnt_options as per other
pools but without edit options: i.e. 'no', if set, rather than 'OFF' as
hard wired.
@schakrava
Copy link
Member

I like this pr a lot. Thanks @phillxnet !

@schakrava schakrava merged commit fff93f7 into rockstor:master Mar 5, 2018
@phillxnet phillxnet deleted the 1896_empty_compression_setting_on_imported_pools_blocks_extra_mount_options branch March 5, 2018 16:11
@phillxnet
Copy link
Member Author

@schakrava Thanks and Cheers.

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