Skip to content

Conversation

@Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Nov 26, 2025

What I did
This PR is for fixing a bug that telemetry_tam object may not be deleted after the default HFT profile removed.

Why I did it
To the default profile, its group is empty, so the original checker isEmpty will be unable to capture the group deletion message.

How I verified it
Add a new vstest to cover this scenario.

Details if related

@Pterosaur Pterosaur requested a review from prsunny as a code owner November 26, 2025 11:32
Copilot AI review requested due to automatic review settings November 26, 2025 11:32
@Pterosaur Pterosaur marked this pull request as draft November 26, 2025 11:32
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copilot finished reviewing on behalf of Pterosaur November 26, 2025 11:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a regression test to verify that a bug related to deleting default HFT (High Frequency Telemetry) configurations has been fixed. The test ensures that TAM_TELEMETRY objects are properly cleaned up from the ASIC database when a default profile with empty object_names and object_counters is deleted.

Key Changes:

  • Added test_hft_empty_default_config test function to verify proper cleanup of TAM_TELEMETRY objects when deleting a default HFT profile with empty configuration
  • Test validates that TAM_TELEMETRY objects exist after creation with empty fields and are properly deleted after profile/group removal

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Ze Gan <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur Pterosaur marked this pull request as ready for review November 27, 2025 01:46
@Pterosaur Pterosaur requested a review from r12f November 27, 2025 01:49
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Ze Gan <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur
Copy link
Contributor Author

Hi @r12f , could you please help to review this PR?

@Pterosaur
Copy link
Contributor Author

Pterosaur commented Nov 27, 2025

Hi @prsunny , could you please help to merge this PR?

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

task_process_status groupTableDel(const std::string &profile_name, const std::string &group_name);
std::shared_ptr<HFTelProfile> getProfile(const std::string &profile_name);
std::shared_ptr<HFTelProfile> tryGetProfile(const std::string &profile_name);
bool isProfileInUse(const std::shared_ptr<HFTelProfile> &profile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big issue, but we can mark this as a const function

Suggested change
bool isProfileInUse(const std::shared_ptr<HFTelProfile> &profile);
bool isProfileInUse(const std::shared_ptr<HFTelProfile> &profile) const;

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Ze Gan <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants