You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/12/09 15:31:13 UTC

[GitHub] [flink-connector-hbase] MartijnVisser opened a new pull request, #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

MartijnVisser opened a new pull request, #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5

   + hotfix to set correct version number


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

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


[GitHub] [flink-connector-hbase] MartijnVisser commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "MartijnVisser (via GitHub)" <gi...@apache.org>.
MartijnVisser commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1576955426

   @ferenc-csaky What would be your preference?


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

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


[GitHub] [flink-connector-hbase] MartijnVisser commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "MartijnVisser (via GitHub)" <gi...@apache.org>.
MartijnVisser commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1463393988

   @ferenc-csaky With @zentol his comments, do you think you could continue working on this? 


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1512740236

   I had some interrupts, but I'll have time this week, so I plan to have a PR by next week.


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

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


[GitHub] [flink-connector-hbase] zentol commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
zentol commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1383961066

   > On the short term the necessary calsses can be copied over to achieve the e2e test externalization ASAP
   
   I'd like to avoid that because realistically a migration just won't happen if we don't force the issue, as it was the case for years.
   
   > downloading artifacts will be required by multiple connector E2E tests
   
   By and large we avoid the need for this in newer connector tests by relying on testcontainers.
   
   > The HBase e2e test also relies on the flink-sql-client-test JAR, which is currently not deployed to maven. Would that be an option to deploy that as well for future releases so it is not necessary to put thogether the deps by hand?
   
   Does it actually need it though? Why would executing a SQL statement require test utils?
   
   > The one and only HBase E2E test is [ignored](https://github.com/apache/flink/blob/b29d1055f9225d189603b5aeeacf3199e4c98636/flink-end-to-end-tests/flink-end-to-end-tests-hbase/src/test/java/org/apache/flink/tests/util/hbase/SQLClientHBaseITCase.java#L70) since 2021 April (according to Jira, that issue should be resolved but we never removed the @Ignore itself), so do we still consider this as a release blocker? It never ran during the whole 1.16 dev. cycle. WDYT?
   
   That's a fair point and I'd actually be fine to continue without it (or creating a completely new one).


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

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


[GitHub] [flink-connector-hbase] zentol closed pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol closed pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo
URL: https://github.com/apache/flink-connector-hbase/pull/5


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

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


[GitHub] [flink-connector-hbase] MartijnVisser commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
MartijnVisser commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1347119725

   > The e2e tests will need to be re-written to not rely on the legacy E2E infrastructure but use the connector e2e framework instead.
   
   @ferenc-csaky Is this something you could pick up? 


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1347163190

   > > The e2e tests will need to be re-written to not rely on the legacy E2E infrastructure but use the connector e2e framework instead.
   > 
   > @ferenc-csaky Is this something you could pick up?
   
   Yep, you can assign the jira to me. Is this blocking the connector release itself or that can happen without finishing this work?


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

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


[GitHub] [flink-connector-hbase] MartijnVisser commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
MartijnVisser commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1347239829

   > Yep, you can assign the jira to me. Is this blocking the connector release itself or that can happen without finishing this work?
   
   I would consider it a blocker because without the e2e tests we can't fully externalize the code


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1530998138

   Hello! The thing is, my original idea of using the `HBaseTestUtility` (HBase minicluster) for the E2E tests did not work out, because I could not integrate it with the dockerized Flink env, wiring back the HBase ZK quorum from the host to the docker img. did not work, so I see 2 options at this point:
   
   1.  Create an HBase testcontainers img (there is a Docker img, but no testcontainers intergration)
   2. Write non-dockerized Flink test env. to the `flink-connector-test-utils`.
   
   IMO 1. makes more sense, so I am currently working on the HBase img. Although, pointing back to my prev. comment, @zentol agreed that missing these E2E tests should not block the connector externalization, cause they were disabled for the whole 1.16 development already. Until the HBase connector itself does not change, it should be okay, which would mean the first external release, I think.


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

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


[GitHub] [flink-connector-hbase] zentol commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1629259709

   Subsumed.


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1348992091

   > The e2e tests will need to be re-written to not rely on the legacy E2E infrastructure but use the connector e2e framework instead.
   
   @zentol Do we have any common utils or such that can be used instead of the JUnit4 based ones in the core e2e common module? Or each and every connector should take care about it on their own?


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

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


[GitHub] [flink-connector-hbase] boring-cyborg[bot] commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1344448646

   Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)
   


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1578242904

   > @ferenc-csaky What would be your preference?
   
   I already started working on it, creating an on the fly HBase img. with `testcontainers` and interact with it via the `hbase shell`, so I reuse the already existing code from the current E2E tests. I had some interrupts, but I plan to open the PR with the containerized solution by the end of this week.


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1354763682

   I started to work on this, but I think there are things to discuss:
   * The HBase (and also Kafka) heavily relies on the custom JUnit4 based resources defined in `flink-end-to-end-tests-common` (e.g. `LocalStandaloneFlinkResource`, `DownloadCache`). On the short term the necessary calsses can be copied over to achieve the e2e test externalization ASAP and at least be able to execute the test, but on the longer term a common module probably would be useful. We can say every connector should take care about these things on their own, but IMO things like running flink, downloading artifacts will be required by multiple connector E2E tests, so eventually they will duplicate it. WDYT?
   * The HBase e2e test also relies on the `flink-sql-client-test` JAR, which is currently not deployed to maven. Would that be an option to deploy that as well for future releases so it is not necessary to put thogether the deps by hand?
   * The one and only HBase E2E test is [ignored](https://github.com/apache/flink/blob/b29d1055f9225d189603b5aeeacf3199e4c98636/flink-end-to-end-tests/flink-end-to-end-tests-hbase/src/test/java/org/apache/flink/tests/util/hbase/SQLClientHBaseITCase.java#L70) since 2021 April (according to Jira, that issue should be resolved but we never removed the `@Ignore` itself), so do we still consider this as a release blocker? It never ran during the whole 1.16 dev. cycle. WDYT?
   
   Maybe it would worth to discuss this in the dev channel in a more general manner, but I felt like posting it here first, cause I phrased everything from the HBase perspective, where these apply.
   
   cc @MartijnVisser @zentol 


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

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


[GitHub] [flink-connector-hbase] zentol commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by GitBox <gi...@apache.org>.
zentol commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1346170903

   The e2e tests will need to be re-written to not rely on the legacy E2E infrastructure but use the connector e2e framework instead.


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1463882888

   > @ferenc-csaky With @zentol his comments, do you think you could continue working on this?
   
   Yep, definitely.


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

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


[GitHub] [flink-connector-hbase] ferenc-csaky commented on pull request #5: [FLINK-30349] Sync missing HBase e2e tests to external repo

Posted by "ferenc-csaky (via GitHub)" <gi...@apache.org>.
ferenc-csaky commented on PR #5:
URL: https://github.com/apache/flink-connector-hbase/pull/5#issuecomment-1597249499

   @MartijnVisser @zentol opened #12 with the dockerized E2E tests. The workflow runs currently seems to fail, I'm not sure why.


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

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