NC | CLI | Updated the CLI response to have more info when account/bucket has been deleted #8723

achouhan09 · 2025-01-24T14:51:27Z

Explain the changes

Improved the response to provide a clearer message about the operations.
Now, the name of the object would also be printed in the response message for more info.

Issues: Fixed #xxx / Gap #xxx

Fixed: NC | NSFS | CLI | Improve Response (To have Details) #8200

Testing Instructions:

Try creating/deleting(or any other operation) on objects(account, bucket, etc.) using noobaa-cli and check the response returned.

Tests added

src/manage_nsfs/manage_nsfs_cli_responses.js

src/cmd/manage_nsfs.js

src/manage_nsfs/manage_nsfs_cli_responses.js

src/cmd/manage_nsfs.js

romayalon · 2025-02-06T10:03:14Z

src/manage_nsfs/manage_nsfs_cli_responses.js

@@ -131,16 +148,19 @@ ManageCLIResponse.LoggingExported = Object.freeze({

 ManageCLIResponse.UpgradeSuccessful = Object.freeze({
    code: 'UpgradeSuccessful',
+    message: 'Upgrade completed successfully',


Can you add to all the Upgrade functions Config Directory Upgrade prefix? for example - Config Directory Upgrade completed successfully etc

romayalon · 2025-02-06T10:14:13Z

src/manage_nsfs/manage_nsfs_cli_responses.js

@@ -150,25 +170,30 @@ ManageCLIResponse.UpgradeHistory = Object.freeze({

 ManageCLIResponse.ConnectionCreated = Object.freeze({
    code: 'ConnectionCreated',
+    message: 'Connection has been successfully created',


I think that here as well maybe Notification Connection
@alphaprinz probably can help with this response details

romayalon · 2025-02-06T10:15:27Z

src/manage_nsfs/manage_nsfs_cli_responses.js

@@ -94,25 +105,30 @@ ManageCLIResponse.AccountList = Object.freeze({

 ManageCLIResponse.BucketCreated = Object.freeze({
    code: 'BucketCreated',
+    message: 'Bucket has been successfully created',


I would put the 'successfully' in the end of the sentance

I think I should do this for all the operations. Thanks

romayalon · 2025-02-06T10:16:49Z

src/manage_nsfs/manage_nsfs_cli_responses.js

    }

    to_string(detail) {
        const json = {
            response: {
                code: this.code,
+                message: detail.name ? `${this.message}: ${detail.name}` : this.message,


does adding the detail after the message makes sense in all cases?
and also, do we always have detail and detail.name?

whenever we want to print the name of the object in the operation then only we are adding detail at the end of message otherwise only the set message would be printed for every operation

the issue I see here is with accessing directly to detail.name while detail can be undefined

Updated to optional chain. Thanks

romayalon · 2025-02-06T10:17:30Z

src/cmd/manage_nsfs.js

@@ -252,7 +252,7 @@ async function delete_bucket(data, force) {
        }
        await native_fs_utils.folder_delete(bucket_temp_dir_path, fs_context_fs_backend, true);
        await config_fs.delete_bucket_config_file(data.name);
-        return { code: ManageCLIResponse.BucketDeleted, detail: '', event_arg: { bucket: data.name } };
+        return { code: ManageCLIResponse.BucketDeleted, detail: { name: data.name }, event_arg: { bucket: data.name } };


are there more cases where we want to add detail?

for now I have added this in delete_account() and delete_bucket() operation only. We can print the name of object in message by setting the name property in detail(for any operation) otherwise the set message would be printed

please fix all the cases in this PR or open another gap for the rest of the cases so we will handle it in the future

Sure, I will open gap if there are more operations.

@romayalon I just verified other then connection_management there is no need add details explicitly. I have updated it. Thanks

For every operation we are printing the name of the object in the message(the code parsed the name automatically if detail.name is defined).

…en deleted Signed-off-by: Aayush Chouhan <[email protected]>

Signed-off-by: Aayush Chouhan <[email protected]>

achouhan09 requested review from liranmauda, romayalon, shirady, a team and vh05 and removed request for a team January 24, 2025 14:51

pull-request-size bot added the size/S label Jan 24, 2025

shirady reviewed Jan 26, 2025

View reviewed changes

src/manage_nsfs/manage_nsfs_cli_responses.js Outdated Show resolved Hide resolved

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved

achouhan09 requested review from nadavMiz and naveenpaul1 January 27, 2025 05:38

nadavMiz reviewed Jan 27, 2025

View reviewed changes

src/manage_nsfs/manage_nsfs_cli_responses.js Outdated Show resolved Hide resolved

src/manage_nsfs/manage_nsfs_cli_responses.js Outdated Show resolved Hide resolved

achouhan09 force-pushed the account-res-fix branch 2 times, most recently from 552ba45 to 390d2b4 Compare January 29, 2025 10:04

achouhan09 requested review from shirady and nadavMiz January 29, 2025 10:06

shirady reviewed Jan 30, 2025

View reviewed changes

src/manage_nsfs/manage_nsfs_cli_responses.js Outdated Show resolved Hide resolved

src/manage_nsfs/manage_nsfs_cli_responses.js Outdated Show resolved Hide resolved

src/manage_nsfs/manage_nsfs_cli_responses.js Outdated Show resolved Hide resolved

romayalon requested changes Jan 30, 2025

View reviewed changes

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved

achouhan09 force-pushed the account-res-fix branch from 390d2b4 to 7a0f9e3 Compare February 5, 2025 12:59

pull-request-size bot added size/M and removed size/S labels Feb 5, 2025

achouhan09 force-pushed the account-res-fix branch from 7a0f9e3 to c3c75b6 Compare February 5, 2025 13:01

achouhan09 requested review from romayalon and shirady February 5, 2025 13:01

romayalon reviewed Feb 6, 2025

View reviewed changes

achouhan09 force-pushed the account-res-fix branch from c3c75b6 to a0129e7 Compare February 7, 2025 13:39

achouhan09 requested a review from romayalon February 7, 2025 13:50

achouhan09 added 3 commits February 10, 2025 16:01

Updated the CLI response to have more info when account/bucket has be…

b30c686

…en deleted Signed-off-by: Aayush Chouhan <[email protected]>

Update cli tests to check for message in account and bucket

9bb78c8

Signed-off-by: Aayush Chouhan <[email protected]>

Added message in response to every operation and logic

89fe7ea

Signed-off-by: Aayush Chouhan <[email protected]>

Updated the msg logic and content

4172231

Signed-off-by: Aayush Chouhan <[email protected]>

achouhan09 force-pushed the account-res-fix branch from a0129e7 to 4172231 Compare February 10, 2025 11:20

Added connection name to be printed in resp msg

6c75de7

Signed-off-by: Aayush Chouhan <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NC | CLI | Updated the CLI response to have more info when account/bucket has been deleted #8723

NC | CLI | Updated the CLI response to have more info when account/bucket has been deleted #8723

achouhan09 commented Jan 24, 2025 •

edited

Loading

romayalon Feb 6, 2025

achouhan09 Feb 6, 2025

romayalon Feb 6, 2025

romayalon Feb 6, 2025

achouhan09 Feb 6, 2025

romayalon Feb 6, 2025

achouhan09 Feb 6, 2025

romayalon Feb 6, 2025

achouhan09 Feb 7, 2025

romayalon Feb 6, 2025

achouhan09 Feb 6, 2025

romayalon Feb 6, 2025

achouhan09 Feb 7, 2025

achouhan09 Feb 10, 2025

NC | CLI | Updated the CLI response to have more info when account/bucket has been deleted #8723

Are you sure you want to change the base?

NC | CLI | Updated the CLI response to have more info when account/bucket has been deleted #8723

Conversation

achouhan09 commented Jan 24, 2025 • edited Loading

Explain the changes

Issues: Fixed #xxx / Gap #xxx

Testing Instructions:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achouhan09 commented Jan 24, 2025 •

edited

Loading