Skip to content

Conversation

@fcb-xiaobo
Copy link
Contributor

Purpose of this pull request

image
When the host parameter is not filled in, the config. get method will throw a null pointer exception

Does this PR introduce any user-facing change?

fix #8841

How was this patch tested?

Check list

@Hisoka-X
Copy link
Member

Hisoka-X commented Mar 3, 2025

When the host parameter is not filled in, the config. get method will throw a null pointer exception

Could you provide the error stack?

@fcb-xiaobo
Copy link
Contributor Author

fcb-xiaobo commented Mar 3, 2025

When the host parameter is not filled in, the config. get method will throw a null pointer exception

Could you provide the error stack?

image

image

stack info
Exception in thread "main" org.apache.seatunnel.core.starter.exception.CommandExecuteException: SeaTunnel job executed failed
at org.apache.seatunnel.core.starter.seatunnel.command.ClientExecuteCommand.execute(ClientExecuteCommand.java:228)
at org.apache.seatunnel.core.starter.SeaTunnel.run(SeaTunnel.java:40)
at org.apache.seatunnel.example.engine.SeaTunnelEngineLocalExample.main(SeaTunnelEngineLocalExample.java:52)
Caused by: org.apache.seatunnel.api.table.factory.FactoryException: ErrorCode:[API-06], ErrorDescription:[Factory initialize failed] - Unable to create a source for identifier 'Redis'.
at org.apache.seatunnel.api.table.factory.FactoryUtil.restoreAndPrepareSource(FactoryUtil.java:168)
at org.apache.seatunnel.api.table.factory.FactoryUtil.createAndPrepareSource(FactoryUtil.java:86)
at org.apache.seatunnel.engine.core.parse.MultipleTableJobConfigParser.parseSource(MultipleTableJobConfigParser.java:376)
at org.apache.seatunnel.engine.core.parse.MultipleTableJobConfigParser.parse(MultipleTableJobConfigParser.java:227)
at org.apache.seatunnel.engine.client.job.ClientJobExecutionEnvironment.getLogicalDag(ClientJobExecutionEnvironment.java:123)
at org.apache.seatunnel.engine.client.job.ClientJobExecutionEnvironment.execute(ClientJobExecutionEnvironment.java:191)
at org.apache.seatunnel.core.starter.seatunnel.command.ClientExecuteCommand.execute(ClientExecuteCommand.java:165)
... 2 more
Caused by: java.lang.NullPointerException
at org.apache.seatunnel.connectors.seatunnel.redis.config.RedisParameters.buildWithConfig(RedisParameters.java:72)
at org.apache.seatunnel.connectors.seatunnel.redis.source.RedisSource.(RedisSource.java:56)
at org.apache.seatunnel.connectors.seatunnel.redis.source.RedisSourceFactory.lambda$createSource$0(RedisSourceFactory.java:45)
at org.apache.seatunnel.api.table.factory.FactoryUtil.createAndPrepareSource(FactoryUtil.java:180)
at org.apache.seatunnel.api.table.factory.FactoryUtil.restoreAndPrepareSource(FactoryUtil.java:136)
... 8 more
[] 2025-03-03 18:10:02,703 DEBUG org.apache.hadoop.util.ShutdownHookManager - ShutdownHookManger completed shutdown.
Disconnected from the target VM, address: '127.0.0.1:55053', transport: 'socket'

Process finished with exit code 1

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

@fcb-xiaobo
Copy link
Contributor Author

Maybe change port from int to Integer is a better option. https://github.com/apache/seatunnel/pull/8881/files#diff-85c72df044b54265682571cdb07752eb396fdf68965cbcf863553efa9a22b34fR49

Okay, thank you for your suggestion. After rounding it to Integer, I can run it locally

public class RedisParameters implements Serializable {
private String host;
private int port;
private Integer port;
Copy link
Member

Choose a reason for hiding this comment

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

Let's add some test case for RedisParameters.

Copy link
Member

Choose a reason for hiding this comment

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

How about update it to redis default port 6379.

Copy link
Member

Choose a reason for hiding this comment

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

How about update it to redis default port 6379.

In cluster mode it should be null.

Copy link
Member

Choose a reason for hiding this comment

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

How about update it to redis default port 6379.

In cluster mode it should be null.

In cluster moe, it won't be use
https://github.com/apache/seatunnel/pull/8841/files#diff-85c72df044b54265682571cdb07752eb396fdf68965cbcf863553efa9a22b34fR175

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's update it to 6379 as default. @fcb-xiaobo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, thank you for your suggestions. I will update the code and documentation later

@liunaijie liunaijie merged commit 7bd5865 into apache:dev Mar 12, 2025
4 checks passed
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