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

Remove dropwizard-jackson dep from core #470

Merged
merged 1 commit into from
Nov 29, 2024

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!

@snazy snazy enabled auto-merge (squash) November 29, 2024 15:43
@snazy snazy merged commit a6197bd into apache:main Nov 29, 2024
5 checks passed
@dimas-b dimas-b deleted the remove-dw-from-core2 branch November 29, 2024 16:42
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