[GitHub] [flink-connector-pulsar] tisonkun commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


tisonkun commented on code in PR #45:
URL: 
https://github.com/apache/flink-connector-pulsar/pull/45#discussion_r1186792428


##
flink-connector-pulsar/archunit-violations/f4d91193-72ba-4ce4-ad83-98f780dce581:
##
@@ -10,3 +10,15 @@ org.apache.flink.connector.pulsar.source.PulsarSourceITCase 
does not satisfy: on
 * reside in a package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class InternalMiniClusterExtension\
 * reside outside of package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class MiniClusterExtension\
  or contain any fields that are public, static, and of type 
MiniClusterWithClientResource and final and annotated with @ClassRule or 
contain any fields that is of type MiniClusterWithClientResource and public and 
final and not static and annotated with @Rule
+org.apache.flink.connector.pulsar.sink.PulsarSinkITCase does not satisfy: only 
one of the following predicates match:\

Review Comment:
   > I thought about it again, and feel a bit that maybe FLINK-31804 should be 
merged into 1.16 and 1.17. Because to some extent, this (without considering 
the MiniClusterTestEnvironment) is a bug for ITCaseRules. If it were backported 
to these versions, there is no need to duplicate it. 🤔 WDYT?
   
   No need to duplicate. But still need to update the message.
   
   I'd prefer to duplicate for now so that we can capture other failure in 
daily CI if any. We can always revert the duplicate when FLINK-31804 properly 
backport, instead of waiting here and letting the daily CI fails fast and 
conceal other possible issues.



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [flink-connector-pulsar] tisonkun commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


tisonkun commented on code in PR #45:
URL: 
https://github.com/apache/flink-connector-pulsar/pull/45#discussion_r1186784205


##
flink-connector-pulsar/archunit-violations/f4d91193-72ba-4ce4-ad83-98f780dce581:
##
@@ -10,3 +10,15 @@ org.apache.flink.connector.pulsar.source.PulsarSourceITCase 
does not satisfy: on
 * reside in a package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class InternalMiniClusterExtension\
 * reside outside of package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class MiniClusterExtension\
  or contain any fields that are public, static, and of type 
MiniClusterWithClientResource and final and annotated with @ClassRule or 
contain any fields that is of type MiniClusterWithClientResource and public and 
final and not static and annotated with @Rule
+org.apache.flink.connector.pulsar.sink.PulsarSinkITCase does not satisfy: only 
one of the following predicates match:\

Review Comment:
   @reswqa Thanks for pointing out the source code! Perhaps I can take a look 
when I have free time (I do _not_ promise it, lol).
   
   Do you think it's reasonable we duplicate a bit the exclusion to suppress 
failing daily workflows?



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [flink-connector-pulsar] tisonkun commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


tisonkun commented on code in PR #45:
URL: 
https://github.com/apache/flink-connector-pulsar/pull/45#discussion_r1186780749


##
flink-connector-pulsar/archunit-violations/f4d91193-72ba-4ce4-ad83-98f780dce581:
##
@@ -10,3 +10,15 @@ org.apache.flink.connector.pulsar.source.PulsarSourceITCase 
does not satisfy: on
 * reside in a package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class InternalMiniClusterExtension\
 * reside outside of package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class MiniClusterExtension\
  or contain any fields that are public, static, and of type 
MiniClusterWithClientResource and final and annotated with @ClassRule or 
contain any fields that is of type MiniClusterWithClientResource and public and 
final and not static and annotated with @Rule
+org.apache.flink.connector.pulsar.sink.PulsarSinkITCase does not satisfy: only 
one of the following predicates match:\

Review Comment:
   > I guess the violation in PulsarSinkITCase caused by non static field 
private final MiniClusterExtension.
   
   Yes. But the actually nested test cases have. I don't think we should tweak 
to bypass the check..



##
flink-connector-pulsar/archunit-violations/f4d91193-72ba-4ce4-ad83-98f780dce581:
##
@@ -10,3 +10,15 @@ org.apache.flink.connector.pulsar.source.PulsarSourceITCase 
does not satisfy: on
 * reside in a package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class InternalMiniClusterExtension\
 * reside outside of package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class MiniClusterExtension\
  or contain any fields that are public, static, and of type 
MiniClusterWithClientResource and final and annotated with @ClassRule or 
contain any fields that is of type MiniClusterWithClientResource and public and 
final and not static and annotated with @Rule
+org.apache.flink.connector.pulsar.sink.PulsarSinkITCase does not satisfy: only 
one of the following predicates match:\

Review Comment:
   > I guess the violation in PulsarSinkITCase caused by non static field 
private final MiniClusterExtension.
   
   Yes. But the actual nested test cases have. I don't think we should tweak to 
bypass the check..



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [flink-connector-pulsar] tisonkun commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


tisonkun commented on code in PR #45:
URL: 
https://github.com/apache/flink-connector-pulsar/pull/45#discussion_r1186769978


##
flink-connector-pulsar/archunit-violations/f4d91193-72ba-4ce4-ad83-98f780dce581:
##
@@ -10,3 +10,15 @@ org.apache.flink.connector.pulsar.source.PulsarSourceITCase 
does not satisfy: on
 * reside in a package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class InternalMiniClusterExtension\
 * reside outside of package 'org.apache.flink.runtime.*' and is annotated with 
@ExtendWith with class MiniClusterExtension\
  or contain any fields that are public, static, and of type 
MiniClusterWithClientResource and final and annotated with @ClassRule or 
contain any fields that is of type MiniClusterWithClientResource and public and 
final and not static and annotated with @Rule
+org.apache.flink.connector.pulsar.sink.PulsarSinkITCase does not satisfy: only 
one of the following predicates match:\

Review Comment:
   @syhily I'd like to know where this check comes from so that I can 
investigate to avoid further conflict.
   
   That is - where is the source code of archunit violation check.



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [flink-connector-pulsar] tisonkun commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


tisonkun commented on code in PR #45:
URL: 
https://github.com/apache/flink-connector-pulsar/pull/45#discussion_r1186769897


##
.github/workflows/daily.yml:
##
@@ -21,10 +21,13 @@ on:
   schedule:
 - cron: "0 0 * * *"
   workflow_dispatch:
+  pull_request: ## !!! TODO DELETED BEFORE MERGED

Review Comment:
   ```suggestion
   ```



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [flink-connector-pulsar] tisonkun commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


tisonkun commented on code in PR #45:
URL: 
https://github.com/apache/flink-connector-pulsar/pull/45#discussion_r1186664577


##
.github/workflows/daily.yml:
##
@@ -21,10 +21,13 @@ on:
   schedule:
 - cron: "0 0 * * *"
   workflow_dispatch:
+  pull_request: ## !!! TODO DELETED BEFORE MERGED

Review Comment:
   TODO
   
   After approved I will remove it. Now let's keep it as is to show the result.



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org