Skip to content

Conversation

gurbuzali
Copy link

@gurbuzali gurbuzali commented Mar 30, 2020

Spring Boot auto-configures Hazelcast IMDG when it finds relevant configuration file on the classpath or root directory. Hazelcast Jet wraps IMDG and uses the very same configuration files(in addition to Hazelcast Jet specific configuration file) to configure and create Hazelcast Jet instances. This PR disables Hazelcast IMDG auto-configuration if Jet presents by checking a certain file (hazelcast-jet-default.yaml) is on the classpath.

See discussion on this PR #19483

fixes hazelcast/hazelcast-jet#1835

See Hazelcast Jet Starter Spring Boot Starter

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 30, 2020
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. If the purpose of this is to make sure HazelcastInstance is not created, I'd rather put that condition on HazelcastAutoConfiguration itself as that's the only thing it does.

@@ -43,6 +46,16 @@ protected HazelcastConfigResourceCondition(String configSystemProperty, String..
this.configSystemProperty = configSystemProperty;
}

@Override
public ConditionOutcome getMatchOutcome(ConditionContext context, AnnotatedTypeMetadata metadata) {
Copy link
Member

Choose a reason for hiding this comment

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

This code doesn't match with the javadoc or name of HazelcastConfigResourceCondition so it shouldn't be located there.

*
* @author Ali Gurbuz
*/
@ClassPathOverrides("com.hazelcast.jet:hazelcast-jet:4.0")
Copy link
Member

@snicoll snicoll Mar 30, 2020

Choose a reason for hiding this comment

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

I prefer no to refer to a concept if we can avoid doing so. This test could be put in the main test with a special ClassLoader that returns the expected resource and shows the hazelcast instance is not created.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Mar 30, 2020
@snicoll
Copy link
Member

snicoll commented Mar 30, 2020

@gurbuzali are you aware a colleague of yours has already submitted an attempt? see #19483. Ignoring the class check, I think it's more aligned with what we need to do there (besides testing as I've mentioned in a comment).

@gurbuzali
Copy link
Author

Hi @snicoll, I've tried to address your review comments, please take a look.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 30, 2020
@snicoll snicoll added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Mar 31, 2020
@snicoll snicoll added this to the 2.3.x milestone Mar 31, 2020
@snicoll snicoll closed this in b9cb1c8 Apr 1, 2020
@snicoll snicoll modified the milestones: 2.3.x, 2.3.0.M4 Apr 1, 2020
@snicoll
Copy link
Member

snicoll commented Apr 1, 2020

Thank you for making your first contribution to Spring Boot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spring Boot Hazelcast AutoConfiguration conflicts with Jet
3 participants