You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/11/17 03:34:02 UTC

[GitHub] [iceberg] stevenzwu opened a new pull request, #6206: Extract Flink package version programmatically for EnvironmentContext…

stevenzwu opened a new pull request, #6206:
URL: https://github.com/apache/iceberg/pull/6206

   … engine-version
   
   This is to address hard-coded Flink version from PR #6184. Hard-code value can be very tricky to detect and fix when we copy the 1.16 module to 1.17 module in the future.
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #6206: Extract Flink package version programmatically for EnvironmentContext…

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #6206:
URL: https://github.com/apache/iceberg/pull/6206#issuecomment-1319624537

   @pvary no problem, it was more of a nit, and therefore I already approved 👍🏻 


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary commented on pull request #6206: Extract Flink package version programmatically for EnvironmentContext…

Posted by GitBox <gi...@apache.org>.
pvary commented on PR #6206:
URL: https://github.com/apache/iceberg/pull/6206#issuecomment-1319616607

   @Fokko: Sorry, I have missed your comment about the unit tests. Based on the approvals just merged the change


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] nastra commented on pull request #6206: Extract Flink package version programmatically for EnvironmentContext…

Posted by GitBox <gi...@apache.org>.
nastra commented on PR #6206:
URL: https://github.com/apache/iceberg/pull/6206#issuecomment-1318927977

   I'm in favor of adding the unit test.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu commented on pull request #6206: Extract Flink package version programmatically for EnvironmentContext…

Posted by GitBox <gi...@apache.org>.
stevenzwu commented on PR #6206:
URL: https://github.com/apache/iceberg/pull/6206#issuecomment-1318867484

   @Fokko Yes, I was thinking about adding a unit test with assertion. I had it locally but was on the fence.
   
   - pro: it can ensure the correctness and address @nastra 's concern. Realistically, this shouldn't be a problem but we never know.
   - con: we would need to update the unit test code whenever we upgrade Flink version (e.g. 1.16.0 -> 1.17.0). This happens probably once every 4 months (Flink release cycle). so it wasn't too bad either.
   
   What's your take? @nastra @Fokko @pvary @rdblue 


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #6206: Extract Flink package version programmatically for EnvironmentContext…

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #6206:
URL: https://github.com/apache/iceberg/pull/6206#issuecomment-1319199002

   I'm not super strong on it, but I would have added one, just to make sure that nothing changes on the Flink side.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary merged pull request #6206: Extract Flink package version programmatically for EnvironmentContext…

Posted by GitBox <gi...@apache.org>.
pvary merged PR #6206:
URL: https://github.com/apache/iceberg/pull/6206


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org