You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/07/18 20:42:22 UTC
[GitHub] [beam] damccorm commented on a diff in pull request #22316: cleaned up types in standard_coders.ts
damccorm commented on code in PR #22316:
URL: https://github.com/apache/beam/pull/22316#discussion_r923843369
##########
sdks/typescript/README.md:
##########
@@ -175,6 +175,12 @@ Install node.js, and then from within `sdks/typescript`.
npm install
```
+To install without rewriting **package-lock.json**, run:
Review Comment:
I assume this came up because running `npm install` currently updates package-lock.json? I'd probably argue that's things working as intended. Package management is always a nightmare, but by specifying minor version pins in the package.json, I'd expect us to be comfortable with dependency updates to the package-lock.json in an arbitrary pr.
FWIW, right now our tests use `npm install` (not `npm ci`), so if you use `npm ci` in local dev you're building something different than what's getting tested in ci. That's probably just us doing the wrong thing in CI though.
##########
sdks/typescript/src/apache_beam/coders/standard_coders.ts:
##########
@@ -28,52 +28,44 @@ import {
} from "./coders";
import { BytesCoder, InstantCoder } from "./required_coders";
import Long from "long";
-import {
- Window,
- Instant,
- IntervalWindow,
- KV,
- PaneInfo,
- Timing,
- WindowedValue,
-} from "../values";
+import { IntervalWindow } from "../values";
// Historical
export * from "./required_coders";
/**
* @fileoverview Defines all of the Apache Beam standard coders.
*
- * Beyond required coders, standard coders provide a efficient ways of encode
+ * Beyond required coders, standard coders provide an efficient way to encode
* data for communication between the runner and various Beam workers for
* types that commonly cross process boundaries. Though none of these coders
- * are strictly necessary, if encodings are given for these types it is highly
+ * is strictly necessary, if encodings are given for these types it is highly
* advised to use these definitions that are interoperable with runners and
* other SDKs.
*
- * For schema-aware transforms RowCoder, which is a coder for rows of data
- * with a predetermined schema, is also advised.
+ * For the schema-aware transform RowCoder, which is a coder for rows of data
+ * with a predetermined schema, it is also advised.
*
* The formal specifications for these coders can be found in
* model/pipeline/src/main/proto/beam_runner_api.proto
*/
-export class StrUtf8Coder implements Coder<String> {
- static URN: string = "beam:coder:string_utf8:v1";
- type: string = "stringutf8coder";
+export class StrUtf8Coder implements Coder<string> {
+ static URN = "beam:coder:string_utf8:v1";
+ type = "stringutf8coder";
encoder = new TextEncoder();
decoder = new TextDecoder();
- encode(element: String, writer: Writer, context: Context) {
- const encodedElement = this.encoder.encode(element as string);
+ encode(element: string, writer: Writer, context: Context) {
+ const encodedElement = this.encoder.encode(element);
BytesCoder.INSTANCE.encode(encodedElement, writer, context);
}
- decode(reader: Reader, context: Context): String {
+ decode(reader: Reader, context: Context): string {
return this.decoder.decode(BytesCoder.INSTANCE.decode(reader, context));
}
- toProto(pipelineContext: ProtoContext): runnerApi.Coder {
+ toProto(): runnerApi.Coder {
Review Comment:
Will this work? Shouldn't this be implementing https://github.com/apache/beam/blob/23423e23d3560185792894c7e4d7ba0d4ca4092d/sdks/typescript/src/apache_beam/coders/coders.ts#L139
##########
sdks/typescript/src/apache_beam/coders/standard_coders.ts:
##########
@@ -28,52 +28,44 @@ import {
} from "./coders";
import { BytesCoder, InstantCoder } from "./required_coders";
import Long from "long";
-import {
- Window,
- Instant,
- IntervalWindow,
- KV,
- PaneInfo,
- Timing,
- WindowedValue,
-} from "../values";
+import { IntervalWindow } from "../values";
// Historical
export * from "./required_coders";
/**
* @fileoverview Defines all of the Apache Beam standard coders.
*
- * Beyond required coders, standard coders provide a efficient ways of encode
+ * Beyond required coders, standard coders provide an efficient way to encode
* data for communication between the runner and various Beam workers for
* types that commonly cross process boundaries. Though none of these coders
- * are strictly necessary, if encodings are given for these types it is highly
+ * is strictly necessary, if encodings are given for these types it is highly
* advised to use these definitions that are interoperable with runners and
* other SDKs.
*
- * For schema-aware transforms RowCoder, which is a coder for rows of data
- * with a predetermined schema, is also advised.
+ * For the schema-aware transform RowCoder, which is a coder for rows of data
+ * with a predetermined schema, it is also advised.
*
* The formal specifications for these coders can be found in
* model/pipeline/src/main/proto/beam_runner_api.proto
*/
-export class StrUtf8Coder implements Coder<String> {
- static URN: string = "beam:coder:string_utf8:v1";
- type: string = "stringutf8coder";
+export class StrUtf8Coder implements Coder<string> {
+ static URN = "beam:coder:string_utf8:v1";
+ type = "stringutf8coder";
encoder = new TextEncoder();
decoder = new TextDecoder();
- encode(element: String, writer: Writer, context: Context) {
- const encodedElement = this.encoder.encode(element as string);
+ encode(element: string, writer: Writer, context: Context) {
+ const encodedElement = this.encoder.encode(element);
BytesCoder.INSTANCE.encode(encodedElement, writer, context);
}
- decode(reader: Reader, context: Context): String {
+ decode(reader: Reader, context: Context): string {
return this.decoder.decode(BytesCoder.INSTANCE.decode(reader, context));
}
- toProto(pipelineContext: ProtoContext): runnerApi.Coder {
+ toProto(): runnerApi.Coder {
Review Comment:
I have a similar question elsewhere in this file for toProto and other functions
--
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