You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "robertwb (via GitHub)" <gi...@apache.org> on 2023/09/20 05:54:32 UTC

[GitHub] [beam] robertwb opened a new pull request, #28553: Typescript changes for Beam 2.51.0

robertwb opened a new pull request, #28553:
URL: https://github.com/apache/beam/pull/28553

   * Bad dependbot upgrades.
   * Missing experiments due to Python dataflow runner change.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger 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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] robertwb commented on a diff in pull request #28553: Typescript changes for Beam 2.51.0

Posted by "robertwb (via GitHub)" <gi...@apache.org>.
robertwb commented on code in PR #28553:
URL: https://github.com/apache/beam/pull/28553#discussion_r1331881064


##########
sdks/typescript/package.json:
##########
@@ -36,7 +36,7 @@
   },
   "dependencies": {
     "@google-cloud/pubsub": "^2.19.4",
-    "@grpc/grpc-js": "^1.8.8",
+    "@grpc/grpc-js": "^1.4.6",

Review Comment:
   Yeah. Updated to restrict to the patch version.



-- 
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] damccorm commented on a diff in pull request #28553: Typescript changes for Beam 2.51.0

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #28553:
URL: https://github.com/apache/beam/pull/28553#discussion_r1331871953


##########
sdks/typescript/src/apache_beam/runners/dataflow.ts:
##########
@@ -33,6 +33,8 @@ export function dataflowRunner(runnerOptions: {
       options: Object = {}
     ): Promise<PipelineResult> {
       var augmentedOptions = { experiments: [] as string[], ...options };
+      augmentedOptions.experiments.push("use_runner_v2");
+      augmentedOptions.experiments.push("use_portable_job_submission");

Review Comment:
   > but it's not generally run due to lack of credentials.
   
   Not sure what this means - https://github.com/apache/beam/actions/workflows/typescript_tests.yml looks like its running consistently



-- 
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] damccorm merged pull request #28553: Typescript changes for Beam 2.51.0

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm merged PR #28553:
URL: https://github.com/apache/beam/pull/28553


-- 
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] robertwb commented on a diff in pull request #28553: Typescript changes for Beam 2.51.0

Posted by "robertwb (via GitHub)" <gi...@apache.org>.
robertwb commented on code in PR #28553:
URL: https://github.com/apache/beam/pull/28553#discussion_r1331836378


##########
sdks/typescript/package.json:
##########
@@ -36,7 +36,7 @@
   },
   "dependencies": {
     "@google-cloud/pubsub": "^2.19.4",
-    "@grpc/grpc-js": "^1.8.8",
+    "@grpc/grpc-js": "^1.4.6",

Review Comment:
   Actually, I'm not sure why this fixed things then. I'm thinking maybe I already had an older grpc in the starter project which is why not forcing the lower bound didn't break things. (I have to admit I'm a bit out of depth with these dependency issues here. It's also curious that the `TypeScript xlang Tests` didn't fail on the depend bot PR. The error I'm struggling with manifests as https://github.com/grpc/grpc-node/issues/2411 ; open to any ideas here.)



-- 
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] robertwb commented on pull request #28553: Typescript changes for Beam 2.51.0

Posted by "robertwb (via GitHub)" <gi...@apache.org>.
robertwb commented on PR #28553:
URL: https://github.com/apache/beam/pull/28553#issuecomment-1727015865

   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] github-actions[bot] commented on pull request #28553: Typescript changes for Beam 2.51.0

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28553:
URL: https://github.com/apache/beam/pull/28553#issuecomment-1727017176

   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] damccorm commented on a diff in pull request #28553: Typescript changes for Beam 2.51.0

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #28553:
URL: https://github.com/apache/beam/pull/28553#discussion_r1331876394


##########
sdks/typescript/package.json:
##########
@@ -36,7 +36,7 @@
   },
   "dependencies": {
     "@google-cloud/pubsub": "^2.19.4",
-    "@grpc/grpc-js": "^1.8.8",
+    "@grpc/grpc-js": "^1.4.6",

Review Comment:
   Probably its actually the package-lock changes that fixed things. Specifically, `"node_modules/@grpc/grpc-js"` went from `1.8.8` to `1.4.6`. But that kinda just means you got lucky 😄 we should change this from `^1.4.6` to `1.4.6` if we're sensitive to the version changing (or at least `~1.4.6` if we don't care about the patch version changing). See https://devhints.io/semver
   
   If we don't do that, the next time someone runs `npm install` (if for example they change a different package), this is liable to change.



##########
sdks/typescript/package.json:
##########
@@ -36,7 +36,7 @@
   },
   "dependencies": {
     "@google-cloud/pubsub": "^2.19.4",
-    "@grpc/grpc-js": "^1.8.8",
+    "@grpc/grpc-js": "^1.4.6",

Review Comment:
   Probably its actually the package-lock changes that fixed things. Specifically, `"node_modules/@grpc/grpc-js"` went from `1.8.8` to `1.4.6`. But that kinda just means you got lucky 😄 we should change this from `^1.4.6` to `1.4.6` if we're sensitive to the version changing (or at least `~1.4.6` if we don't care about the patch version changing). See https://devhints.io/semver
   
   If we don't do that, the next time someone runs `npm install` (if for example they change a different package), this is liable to 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: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] damccorm commented on a diff in pull request #28553: Typescript changes for Beam 2.51.0

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #28553:
URL: https://github.com/apache/beam/pull/28553#discussion_r1331668422


##########
sdks/typescript/package.json:
##########
@@ -36,7 +36,7 @@
   },
   "dependencies": {
     "@google-cloud/pubsub": "^2.19.4",
-    "@grpc/grpc-js": "^1.8.8",
+    "@grpc/grpc-js": "^1.4.6",

Review Comment:
   I'm kinda confused by this one: isn't `^1.4.6` just a more permissive superset of `^1.8.8`? Should we be pinning to an explicit version instead of using the `^`?
   
   In semver `^1.4.6` is equivalent to `>=1.4.6, <2.0.0`



##########
sdks/typescript/src/apache_beam/runners/dataflow.ts:
##########
@@ -33,6 +33,8 @@ export function dataflowRunner(runnerOptions: {
       options: Object = {}
     ): Promise<PipelineResult> {
       var augmentedOptions = { experiments: [] as string[], ...options };
+      augmentedOptions.experiments.push("use_runner_v2");
+      augmentedOptions.experiments.push("use_portable_job_submission");

Review Comment:
   Would typescript tests have caught this? If yes, should we add python runner paths to the workflow trigger so that the tests run against python dataflow runner changes?



-- 
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] robertwb commented on a diff in pull request #28553: Typescript changes for Beam 2.51.0

Posted by "robertwb (via GitHub)" <gi...@apache.org>.
robertwb commented on code in PR #28553:
URL: https://github.com/apache/beam/pull/28553#discussion_r1331830222


##########
sdks/typescript/src/apache_beam/runners/dataflow.ts:
##########
@@ -33,6 +33,8 @@ export function dataflowRunner(runnerOptions: {
       options: Object = {}
     ): Promise<PipelineResult> {
       var augmentedOptions = { experiments: [] as string[], ...options };
+      augmentedOptions.experiments.push("use_runner_v2");
+      augmentedOptions.experiments.push("use_portable_job_submission");

Review Comment:
   Yes, the TypeScript Dataflow Tests should have caught this, but it's not generally run due to lack of credentials. 



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