You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "gabrywu (via GitHub)" <gi...@apache.org> on 2023/08/31 12:00:34 UTC
[GitHub] [beam] gabrywu opened a new pull request, #28253: pass runtime configs & variables to BeamSqlSeekableTable
gabrywu opened a new pull request, #28253:
URL: https://github.com/apache/beam/pull/28253
fixes #28145
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #28253: pass runtime configs & variables to BeamSqlSeekableTable
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1700967806
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] gabrywu commented on a diff in pull request #28253: Pass runtime configs & variables to BeamSqlSeekableTable
Posted by "gabrywu (via GitHub)" <gi...@apache.org>.
gabrywu commented on code in PR #28253:
URL: https://github.com/apache/beam/pull/28253#discussion_r1314089009
##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/BeamSqlSeekableTable.java:
##########
@@ -29,6 +31,12 @@ public interface BeamSqlSeekableTable extends Serializable {
/** prepare the instance. */
default void setUp() {};
+ default void startBundle(
+ DoFn<Row, Row>.StartBundleContext context, PipelineOptions pipelineOptions) {};
Review Comment:
remvoed
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] liferoad commented on pull request #28253: pass runtime configs & variables to BeamSqlSeekableTable
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1701219858
R: @Abacn
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] liferoad commented on pull request #28253: pass runtime configs & variables to BeamSqlSeekableTable
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1701220316
R: @bvolpato
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #28253: pass runtime configs & variables to BeamSqlSeekableTable
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1701222434
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Abacn commented on a diff in pull request #28253: Pass runtime configs & variables to BeamSqlSeekableTable
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #28253:
URL: https://github.com/apache/beam/pull/28253#discussion_r1314002936
##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/BeamSqlSeekableTable.java:
##########
@@ -29,6 +31,12 @@ public interface BeamSqlSeekableTable extends Serializable {
/** prepare the instance. */
default void setUp() {};
+ default void startBundle(
+ DoFn<Row, Row>.StartBundleContext context, PipelineOptions pipelineOptions) {};
Review Comment:
minor - unnecessary semicolon
same as existing setUp() and tearDown()
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Abacn merged pull request #28253: Pass runtime configs & variables to BeamSqlSeekableTable
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn merged PR #28253:
URL: https://github.com/apache/beam/pull/28253
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] liferoad commented on pull request #28253: pass runtime configs & variables to BeamSqlSeekableTable
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1701220601
R: @kennknowles
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] gabrywu commented on pull request #28253: pass runtime configs & variables to BeamSqlSeekableTable
Posted by "gabrywu (via GitHub)" <gi...@apache.org>.
gabrywu commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1702566329
precommit
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] gabrywu commented on pull request #28253: Pass runtime configs & variables to BeamSqlSeekableTable
Posted by "gabrywu (via GitHub)" <gi...@apache.org>.
gabrywu commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1703999204
> LGTM. It's totally reasonable to add the method as they corresponding the dofn lifecycle methods. Probably we should add them at the beginning.
>
> * One thing to note when implementing youe own BeamSqlSeekableTable. startBundle can be called many times, so if initializing a connection in it, it's good to use a connection pool otherwise there could be too much connection - a common issue seen in Beam IO implementation
>
> also, its nice to have a test checking dofn lifecycle is effective with BeamSqlSeekableTable methods.
thanks very much. I will add initialized flag to startBundle in our own BeamSqlSeekableTable to avoid too much connection.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] bvolpato commented on pull request #28253: Pass runtime configs & variables to BeamSqlSeekableTable
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on PR #28253:
URL: https://github.com/apache/beam/pull/28253#issuecomment-1703913184
Thanks! No major concerns from me -- just one note is that relying on PipelineOptions will allow configuring behavior for the entire job.
Sometimes it's better to use a specific configuration class / instance, so you could likely have multiple JOINs in the same job and each handle different things.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org