-
Notifications
You must be signed in to change notification settings - Fork 294
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: shutdown hook configuration is using wrong config key #1621
fix: shutdown hook configuration is using wrong config key #1621
Conversation
6a05bca
to
16d1630
Compare
@petermetz: UTs are added and during debugging of the new UTs 2 additional bugs came up -> I will create corresponding bugs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m-courtin Cheers, LGTM, I will merge this once the tests are passing (once the prerequisite PR was merged)
16d1630
to
7d639a1
Compare
🎉 Great news! Looks like all the dependencies have been resolved: 💡 To add or remove a dependency please update this issue/PR description. Brought to you by Dependent Issues (:robot: ). Happy coding! |
In the api-server constructor the evaluation for the activation / deactivation of the shutdown hook is based on the wrong configuration property so that it always is falling back to the default value and pre-configured value is not taken. Closes: hyperledger-cacti#1619 Signed-off-by: Michael Courtin <[email protected]>
Need to set apiServerOptions.configFile to empty string as otherwise the build pipeline is failing Closes: hyperledger-cacti#1619 Signed-off-by: Michael Courtin <[email protected]>
00c61c0
to
edc463c
Compare
In the api-server constructor the evaluation for the activation /
deactivation of the shutdown hook is based on the wrong configuration
property so that it always is falling back to the default value and
pre-configured value is not taken.
Depends on #1649
#1649 fix for faulty shutdownHook definition in the Config-Schema is needed to be approved first as this is enabling the unit tests within this PR to work in correct way-
Closes: #1619
Signed-off-by: Michael Courtin [email protected]