You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by GitBox <gi...@apache.org> on 2022/11/04 02:03:09 UTC
[GitHub] [incubator-streampark] wolfboys opened a new pull request, #1961: [Improve] k8s mode job remapping Improvement
wolfboys opened a new pull request, #1961:
URL: https://github.com/apache/incubator-streampark/pull/1961
[Improve] k8s mode job remapping Improvement
## Contribution Checklist
- If this is your first time, please read our contributor guidelines: [Submit Code](https://streampark.apache.org/community/submit_guide/submit_code).
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/streampark/issues).
- Name the pull request in the form "[Feature] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- If the PR is unfinished, add `[WIP]` in your PR title, e.g., `[WIP][Feature] Title of the pull request`.
-->
## What changes were proposed in this pull request
Issue Number: close #xxx <!-- REMOVE this line if no issue to close -->
<!--(For example: This pull request proposed to add checkstyle plugin).-->
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
## Verifying this change
<!--*(Please pick either of the following options)*-->
This change is a trivial rework / code cleanup without any test coverage.
*(or)*
This change is already covered by existing tests, such as *(please describe tests)*.
*(or)*
This change added tests and can be verified as follows:
<!--*(example:)*
- *Added integration tests for end-to-end.*
- *Added *Test to verify the change.*
- *Manually verified the change by testing locally.* -->
## Does this pull request potentially affect one of the following parts
- Dependencies (does it add or upgrade a dependency): no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wangsizhu0504 commented on a diff in pull request #1961: [Improve] k8s mode job remapping Improvement
Posted by GitBox <gi...@apache.org>.
wangsizhu0504 commented on code in PR #1961:
URL: https://github.com/apache/incubator-streampark/pull/1961#discussion_r1013564774
##########
streampark-console/streampark-console-newui/src/views/flink/app/hooks/useApp.tsx:
##########
@@ -276,45 +276,63 @@ export const useFlinkApplication = (openStartModal: Fn) => {
'Mapping Application',
],
content: () => {
- return (
- <Form
- class="!pt-40px"
- ref={mappingRef}
- name="mappingForm"
- labelCol={{ lg: { span: 7 }, sm: { span: 7 } }}
- wrapperCol={{ lg: { span: 16 }, sm: { span: 4 } }}
- v-model:model={formValue}
- >
- <Form.Item label="Application Name">
- <Alert message={app.jobName} type="info" />
- </Form.Item>
- <Form.Item
- label="Application Id"
- name="appId"
- rules={[{ required: true, message: 'ApplicationId is required' }]}
- >
- <Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
- </Form.Item>
- <Form.Item
- label="JobId"
- name="jobId"
- rules={[{ required: true, message: 'ApplicationId is required' }]}
- >
- <Input type="text" placeholder="JobId" v-model:value={formValue.jobId} />
- </Form.Item>
- </Form>
- );
+ if ([2, 3, 4].includes(app.executionMode)) {
+ return (
+ <Form
+ class="!pt-40px"
+ ref={mappingRef}
+ name="mappingForm"
+ labelCol={{ lg: { span: 7 }, sm: { span: 7 } }}
+ wrapperCol={{ lg: { span: 16 }, sm: { span: 4 } }}
+ v-model:model={formValue}>
+ <Form.Item label="Application Name">
+ <Alert message={app.jobName} type="info" />
+ </Form.Item>
+ <Form.Item
+ label="YARN Application Id"
+ name="appId"
+ rules={[{ required: true, message: 'YARN ApplicationId is required' }]}>
+ <Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
+ </Form.Item>
Review Comment:
> The syntax of jsx is used here
There is actually a better way to write this, like this
content: () => {
return(
<Form
........
{/* What I mean here is that I render the elements behind me when I meet certain conditions */}
{[2, 3, 4].includes(app.executionMode) && (
<Form.Item
label="YARN Application Id"
name="appId"
rules={[{ required: true, message: 'YARN ApplicationId is required' }]}
>
<Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
</Form.Item>
)}
.......
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #1961: [Improve] k8s mode job remapping Improvement
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #1961:
URL: https://github.com/apache/incubator-streampark/pull/1961#discussion_r1013613168
##########
streampark-console/streampark-console-newui/src/views/flink/app/hooks/useApp.tsx:
##########
@@ -276,45 +276,63 @@ export const useFlinkApplication = (openStartModal: Fn) => {
'Mapping Application',
],
content: () => {
- return (
- <Form
- class="!pt-40px"
- ref={mappingRef}
- name="mappingForm"
- labelCol={{ lg: { span: 7 }, sm: { span: 7 } }}
- wrapperCol={{ lg: { span: 16 }, sm: { span: 4 } }}
- v-model:model={formValue}
- >
- <Form.Item label="Application Name">
- <Alert message={app.jobName} type="info" />
- </Form.Item>
- <Form.Item
- label="Application Id"
- name="appId"
- rules={[{ required: true, message: 'ApplicationId is required' }]}
- >
- <Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
- </Form.Item>
- <Form.Item
- label="JobId"
- name="jobId"
- rules={[{ required: true, message: 'ApplicationId is required' }]}
- >
- <Input type="text" placeholder="JobId" v-model:value={formValue.jobId} />
- </Form.Item>
- </Form>
- );
+ if ([2, 3, 4].includes(app.executionMode)) {
+ return (
+ <Form
+ class="!pt-40px"
+ ref={mappingRef}
+ name="mappingForm"
+ labelCol={{ lg: { span: 7 }, sm: { span: 7 } }}
+ wrapperCol={{ lg: { span: 16 }, sm: { span: 4 } }}
+ v-model:model={formValue}>
+ <Form.Item label="Application Name">
+ <Alert message={app.jobName} type="info" />
+ </Form.Item>
+ <Form.Item
+ label="YARN Application Id"
+ name="appId"
+ rules={[{ required: true, message: 'YARN ApplicationId is required' }]}>
+ <Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
+ </Form.Item>
Review Comment:
> > The syntax of tsx is used here
>
> There is actually a better way to write this, like this
>
> ```
> content: () => {
> return(
> <Form
> ........
> {/* What I mean here is that I render the elements behind me when I meet certain conditions */}
> {[2, 3, 4].includes(app.executionMode) && (
> <Form.Item
> label="YARN Application Id"
> name="appId"
> rules={[{ required: true, message: 'YARN ApplicationId is required' }]}
> >
> <Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
> </Form.Item>
> )}
> .......
> ```
thx, done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] MonsterChenzhuo merged pull request #1961: [Improve] k8s mode job remapping Improvement
Posted by GitBox <gi...@apache.org>.
MonsterChenzhuo merged PR #1961:
URL: https://github.com/apache/incubator-streampark/pull/1961
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on pull request #1961: [Improve] k8s mode job remapping Improvement
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #1961:
URL: https://github.com/apache/incubator-streampark/pull/1961#issuecomment-1302866877
cc @MonsterChenzhuo PTAL, thx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wangsizhu0504 commented on a diff in pull request #1961: [Improve] k8s mode job remapping Improvement
Posted by GitBox <gi...@apache.org>.
wangsizhu0504 commented on code in PR #1961:
URL: https://github.com/apache/incubator-streampark/pull/1961#discussion_r1013564774
##########
streampark-console/streampark-console-newui/src/views/flink/app/hooks/useApp.tsx:
##########
@@ -276,45 +276,63 @@ export const useFlinkApplication = (openStartModal: Fn) => {
'Mapping Application',
],
content: () => {
- return (
- <Form
- class="!pt-40px"
- ref={mappingRef}
- name="mappingForm"
- labelCol={{ lg: { span: 7 }, sm: { span: 7 } }}
- wrapperCol={{ lg: { span: 16 }, sm: { span: 4 } }}
- v-model:model={formValue}
- >
- <Form.Item label="Application Name">
- <Alert message={app.jobName} type="info" />
- </Form.Item>
- <Form.Item
- label="Application Id"
- name="appId"
- rules={[{ required: true, message: 'ApplicationId is required' }]}
- >
- <Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
- </Form.Item>
- <Form.Item
- label="JobId"
- name="jobId"
- rules={[{ required: true, message: 'ApplicationId is required' }]}
- >
- <Input type="text" placeholder="JobId" v-model:value={formValue.jobId} />
- </Form.Item>
- </Form>
- );
+ if ([2, 3, 4].includes(app.executionMode)) {
+ return (
+ <Form
+ class="!pt-40px"
+ ref={mappingRef}
+ name="mappingForm"
+ labelCol={{ lg: { span: 7 }, sm: { span: 7 } }}
+ wrapperCol={{ lg: { span: 16 }, sm: { span: 4 } }}
+ v-model:model={formValue}>
+ <Form.Item label="Application Name">
+ <Alert message={app.jobName} type="info" />
+ </Form.Item>
+ <Form.Item
+ label="YARN Application Id"
+ name="appId"
+ rules={[{ required: true, message: 'YARN ApplicationId is required' }]}>
+ <Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
+ </Form.Item>
Review Comment:
> The syntax of tsx is used here
There is actually a better way to write this, like this
content: () => {
return(
<Form
........
{/* What I mean here is that I render the elements behind me when I meet certain conditions */}
{[2, 3, 4].includes(app.executionMode) && (
<Form.Item
label="YARN Application Id"
name="appId"
rules={[{ required: true, message: 'YARN ApplicationId is required' }]}
>
<Input type="text" placeholder="ApplicationId" v-model:value={formValue.appId} />
</Form.Item>
)}
.......
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org