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

2023-05-07 Thread via GitHub


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


##
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'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](https://issues.apache.org/jira/browse/FLINK-31804) properly 
backport, instead of waiting here and letting the daily CI fails fast and 
conceal other possible issues.
   
   Yes, make sense. If we decided to backport `FLINK-31804` finally, let's 
revert the duplicate.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-07 Thread via GitHub


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


##
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'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](https://issues.apache.org/jira/browse/FLINK-31804) properly 
backport, instead of waiting here and letting the daily CI fails fast and 
conceal other possible issues.
   
   Yes, make sense.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-07 Thread via GitHub


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


##
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?



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-07 Thread via GitHub


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


##
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. 樂 



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-07 Thread via GitHub


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


##
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:
   The reason why we need to duplicate rather than directly modify is that our 
CI will support multiple versions of flink? 
   
   Given that `FLINK-31804` has only been merged into the master(1.18), all 
external connectors that need to suppress this violation should also have the 
same problem.
   
   Perhaps temporary duplicate is acceptable, as it will continue until the 
workflow for `flink_version < 1.18` is no longer running.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-07 Thread via GitHub


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


##
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, wouldn't there be no such duplicate. 樂 



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


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


##
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:
   The reason why we need to duplicate rather than directly modify is that our 
CI will support multiple versions of flink? 
   
   Given that `FLINK-31804` has only been merged into the master(1.18), all 
external connectors that need to suppress this violation should also have the 
same problem.
   
   Perhaps temporary duplicate is acceptable, as it will continue until the 
workflow for flink_version < 1.18 is no longer running.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


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


##
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:
   Maybe we can improve `ITCaseRules` in `apache/flink` repo to support our 
situation (Introducing `MiniCluster` only in nested classes). 樂 



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


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


##
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.
   
   This guess is incorrect  . The real reason is that the top level class must 
have a qualified mini cluster field and the nested classes are not considered.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


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


##
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.
   
   After testing, I found that this guess is incorrect. The real reason is that 
the top level class must have a qualified mini cluster field and the nested 
classes are not considered.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


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


##
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`.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


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


##
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 private final 
MiniClusterExtension.



-- 
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] reswqa commented on a diff in pull request #45: [hotfix] Workaround new violations message

2023-05-06 Thread via GitHub


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


##
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:
   Do you mean the source code here?
   
   
https://github.com/apache/flink/blob/3379c2c085da43bb452536c981a7fc13f39482ee/flink-architecture-tests/flink-architecture-tests-test/src/main/java/org/apache/flink/architecture/rules/ITCaseRules.java#L70-L140



-- 
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