robertwb commented on code in PR #17732:
URL: https://github.com/apache/beam/pull/17732#discussion_r886083593
##########
sdks/typescript/src/apache_beam/worker/pardo_context.ts:
##########
@@ -65,21 +63,23 @@ export class ParamProviderImpl implements ParamProvider {
// if they are widely shared.
augmentContext(context: any) {
this.prefetchCallbacks = [];
- if (typeof context != "object") {
+ if (typeof context !== "object") {
return context;
}
const result = Object.create(context);
for (const [name, value] of Object.entries(context)) {
// Is this the best way to check post serialization?
if (
- typeof value == "object" &&
- value != null &&
- value["parDoParamName"] != undefined
+ typeof value === "object" &&
+ value !== null &&
+ value !== undefined &&
Review Comment:
This will never be the case if value === object.
##########
sdks/typescript/src/apache_beam/worker/worker.ts:
##########
@@ -194,7 +194,7 @@ export class Worker {
this.bundleProcessors.set(descriptorId, []);
}
const processor = this.bundleProcessors.get(descriptorId)?.pop();
- if (processor != undefined) {
+ if (processor !== null && processor !== undefined) {
Review Comment:
Let's change this to `if (processor)`.
##########
sdks/typescript/src/apache_beam/worker/worker.ts:
##########
@@ -321,8 +321,8 @@ export class BundleProcessor {
}
getStateProvider() {
- if (this.stateProvider == undefined) {
- if (typeof this.getStateChannel == "function") {
+ if (this.stateProvider === null || this.stateProvider === undefined) {
Review Comment:
same
##########
sdks/typescript/src/apache_beam/runners/portable_runner/runner.ts:
##########
@@ -89,7 +89,11 @@ class PortableRunnerPipelineResult implements PipelineResult
{
let pollMillis = 10;
while (!PortableRunnerPipelineResult.isTerminal(state)) {
const now = Date.now();
- if (duration !== undefined && now - start > duration) {
+ if (
+ duration !== null &&
Review Comment:
I don't think we need to allow null here.
##########
sdks/typescript/src/apache_beam/runners/direct_runner.ts:
##########
@@ -304,8 +304,9 @@ function rewriteSideInputs(p: runnerApi.Pipeline,
pipelineStateRef: string) {
const transforms = p.components!.transforms;
for (const [transformId, transform] of Object.entries(transforms)) {
if (
- transform.spec != undefined &&
- transform.spec.urn == parDo.urn &&
+ transform.spec !== null &&
Review Comment:
This should probably be `transform?.spec.urn` rather than an explicit check.
##########
sdks/typescript/src/apache_beam/transforms/sql.ts:
##########
@@ -49,7 +49,7 @@ export function sqlTransform<
// TOOD: (API) (Typescript): How to infer input_types, or at least make it
optional.
async function expandInternal(input: InputT): Promise<PCollection<any>> {
function withCoder<T>(pcoll: PCollection<T>, type): PCollection<T> {
- if (type == null) {
+ if (type === null || type === undefined) {
Review Comment:
I think we can safely write this `if (type)`
##########
sdks/typescript/src/apache_beam/runners/runner.ts:
##########
@@ -27,15 +27,19 @@ export interface PipelineResult {
export function createRunner(options): Runner {
let runnerConstructor: (any) => Runner;
- if (options.runner == undefined || options.runner == "default") {
+ if (
+ options.runner === null ||
Review Comment:
No need to allow null.
--
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]
