Yingjianw/aurora jdbc #658
Conversation
yingjianwu98
force-pushed
the
yingjianw/auroraJDBC
branch
8 times, most recently
from
5ff874e to
5712246
Compare
| * @param env Spring Environment | ||
| * @param isAuroraEnabled isAuroraEnabled | ||
| */ | ||
| public MetacatProperties(final Environment env, final boolean isAuroraEnabled) { |
There was a problem hiding this comment.
nit: It might be good to add a new constructor rather than changing the existing one. I think you can keep the original and just proxy that through to this one, with isAuroraEnabled set to null. I think that would decrease the change's footprint.
| facets { | ||
| functionalTest { | ||
| parentSourceSet = "test" | ||
| includeInCheckLifecycle = true | ||
| testTaskName = 'functionalTestWithDifferentDB' | ||
| includeInCheckLifecycle = false |
There was a problem hiding this comment.
Why are we switching the value of includeInCheckLifecycle?
There was a problem hiding this comment.
the reason is
./gradlew clean build will run the functional test
but we already have an action for
./gradlew clean build functionalTest in our github action
| @@ -61,21 +64,29 @@ public void deleteDatabase(final String dbName) { | |||
| } | |||
|
|
|||
| @Override | |||
| @Transactional(propagation = Propagation.SUPPORTS) | |||
| @Transactional(propagation = Propagation.REQUIRED) | |||
There was a problem hiding this comment.
Why are we shifting these to REQUIRED?
| */ | ||
| protected <T> Slice<T> getAllDatabasesForCurrentPage( | ||
| final String dbNamePrefix, final Pageable page, final boolean selectAllColumns) { | ||
| return null; |
There was a problem hiding this comment.
nit: Might be good to throw a NotImplemented exception here.
| return (List<PolarisDatabaseEntity>) dbRepo.getAllDatabases(dbNamePrefix, sort, pageSize, true); | ||
| final int pageSize, | ||
| final boolean isAuroraEnabled) { | ||
| return (List<PolarisDatabaseEntity>) |
There was a problem hiding this comment.
nit: I'd recommend adding a function like getDBRepo(isAuroraEnabled) and getTblRepo(isAuroraEnabled) that return BasePolarisDatabaseReplicaRepository and BasePolarisTableReplicaRepository respectively. Then you can do the ternary operator there and return the appropriate value and just call getAllDatabases on the return value. It might simplify the code a little bit.
rebase wip wip wip Asda Asda Asda asda asda asda asda asda asda asda asda asda asda asda asda asda asda asda asda asda asda asda asda Adas Adas Adas Adas Adas Adas Adas wip wip wip Adas wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip wip
yingjianwu98
force-pushed
the
yingjianw/auroraJDBC
branch
from
be30382 to
3ac4408
Compare
Set Up Aurora Storage for Metacat
Overview
This PR sets up Aurora storage for Metacat, introducing several key changes to ensure compatibility and performance.
Changes
JDBC Template for Aurora Reader
entityManagerdoesn't support twoentityManagerspointing to the same entity, where it looks like it is always the primary entityManager will be used, which means the replica won't be used in this case.entityManageris sufficient, usingread_followerin SQL queries to route to replicas.metacat-connector-polaris.Aurora Compatibility and Control Flag
metacat-common-server/src.Functional Test Refactoring
functionalTestundermetacat-connector-polaris.Functional Tests for CRDB and Aurora
metacat-functional-tests.Some minor improvements to allow test report to be downloadable for debugging purposes