Skip to content

Conversation

qvistgaard
Copy link

This allows the default cassandra builder to be overwritten. This is useful for example, when implementing jaeger/opentracing

the default cassandra builder to be overwritten. This is useful for
example when implementing jaeger/opentracing
@pivotal-issuemaster
Copy link

@ssoerensen Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@ssoerensen Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 2, 2019
@snicoll
Copy link
Member

snicoll commented May 3, 2019

This is useful for example, when implementing jaeger/opentracing

Is that using a decorator pattern of some sort? I don't think increasing the surface area with an extra callback is actually worth it. Have you tried to use a BeanPostProcessor to wrap the Cluster bean instead?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label May 3, 2019
@qvistgaard
Copy link
Author

It was my first thought to use a BeanPostProcessor. However the way the opentracing cassandra driver is made prevents that. Basically they have a TracingCluster class which extends the original Cluster class. it is instantiated like so:

// Instantiate Tracing Cluster
Cluster cluster = new TracingCluster(builder, tracer);

@snicoll
Copy link
Member

snicoll commented May 5, 2019

Thanks for the feedback. I am still not convinced we should create an interface for that specific use case. Perhaps we could expose the Builder and then let you inject it and create the cluster any way you like.

Flagging for team attention to see what the rest of the team thinks.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label May 5, 2019
@philwebb philwebb added for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels May 8, 2019
@philwebb philwebb added this to the 2.2.x milestone May 8, 2019
@wilkinsona wilkinsona self-assigned this May 9, 2019
@wilkinsona wilkinsona closed this in 40e1c19 May 9, 2019
@wilkinsona wilkinsona modified the milestones: 2.2.x, 2.2.0.M3 May 9, 2019
@wilkinsona
Copy link
Member

@ssoerensen Thank you very much for making your first contribution to Spring Boot. The proposed changes have been merged into master, along with a polish commit. The two main changes were to remove the default implementation of the new interface and to add a test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants