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

Configuration bug #552

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Configuration bug #552

merged 1 commit into from
Apr 2, 2024

Conversation

Bdegraaf1234
Copy link
Member

This pull request is to fix an apparent bug (#551) in the output restructure configuration process.

I added a test but cannot capture the problem in a proper unit test so added an integration test.

@Bdegraaf1234 Bdegraaf1234 changed the base branch from security-updates to dev March 29, 2024 15:01
@Bdegraaf1234 Bdegraaf1234 self-assigned this Mar 29, 2024
@Bdegraaf1234
Copy link
Member Author

Bdegraaf1234 commented Mar 29, 2024

in 6814c16 You can see a log message which indicates that the two levels of configuration are the same for the unittest added in this pull request (recordPathFactoryTest.kt, as well as the other unit tests that I checked) but NOT for the integration test.

image

https://github.com/RADAR-base/radar-output-restructure/actions/runs/8482449189/job/23241706408?pr=552#step:6:420

https://github.com/RADAR-base/radar-output-restructure/actions/runs/8482449189/job/23241706408?pr=552#step:6:589

@Bdegraaf1234 Bdegraaf1234 force-pushed the bugfix-pathconfig branch 4 times, most recently from 0144ab4 to ae98a17 Compare March 29, 2024 15:49
@Bdegraaf1234 Bdegraaf1234 marked this pull request as ready for review April 2, 2024 09:20
Copy link

Choose a reason for hiding this comment

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

Thanks for fixing this !

@Bdegraaf1234
Copy link
Member Author

@pvannierop and I looked more closely at this and found the issue was located in the PathConfig.createFactory() method, which in its current implementation could not pass any configuration to the created factory.path property.

@Bdegraaf1234 Bdegraaf1234 merged commit f6ad635 into dev Apr 2, 2024
2 checks passed
@Bdegraaf1234 Bdegraaf1234 deleted the bugfix-pathconfig branch May 23, 2024 17:05
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