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

fix(spring-boot): avoid logging admin user on INFO #3957

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

tasso94
Copy link
Member

@tasso94 tasso94 commented Nov 16, 2023

Logs admin user on DEBUG.

related to #3956

Logs admin user on `DEBUG`.

related to #3956
@tasso94 tasso94 added the ci:spring-boot Runs the integration tests for the Spring Boot starter. label Nov 16, 2023
@tasso94 tasso94 self-assigned this Nov 16, 2023
@tasso94 tasso94 requested a review from mboskamp November 17, 2023 12:58
Copy link
Member

@mboskamp mboskamp 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, nice improvement.
🙃 I would like to change the title of the issue to In Spring Boot Starter, admin user information is logged on INFO (added information). I think it is clearer what kind of data is logged this way.
🔧 We covered only one of the three affected loggers from the Expected Behavior section of this ticket. I think we could add tests for the other affected loggers as well.

@tasso94
Copy link
Member Author

tasso94 commented Nov 22, 2023

🔧 We covered only one of the three affected loggers from the Expected Behavior section of this ticket. I think we could add tests for the other affected loggers as well.

Do you have an idea what these tests could look like? The log that writes all registered process engine plugins is written before the CreateAdminUserConfiguration is actually registered, so it doesn't contain it. Asserting this log would require restructuring the test design. For the log that indicates creating the admin user has been skipped, as a black box test, we would need to persist the database and restart the engine. Both tests are technically possible but require a certain amount of effort. I think that investing in this effort pays little. Let me know if I'm missing something, and writing these tests is easier than I anticipated.

🙃 I would like to change the title of the issue to In Spring Boot Starter, admin user information is logged on INFO (added information). I think it is clearer what kind of data is logged this way.

I adjusted the title.

@tasso94 tasso94 requested a review from mboskamp November 22, 2023 08:30
@mboskamp
Copy link
Member

You are probably right. It's too much effort. I agree we should not do it.

@tasso94 tasso94 merged commit 9f3e0d0 into master Nov 22, 2023
2 checks passed
@tasso94 tasso94 deleted the 3956-avoid-logging-admin-user-on-info branch November 22, 2023 12:03
psavidis pushed a commit that referenced this pull request Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:spring-boot Runs the integration tests for the Spring Boot starter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants