-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[BUG][Connector-V2][Http] fix bug http config no schema option and improve e2e test add case #5939
Conversation
...ttp-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSource.java
Show resolved
Hide resolved
@@ -67,7 +67,7 @@ public class HttpConfig { | |||
public static final Option<ResponseFormat> FORMAT = | |||
Options.key("format") | |||
.enumType(ResponseFormat.class) | |||
.defaultValue(ResponseFormat.JSON) | |||
.noDefaultValue() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should not change the default value of format
. Because it only worked when user configure Http source with schema
config. Please refer https://github.com/apache/seatunnel/pull/5939/files#diff-7c704c6a3927123b31b41483102f286339da3832309810496f19410c2548f06fR118-R150
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I see. How about add new ResponseFormat TEXT
? Then set it as default value. It used when no schema config. Also please update the doc. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done PTAL @Hisoka-X
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @jackyyyyyssss
…prove e2e test add case (apache#5939)
…prove e2e test add case (apache#5939)
Purpose of this pull request
fix bug http config no schema option and improve e2e test add case cover
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide
release-note
.