robertwb commented on code in PR #29889:
URL: https://github.com/apache/beam/pull/29889#discussion_r1447621920
##########
sdks/typescript/src/apache_beam/utils/service.ts:
##########
@@ -414,7 +414,7 @@ export class PythonService extends SubprocessService {
const result = childProcess.spawnSync(
PythonService.whichPython(),
[bootstrapScript],
- { encoding: "latin1" },
+ { encoding: "utf-8" },
Review Comment:
To clarify, how do we know the subprocess will always produce valid utf-8?
(I chose latin1 because it has no "invalid" encodings.) If we can guarantee
this, or at least it fails gracefully on invalid byte sequences, this is a
definite improvement.
##########
sdks/typescript/src/apache_beam/utils/service.ts:
##########
@@ -260,7 +260,7 @@ export class JavaJarService extends SubprocessService {
const tmp = dest + ".tmp" + Math.random();
return new Promise((resolve, reject) => {
const fout = fs.createWriteStream(tmp);
- console.warn("Downloading", urlOrPath);
+ console.info("Downloading", urlOrPath);
Review Comment:
Let's leave it for now. I'd rather be too verbose (once) than leave people
hanging.
##########
sdks/typescript/package.json:
##########
@@ -35,30 +39,33 @@
"lint": "eslint . --ext .ts"
},
"dependencies": {
- "@google-cloud/pubsub": "^2.19.4",
- "@grpc/grpc-js": "~1.4.6",
- "@protobuf-ts/grpc-transport": "^2.1.0",
- "@protobuf-ts/plugin": "^2.1.0",
- "bson": "^4.6.0",
- "capture-console": "^1.0.1",
+ "@google-cloud/pubsub": "^4.1.0",
+ "@grpc/grpc-js": "~1.9.13",
+ "@protobuf-ts/grpc-transport": "^2.9.3",
+ "@protobuf-ts/plugin": "^2.9.3",
+ "@types/node": "^20.10.6",
+ "bson": "^6.2.0",
+ "capture-console": "^1.0.2",
"chai": "^4.3.4",
- "date-fns": "^2.28.0",
+ "date-fns": "^3.0.6",
"fast-deep-equal": "^3.1.3",
"find-git-root": "^1.0.4",
- "long": "^4.0.0",
- "protobufjs": "~6.11.3",
+ "long": "^5.2.3",
+ "protobufjs": "~7.2.5",
"queue-typescript": "^1.0.1",
- "serialize-closures": "^0.2.7",
- "ts-closure-transform": "^0.1.7",
- "ttypescript": "^1.5.15",
- "uuid": "^8.3.2"
+ "serialize-closures": "^0.3.0",
+ "ts-closure-transform": "^0.2.0",
+ "typescript": "5.3",
+ "uuid": "^9.0.1"
},
"main": "./dist/src/apache_beam/index.js",
"exports": {
".": "./dist/src/apache_beam/index.js",
"./io": "./dist/src/apache_beam/io/index.js",
"./runners": "./dist/src/apache_beam/index.js",
"./transforms": "./dist/src/apache_beam/transforms/index.js",
+ "./coders": "./dist/src/apache_beam/coders/index.js",
Review Comment:
I'm curious, what for?
##########
sdks/typescript/src/apache_beam/io/pubsub.ts:
##########
@@ -121,9 +121,13 @@ function writeToPubSubRaw(
export function writeToPubSub(topic: string, options: WriteOptions = {}) {
return async function writeToPubSub(dataPColl: beam.PCollection<Uint8Array>)
{
- return dataPColl //
- .map((data) =>
- PubSub.protos.google.pubsub.v1.PubsubMessage.encode({ data }).finish(),
+ return dataPColl
+ .map(
+ withName("writeToPubSubEncode", (data) =>
Review Comment:
This is just to make messages, etc. more meaningful to the users. This is
good (with the nit that convention is to us CapitalCamelCase for stage names).
##########
sdks/typescript/src/apache_beam/transforms/pardo.ts:
##########
@@ -85,11 +85,7 @@ export interface DoFn<InputT, OutputT, ContextT = undefined>
{
/**
* Creates a PTransform that applies a `DoFn` to a PCollection.
*/
-export function parDo<
- InputT,
- OutputT,
- ContextT extends Object | undefined = undefined,
->(
+export function parDo<InputT, OutputT, ContextT = undefined>(
Review Comment:
Oh, I'm seeing the change now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
