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

[BugFix][FlinkSQL] V1.1.0 version Keberos bug related repair, SQL SET value does not take effect, etc #3875

Merged
merged 24 commits into from
Nov 12, 2024

Conversation

chenhaipeng
Copy link
Contributor

@chenhaipeng chenhaipeng commented Oct 17, 2024

Fix the following issues:

Feature:

  1. Added getJobInstanceListapi in APIController.
  2. table.topic.map, using topic:tableName,eg: topic1:tableA;topic2:tableB,tableC, can reduce the creation of topics.

Fix:

  1. FlinkAPI Kerberos might start with http or https.
  2. In ExecutorConfig, if the address starts with http://xx, the splitting is incorrect.
  3. Fixed value passing issue in KafkaSinkBuilder, where DeliveryGuarantee was always set to EXACTLY_ONCE because the default submission is in Dinky.
  4. Fixed the issue where the environment variable set 'xxxx' in MysqlCDCBuilder SQL did not take effect.
  5. Fixed the redirection issue in JobOperator.tsx.

Pending:

#3874 (comment)
PS: Thanks to the community for contributing such a useful tool!

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

1.APIController 新增getJobInstanceListapi
2.table.topic.map , 以topic:tableName ,可以减少topic 创建
fix:
1. FlinkAPI kerberos 可能以http或者https 开头
2. ExecutorConfig , address 如果是http://xx分割不对
3. KafkaSinkBuilder 传值问题
4. JobOperator.tsx 跳转问题
@github-actions github-actions bot changed the title v1.10 版本keberos bug 修复, v1.10 version keberos bug fixed, Oct 17, 2024
@chenhaipeng chenhaipeng changed the title v1.10 version keberos bug fixed, v1.10 版本keberos bug 相关修复 Oct 17, 2024
@chenhaipeng chenhaipeng changed the title v1.10 版本keberos bug 相关修复 v1.10 版本keberos bug 相关修复, sql set 值不生效等问题 Oct 17, 2024
@chenhaipeng chenhaipeng changed the title v1.10 版本keberos bug 相关修复, sql set 值不生效等问题 v1.1.0 版本keberos bug 相关修复, sql set 值不生效等问题 Oct 17, 2024
@chenhaipeng chenhaipeng changed the title v1.1.0 版本keberos bug 相关修复, sql set 值不生效等问题 V1.1.0 version Keberos bug related repair, SQL SET value does not take effect, etc. #3875 Oct 17, 2024
@@ -587,4 +590,26 @@ protected List<String> getPKList(Table table) {
protected ZoneId getSinkTimeZone() {
return this.sinkTimeZone;
}

protected Map<String, String> getTableTopicMap() {
Copy link
Contributor

Choose a reason for hiding this comment

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

不应该在 dinky-cdc/dinky-cdc-core/src/main/java/org/dinky/cdc/AbstractSinkBuilder.java 中写此方法 因为不属于公共抽象方法 建议下沉到 kafkasinkbuilder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

有好几个KakfaSinkBuilder, KafkaSinkJsonBuilder , 不放这里需要写好几次

Copy link
Contributor

Choose a reason for hiding this comment

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

Add AbstractKafkaSinkBuilder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add AbstractKafkaSinkBuilder.

it just a single method , Is it necessary to add an AbstractKafkaSinkBuilder class?

Copy link
Contributor

Choose a reason for hiding this comment

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

If sinks other than kafka use this method, your current code logic is feasible.

Copy link
Contributor

@aiwenmo aiwenmo left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request.

Comment on lines +134 to +135
} else {
properties.setProperty(entry.getKey(), entry.getValue());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why set other keys that do not start with an attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because there's no place in the constructor of kafkaSinkBuilder to set properties.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Thx. I get it.

@aiwenmo
Copy link
Contributor

aiwenmo commented Oct 23, 2024

#3874 will be fixed by #3889

@Zzm0809
Copy link
Contributor

Zzm0809 commented Nov 9, 2024

@chenhaipeng 你好, 请合并主仓库最新代码 解决 CI 过程出现的问题, 并将您的 Pr title 进行简单规范, 再次感谢您的贡献

@chenhaipeng
Copy link
Contributor Author

@chenhaipeng 你好, 请合并主仓库最新代码 解决 CI 过程出现的问题, 并将您的 Pr title 进行简单规范, 再次感谢您的贡献

OK

Copy link
Contributor

@aiwenmo aiwenmo left a comment

Choose a reason for hiding this comment

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

LGTM

donotcoffee and others added 9 commits November 12, 2024 16:27
…ble.topic.map, using topic:tableName,eg: topic1:tableA;topic2:tableB,tableC, can reduce the creation of topics.

[Fix] 1.FlinkAPI Kerberos might start with http or https. 2.In ExecutorConfig, if the address starts with http://xx, the splitting is incorrect.
Fixed value passing issue in KafkaSinkBuilder, whereDeliveryGuarantee was always set to EXACTLY_ONCE because the default submission is in Dinky. 3.Fixed the issue where the environment variable set 'xxxx' in MysqlCDCBuilder SQL did not take effect. 4.Fixed the redirection issue in JobOperator.tsx.
gaoyan1998 and others added 12 commits November 12, 2024 16:47
…e the user experience (DataLinkDC#3854)

Signed-off-by: Zzm0809 <[email protected]>
Co-authored-by: zackyoungh <[email protected]>
Co-authored-by: Zzm0809 <[email protected]>
Co-authored-by: gaoyan <[email protected]>
Co-authored-by: gaoyan1998 <[email protected]>
Co-authored-by: zhangyuhang <[email protected]>
Co-authored-by: LUOSHANGJIE\71826 <[email protected]>
Co-authored-by: 18216499322 <[email protected]>
Co-authored-by: Zzm0809 <[email protected]>
@chenhaipeng
Copy link
Contributor Author

@chenhaipeng 你好, 请合并主仓库最新代码 解决 CI 过程出现的问题, 并将您的 Pr title 进行简单规范, 再次感谢您的贡献

我已经重新合并了,pr comit title 简单修改了

@zackyoungh zackyoungh changed the title V1.1.0 version Keberos bug related repair, SQL SET value does not take effect, etc. #3875 [BugFix][FlinkSQL] V1.1.0 version Keberos bug related repair, SQL SET value does not take effect, etc Nov 12, 2024
@zackyoungh zackyoungh added the Bug Something isn't working label Nov 12, 2024
@zackyoungh zackyoungh added this to the 1.2.0 milestone Nov 12, 2024
@zackyoungh zackyoungh merged commit 237b45a into DataLinkDC:dev Nov 12, 2024
17 checks passed
@@ -65,6 +65,8 @@ public void run() throws Exception {
}
GatewayResult gatewayResult = null;
config.addGatewayConfig(executor.getSetConfig());
config.addGatewayConfig(
executor.getCustomTableEnvironment().getConfig().getConfiguration());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

flink sql set 参数不生效,我加了这一行放到这里是否合理,麻烦确认下

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] [HIVE] Did not correctly interpret the Hive table creation statement, but the older version 0.7.1 works.
10 participants