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

[improve] update activemq connector config option #8580

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

liunaijie
Copy link
Member

Purpose of this pull request

subtask of #8576

Does this PR introduce any user-facing change?

How was this patch tested?

Check list


private final Map<String, Object> sinkOptionProps = new HashMap<>();

public static final Option<String> HOST =
Copy link
Member Author

Choose a reason for hiding this comment

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

Those parameters are defined, but not used. So I delete them.

@Hisoka-X
Copy link
Member

I think we should add a test case to verify the all connector has xxxSinkOptions or xxxSourceOptions.

@liunaijie
Copy link
Member Author

I think we should add a test case to verify the all connector has xxxSinkOptions or xxxSourceOptions.

Good point, I will write the test.

@@ -42,11 +40,6 @@ public void write(SeaTunnelRow element) {
activeMQClient.write(serializationSchema.serialize(element));
}

@Override
Copy link

@akulabs8 akulabs8 Jan 27, 2025

Choose a reason for hiding this comment

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

why did you remove this entire bit of code for optional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants