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

NC | NSFS | CLI | Separate Bucket and Account List Functions #8747

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

shirady
Copy link
Contributor

@shirady shirady commented Feb 3, 2025

Explain the changes

  1. Separate the list function - from list_config_files to list_bucket_config_files and list_account_config_files.
  2. Organize the file and add titles to the parts (buckets, accounts, etc.).
  3. Move the functions filter_bucket and list_bucket_config_files under the buckets part, and move list_connections to the notifications part.
  4. Remove the function filter_list_item as we use the functions filter_account and filter_bucket directly.
  5. Added 2 cases in the file test_nc_nsfs_bucket_cli.test.js (after adding them I saw that we had them in the file test_nc_nsfs_cli.js but decided to keep them).
  6. Fix a typo in a comment ("initialization").

Issues:

  1. It is a partial fix as a part of NC | Maintenance and Short Refactoring Tasks #8264 "In manage_nsfs: split the implementation of list config file to a function for accounts and a function for buckets.", and was done as step in NC | Make list accounts/buckets and update account/bucket tolerant to errors like missing master key, permissions etc #8104 that part of the solution would be to avoid decryption when there are issues related to the master key.

Gaps:

  1. Rename CLI test files in Jest - I added in issue NC | Unit tests suggestions and refactoring #8056 (was raised in this comment).

Testing Instructions:

Automatic tests

Please run:

  1. sudo npx jest test_nc_nsfs_account_cli.test.js
  2. sudo npx jest test_nc_nsfs_bucket_cli.test.js (here 2 cases were added)
  3. sudo NC_CORETEST=true node node_modules/mocha/bin/mocha src/test/unit_tests/test_nc_nsfs_cli.js
  • Doc added/updated
  • Tests added

@shirady shirady self-assigned this Feb 3, 2025
Copy link
Contributor

@romayalon romayalon left a comment

Choose a reason for hiding this comment

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

A few minor comments, LGTM in general

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
@shirady shirady force-pushed the nsfs-nc-list-separate branch 2 times, most recently from 9ef917a to c76b9bd Compare February 9, 2025 14:23
@shirady shirady force-pushed the nsfs-nc-list-separate branch from c76b9bd to b11aa5d Compare February 10, 2025 06:50
@shirady shirady merged commit a3bc7dd into noobaa:master Feb 10, 2025
11 checks passed
@shirady shirady deleted the nsfs-nc-list-separate branch February 10, 2025 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants