profiles support global env value about shardingsphere HOT 13 CLOSED

@linghengqian I've submitted a new pull request. Thanks. #29704

from shardingsphere.

Let's see if you have any other solutions to solve this problem.

• I need some time to think about the design. There are some optional symbols to replace ${}.

from shardingsphere.

• To be honest, I hit a limitation of Java itself while trying to write a unit test for System.getenv(). Of course I know that System.getenv() almost certainly serves the usage process of Docker Image.
  • @org.junitpioneer.jupiter.SetEnvironmentVariable of org.junit-pioneer:junit-pioneer:2.2.0 requires additional --add-open for maven-surefire-plugin on JDK21, which is something I hope to avoid at this stage because it involves questionable changes on the JDK22-ea milestone.
  • uk.org.webcompere:system-stubs-jupiter:2.1.6 will break ShardingSphere's unit test CI for JDK8 because it uses JDK11's bytecode.
  • com.github.stefanbirkner:system-rules:1.19.0 and com.github.stefanbirkner:system-lambda:1.2.1 will break ShardingSphere's unit test CI for JDK17+. They lack maintenance.
  • For Mockito 4, I personally don't think the method of Mockito.spy is appropriate, and doing Mockito.spy on ShardingSphereURLManager is too weird. Of course we can do this, but unit testing will look a lot more obscure.
  • It's hard for me to imagine that there will be a good way to simulate environment variables on JDK8-JDK21 before ShardingSphere improves the JDK baseline to JDK11. Yes, the runtime requirement for ShardingSphere has always been JDK8.
• Rather than using Environment Variables(System.getenv()), I personally prefer to use System Property Variables(System.getProperty()). I anticipate adjusting the final outcome in this direction. Another idea is to set Args/buildArgs directly on the ShardingSphere URL string.
• The regular expression of \$\$\{(.+?)\}$ also looks good. Although more can be restricted. Expecting a match like

dataSources:
  ds_0:
    dataSourceClassName: com.zaxxer.hikari.HikariDataSource
    driverClassName: org.h2.Driver
    jdbcUrl: $${FIXTURE_JDBC_URL::jdbc:h2:mem:foo_ds_do_not_use}
    username: sa
    password:

• I'm still organizing a PR for this issue. But my vacations are always full of fun things to distract me.

from shardingsphere.

• @xiaomizhou2 I still chose mock environment variables. I proposed another regex in #29907 .

from shardingsphere.

• After several refactorings, this became an optional JDBC URL parameter of placeholder-type. See https://shardingsphere.apache.org/document/current/en/user-manual/shardingsphere-jdbc/yaml-config/jdbc-driver/known-implementation/ .

• I'm not discouraging potential implementations of another placeholder operation, I'm just documenting how to handle custom implementations as much as possible.

• That's why the documentation mentions that you can try to manually convert from XML or JSON to YAML in a custom implementation. This requires the use of Jackson data binding.

from shardingsphere.

• This only requires reading the YAML content when implementing the org.apache.shardingsphere.driver.jdbc.core.driver.ShardingSphereURLProvider SPI, and then replacing it with system variables.

• The SPI of org.apache.shardingsphere.driver.jdbc.core.driver.ShardingSphereURLProvider provides the ability to dynamically read YAML files, but after reading the file, the configuration will still overwrite the metadata of the local or remote registry according to the configuration of Mode Config. For a simple example, refer to https://github.com/apache/shardingsphere-plugin/tree/main/jdbc%2Fdriver%2Fapollo .

• This is actually a hidden subtask of #29052 , because the integration of testcontainers-java cannot always use a fixed port, and a dynamic YAML configuration method needs to be provided.

• Considering that the Christmas holidays are not over yet, does your raising of the current issue mean that you are ready to submit a PR?

from shardingsphere.

I can try to implement these features.

from shardingsphere.

@linghengqian Hello,I have completed the handling of global variables. PR:https://github.com/apache/shardingsphere/pull/29682

from shardingsphere.

• The most embarrassing thing for me is that I will not be back from my Christmas holiday until next Saturday, so I may not have time to comment on the PR. I noticed that the SPI involved in the relevant PR lacks a document, and I may add the corresponding document in the near future.

• The PR did not pass the code formatting validation. You can execute first ./mvnw spotless:apply -Pcheck -T1C. Then execute ./mvnw checkstyle:check -Pcheck -T1C to manually adjust to repair the CI.

from shardingsphere.

Okay, I'll fix it.

from shardingsphere.

Let's see if you have any other solutions to solve this problem.

from shardingsphere.

• Fixed by #29907 and #29909.

from shardingsphere.

@linghengqian I just finished my Chinese New Year holiday.I prefer using the ENV() function to replace global variables. While ${} is already a keyword in shardingsphere, $${} can also be used, but I feel it may lead to ambiguity.

from shardingsphere.