Dev 1.1.15 webank streamis fink load yaml #303
Merged
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
yangwenzea
changed the base branch from
dev-1.1.16-webank
to
dev-1.1.15-webank
Alexkun
changed the title
casionone
reviewed
Oct 8, 2023
| @@ -232,6 +242,96 @@ class FlinkEngineConnFactory extends MultiExecutorEngineConnFactory with Logging | |||
| context | |||
| } | |||
|
|
|||
| protected def getExtractJavaOpts(envJavaOpts: String): String = { | |||
There was a problem hiding this comment.
调整为private 并添加单测
单测使用org.junit.jupiter.api.Assertions.assertEquals 来断言判断
单测目录应该和src/main 同级 即 src/test
casionone
reviewed
Oct 8, 2023
| val yamlContent = source.mkString | ||
| val yaml = new Yaml() | ||
| val configMap = yaml.loadAs(yamlContent, classOf[util.LinkedHashMap[String, Object]]) | ||
| if (configMap.containsKey("env.java.opts")) { |
casionone
reviewed
Oct 8, 2023
| } | ||
|
|
||
|
|
||
| protected def mergeAndDeduplicate(str1: String, str2: String): String = { |
There was a problem hiding this comment.
补充必要的注解 必要的地方可以加示例
字符命名优化 str1 和str2 这种不好理解
casionone
reviewed
Oct 8, 2023
| val merged = mergeAndDeduplicate(defaultJavaOpts, envJavaOpts) | ||
| } | ||
|
|
||
| protected def mergeAndDeduplicate(str1: String, str2: String): String = { |
There was a problem hiding this comment.
这里重复定义funtion 的话 没有验证到实际的代码块 且后续修改需要两个地方同步修改，考虑修改为private
casionone
reviewed
Oct 8, 2023
| def testMergeAndDeduplicate: Unit = { | ||
| var defaultJavaOpts = "-Da=3 -Db=4 -XXc=5 -Dk=a1=b"; | ||
| var envJavaOpts = "-DjobName=0607_1 -Dlog4j.configuration=./log4j.properties -Da=1 -Dk=a1=c"; | ||
| val merged = mergeAndDeduplicate(defaultJavaOpts, envJavaOpts) |
casionone
reviewed
Oct 11, 2023
| val yaml = new Yaml() | ||
| val configMap = yaml.loadAs(yamlContent, classOf[util.LinkedHashMap[String, Object]]) | ||
| if (configMap.containsKey("env.java.opts")) { | ||
| defaultJavaOpts = configMap.get("env.java.opts").toString |
casionone
approved these changes
Oct 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What is the purpose of the change
Support param 'env.java.opts' in default flink config
Brief change log
Checklist