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-64298: rotate command log files. #144

Merged
merged 3 commits into from
Dec 14, 2023

Conversation

moodiRealist
Copy link
Collaborator

What is the purpose of this change?

To rotate command log files using logback functionality.

How was this change implemented?

updated logback-spring.xml and removed previous manual logging functionality.

How was this change tested?

Ran the code multiple times to see logs being rotated and eventually removed:
Screenshot 2023-12-14 at 2 29 15 PM

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

Also added a new attribute eventManagementId to commandLogs message to satisfy a requirement

Comment on lines 90 to 96
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.10.1</version>
</dependency>
</dependencies>
Copy link
Contributor

Choose a reason for hiding this comment

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

☁️ I was wondering if adding another json processing library is needed. I think we already have jackson library available to ema, can we reuse that library?

Copy link
Collaborator

@gregmeldrum gregmeldrum left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link

SonarQube Quality Gate

Quality Gate failed

Failed condition 33.3% 33.3% Coverage on New Code (is less than 80%)

See analysis details on SonarQube

@moodiRealist moodiRealist merged commit e686c6e into main Dec 14, 2023
5 of 6 checks passed
@mo-radwan1 mo-radwan1 deleted the moodiRealist/DATAGO-64298-remove-old-logs branch May 7, 2024 18:21
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.

3 participants