-
Notifications
You must be signed in to change notification settings - Fork 130
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
Resolve type id 'eclipse-link' as a subtype of MetaStoreManagerFactory #426
base: main
Are you sure you want to change the base?
Resolve type id 'eclipse-link' as a subtype of MetaStoreManagerFactory #426
Conversation
Hello @fivetran-arunsuri , That is expected as Polaris is not enable eclipse-link by default. It is possible to enable it as part of the build. See following for detail (same can be found in https://polaris.apache.org/in-dev/unreleased/metastores/):
This is archived by the same file you put the PR with following snippet:
Reasoning for this is by default eclipselink is configured to use h2 database which is not very useful for prod graded deployment. Due to licensing concern as well as various diff backends end-users may preferred, none of the other database drivers are included as of now. There is a purpose to use quarkus to close this gap (#392), however, that is not yet completed as of today. Thanks, |
Hey Yong Zheng,
in polaris-server.yml, which currently pulls EclipseLink from the build options as you mentioned, creating confusion. Adding the dependency directly in the PR, as I’ve done, should prevent any errors mentioned in the documentation and keep things straightforward. We could also update the documentation to specify that in case additional backends require dependencies for eg postgres here that can be added to the build file, as our team is currently handling it now. Since we’ll be modifying persistence.xml anyway, this approach seems practical. I'm unclear on why making EclipseLink optional is necessary. Regardless of the database backend used, this property would still be required for production cases and not the in memory obe, wouldn’t it? Let me know your thoughts. Thanks, |
Hello, So my first PR to Polaris is also making that a must: https://github.com/apache/polaris/pull/47/files. Later on this change got accidently rolled off due to PR #114. Then back to your question, I think it is more likes a preference thing regarding if EclipseLink should not enabled or not by default. IMO, is it not very useful when we can't use persistent backend for a catalog other than some simple testing etc. I took a quick look at the production doc you mentioned above, it does stated the following on how to enable it:
Then for sure we will need persistent backend. How to add additional libs for persistent backend is document in the main READ.me instead of production doc. I think there may be some documentation improvement needed (feel free to PR those or I can take care it later this weekend). It may be better to raise a discussion with community on weather to enable this by default. Based on my use case, it is part of build on my pipeline. So this is always included along with additional jar dependencies for the db client. One potential use case people may have with not wanting this enabled by default is using their own version of EclipseLink with custom implementation and different backend requirements (e.g. MongoDB as backend). The current EcliseLink plugin won't work with document stores database. Thanks, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes sense to have eclipselink dep defined here.
polaris-service
and extensions should stay "decoupled".
+1 to decoupling service classes and extensions. Taking this one step further, we may want to consider build-time options for producing servers (e.g. docker images) for a particular combination of extensions. I think it is quite doable with gradle. |
Description
Resolve type id 'eclipse-link' as a subtype of MetaStoreManagerFactory. Right now with the main branch as it is when we try to configure and run polaris server for production use case with eclipse link as metaStoreManager and postgres as db, it fails with following error
Failed to parse configuration at: metaStoreManager; Could not resolve type id 'eclipse-link' as a subtype of
org.apache.polaris.core.persistence.MetaStoreManagerFactory
: known type ids = [in-memory] (for POJO property 'metaStoreManager')at [Source: UNKNOWN; byte offset: #UNKNOWN] (through reference chain: org.apache.polaris.service.config.PolarisApplicationConfig["metaStoreManager"])
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist:
Please delete options that are not relevant.