[Multi-Database Support][postgre] add postgre sql and docs #4782
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #4782 +/- ##
============================================
+ Coverage 47.26% 47.29% +0.03%
+ Complexity 1661 1660 -1
============================================
Files 346 346
Lines 10683 10683
Branches 1063 1063
============================================
+ Hits 5049 5053 +4
+ Misses 5327 5322 -5
- Partials 307 308 +1 see 5 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
nobodyiam
reviewed
Mar 9, 2023
| /* # CREATE apolloconfigdb; */ | ||
| /* # ------------------------------------------------------------ */ | ||
|
|
||
| CREATE DATABASE "ApolloPortalDB"; |
There was a problem hiding this comment.
It looks like Postgres is very different from MySQL, which would make future contributions harder.
I'm thinking how about we abstract a storage layer this time and move the Postgres support to a new git repository, e.g. apollo-storage-extensions? The apollo-storage-extensions repository would target a specific version of apollo and doesn't have to be updated every time a feature is added. It would instead be updated by some Postgres user from time to time when they think is necessary.
cc @apolloconfig/committers what do you think?
There was a problem hiding this comment.
I humbly suggest that we create a separate folder for now, as we are not entirely clear on which boundaries should be placed between multiple databases in the main code and extensions. My recommendation is that we start by placing PostgreSQL and other databases in a separate folder. Once we have successfully achieved compatibility with one or two databases, we can then begin to separate the repository.
cc @apolloconfig/committers
| @@ -248,3 +248,118 @@ Apollo Config Demo. Please input key to get the value. Input quit to exit. | |||
| app.id=你的appId | |||
| ``` | |||
| 运行`./demo.sh client`启动Demo客户端即可。 | |||
|
|
|||
There was a problem hiding this comment.
what's the default database for quick start?
There was a problem hiding this comment.
@nobodyiam I think it will be mysql, since we haven't finish h2 feature.
There was a problem hiding this comment.
If mysql is the default, then section 5.2 is not necessary?
shoothzj
force-pushed
the
postgre-support
branch
2 times, most recently
from
41c65a9 to
7d2227a
Compare
|
I'm not familiar with postgre but when I tried to import the sql files, some errors occurred: |
nobodyiam
reviewed
Mar 10, 2023
docs/zh/deployment/quick-start.md
Outdated
| |-------------|---------|-------|--------------------| | ||
| | 1 | timeout | 100 | sample timeout配置 | | ||
|
|
||
| ## 5.3 配置其他数据库连接信息 |
There was a problem hiding this comment.
We need to list detailed instructions to guide the users on how the change the demo.sh, e.g. how to change the spring_profiles_group_github.
There was a problem hiding this comment.
This is the instructions to enable postgre, so it's better to merge it to 5.1
shoothzj
force-pushed
the
postgre-support
branch
2 times, most recently
from
52a1d7f to
0703020
Compare
nobodyiam
requested changes
Mar 11, 2023
docs/zh/deployment/quick-start.md
Outdated
| |-------------|---------|-------|--------------------| | ||
| | 1 | timeout | 100 | sample timeout配置 | | ||
|
|
||
| ## 5.3 配置其他数据库连接信息 |
There was a problem hiding this comment.
This is the instructions to enable postgre, so it's better to merge it to 5.1
docs/zh/deployment/quick-start.md
Outdated
|
|
||
| ``` | ||
|
|
||
| ## 5.4 配置mysql数据库连接信息 |
There was a problem hiding this comment.
This is the instructions to enable mysql, so it's better to merge it to 5.2
|
BTW, apollo could start with pg now, but the following errors occurred when I tried to log in. |
Co-authored-by: Jason Song <nobodyiam@gmail.com>
Co-authored-by: Jason Song <nobodyiam@gmail.com>
shoothzj
force-pushed
the
postgre-support
branch
from
8b7b28e to
18d4eb6
Compare
5 tasks
nobodyiam
requested changes
Mar 15, 2023
| VALUES ('apollo', 'ROLE_user'); | ||
|
|
||
| -- spring session (https://github.com/spring-projects/spring-session/blob/main/spring-session-jdbc/src/main/resources/org/springframework/session/jdbc/schema-postgresql.sql) | ||
| CREATE TABLE apolloportal.SPRING_SESSION |
There was a problem hiding this comment.
Is it better to quote the table name just like we did for other tables?
There was a problem hiding this comment.
It's copied from spring-session repo. I suggest we not change this. And spring-session connect to lower-case table.
| CREATE INDEX SPRING_SESSION_IX2 ON apolloportal.SPRING_SESSION (EXPIRY_TIME); | ||
| CREATE INDEX SPRING_SESSION_IX3 ON apolloportal.SPRING_SESSION (PRINCIPAL_NAME); | ||
|
|
||
| CREATE TABLE apolloportal.SPRING_SESSION_ATTRIBUTES |
There was a problem hiding this comment.
Is it better to quote the table name just like we did for other tables?
There was a problem hiding this comment.
It's copied from spring-session repo. I suggest we not change this. And spring-session connect to lower-case table.
Co-authored-by: Jason Song <nobodyiam@gmail.com>
Are we considering put these jpa sql in one repo. Like apollo-storage-rds? |
Yes, I'm seriously considering creating a new repo to host schema/codes for different storages. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 14 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Brief changelog
Add postgre sql and docs
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.