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

DATAGO-93783: Fix cfg push cycle time bug by putting back the setting of the createdTime #231

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

moodiRealist
Copy link
Collaborator

What is the purpose of this change?

To fix a bug that prevented reporting configPush cycle time

How was this change implemented?

There was a change in this PR
Screenshot 2025-02-10 at 2 50 29 PM

Which was removed as part of this later PR
Screenshot 2025-02-10 at 2 51 21 PM

How was this change tested?

Manually by validating DD metric values showing up:
Screenshot 2025-02-10 at 2 48 02 PM

Is there anything the reviewers should focus on/be aware of?

...

@@ -91,6 +92,7 @@ public void execute(CommandMessage request) {
Validate.notEmpty(request.getCommandBundles(), "CommandBundles must not be empty");
Validate.notEmpty(request.getCommandBundles().get(0).getCommands(), "At least one command must be present");
CommandRequest requestBO = commandMapper.map(request);
requestBO.setCreatedTime(Instant.now());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not fix this on the mapper?

Copy link
Collaborator Author

@moodiRealist moodiRealist Feb 11, 2025

Choose a reason for hiding this comment

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

Addressed as we discussed. Metrics look good:
Screenshot 2025-02-10 at 5 44 04 PM

based on the new implementation, I couldn't have a status tag. @CameronRushton would that be a deal breaker considering that this PR is only registering success scenarios? I might be able to hard code it and register failure scenarios too. Thoughts?
Never mind, I will refactor and implement it so that it tags the status and registers failures as well.

Copy link

Quality Gate failed Quality Gate failed

Failed conditions
1 New issue

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube IDE SonarQube IDE

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.

2 participants