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

Enhance list-checkpoints CLI #3746

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

xincunli-sonic
Copy link
Contributor

@xincunli-sonic xincunli-sonic commented Jan 31, 2025

What I did

Add a new parameter -t to add last modify datetime of each checkpoint to the list-checkpoint CLI.

ADO: 30788686

How I did it

Read each checkpoint file last modify time, return checkpoint name and its last modify datetime

[
    {"name": "checkpoint1", "time": "timestamp"},
    {"name": "checkpoint2", "time": "timestamp"}
]

How to verify it

Applied change to lab device, then run sudo config list-checkpoints

Previous command output (if the output of a command-line utility has changed)

admin@str2-7250-lc1-2:~/xincun$ sudo config list-checkpoints
[
    "xincun2",
    "xincun1"
]

New command output (if the output of a command-line utility has changed)

admin@str2-7250-lc1-2:~/xincun$ sudo config list-checkpoints
[
    "xincun2",
    "xincun1"
]
admin@str2-7250-lc1-2:~/xincun$ sudo config list-checkpoints -t
[
    {
        "name": "xincun2",
        "time": "2025-02-11T00:55:06.521339+00:00"
    },
    {
        "name": "xincun1",
        "time": "2025-02-11T00:54:45.803878+00:00"
    }
]

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xincunli-sonic xincunli-sonic changed the title Add datetime to list checkpoints Enhance list-checkpoints CLI Jan 31, 2025
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -1825,7 +1825,11 @@ def setUp(self):
self.any_target_config = {"PORT": {}}
self.any_target_config_as_text = json.dumps(self.any_target_config)
self.any_checkpoint_name = "any_checkpoint_name"
self.any_checkpoints_list = ["checkpoint1", "checkpoint2", "checkpoint3"]
self.any_checkpoints_list = [
{"name": "checkpoint1", "time": datetime.datetime.now(timezone.utc).isoformat()},
Copy link
Contributor

Choose a reason for hiding this comment

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

name

break backward-compatibility. suggest a new parameter.

@@ -1825,7 +1825,11 @@ def setUp(self):
self.any_target_config = {"PORT": {}}
self.any_target_config_as_text = json.dumps(self.any_target_config)
self.any_checkpoint_name = "any_checkpoint_name"
self.any_checkpoints_list = ["checkpoint1", "checkpoint2", "checkpoint3"]
self.any_checkpoints_list = [
Copy link
Contributor

@wen587 wen587 Feb 6, 2025

Choose a reason for hiding this comment

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

Will it has command show change?
e.g.
Current command output:

admin@str3-msn4280-02:~$ sudo config checkpoint test
...
Checkpoint created successfully.
admin@str3-msn4280-02:~$ sudo config checkpoint test1
...
Checkpoint created successfully.
admin@str3-msn4280-02:~$  sudo config list-checkpoints 
[
    "test1",
    "test"
]
admin@str3-msn4280-02:~$ sudo config rollback test
...

What would it be after this change? Especially when rollback

@@ -244,14 +245,29 @@ def list_checkpoints(self):
self.logger.log_info("Getting checkpoints in checkpoints directory.")
checkpoint_names = self.util.get_checkpoint_names()

checkpoints_len = len(checkpoint_names)
self.logger.log_info(f"Found {checkpoints_len} checkpoint{'s' if checkpoints_len != 1 else ''}{':' if checkpoints_len > 0 else '.'}")
checkpoints = []
Copy link
Contributor

Choose a reason for hiding this comment

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

do you need to run sonic-mgmt test as well, since 'config list-checkpoints' is used in test code https://github.com/search?q=repo%3Asonic-net%2Fsonic-mgmt%20config%20list-checkpoints&type=code

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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.

5 participants