Skip to content

Conversation

@dimas-b
Copy link
Contributor

@dimas-b dimas-b commented Nov 23, 2024

This is a simplification / cleanup. The dependency does not appear to be required in polaris-core

Add custom code to PolarisApplication find classes directly listed in the Discoverable service descriptor and register them with the ObjectMapper. This approach to finding sub-types is consistent both with the java service descriptors (listed types actually implement the service interface) and at the same time allows moving the Dropwizard Discoverable dependencies to the polaris-service module that actually integrates with Dropwizard.

Move leaf metastore classes to the Discoverable service descriptor in their respective module.

Note: this fixes the cross-jar leak of EclipseLinkPolarisMetaStoreManagerFactory in service descriptors.

@dimas-b
Copy link
Contributor Author

dimas-b commented Nov 23, 2024

This is an alternative to #435

Copy link
Contributor

@eric-maynard eric-maynard left a comment

Choose a reason for hiding this comment

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

This may be better than the approach in #435, thanks for iterating on this. LGTM

@dimas-b
Copy link
Contributor Author

dimas-b commented Nov 26, 2024

@collado-mike : What's your take on this alternative approach?

This is a simplification / cleanup. The dependency does not appear to be required in `polaris-core`

Add custom code to `PolarisApplication` find classes directly listed in the `Discoverable`
service descriptor and register them with the ObjectMapper. This approach to finding sub-types
is consistent both with the java service descriptors (listed types actually implement the
service interface) and at the same time allows moving the Dropwizard `Discoverable` dependencies
to the polaris-service module that actually integrates with Dropwizard.

Move leaf metastore classes to the `Discoverable` service descriptor in their respective module.

Note: this fixes the cross-jar leak of EclipseLinkPolarisMetaStoreManagerFactory in service descriptors.
@dimas-b dimas-b force-pushed the remove-dw-from-core2 branch from 39c3f05 to a29a910 Compare November 29, 2024 15:29
@dimas-b
Copy link
Contributor Author

dimas-b commented Nov 29, 2024

rebased and resolved conflicts

Copy link
Member

@snazy snazy left a comment

Choose a reason for hiding this comment

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

Thanks! 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.

5 participants