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.

Sign up for GitHub

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

Jump to bottom

ISPN-16696 Configurable logging pattern #2157

Merged
merged 1 commit into from
Oct 2, 2024
Merged

ISPN-16696 Configurable logging pattern #2157

merged 1 commit into from
Oct 2, 2024

Conversation

karesti
Copy link
Contributor

@karesti karesti commented Sep 30, 2024 •
edited
Loading

@karesti karesti added the Preview WIP. Don't merge yet label Sep 30, 2024
@karesti
Copy link
Contributor Author

karesti commented Sep 30, 2024

@ryanemerson pr to know if I'm in the good direction here

ryanemerson
ryanemerson reviewed Sep 30, 2024
View reviewed changes
Copy link
Contributor

@ryanemerson ryanemerson left a comment

Choose a reason for hiding this comment

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

Good start @karesti 🎉

As well as the changes, the next step is to create a test that ensures a custom pattern is applied to the ConfigMap. It would consist of:

  1. Create Infinispan CR with 1 replica and a custom log pattern
  2. Wait for the cluster to form (see TestBaseFunctionality)
  3. Retrieve the i.GetConfigName() and check that the log4j.xml contains the expected log pattern.

api/v1/infinispan_types.go Outdated Show resolved Hide resolved
api/v1/types_util.go Outdated Show resolved Hide resolved
api/v1/types_util.go Outdated Show resolved Hide resolved
@karesti karesti force-pushed the ISPN-16696 branch 3 times, most recently from 61bdbf1 to a72eacb Compare October 1, 2024 16:32
@karesti karesti removed the Preview WIP. Don't merge yet label Oct 1, 2024
@karesti
Copy link
Contributor Author

karesti commented Oct 1, 2024

@ryanemerson I reported your changes and added docs and tests

ryanemerson
ryanemerson reviewed Oct 2, 2024
View reviewed changes
Copy link
Contributor

@ryanemerson ryanemerson left a comment

Choose a reason for hiding this comment

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

Great work @karesti, just a few very minor comments.

test/e2e/infinispan/smoke_test.go Outdated Show resolved Hide resolved
test/e2e/infinispan/smoke_test.go Outdated Show resolved Hide resolved
test/e2e/infinispan/smoke_test.go Outdated Show resolved Hide resolved
test/e2e/infinispan/smoke_test.go Outdated Show resolved Hide resolved
test/e2e/infinispan/smoke_test.go Outdated Show resolved Hide resolved
test/e2e/infinispan/smoke_test.go Outdated Show resolved Hide resolved
api/v1/types_util.go Outdated Show resolved Hide resolved
api/v1/infinispan_types.go Show resolved Hide resolved
@karesti
Copy link
Contributor Author

karesti commented Oct 2, 2024

@ryanemerson I think I reviewed all the comments

@ryanemerson ryanemerson merged commit 20d600c into infinispan:main Oct 2, 2024
10 checks passed
@ryanemerson
Copy link
Contributor

I didn't notice the ISPN- ticket in the commit message before. We use GH issues for upstream operator work, so I created #2162 to track this and have updated the commit message accordingly.

Thanks for your first contribution to the Operator @karesti 🎉

@karesti karesti deleted the ISPN-16696 branch October 2, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanemerson ryanemerson ryanemerson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@karesti @ryanemerson