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

[Multi-Database Support] Introduce h2 postgre profile properties to let user config database config #4766

Merged

Conversation

shoothzj
Copy link
Member

@shoothzj shoothzj commented Mar 3, 2023

Brief changelog

Introduce h2 postgre profile properties to let user config database config

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@codecov
Copy link

codecov bot commented Mar 3, 2023

Codecov Report

Merging #4766 (9df198f) into master (9bed634) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #4766      +/-   ##
============================================
- Coverage     47.26%   47.21%   -0.05%     
- Complexity     1657     1658       +1     
============================================
  Files           346      346              
  Lines         10683    10683              
  Branches       1063     1063              
============================================
- Hits           5049     5044       -5     
- Misses         5328     5331       +3     
- Partials        306      308       +2     
Impacted Files Coverage Δ
...oServiceRegistryDeregisterApplicationListener.java 30.00% <0.00%> (-45.00%) ⬇️
...mework/apollo/openapi/service/ConsumerService.java 58.46% <0.00%> (+1.53%) ⬆️
...framework/apollo/openapi/entity/ConsumerAudit.java 48.48% <0.00%> (+6.06%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shoothzj shoothzj force-pushed the introduce-h2-posrgre-properties branch from 6a82605 to b15465c Compare March 3, 2023 04:52
@nobodyiam
Copy link
Member

It looks like the content is the same among h2/postgres/mysql, I'm thinking if it's possible to leave the datasource config in application-github.properties and config database specific properties in application-mysql.properties/application-h2.properties/application-postgres.properties?

@shoothzj
Copy link
Member Author

shoothzj commented Mar 3, 2023

@nobodyiam Yes we can, But that would require udpated users add a mysql profile.
This is also a viable direction, I think we will eventually rename the github profile, give it a proper name.

  • If we move database specific properties out, that's jpa
  • If we keep database specific properties still in here, that's mysql

@nobodyiam
Copy link
Member

I have given some thoughts on this and here are my suggestions.

  1. Keep the github profile
    The github profile was designed to distinguish the internal version and the github version, the major difference is the datasource configuration part. So that's why the only configurations in the file are spring.datasource.*.
    Since Apollo is now widely deployed in many production sites, it's better we keep the github profile so that existing users could upgrade seamlessly, e.g. they could keep the previous startup scripts and only update the executables.

Another benefit is the current documentation could be reused, we don't need to modify the previous instructions.

  1. Separate data source connection configuration and database specific configuration
    I suppose the data source connection configuration is the same for mysql/h2/postgres and since the user needs to change them, so we leave it in the application-github.properties.
# DataSource
spring.datasource.url = ${spring_datasource_url}
spring.datasource.username = ${spring_datasource_username}
spring.datasource.password = ${spring_datasource_password}

However, other configurations like init sql could be different for different databases and normally users don't need to change them, so we put them in database specific properties, e.g. application-mysql.properties

# Configurations for MySQL
spring.datasource.hikari.connection-init-sql = SET NAMES utf8mb4
  1. Database default to MySQL
    It's important to make Apollo backward compatible and reduce the upgrading effort, so it's necessary to make the MySQL as the default database. To achieve this, we could configure the github profile default to include the mysql profile in the application.properties and document the steps about how to enable use h2 or postgres.
# You may change the following config to activate different database profiles like h2/postgres
spring.profiles.group.github = mysql

@shoothzj shoothzj force-pushed the introduce-h2-posrgre-properties branch from b15465c to a3084b2 Compare March 4, 2023 03:02
@shoothzj
Copy link
Member Author

shoothzj commented Mar 4, 2023

@nobodyiam spring.profiles.group.github is cool!!! Fixed

@shoothzj shoothzj force-pushed the introduce-h2-posrgre-properties branch from 62e1bb1 to 5163e75 Compare March 4, 2023 07:10
@shoothzj
Copy link
Member Author

shoothzj commented Mar 4, 2023

@nobodyiam Sorry, maybe I need a cup of coffee. The quality of the patch I was working on earlier was poor, and I was a bit distracted. But it's fixed now.

@shoothzj shoothzj force-pushed the introduce-h2-posrgre-properties branch 2 times, most recently from fc04f5f to 3ec58d8 Compare March 4, 2023 07:15
@shoothzj shoothzj force-pushed the introduce-h2-posrgre-properties branch from 3ec58d8 to 9df198f Compare March 4, 2023 07:16
Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nobodyiam nobodyiam merged commit 2f311ae into apolloconfig:master Mar 4, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 4, 2023
@shoothzj shoothzj deleted the introduce-h2-posrgre-properties branch March 4, 2023 07:37
@nobodyiam nobodyiam added this to the 2.2.0 milestone Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants