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: shutdown hook configuration is using wrong config key #1621

Conversation

m-courtin
Copy link
Contributor

@m-courtin m-courtin commented Dec 3, 2021

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]

@petermetz petermetz removed the request for review from jonathan-m-hamilton December 6, 2021 21:49
@petermetz petermetz added API_Server bug Something isn't working labels Dec 6, 2021
@m-courtin m-courtin force-pushed the fix-1619-shutdown-hook-configuration branch 3 times, most recently from 6a05bca to 16d1630 Compare December 10, 2021 12:55
@m-courtin m-courtin requested a review from petermetz December 10, 2021 12:58
@m-courtin
Copy link
Contributor Author

@petermetz: UTs are added and during debugging of the new UTs 2 additional bugs came up -> I will create corresponding bugs

Copy link
Contributor

@petermetz petermetz left a 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)

@m-courtin m-courtin force-pushed the fix-1619-shutdown-hook-configuration branch from 16d1630 to 7d639a1 Compare December 15, 2021 08:21
@github-actions
Copy link

🎉 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]>
@m-courtin m-courtin force-pushed the fix-1619-shutdown-hook-configuration branch from 00c61c0 to edc463c Compare December 15, 2021 16:04
@petermetz petermetz merged commit 5c5a1e1 into hyperledger-cacti:main Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API_Server bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(api-server): shutdown hook configuration is using wrong config key
3 participants