You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by "Al-assad (via GitHub)" <gi...@apache.org> on 2023/07/31 16:28:02 UTC
[GitHub] [incubator-streampark] Al-assad opened a new pull request, #2916: [WIP] [Flink-K8s-V2] Port the new module codes to Streampark-flink-kubernetes-v2
Al-assad opened a new pull request, #2916:
URL: https://github.com/apache/incubator-streampark/pull/2916
## What changes were proposed in this pull request
This is a **WIP** PR.
Issue Number: close #2881 <!-- REMOVE this line if no issue to close -->
<!--(For example: This pull request proposed to add checkstyle plugin).-->
<!--*(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): (yes / 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] Al-assad commented on a diff in pull request #2916: [Flink-K8s-V2] Port the new module codes to Streampark-flink-kubernetes-v2
Posted by "Al-assad (via GitHub)" <gi...@apache.org>.
Al-assad commented on code in PR #2916:
URL: https://github.com/apache/incubator-streampark/pull/2916#discussion_r1284019489
##########
streampark-common/pom.xml:
##########
@@ -128,6 +128,32 @@
<version>${streampark.shaded.version}</version>
</dependency>
+ <!-- ZIO -->
+ <dependency>
Review Comment:
The interfaces exposed by streampark-flink-kubernetes are all zio or zstream structures, and external calls must refer to zio. I'll be writing more zio code in other modules later on.ZIO is a basic, pure-scala, zero-dependency library, so placing it in comon doesn't seem like a problem.
--
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 #2916: [Flink-K8s-V2] Port the new module codes to Streampark-flink-kubernetes-v2
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on PR #2916:
URL: https://github.com/apache/incubator-streampark/pull/2916#issuecomment-1665057434
Overall, it looks good, but there are still some minor issues. We can merge this pull request first and improve it later.
--
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 #2916: [Flink-K8s-V2] Port the new module codes to Streampark-flink-kubernetes-v2
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2916:
URL: https://github.com/apache/incubator-streampark/pull/2916#discussion_r1283439679
##########
streampark-common/pom.xml:
##########
@@ -128,6 +128,32 @@
<version>${streampark.shaded.version}</version>
</dependency>
+ <!-- ZIO -->
+ <dependency>
Review Comment:
"common" is a public module that many other modules depend on. It is not a good idea to add ZIO dependency in the common module.
--
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 merged pull request #2916: [Flink-K8s-V2] Port the new module codes to Streampark-flink-kubernetes-v2
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys merged PR #2916:
URL: https://github.com/apache/incubator-streampark/pull/2916
--
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