Enable eclipselink for metastore

[MonkeyCanCode]

Description

This PR will enable eclipselink by default with H2 as backend metastore. This is based off changes from #41.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

This had being tested locally. With the changes in the PR, an end-user will now be able to use eclipselink for the metastore (h2 for now). To verified that is working, I verified this by following:

• checked if /tmp/eclipseLink/ is created and if h2 db files are created
• checked the log for "Initializing PolarisCallContextCatalogFactory for baseCatalogType polaris, metaStoreManagerType io.polaris.extension.persistence.impl.eclipselink.EclipseLinkPolarisMetaStoreManagerFactory"

Test Configuration:

• Firmware version:
• Hardware:
• Toolchain:
• SDK:

Checklist:

Please delete options that are not relevant.

• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• If adding new functionality, I have discussed my implementation with the community using the linked GitHub issue
• I have signed and submitted the ICLA and if needed, the CCLA. See Contributing for details.

[guitcastro]

I think you created a circular dependency between polaris-eclipselink and polaris-service. Try to remove the polaris-service from polaris-eclipselink dependencies.

[jbonofre]

It looks like a good idea to me to have eclipselink available by default (even if not enabled). It would simplify the life of our users.

@takidau @dennishuo @collado-mike what do you think guy about shipping eclipselink extension by default ?

[sfc-gh-aixu]

It looks like a good idea to me to have eclipselink available by default (even if not enabled). It would simplify the life of our users.

@takidau @dennishuo @collado-mike what do you think guy about shipping eclipselink extension by default ?

+1 to enable eclipselink by default since that is more closer to what the users will choose to use.

[dennishuo]

It looks like a good idea to me to have eclipselink available by default (even if not enabled). It would simplify the life of our users.

@takidau @dennishuo @collado-mike what do you think guy about shipping eclipselink extension by default ?

+1, having it ready-to-run out of the box will be nice

[guitcastro]

@MonkeyCanCode Can you please fix the branchs conflicts?

[MonkeyCanCode]

@MonkeyCanCode Can you please fix the branchs conflicts?

This is completed.

[guitcastro]

@MonkeyCanCode Even using runtimeOnly the cyclic reference stills exists. You should just remove polaris-service

Edit: Never mind, you can actually build using runtimeOnly.

[MonkeyCanCode]

polaris-service

Without adding that, it will fail on the test:

> Task :polaris-eclipselink:test

PolarisEclipseLinkMetaStoreTest > testCreateTestCatalog() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testDropEntities() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > validateBootstrap() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testCreateTestCatalogWithRetry() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testEntityCache() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testCreateTasksInParallel() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testLoadTasksInParallel() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testBrowse() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testCreateEntitiesWithConflict() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testRename() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testCreateEntitiesAlreadyExisting() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testUpdateEntities() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testCreateEntities() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testPrivileges() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

PolarisEclipseLinkMetaStoreTest > testLoadTasks() FAILED
    jakarta.persistence.PersistenceException at PolarisEclipseLinkMetaStoreTest.java:43

15 tests completed, 15 failed

> Task :polaris-eclipselink:test FAILED

FAILURE: Build failed with an exception.

[guitcastro]

@MonkeyCanCode I've edit my comment, you are right

[MonkeyCanCode]

@MonkeyCanCode I've edit my comment, you are right

Thanks for checking.

[eric-maynard]

I think we should treat changing the default metastore manager as a separate effort -- we will have to make changes to the docs, and we should also consider add more test coverage for eclipse-link before doing that.

Besides the change for the default metastore manager, this PR LGTM

[guitcastro]

This will not be enough to use eclipselink out of box because this code is expecting a resource from inside the class path instead of a local file:

InputStream input = this.getClass().getClassLoader().getResourceAsStream(confFile);

[MonkeyCanCode]

This will not be enough to use eclipselink out of box because this code is expecting a resource from inside the class path instead of a local file:

InputStream input = this.getClass().getClassLoader().getResourceAsStream(confFile);

So with this line, it will try to read config from https://github.com/polaris-catalog/polaris/blob/main/polaris-service/src/main/resources/META-INF. Then back to the change for the following:

metaStoreManager:
  type: in-memory
  # type: eclipse-link # uncomment to use eclipse-link as metastore
  # persistence-unit: polaris-dev

If you uncommented the eclipse-link (and commented out the in-memory one), it will then use h2 for eclipse-link defined in the default config.

To use a diff config file under resource dir, you can then do following (assuming you created a new persistence file named persistence-new.xml with persistence-unit named polaris-dev under path https://github.com/polaris-catalog/polaris/tree/main/polaris-service/src/main/resources/META-INF):

metaStoreManager:
  type: eclipse-link
  conf-file: META-INF/persistence-new.xml
  persistence-unit: polaris-dev

As yes, as of now, the current code doesn't support reading config outside the resources dir. I had created #45 earlier for asking how to load this file outside. However, with current code, that doesn't appear seems to be possible.