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

CORE-17437: State Manager Configuration Section #4780

Merged
merged 13 commits into from
Oct 10, 2023

Conversation

jujoramos
Copy link
Contributor

Promote State Manager configuration to its own section and remove it
from under the Messaging configuration.

@jujoramos jujoramos force-pushed the feature/CORE-17437 branch 2 times, most recently from 0c1b4df to 3a630c9 Compare October 5, 2023 16:51
@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Oct 5, 2023

Jenkins build for PR 4780 build 19

Build Successful:
Jar artifact version produced by this PR: 5.1.0.0-alpha-1696887563598
Helm chart version produced by this PR: 5.1.0-alpha.1696887563598
Helm chart pushed to: oci://corda-os-docker-dev.software.r3.com/helm-charts/pr-4780/corda

@jujoramos
Copy link
Contributor Author

Related api change: corda/corda-api#1277

@jujoramos jujoramos marked this pull request as ready for review October 5, 2023 20:41
@jujoramos jujoramos requested review from a team as code owners October 5, 2023 20:41
JamesHR3
JamesHR3 previously approved these changes Oct 9, 2023
mbrkic-r3
mbrkic-r3 previously approved these changes Oct 9, 2023
@jujoramos jujoramos dismissed stale reviews from mbrkic-r3 and JamesHR3 via 78d7fa8 October 9, 2023 16:14
JamesHR3
JamesHR3 previously approved these changes Oct 9, 2023
conalsmith-r3
conalsmith-r3 previously approved these changes Oct 9, 2023
mbrkic-r3
mbrkic-r3 previously approved these changes Oct 9, 2023
@@ -40,17 +40,14 @@ class FlowMaintenanceImpl @Activate constructor(
private var stateManager: StateManager? = null

override fun onConfigChange(config: Map<String, SmartConfig>) {
// TODO - fix config key (CORE-17437).
if(config.containsKey(ConfigKeys.MESSAGING_CONFIG)) {
if (config.containsKey(ConfigKeys.STATE_MANAGER_CONFIG)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct as this assumes that the messaging config is always already there when the state manager config arrives.

I think they need to be treated separate.

Copy link
Contributor

@driessamyn driessamyn left a comment

Choose a reason for hiding this comment

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

LGTM

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.

6 participants