-
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
[Feature] [rest-api] Support Rest Api to upload file and submit task #8442
Conversation
cc @liunaijie |
2797c7d
to
a7f340a
Compare
cc @liugddx |
HttpEntity multipart = builder.build(); | ||
uploadFile.setEntity(multipart); | ||
int statusCode = httpClient.execute(uploadFile).getStatusLine().getStatusCode(); | ||
Assertions.assertEquals(200, statusCode); |
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.
Add test cases in ClusterSeaTunnelEngineContainer
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, thank you for your suggestion
@@ -89,4 +92,7 @@ public class RestConstant { | |||
public static final String REST_URL_OPEN_METRICS = "/openmetrics"; | |||
// api path end | |||
|
|||
// metrics |
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.
This is useless, please delete
@@ -65,6 +75,8 @@ public class RestApiIT { | |||
|
|||
private static final String HOST = "http://localhost:"; | |||
|
|||
private static final Integer PORT = 8080; |
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.
we can use node1Config.getEngineConfig().getHttpConfig().getPort()
to get port, if hardcode here and config updated, the test will be fail.
ConfigFactory.parseString( | ||
content, ConfigParseOptions.defaults().setSyntax(ConfigSyntax.JSON)); | ||
} else { | ||
config = ConfigFactory.parseString(content); |
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.
Now we only support json and conf file, can we add file name suffix check here, if not conf and json config, throw exception.
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.
Now we only support json and conf file, can we add file name suffix check here, if not conf and json config, throw exception.
Okay, thank you for your suggestion. My idea is that the configuration file format supports JSON or HOCON, so the configuration file suffix only needs to distinguish JSON from others, such as. conf, config, etc. This type of content can be parsed as long as it is HOCON, rather than limiting the. conf suffix
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.
how about add a parameters named format
? Just like submit job api? Please refer https://github.com/apache/seatunnel/blob/dev/docs/en/seatunnel-engine/rest-api-v2.md#parameters-6
Also please add document about this feature, thanks. |
@liugddx hello,Please have time to help continue reviewing |
070082f
to
6772a51
Compare
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.
+1
Purpose of this pull request
close #8246
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide
release-note
.