[GitHub] [beam] lostluck merged pull request #12948: [BEAM-10959] Go SDK: Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck merged pull request #12948:
URL: https://github.com/apache/beam/pull/12948


   



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.

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

[GitHub] [beam] lostluck commented on pull request #12948: [BEAM-10959] Go SDK: Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on pull request #12948:
URL: https://github.com/apache/beam/pull/12948#issuecomment-699340212


   Thanks! Have a good weekend.



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12952: [WIP] Draft - Add bundle finalization for flink.

[GitBox]

codecov[bot] edited a comment on pull request #12952:
URL: https://github.com/apache/beam/pull/12952#issuecomment-699286177


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=h1) Report
   > Merging 
[#12952](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/f1193197c56ce11bf1e9721c7a28ffa8586ffe0f?el=desc)
 will **decrease** coverage by `0.39%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12952/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12952  +/-   ##
   ==
   - Coverage   82.72%   82.32%   -0.40% 
   ==
 Files 454  455   +1 
 Lines   5578754650-1137 
   ==
   - Hits4614944990-1159 
   - Misses   9638 9660  +22 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/typehints/opcodes.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL29wY29kZXMucHk=)
 | `87.91% <0.00%> (-4.49%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-3.85%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==)
 | `90.77% <0.00%> (-3.02%)` | :arrow_down: |
   | 
[.../python/apache\_beam/typehints/trivial\_inference.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL3RyaXZpYWxfaW5mZXJlbmNlLnB5)
 | `89.44% <0.00%> (-2.67%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==)
 | `88.68% <0.00%> (-2.35%)` | :arrow_down: |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.93% <0.00%> (-1.49%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5)
 | `88.63% <0.00%> (-1.24%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | ... and [43 
more](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=footer). Last 
update 
[126554d...4980e35](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] kennknowles commented on pull request #12947: [Beam-10903] Fix Java nightly snapshot build.

[GitBox]

kennknowles commented on pull request #12947:
URL: https://github.com/apache/beam/pull/12947#issuecomment-699304359


   Makes sense. How did you track it down, out of curiosity? Browsing 
https://ci-beam.apache.org/job/beam_Release_NightlySnapshot/935/ and 
thereabouts didn't yield such a clear result while 
https://ci-beam.apache.org/job/beam_PostRelease_NightlySnapshot/ is healthy (I 
don't know what exactly these jobs actually do in detail, and particularly the 
latter name makes no sense to me)



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12952: [WIP] Draft - Add bundle finalization for flink.

[GitBox]

codecov[bot] edited a comment on pull request #12952:
URL: https://github.com/apache/beam/pull/12952#issuecomment-699286177


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=h1) Report
   > Merging 
[#12952](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/f1193197c56ce11bf1e9721c7a28ffa8586ffe0f?el=desc)
 will **decrease** coverage by `0.39%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12952/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12952  +/-   ##
   ==
   - Coverage   82.72%   82.32%   -0.40% 
   ==
 Files 454  455   +1 
 Lines   5578754650-1137 
   ==
   - Hits4614944990-1159 
   - Misses   9638 9660  +22 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/typehints/opcodes.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL29wY29kZXMucHk=)
 | `87.91% <0.00%> (-4.49%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-3.85%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==)
 | `90.77% <0.00%> (-3.02%)` | :arrow_down: |
   | 
[.../python/apache\_beam/typehints/trivial\_inference.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL3RyaXZpYWxfaW5mZXJlbmNlLnB5)
 | `89.44% <0.00%> (-2.67%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==)
 | `88.68% <0.00%> (-2.35%)` | :arrow_down: |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.93% <0.00%> (-1.49%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5)
 | `88.63% <0.00%> (-1.24%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | ... and [43 
more](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=footer). Last 
update 
[126554d...4980e35](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] lostluck commented on pull request #12903: [BEAM-9616] Add RegisterDoFn

[GitBox]

lostluck commented on pull request #12903:
URL: https://github.com/apache/beam/pull/12903#issuecomment-699292528


   Heh no worries.
   
   On Fri, Sep 25, 2020, 8:18 PM Daniel Oliveira 
   wrote:
   
   > *@youngoli* commented on this pull request.
   > --
   >
   > In sdks/go/pkg/beam/core/runtime/genx/genx.go
   > :
   >
   > > +for _, t := range ts {
   > +  runtime.RegisterType(t)
   > +  }
   > +}
   > +
   > +// registerDoFn returns all types associated with the provided DoFn.
   > +// If passed a functional DoFn, the first return is a Function to
   > +// register with runtime.RegisterFunction.
   > +// The second return is all types to register with runtime.RegisterType.
   > +// Returns an error if the passed in values are not DoFns.
   > +func registerDoFn(dofn interface{}) (interface{}, []reflect.Type, error) {
   > +  if rt, ok := dofn.(reflect.Type); ok {
   > +  if rt.Kind() == reflect.Ptr {
   > +  rt = rt.Elem()
   > +  }
   > +  dofn = reflect.New(rt).Interface()
   >
   > Oh, duh. I had a brain-fart that this statement only happens if the dofn
   > is a reflect.Type. I was imagining turning an actual DoFn instance into a
   > reflect.Type and back. Makes complete sense now.
   >
   > —
   > You are receiving this because you modified the open/close state.
   > Reply to this email directly, view it on GitHub
   > , or
   > unsubscribe
   > 

   > .
   >
   



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.

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

[GitHub] [beam] youngoli commented on a change in pull request #12903: [BEAM-9616] Add RegisterDoFn

[GitBox]

youngoli commented on a change in pull request #12903:
URL: https://github.com/apache/beam/pull/12903#discussion_r495407788



##
File path: sdks/go/pkg/beam/core/runtime/genx/genx.go
##
@@ -0,0 +1,172 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package genx is a convenience package to better support the code
+// generator. It can be depended on by the user facing beam package
+// and be refered to by generated code.
+//
+// Similarly, it can depend on beam internals and access the canonical
+// method list in the graph package, or other packages to filter out
+// types that aren't necessary for registration (like context.Context).
+package genx
+
+import (
+   "reflect"
+
+   "github.com/apache/beam/sdks/go/pkg/beam/core/funcx"
+   "github.com/apache/beam/sdks/go/pkg/beam/core/graph"
+   "github.com/apache/beam/sdks/go/pkg/beam/core/runtime"
+)
+
+// RegisterDoFn is a convenience function for registering DoFns.
+// Differs from RegisterFunction and RegisterType by introspecting
+// all parameters and returns of Lifecycle methods on the dofn,
+// and registers those types for you.
+//
+// Panics if not passed a dofn.
+func RegisterDoFn(dofn interface{}) {
+   f, ts, err := registerDoFn(dofn)
+   if err != nil {
+   panic(err)
+   }
+   if f != nil {
+   runtime.RegisterFunction(f)
+   }
+   for _, t := range ts {
+   runtime.RegisterType(t)
+   }
+}
+
+// registerDoFn returns all types associated with the provided DoFn.
+// If passed a functional DoFn, the first return is a Function to
+// register with runtime.RegisterFunction.
+// The second return is all types to register with runtime.RegisterType.
+// Returns an error if the passed in values are not DoFns.
+func registerDoFn(dofn interface{}) (interface{}, []reflect.Type, error) {
+   if rt, ok := dofn.(reflect.Type); ok {
+   if rt.Kind() == reflect.Ptr {
+   rt = rt.Elem()
+   }
+   dofn = reflect.New(rt).Interface()

Review comment:
   Oh, duh. I had a brain-fart that this statement only happens if the dofn 
is a reflect.Type. I was imagining turning an actual DoFn instance into a 
reflect.Type and back. Makes complete sense 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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12952: [WIP] Draft - Add bundle finalization for flink.

[GitBox]

codecov[bot] edited a comment on pull request #12952:
URL: https://github.com/apache/beam/pull/12952#issuecomment-699286177


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=h1) Report
   > Merging 
[#12952](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/f1193197c56ce11bf1e9721c7a28ffa8586ffe0f?el=desc)
 will **decrease** coverage by `0.39%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12952/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12952  +/-   ##
   ==
   - Coverage   82.72%   82.32%   -0.40% 
   ==
 Files 454  455   +1 
 Lines   5578754650-1137 
   ==
   - Hits4614944990-1159 
   - Misses   9638 9660  +22 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/typehints/opcodes.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL29wY29kZXMucHk=)
 | `87.91% <0.00%> (-4.49%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-3.85%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==)
 | `90.77% <0.00%> (-3.02%)` | :arrow_down: |
   | 
[.../python/apache\_beam/typehints/trivial\_inference.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL3RyaXZpYWxfaW5mZXJlbmNlLnB5)
 | `89.44% <0.00%> (-2.67%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==)
 | `88.68% <0.00%> (-2.35%)` | :arrow_down: |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.93% <0.00%> (-1.49%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5)
 | `88.63% <0.00%> (-1.24%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | ... and [43 
more](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=footer). Last 
update 
[126554d...4980e35](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] commented on pull request #12952: [WIP] Draft - Add bundle finalization for flink.

[GitBox]

codecov[bot] commented on pull request #12952:
URL: https://github.com/apache/beam/pull/12952#issuecomment-699286177


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=h1) Report
   > Merging 
[#12952](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/f1193197c56ce11bf1e9721c7a28ffa8586ffe0f?el=desc)
 will **decrease** coverage by `0.39%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12952/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12952  +/-   ##
   ==
   - Coverage   82.72%   82.32%   -0.40% 
   ==
 Files 454  455   +1 
 Lines   5578754650-1137 
   ==
   - Hits4614944990-1159 
   - Misses   9638 9660  +22 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/typehints/opcodes.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL29wY29kZXMucHk=)
 | `87.91% <0.00%> (-4.49%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-3.85%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==)
 | `90.77% <0.00%> (-3.02%)` | :arrow_down: |
   | 
[.../python/apache\_beam/typehints/trivial\_inference.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL3RyaXZpYWxfaW5mZXJlbmNlLnB5)
 | `89.44% <0.00%> (-2.67%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[.../runners/portability/fn\_api\_runner/translations.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3RyYW5zbGF0aW9ucy5weQ==)
 | `88.68% <0.00%> (-2.35%)` | :arrow_down: |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.93% <0.00%> (-1.49%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5)
 | `88.63% <0.00%> (-1.24%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | ... and [43 
more](https://codecov.io/gh/apache/beam/pull/12952/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=footer). Last 
update 
[126554d...4980e35](https://codecov.io/gh/apache/beam/pull/12952?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] boyuanzz opened a new pull request #12952: [WIP] Draft - Add bundle finalization for flink.

[GitBox]

boyuanzz opened a new pull request #12952:
URL: https://github.com/apache/beam/pull/12952


   **Please** add a meaningful description for your change here
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 
Status](https

[GitHub] [beam] udim commented on pull request #12951: [BEAM-7463] Fix BQ IT flake with streaming inserts

[GitBox]

udim commented on pull request #12951:
URL: https://github.com/apache/beam/pull/12951#issuecomment-699270691


   R: @pabloem 



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.

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

[GitHub] [beam] udim opened a new pull request #12951: [BEAM-7463] Fix BQ IT flake with streaming inserts

[GitBox]

udim opened a new pull request #12951:
URL: https://github.com/apache/beam/pull/12951


   - In short: streaming inserts are not immediately available after
 InsertAll RPC is done. See bug for details.
   - Added cumulative time limit option to retry mechanism. This may also
 be useful for ITs where the matcher needs to wait for a streaming
 pipeline to complete writing its output.
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCo

[GitHub] [beam] chadrik commented on pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

chadrik commented on pull request #12883:
URL: https://github.com/apache/beam/pull/12883#issuecomment-699267089


   Lint issues have been fixed.  Codecov warnings look bogus.  Remaining 
unittest failures appear have nothing to do with my 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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12950: Update test_get_python_sdk_name to supported python version

[GitBox]

codecov[bot] edited a comment on pull request #12950:
URL: https://github.com/apache/beam/pull/12950#issuecomment-699259698


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=h1) Report
   > Merging 
[#12950](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **increase** coverage by `0.02%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12950/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12950  +/-   ##
   ==
   + Coverage   82.31%   82.34%   +0.02% 
   ==
 Files 455  455  
 Lines   5460354650  +47 
   ==
   + Hits4494545000  +55 
   + Misses   9658 9650   -8 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...ython/apache\_beam/io/gcp/experimental/spannerio.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2V4cGVyaW1lbnRhbC9zcGFubmVyaW8ucHk=)
 | `88.23% <ø> (ø)` | |
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <0.00%> (ø)` | |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.80% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <0.00%> (+0.23%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5)
 | `84.03% <0.00%> (+0.28%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=)
 | `89.20% <0.00%> (+0.44%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `89.60% <0.00%> (+0.91%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `89.98% <0.00%> (+1.17%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=footer). Last 
update 
[4818697...a9bdc07](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12950: Update test_get_python_sdk_name to supported python version

[GitBox]

codecov[bot] edited a comment on pull request #12950:
URL: https://github.com/apache/beam/pull/12950#issuecomment-699259698


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=h1) Report
   > Merging 
[#12950](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **increase** coverage by `0.02%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12950/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12950  +/-   ##
   ==
   + Coverage   82.31%   82.34%   +0.02% 
   ==
 Files 455  455  
 Lines   5460354650  +47 
   ==
   + Hits4494545000  +55 
   + Misses   9658 9650   -8 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...ython/apache\_beam/io/gcp/experimental/spannerio.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2V4cGVyaW1lbnRhbC9zcGFubmVyaW8ucHk=)
 | `88.23% <ø> (ø)` | |
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <0.00%> (ø)` | |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.80% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <0.00%> (+0.23%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5)
 | `84.03% <0.00%> (+0.28%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=)
 | `89.20% <0.00%> (+0.44%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `89.60% <0.00%> (+0.91%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `89.98% <0.00%> (+1.17%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=footer). Last 
update 
[4818697...a9bdc07](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12950: Update test_get_python_sdk_name to supported python version

[GitBox]

codecov[bot] edited a comment on pull request #12950:
URL: https://github.com/apache/beam/pull/12950#issuecomment-699259698


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=h1) Report
   > Merging 
[#12950](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **increase** coverage by `0.02%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12950/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12950  +/-   ##
   ==
   + Coverage   82.31%   82.34%   +0.02% 
   ==
 Files 455  455  
 Lines   5460354650  +47 
   ==
   + Hits4494545000  +55 
   + Misses   9658 9650   -8 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...ython/apache\_beam/io/gcp/experimental/spannerio.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2V4cGVyaW1lbnRhbC9zcGFubmVyaW8ucHk=)
 | `88.23% <ø> (ø)` | |
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <0.00%> (ø)` | |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.80% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <0.00%> (+0.23%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5)
 | `84.03% <0.00%> (+0.28%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=)
 | `89.20% <0.00%> (+0.44%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `89.60% <0.00%> (+0.91%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `89.98% <0.00%> (+1.17%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=footer). Last 
update 
[4818697...a9bdc07](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] commented on pull request #12950: Update test_get_python_sdk_name to supported python version

[GitBox]

codecov[bot] commented on pull request #12950:
URL: https://github.com/apache/beam/pull/12950#issuecomment-699259698


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=h1) Report
   > Merging 
[#12950](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **increase** coverage by `0.02%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12950/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12950  +/-   ##
   ==
   + Coverage   82.31%   82.34%   +0.02% 
   ==
 Files 455  455  
 Lines   5460354650  +47 
   ==
   + Hits4494545000  +55 
   + Misses   9658 9650   -8 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...ython/apache\_beam/io/gcp/experimental/spannerio.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2V4cGVyaW1lbnRhbC9zcGFubmVyaW8ucHk=)
 | `88.23% <ø> (ø)` | |
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <0.00%> (ø)` | |
   | 
[...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=)
 | `76.80% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <0.00%> (+0.23%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5)
 | `84.03% <0.00%> (+0.28%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=)
 | `89.20% <0.00%> (+0.44%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `89.60% <0.00%> (+0.91%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12950/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `89.98% <0.00%> (+1.17%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=footer). Last 
update 
[4818697...a9bdc07](https://codecov.io/gh/apache/beam/pull/12950?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12949: [BEAM-10978] Fix bug with map type inference.

[GitBox]

codecov[bot] edited a comment on pull request #12949:
URL: https://github.com/apache/beam/pull/12949#issuecomment-699256196


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=h1) Report
   > Merging 
[#12949](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/64d8c8006c4aa633438863eab15078b22b6d85ac?el=desc)
 will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12949/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master   #12949   +/-   ##
   ===
 Coverage   82.34%   82.34%   
   ===
 Files 455  455   
 Lines   5465054650   
   ===
   + Hits4499945001+2 
   + Misses   9651 9649-2 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/typehints/opcodes.py](https://codecov.io/gh/apache/beam/pull/12949/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL29wY29kZXMucHk=)
 | `87.91% <100.00%> (ø)` | |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12949/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.58% <0.00%> (+0.13%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12949/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `90.14% <0.00%> (+0.16%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=footer). Last 
update 
[64d8c80...7856a21](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] commented on pull request #12949: [BEAM-10978] Fix bug with map type inference.

[GitBox]

codecov[bot] commented on pull request #12949:
URL: https://github.com/apache/beam/pull/12949#issuecomment-699256196


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=h1) Report
   > Merging 
[#12949](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/64d8c8006c4aa633438863eab15078b22b6d85ac?el=desc)
 will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12949/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master   #12949   +/-   ##
   ===
 Coverage   82.34%   82.34%   
   ===
 Files 455  455   
 Lines   5465054650   
   ===
   + Hits4499945001+2 
   + Misses   9651 9649-2 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/typehints/opcodes.py](https://codecov.io/gh/apache/beam/pull/12949/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL29wY29kZXMucHk=)
 | `87.91% <100.00%> (ø)` | |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12949/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.58% <0.00%> (+0.13%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12949/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `90.14% <0.00%> (+0.16%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=footer). Last 
update 
[64d8c80...7856a21](https://codecov.io/gh/apache/beam/pull/12949?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] y1chi opened a new pull request #12950: Update test_get_python_sdk_name to supported python version

[GitBox]

y1chi opened a new pull request #12950:
URL: https://github.com/apache/beam/pull/12950


   **Please** add a meaningful description for your change here
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 
Status](https://

[GitHub] [beam] y1chi commented on pull request #12950: Update test_get_python_sdk_name to supported python version

[GitBox]

y1chi commented on pull request #12950:
URL: https://github.com/apache/beam/pull/12950#issuecomment-699255171


   R: @robinyqiu @tvalentyn 
   to fix failing test on release branch
   
https://ci-beam.apache.org/job/beam_PreCommit_Python_Phrase/2215/testReport/apache_beam.runners.dataflow.internal.apiclient_test/UtilTest/test_get_python_sdk_name/



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

codecov[bot] edited a comment on pull request #12883:
URL: https://github.com/apache/beam/pull/12883#issuecomment-699197950


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=h1) Report
   > Merging 
[#12883](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc)
 will **decrease** coverage by `0.02%`.
   > The diff coverage is `76.19%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12883/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12883  +/-   ##
   ==
   - Coverage   82.34%   82.31%   -0.03% 
   ==
 Files 452  455   +3 
 Lines   5401654688 +672 
   ==
   + Hits4448145019 +538 
   - Misses   9535 9669 +134 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/metrics/cells.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9jZWxscy5weQ==)
 | `80.73% <50.00%> (-1.32%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/metrics/monitoring\_infos.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tb25pdG9yaW5nX2luZm9zLnB5)
 | `96.64% <50.00%> (-0.53%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/execution.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9leGVjdXRpb24ucHk=)
 | `87.69% <76.00%> (-2.58%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWMucHk=)
 | `95.31% <90.90%> (-1.36%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metricbase.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWNiYXNlLnB5)
 | `88.23% <100.00%> (+0.35%)` | :arrow_up: |
   | 
[...am/runners/interactive/options/capture\_limiters.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9vcHRpb25zL2NhcHR1cmVfbGltaXRlcnMucHk=)
 | `90.47% <0.00%> (-3.08%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-1.01%)` | :arrow_down: |
   | ... and [48 
more](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=footer). Last 
update 
[067cba8...513693b](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] robertwb commented on pull request #12949: [BEAM-10978] Fix bug with map type inference.

[GitBox]

robertwb commented on pull request #12949:
URL: https://github.com/apache/beam/pull/12949#issuecomment-699251249


   R: @udim 
   CC: @robinyqiu This will probably need a cherry-pick to not break people.



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.

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

[GitHub] [beam] robertwb opened a new pull request #12949: [BEAM-10978] Fix bug with map type inference.

[GitBox]

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


   Only half the arguments were popped off the top.
   
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 
Status](https://ci-be

[GitHub] [beam] lostluck commented on pull request #12948: [BEAM-10959] Go SDK: Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on pull request #12948:
URL: https://github.com/apache/beam/pull/12948#issuecomment-699250223


   R: @youngoli 
   R: @lukecwik 



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.

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

[GitHub] [beam] lostluck opened a new pull request #12948: [BEAM-10959] Go SDK: Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck opened a new pull request #12948:
URL: https://github.com/apache/beam/pull/12948


   Add a touch more state to prevent race conditions on the runner side.
   
   Retains a list of the most recent N (currently 1000) process bundle 
instructions.
   
   The flow is: 
   Receive ProcessBundleRequest INST-> Add INST to inactive.
   Start executing INST -> remove INST from inactive and add INST to active.
   Finish executing INST -> remove INST from active, add INST to inactive
 if inactive queue > N, then remove the oldest inst from inactive first.
 if INST failed -> also add INST to failed.
   
   Then for either Split or Progress requests:
INST is active -> return the appropriate populated response.
INST is failed -> return the cached error
INST is inactive -> return empty response.
   
   https://issues.apache.org/jira/browse/BEAM-10976 is tracking handling Bundle 
Finalization which would require an additional queue to avoid early inactive 
eviction. (or similarly to the  "known but not yet started case", not have the 
removal queue populated in that instance, preventing removal until after 
finalization is handled.) Moot until that feature is in the SDK.
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/

[GitHub] [beam] codecov[bot] edited a comment on pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

codecov[bot] edited a comment on pull request #12883:
URL: https://github.com/apache/beam/pull/12883#issuecomment-699197950


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=h1) Report
   > Merging 
[#12883](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc)
 will **decrease** coverage by `0.02%`.
   > The diff coverage is `76.19%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12883/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12883  +/-   ##
   ==
   - Coverage   82.34%   82.31%   -0.03% 
   ==
 Files 452  455   +3 
 Lines   5401654688 +672 
   ==
   + Hits4448145019 +538 
   - Misses   9535 9669 +134 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/metrics/cells.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9jZWxscy5weQ==)
 | `80.73% <50.00%> (-1.32%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/metrics/monitoring\_infos.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tb25pdG9yaW5nX2luZm9zLnB5)
 | `96.64% <50.00%> (-0.53%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/execution.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9leGVjdXRpb24ucHk=)
 | `87.69% <76.00%> (-2.58%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWMucHk=)
 | `95.31% <90.90%> (-1.36%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metricbase.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWNiYXNlLnB5)
 | `88.23% <100.00%> (+0.35%)` | :arrow_up: |
   | 
[...am/runners/interactive/options/capture\_limiters.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9vcHRpb25zL2NhcHR1cmVfbGltaXRlcnMucHk=)
 | `90.47% <0.00%> (-3.08%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-1.01%)` | :arrow_down: |
   | ... and [48 
more](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=footer). Last 
update 
[067cba8...513693b](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495348238



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -543,15 +602,19 @@ def process_bundle_split(self,
instruction_id  # type: str
   ):
 # type: (...) -> beam_fn_api_pb2.InstructionResponse
-processor = self.bundle_processor_cache.lookup(request.instruction_id)
-if processor:
-  return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  process_bundle_split=processor.try_split(request))
-else:
+try:
+  processor = self.bundle_processor_cache.lookup(request.instruction_id)
+except RuntimeError:
   return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  error='Instruction not running: %s' % instruction_id)
+  instruction_id=instruction_id, error=traceback.format_exc())
+# Return an empty response if we aren't running. This can happen
+# if the ProcessBundleRequest has not started or already finished.
+process_bundle_split = (
+processor.try_split(request)
+if processor else beam_fn_api_pb2.ProcessBundleSplitResponse())

Review comment:
   The Go SDK already differentiates the failed case and re-forwards the 
cached error.





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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

codecov[bot] edited a comment on pull request #12883:
URL: https://github.com/apache/beam/pull/12883#issuecomment-699197950


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=h1) Report
   > Merging 
[#12883](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc)
 will **decrease** coverage by `0.02%`.
   > The diff coverage is `76.19%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12883/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12883  +/-   ##
   ==
   - Coverage   82.34%   82.31%   -0.03% 
   ==
 Files 452  455   +3 
 Lines   5401654688 +672 
   ==
   + Hits4448145019 +538 
   - Misses   9535 9669 +134 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/metrics/cells.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9jZWxscy5weQ==)
 | `80.73% <50.00%> (-1.32%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/metrics/monitoring\_infos.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tb25pdG9yaW5nX2luZm9zLnB5)
 | `96.64% <50.00%> (-0.53%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/execution.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9leGVjdXRpb24ucHk=)
 | `87.69% <76.00%> (-2.58%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWMucHk=)
 | `95.31% <90.90%> (-1.36%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metricbase.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWNiYXNlLnB5)
 | `88.23% <100.00%> (+0.35%)` | :arrow_up: |
   | 
[...am/runners/interactive/options/capture\_limiters.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9vcHRpb25zL2NhcHR1cmVfbGltaXRlcnMucHk=)
 | `90.47% <0.00%> (-3.08%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-1.01%)` | :arrow_down: |
   | ... and [47 
more](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=footer). Last 
update 
[067cba8...513693b](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] tysonjh commented on pull request #12947: [Beam-10903] Fix Java nightly snapshot build.

[GitBox]

tysonjh commented on pull request #12947:
URL: https://github.com/apache/beam/pull/12947#issuecomment-699237958


   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.

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

[GitHub] [beam] ajamato commented on a change in pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

ajamato commented on a change in pull request #12883:
URL: https://github.com/apache/beam/pull/12883#discussion_r495336721



##
File path: sdks/python/apache_beam/metrics/metricbase.py
##
@@ -78,7 +80,7 @@ def __hash__(self):
 
 class Metric(object):
   """Base interface of a metric object."""
-  pass
+  metric_name = None  # type: MetricName

Review comment:
   Sounds good. I wouldn't want to scope creep that into your PR. 
Definitely for another day :).





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.

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

[GitHub] [beam] tysonjh opened a new pull request #12947: [Beam-10903] Fix Java nightly snapshot build.

[GitBox]

tysonjh opened a new pull request #12947:
URL: https://github.com/apache/beam/pull/12947


   After upgrading to Gradle 6.6.1 (commit
   1db383c58c214ec2e10edec72e7fc427c74ebb1c) the snapshot builds started
   failing. This is because the maven plugin now publishes Maven 3
   metadata, where previously it published Maven 2 metadata. From the
   userguide [1]:
   
   The Maven Plugin used to publish the highly outdated Maven 2 metadata 
format. This has
   been changed and it will now publish Maven 3 metadata, just like the 
Maven Publish Plugin.
   With the removal of Maven 2 support, the methods that configure unique 
snapshot behavior
   have also been removed. Maven 3 only supports unique snapshots, so we 
decided to remove
   them.
   
   [1]: https://docs.gradle.org/current/userguide/userguide.pdf
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/be

[GitHub] [beam] codecov[bot] edited a comment on pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

codecov[bot] edited a comment on pull request #12883:
URL: https://github.com/apache/beam/pull/12883#issuecomment-699197950


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=h1) Report
   > Merging 
[#12883](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc)
 will **decrease** coverage by `0.02%`.
   > The diff coverage is `76.19%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12883/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12883  +/-   ##
   ==
   - Coverage   82.34%   82.31%   -0.03% 
   ==
 Files 452  455   +3 
 Lines   5401654688 +672 
   ==
   + Hits4448145019 +538 
   - Misses   9535 9669 +134 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/metrics/cells.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9jZWxscy5weQ==)
 | `80.73% <50.00%> (-1.32%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/metrics/monitoring\_infos.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tb25pdG9yaW5nX2luZm9zLnB5)
 | `96.64% <50.00%> (-0.53%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/execution.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9leGVjdXRpb24ucHk=)
 | `87.69% <76.00%> (-2.58%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWMucHk=)
 | `95.31% <90.90%> (-1.36%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metricbase.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWNiYXNlLnB5)
 | `88.23% <100.00%> (+0.35%)` | :arrow_up: |
   | 
[...am/runners/interactive/options/capture\_limiters.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9vcHRpb25zL2NhcHR1cmVfbGltaXRlcnMucHk=)
 | `90.47% <0.00%> (-3.08%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-1.01%)` | :arrow_down: |
   | ... and [47 
more](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=footer). Last 
update 
[067cba8...078cb50](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] chadrik commented on a change in pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

chadrik commented on a change in pull request #12883:
URL: https://github.com/apache/beam/pull/12883#discussion_r495318636



##
File path: sdks/python/apache_beam/metrics/metricbase.py
##
@@ -78,7 +80,7 @@ def __hash__(self):
 
 class Metric(object):
   """Base interface of a metric object."""
-  pass
+  metric_name = None  # type: MetricName

Review comment:
   >I recommend adding init with a metric_name parameter to Metric and its 
subclasses (Counter, Distribution and Gauge). This already exists on 
DelegatingX classes, but could be done in the base Metric init instead.
   
   Adding `metric_name` to `Metric.__init__` definitely seems like the best 
solution to me, so I did that. 
   
   > If it were up to me we would remove Counter, Distribution and Gauge 
classes and just have the DelegatingCounter DelegatingDistribution and 
DelegatingGauge classes.
   
   Leaving this for a future task.
   





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.

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

[GitHub] [beam] chadrik commented on a change in pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

chadrik commented on a change in pull request #12883:
URL: https://github.com/apache/beam/pull/12883#discussion_r495317252



##
File path: sdks/python/apache_beam/metrics/cells.py
##
@@ -29,8 +29,12 @@
 import threading
 import time
 from builtins import object
+from typing import TYPE_CHECKING
 from typing import Optional
 
+if TYPE_CHECKING:
+  from apache_beam.metrics.metricbase import MetricName

Review comment:
   leftover cruft. fixed





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.

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

[GitHub] [beam] chadrik commented on a change in pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

chadrik commented on a change in pull request #12883:
URL: https://github.com/apache/beam/pull/12883#discussion_r495317141



##
File path: sdks/python/apache_beam/metrics/metric.py
##
@@ -101,23 +121,26 @@ def gauge(namespace, name):
   class DelegatingCounter(Counter):
 """Metrics Counter that Delegates functionality to MetricsEnvironment."""
 def __init__(self, metric_name):
+  # type: (MetricName) -> None
   super(Metrics.DelegatingCounter, self).__init__()
   self.metric_name = metric_name
-  self.inc = MetricUpdater(cells.CounterCell, metric_name, default=1)
+  self.inc = MetricUpdater(cells.CounterCell, metric_name, default=1)  # 
type: ignore[assignment]

Review comment:
   Here's the workaround:
   
   ```python
 class DelegatingCounter(Counter):
   """Metrics Counter that Delegates functionality to MetricsEnvironment."""
   def __init__(self, metric_name):
 # type: (MetricName) -> None
 super(Metrics.DelegatingCounter, self).__init__(metric_name)
 self._inc = MetricUpdater(cells.CounterCell, metric_name, default=1)
   
   def inc(value=None):
 self._inc(value)
   ```
   
   I don't know the reasoning behind this limitation.  Even the simplest 
scenario fails on the latest version of mypy:
   
   ```python
   class Foo(object):
 x = None  # type: Callable[[], None]
   
   def blah():
 # type: () -> None
 pass
   
   class Bar(Foo):
 def __init__(self):
   # type: () -> None
   self.x = blah
   ```
   
   Note that it does work without complaint if the callable is defined on the 
class:
   
   ```python
   class Bar(Foo):
  x = blah
   ```
   
   but we can't do that because `MetricUpdater` needs the `metric_name`.
   
   So, since what's being done in this metrics code is valid python, I thought 
ignoring the error was the most expedient path.  If mypy fixes this in a future 
version, these lines will generate an "unused type ignore" error and we can get 
rid of them.
   
   





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.

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

[GitHub] [beam] aaltay merged pull request #12944: [BEAM-10977] Disable codecov annotations in GH

[GitBox]

aaltay merged pull request #12944:
URL: https://github.com/apache/beam/pull/12944


   



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.

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

[GitHub] [beam] robinyqiu commented on pull request #12943: Cherry-pick [BEAM-10975] Remove capture_output argument in sdk_container_builder …

[GitBox]

robinyqiu commented on pull request #12943:
URL: https://github.com/apache/beam/pull/12943#issuecomment-699203149


   I think the failing Python tests are just flaky? Anyway, I just run them 
again.



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

codecov[bot] edited a comment on pull request #12883:
URL: https://github.com/apache/beam/pull/12883#issuecomment-699197950


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=h1) Report
   > Merging 
[#12883](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc)
 will **decrease** coverage by `0.02%`.
   > The diff coverage is `76.19%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12883/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12883  +/-   ##
   ==
   - Coverage   82.34%   82.31%   -0.03% 
   ==
 Files 452  455   +3 
 Lines   5401654688 +672 
   ==
   + Hits4448145019 +538 
   - Misses   9535 9669 +134 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/metrics/cells.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9jZWxscy5weQ==)
 | `80.73% <50.00%> (-1.32%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/metrics/monitoring\_infos.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tb25pdG9yaW5nX2luZm9zLnB5)
 | `96.64% <50.00%> (-0.53%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/execution.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9leGVjdXRpb24ucHk=)
 | `87.69% <76.00%> (-2.58%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWMucHk=)
 | `95.31% <90.90%> (-1.36%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metricbase.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWNiYXNlLnB5)
 | `88.23% <100.00%> (+0.35%)` | :arrow_up: |
   | 
[...am/runners/interactive/options/capture\_limiters.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9vcHRpb25zL2NhcHR1cmVfbGltaXRlcnMucHk=)
 | `90.47% <0.00%> (-3.08%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-1.01%)` | :arrow_down: |
   | ... and [47 
more](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=footer). Last 
update 
[067cba8...078cb50](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] commented on pull request #12883: [BEAM-7746] Add more typing to metrics

[GitBox]

codecov[bot] commented on pull request #12883:
URL: https://github.com/apache/beam/pull/12883#issuecomment-699197950


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=h1) Report
   > Merging 
[#12883](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc)
 will **decrease** coverage by `0.02%`.
   > The diff coverage is `76.19%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12883/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12883  +/-   ##
   ==
   - Coverage   82.34%   82.31%   -0.03% 
   ==
 Files 452  455   +3 
 Lines   5401654688 +672 
   ==
   + Hits4448145019 +538 
   - Misses   9535 9669 +134 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/metrics/cells.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9jZWxscy5weQ==)
 | `80.73% <50.00%> (-1.32%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/metrics/monitoring\_infos.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tb25pdG9yaW5nX2luZm9zLnB5)
 | `96.64% <50.00%> (-0.53%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/execution.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9leGVjdXRpb24ucHk=)
 | `87.69% <76.00%> (-2.58%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWMucHk=)
 | `95.31% <90.90%> (-1.36%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/metrics/metricbase.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWV0cmljcy9tZXRyaWNiYXNlLnB5)
 | `88.23% <100.00%> (+0.35%)` | :arrow_up: |
   | 
[...am/runners/interactive/options/capture\_limiters.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9vcHRpb25zL2NhcHR1cmVfbGltaXRlcnMucHk=)
 | `90.47% <0.00%> (-3.08%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/dataframe/schemas.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL3NjaGVtYXMucHk=)
 | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/gcp/pubsub.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL3B1YnN1Yi5weQ==)
 | `92.30% <0.00%> (-1.28%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[...dks/python/apache\_beam/runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | `92.80% <0.00%> (-1.01%)` | :arrow_down: |
   | ... and [47 
more](https://codecov.io/gh/apache/beam/pull/12883/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=footer). Last 
update 
[067cba8...078cb50](https://codecov.io/gh/apache/beam/pull/12883?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] y1chi commented on pull request #12943: Cherry-pick [BEAM-10975] Remove capture_output argument in sdk_container_builder …

[GitBox]

y1chi commented on pull request #12943:
URL: https://github.com/apache/beam/pull/12943#issuecomment-699196211


   Run Python 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.

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

[GitHub] [beam] kileys opened a new pull request #12946: Compile Java 11 validates runner jobs with Java 11

[GitBox]

kileys opened a new pull request #12946:
URL: https://github.com/apache/beam/pull/12946


   Update Java 11 validates runner jenkins job to compile and run with Java 11
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 

[GitHub] [beam] pabloem edited a comment on pull request #12734: Add authentication story via pipeline options and tests using Azurite.

[GitBox]

pabloem edited a comment on pull request #12734:
URL: https://github.com/apache/beam/pull/12734#issuecomment-699192199


   I have asked @AldairCoronel to please push this through to the end. He's 
busy this week, but we'll try to finalize it starting next week.
   
   @tanya-borisova thanks for the interest. I hope we'll be able to wrap this 
up soon, and have it available for Beam 2.26 or 2.27. Will that work for you? : 
)
   
   It should be possible to have a relative shortcut to simply pass the 
connection string via pipeline options, and have that available soon.



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.

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

[GitHub] [beam] pabloem commented on pull request #12734: Add authentication story via pipeline options and tests using Azurite.

[GitBox]

pabloem commented on pull request #12734:
URL: https://github.com/apache/beam/pull/12734#issuecomment-699192199


   I have asked @AldairCoronel to please push this through to the end. He's 
busy this week, but we'll try to finalize it starting next week.
   
   @tanya-borisova thanks for the interest. I hope we'll be able to wrap this 
up soon, and have it available for Beam 2.26 or 2.27. Will that work for you? : 
)
   
   It should be possible to have a relative shortcut to simply pass the 
connection string via options sometimes.



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.

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

[GitHub] [beam] ibzib opened a new pull request #12945: [BEAM-10971] Redirect contribute/design-documents to cwiki.

[GitBox]

ibzib opened a new pull request #12945:
URL: https://github.com/apache/beam/pull/12945


   **Please** add a meaningful description for your change here
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 
Status](https://

[GitHub] [beam] pabloem commented on pull request #12855: Update BigQueryAvroUtils.java to properly support BOOL type of Big Query

[GitBox]

pabloem commented on pull request #12855:
URL: https://github.com/apache/beam/pull/12855#issuecomment-699186422


   Thanks @xasm83 - we appreciate your contribution! 
   
   We have a test that expects the wrong type name: 
https://ci-beam.apache.org/job/beam_PreCommit_Java_Commit/13598/testReport/junit/org.apache.beam.sdk.io.gcp.bigquery/BigQueryAvroUtilsTest/testConvertGenericRecordToTableRow/
   
   Could you allow "BOOL" and "BOOLEAN" types in your change, in case we rely 
on the wrong time somewhere? And please change the test in 
BigQueryAvroUtilsTest?
   
   Thanks again!



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12562: [BEAM-10200] Respect profile_memory option and add memory profiler to…

[GitBox]

codecov[bot] edited a comment on pull request #12562:
URL: https://github.com/apache/beam/pull/12562#issuecomment-696271535


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12562?src=pr&el=h1) Report
   > Merging 
[#12562](https://codecov.io/gh/apache/beam/pull/12562?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/e164d170eb6b5ec199f09e79dfb0147b84ae?el=desc)
 will **increase** coverage by `47.83%`.
   > The diff coverage is `6.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12562/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12562?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff @@
   ##   master   #12562   +/-   ##
   ===
   + Coverage   34.47%   82.30%   +47.83% 
   ===
 Files 684  455  -229 
 Lines   8148354583-26900 
 Branches 91800 -9180 
   ===
   + Hits2809044927+16837 
   + Misses  52972 9656-43316 
   + Partials  4210  -421 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12562?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[sdks/python/apache\_beam/utils/profiler.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvcHJvZmlsZXIucHk=)
 | `28.12% <6.00%> (ø)` | |
   | 
[io/gcp/dicomclient.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-aW8vZ2NwL2RpY29tY2xpZW50LnB5)
 | | |
   | 
[runners/job/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-cnVubmVycy9qb2IvX19pbml0X18ucHk=)
 | | |
   | 
[dataframe/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-ZGF0YWZyYW1lL19faW5pdF9fLnB5)
 | | |
   | 
[...pets/transforms/elementwise/withtimestamps\_test.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-ZXhhbXBsZXMvc25pcHBldHMvdHJhbnNmb3Jtcy9lbGVtZW50d2lzZS93aXRodGltZXN0YW1wc190ZXN0LnB5)
 | | |
   | 
[runners/portability/flink\_runner.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-cnVubmVycy9wb3J0YWJpbGl0eS9mbGlua19ydW5uZXIucHk=)
 | | |
   | 
[examples/complete/tfidf\_test.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-ZXhhbXBsZXMvY29tcGxldGUvdGZpZGZfdGVzdC5weQ==)
 | | |
   | 
[runners/pipeline\_context.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-cnVubmVycy9waXBlbGluZV9jb250ZXh0LnB5)
 | | |
   | 
[runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-cnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==)
 | | |
   | 
[coders/observable\_test.py](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree#diff-Y29kZXJzL29ic2VydmFibGVfdGVzdC5weQ==)
 | | |
   | ... and [1130 
more](https://codecov.io/gh/apache/beam/pull/12562/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12562?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12562?src=pr&el=footer). Last 
update 
[e44c3c2...b29fdcc](https://codecov.io/gh/apache/beam/pull/12562?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] kennknowles commented on a change in pull request #12899: [BEAM-8024] Add JPMS E2E test

[GitBox]

kennknowles commented on a change in pull request #12899:
URL: https://github.com/apache/beam/pull/12899#discussion_r495269114



##
File path: sdks/java/testing/jpms-tests/build.gradle
##
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+plugins {
+  id 'org.apache.beam.module'
+}
+javaVersion="1.11"
+applyJavaNature(
+  enableChecker:false,
+  exportJavadoc: false,
+  publish: false,
+  disableLintWarnings: ['requires-transitive-automatic', 'requires-automatic']
+)
+provideIntegrationTestingDependencies()
+enableJavaPerformanceTesting()
+
+description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
+ext.summary = "E2E test for Java 9 modules"
+
+dependencies {
+  compile project(path: ":sdks:java:core", configuration: "shadow")
+  compile project(path: ":runners:direct-java")

Review comment:
   I believe this would be `runtime` or potentially `testRuntimeOnly`.
   
   If I understand this one correctly right now, it runs the cloned WordCount 
on the DirectRunner. You may want to do some of the hacking done 
[here](https://github.com/apache/beam/blob/38403698934c422968320a0bdd834e8b9ae598a3/examples/java/build.gradle#L99)
 to run it on a variety of runners. I don't know which might (1) be able to run 
it or (2) have infrastructures that would make this test check anything 
meaningful.





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.

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

[GitHub] [beam] ibzib commented on pull request #12907: [BEAM-10953] Add errorprone-slf4j plugin.

[GitBox]

ibzib commented on pull request #12907:
URL: https://github.com/apache/beam/pull/12907#issuecomment-699172961


   > FWIW WindowTracing does not have to be the way it is. It is redundant with 
just having a variety of loggers in WindowTracing in which case all format 
strings could be constants. In a way, WindowTracing is circumventing the whole 
point of having the ability to make different loggers.
   
   I can see about removing it then.
   
   Current status: I ran into at least one place where we will need to make an 
exemption. Haven't checked yet if the plugin makes that possible.
   
   
https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowWorkerHarnessHelper.java#L83



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.

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

[GitHub] [beam] kennknowles commented on pull request #12907: [BEAM-10953] Add errorprone-slf4j plugin.

[GitBox]

kennknowles commented on pull request #12907:
URL: https://github.com/apache/beam/pull/12907#issuecomment-699171704


   FWIW WindowTracing does not have to be the way it is. It is redundant with 
just having a variety of loggers in WindowTracing in which case all format 
strings could be constants. In a way, WindowTracing is circumventing the whole 
point of having the ability to make different loggers.



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.

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

[GitHub] [beam] ibzib edited a comment on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.

[GitBox]

ibzib edited a comment on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629


   I suspect this caused portable Flink postcommit to fail: BEAM-10964, 
https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
   
   cc: @robinyqiu 



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.

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

[GitHub] [beam] ibzib edited a comment on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.

[GitBox]

ibzib edited a comment on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629


   I suspect this caused portable Flink postcommit to fail: BEAM-10974, 
https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
   
   cc: @robinyqiu 



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.

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

[GitHub] [beam] aaltay opened a new pull request #12944: [BEAM-10977] Disable codecov annotations in GH

[GitBox]

aaltay opened a new pull request #12944:
URL: https://github.com/apache/beam/pull/12944


   **Please** add a meaningful description for your change here
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 
Status](https:/

[GitHub] [beam] ibzib edited a comment on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.

[GitBox]

ibzib edited a comment on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629


   I suspect this caused portable Flink postcommit to fail: 
https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
   
   cc: @robinyqiu 



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.

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

[GitHub] [beam] ibzib commented on pull request #12708: [BEAM-10670] Update Flink to be opt-out for SplittableDoFn powering the Read transform.

[GitBox]

ibzib commented on pull request #12708:
URL: https://github.com/apache/beam/pull/12708#issuecomment-699167629


   I suspect this caused Flink postcommit to fail: 
https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/5393/
   
   cc: @robinyqiu 



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.

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

[GitHub] [beam] lukecwik merged pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik merged pull request #12934:
URL: https://github.com/apache/beam/pull/12934


   



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.

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

[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495229524



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[
+with self._lock:
+  try:
+# pop() is threadsafe
+processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
+self.active_bundle_processors[
+  instruction_id] = bundle_descriptor_id, processor
+try:
+  del self.known_not_running_instruction_ids[instruction_id]

Review comment:
   Ack. Now that I understand the semantics as the value existing in both 
before and after processing this makes sense. 





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.

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

[GitHub] [beam] lukecwik commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495223102



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[

Review comment:
   The `pop` is thread safe but accessing 
`self.cached_bundle_processors[bundle_descriptor_id]` is not guaranteed to be 
so your right that some could be saved based upon your suggestion but it still 
requires a self.lock surrounding
   ```
   processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
   ```
   
   which will lead to us acquiring the mutex twice for the case when there is a 
cached instance while the current imlementation only needs the mutex twice if 
we need to create the BundleProcessor.





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.

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

[GitHub] [beam] lukecwik commented on pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on pull request #12934:
URL: https://github.com/apache/beam/pull/12934#issuecomment-699144906


   > This this bug is unrelated to SDF, right? Would this be erroneously 
reporting successful bundle processing, or is it a minor issue since the 
pipeline is being shut down?
   
   @udim It causes a race for users/tests that finalization might be in 
progress when the pipeline is shutting down.



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.

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

[GitHub] [beam] lukecwik commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495219590



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -543,15 +602,19 @@ def process_bundle_split(self,
instruction_id  # type: str
   ):
 # type: (...) -> beam_fn_api_pb2.InstructionResponse
-processor = self.bundle_processor_cache.lookup(request.instruction_id)
-if processor:
-  return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  process_bundle_split=processor.try_split(request))
-else:
+try:
+  processor = self.bundle_processor_cache.lookup(request.instruction_id)
+except RuntimeError:
   return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  error='Instruction not running: %s' % instruction_id)
+  instruction_id=instruction_id, error=traceback.format_exc())
+# Return an empty response if we aren't running. This can happen
+# if the ProcessBundleRequest has not started or already finished.
+process_bundle_split = (
+processor.try_split(request)
+if processor else beam_fn_api_pb2.ProcessBundleSplitResponse())

Review comment:
   You'll want to differentiate the failed case and once the Go SDK handles 
bundle finalization consider that as well (i.e. keep INST live until the 
finalize request comes or a user specified timeout happens)





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.

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

[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495217147



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -543,15 +602,19 @@ def process_bundle_split(self,
instruction_id  # type: str
   ):
 # type: (...) -> beam_fn_api_pb2.InstructionResponse
-processor = self.bundle_processor_cache.lookup(request.instruction_id)
-if processor:
-  return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  process_bundle_split=processor.try_split(request))
-else:
+try:
+  processor = self.bundle_processor_cache.lookup(request.instruction_id)
+except RuntimeError:
   return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  error='Instruction not running: %s' % instruction_id)
+  instruction_id=instruction_id, error=traceback.format_exc())
+# Return an empty response if we aren't running. This can happen
+# if the ProcessBundleRequest has not started or already finished.
+process_bundle_split = (
+processor.try_split(request)
+if processor else beam_fn_api_pb2.ProcessBundleSplitResponse())

Review comment:
   So to be clear, the semantics are required to be:
   
   Receive PB request for INST. -> Empty Splits & and Progress
   INST starts having results -> Return results.
   INST finishes -> EmptySplits and progress until "expired".
   And the "I don't know" is an error for  every other circumstance?
   
   In the case where the bundle finishes, but requires Finalization, it INST 
should remain valid and Empties until finalization occurs.
   





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.

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

[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495219312



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -543,15 +602,19 @@ def process_bundle_split(self,
instruction_id  # type: str
   ):
 # type: (...) -> beam_fn_api_pb2.InstructionResponse
-processor = self.bundle_processor_cache.lookup(request.instruction_id)
-if processor:
-  return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  process_bundle_split=processor.try_split(request))
-else:
+try:
+  processor = self.bundle_processor_cache.lookup(request.instruction_id)
+except RuntimeError:
   return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  error='Instruction not running: %s' % instruction_id)
+  instruction_id=instruction_id, error=traceback.format_exc())
+# Return an empty response if we aren't running. This can happen
+# if the ProcessBundleRequest has not started or already finished.
+process_bundle_split = (
+processor.try_split(request)
+if processor else beam_fn_api_pb2.ProcessBundleSplitResponse())

Review comment:
   Filed https://issues.apache.org/jira/browse/BEAM-10976 for implementing 
Go SDK bundle finalization along with a reminder about this issue regarding the 
active status.





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.

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

[GitHub] [beam] y1chi opened a new pull request #12943: Cherry-pick [BEAM-10975] Remove capture_output argument in sdk_container_builder …

[GitBox]

y1chi opened a new pull request #12943:
URL: https://github.com/apache/beam/pull/12943


   …(#12942)
   
   **Please** add a meaningful description for your change here
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 

[GitHub] [beam] y1chi commented on pull request #12943: Cherry-pick [BEAM-10975] Remove capture_output argument in sdk_container_builder …

[GitBox]

y1chi commented on pull request #12943:
URL: https://github.com/apache/beam/pull/12943#issuecomment-699142798


   R: @robinyqiu 



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.

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

[GitHub] [beam] lukecwik commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495218675



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[
+with self._lock:
+  try:
+# pop() is threadsafe
+processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
+self.active_bundle_processors[
+  instruction_id] = bundle_descriptor_id, processor
+try:
+  del self.known_not_running_instruction_ids[instruction_id]

Review comment:
   The `instruction_id` refers to the original ProcessBundleRequest 
`instruction_id` which is different then the current requests `instruction_id` 
found on `request.instruction_id`





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.

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

[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495217147



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -543,15 +602,19 @@ def process_bundle_split(self,
instruction_id  # type: str
   ):
 # type: (...) -> beam_fn_api_pb2.InstructionResponse
-processor = self.bundle_processor_cache.lookup(request.instruction_id)
-if processor:
-  return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  process_bundle_split=processor.try_split(request))
-else:
+try:
+  processor = self.bundle_processor_cache.lookup(request.instruction_id)
+except RuntimeError:
   return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  error='Instruction not running: %s' % instruction_id)
+  instruction_id=instruction_id, error=traceback.format_exc())
+# Return an empty response if we aren't running. This can happen
+# if the ProcessBundleRequest has not started or already finished.
+process_bundle_split = (
+processor.try_split(request)
+if processor else beam_fn_api_pb2.ProcessBundleSplitResponse())

Review comment:
   So to be clear, the semantics are required to be:
   
   Receive PB request for INST. -> Empty Splits & and Progress
   INST starts having results -> Return results.
   INST finishes -> EmptySplits and progress until "expired".
   And the "I don't know" is an error for  every other circumstance?
   
   In the case where the bundle finishes, but requires Finalization, it INST 
should remain valid and return the it's current set of responses for splits and 
progress as usual, rather than the empties.
   





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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

codecov[bot] edited a comment on pull request #12934:
URL: https://github.com/apache/beam/pull/12934#issuecomment-698657357


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=h1) Report
   > Merging 
[#12934](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/967d7288cd5069f21617c98a9156dd4076bb9fb9?el=desc)
 will **decrease** coverage by `0.13%`.
   > The diff coverage is `92.36%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12934/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12934  +/-   ##
   ==
   - Coverage   82.45%   82.32%   -0.14% 
   ==
 Files 454  455   +1 
 Lines   5460754650  +43 
   ==
   - Hits4502844990  -38 
   - Misses   9579 9660  +81 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <80.00%> (+0.23%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `89.98% <89.70%> (+1.17%)` | :arrow_up: |
   | 
[...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5)
 | `78.85% <93.75%> (-0.16%)` | :arrow_down: |
   | 
[...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==)
 | `89.45% <97.05%> (+0.02%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vX19pbml0X18ucHk=)
 | `85.71% <100.00%> (ø)` | |
   | 
[...ache\_beam/runners/interactive/recording\_manager.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9yZWNvcmRpbmdfbWFuYWdlci5weQ==)
 | `98.88% <100.00%> (-0.20%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5)
 | `83.75% <0.00%> (-0.58%)` | :arrow_down: |
   | ... and [25 
more](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=footer). Last 
update 
[1a09a75...28b604c](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] tvalentyn merged pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

tvalentyn merged pull request #12942:
URL: https://github.com/apache/beam/pull/12942


   



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.

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

[GitHub] [beam] tvalentyn commented on a change in pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

tvalentyn commented on a change in pull request #12942:
URL: https://github.com/apache/beam/pull/12942#discussion_r495211695



##
File path: sdks/python/apache_beam/runners/portability/sdk_container_builder.py
##
@@ -149,7 +149,6 @@ def invoke_docker_build_and_push(self, 
container_image_name):
   _LOGGER.info("Building sdk container, this may take a few minutes...")
   now = time.time()
   subprocess.run(['docker', 'build', '.', '-t', container_image_name],
- capture_output=True,

Review comment:
   Not capturing any output should help. But may be noisy. We can consider 
to capture the output but print it only if something goes wrong. 
   Leaving the change as is SGMT for 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.

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

[GitHub] [beam] tvalentyn commented on a change in pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

tvalentyn commented on a change in pull request #12942:
URL: https://github.com/apache/beam/pull/12942#discussion_r495211141



##
File path: sdks/python/apache_beam/runners/portability/sdk_container_builder.py
##
@@ -149,7 +149,6 @@ def invoke_docker_build_and_push(self, 
container_image_name):
   _LOGGER.info("Building sdk container, this may take a few minutes...")
   now = time.time()
   subprocess.run(['docker', 'build', '.', '-t', container_image_name],
- capture_output=True,

Review comment:
   If there is an build error, how will users see it?





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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

codecov[bot] edited a comment on pull request #12942:
URL: https://github.com/apache/beam/pull/12942#issuecomment-699116630


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=h1) Report
   > Merging 
[#12942](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **increase** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12942/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master   #12942   +/-   ##
   ===
 Coverage   82.31%   82.31%   
   ===
 Files 455  455   
 Lines   5460354603   
   ===
   + Hits4494544948+3 
   + Misses   9658 9655-3 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <0.00%> (ø)` | |
   | 
[.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5)
 | `88.63% <0.00%> (-4.55%)` | :arrow_down: |
   | 
[...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=)
 | `90.90% <0.00%> (-1.82%)` | :arrow_down: |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=)
 | `89.20% <0.00%> (+0.44%)` | :arrow_up: |
   | 
[...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=)
 | `79.34% <0.00%> (+1.81%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=footer). Last 
update 
[806e67d...7691022](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] aaltay merged pull request #12940: spannerio.py doc typo fix

[GitBox]

aaltay merged pull request #12940:
URL: https://github.com/apache/beam/pull/12940


   



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.

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

[GitHub] [beam] aaltay commented on pull request #12940: spannerio.py doc typo fix

[GitBox]

aaltay commented on pull request #12940:
URL: https://github.com/apache/beam/pull/12940#issuecomment-699132490


   Thank you.



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.

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

[GitHub] [beam] lukecwik commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495208617



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -543,15 +602,19 @@ def process_bundle_split(self,
instruction_id  # type: str
   ):
 # type: (...) -> beam_fn_api_pb2.InstructionResponse
-processor = self.bundle_processor_cache.lookup(request.instruction_id)
-if processor:
-  return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  process_bundle_split=processor.try_split(request))
-else:
+try:
+  processor = self.bundle_processor_cache.lookup(request.instruction_id)
+except RuntimeError:
   return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  error='Instruction not running: %s' % instruction_id)
+  instruction_id=instruction_id, error=traceback.format_exc())
+# Return an empty response if we aren't running. This can happen
+# if the ProcessBundleRequest has not started or already finished.
+process_bundle_split = (
+processor.try_split(request)
+if processor else beam_fn_api_pb2.ProcessBundleSplitResponse())

Review comment:
   Yes, but you should strive to return an error for unknown/failed 
instructions.
   
   This PR also keeps a fixed number of completed/failed instructions in a LRU 
like object so that even after the bundle completes we can still return a 
useful answer before we start sending the `I don't know` answer.





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.

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

[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495207572



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -543,15 +602,19 @@ def process_bundle_split(self,
instruction_id  # type: str
   ):
 # type: (...) -> beam_fn_api_pb2.InstructionResponse
-processor = self.bundle_processor_cache.lookup(request.instruction_id)
-if processor:
-  return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  process_bundle_split=processor.try_split(request))
-else:
+try:
+  processor = self.bundle_processor_cache.lookup(request.instruction_id)
+except RuntimeError:
   return beam_fn_api_pb2.InstructionResponse(
-  instruction_id=instruction_id,
-  error='Instruction not running: %s' % instruction_id)
+  instruction_id=instruction_id, error=traceback.format_exc())
+# Return an empty response if we aren't running. This can happen
+# if the ProcessBundleRequest has not started or already finished.
+process_bundle_split = (
+processor.try_split(request)
+if processor else beam_fn_api_pb2.ProcessBundleSplitResponse())

Review comment:
   So, the correct response if something hasn't previously failed, and 
isn't running is an empty response for both Splits and Progress requests?
   
   That's easy enough to fix. Go current fails, but it's very easy to swap that 
up.





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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

codecov[bot] edited a comment on pull request #12934:
URL: https://github.com/apache/beam/pull/12934#issuecomment-698657357


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=h1) Report
   > Merging 
[#12934](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/967d7288cd5069f21617c98a9156dd4076bb9fb9?el=desc)
 will **decrease** coverage by `0.13%`.
   > The diff coverage is `92.36%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12934/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12934  +/-   ##
   ==
   - Coverage   82.45%   82.32%   -0.14% 
   ==
 Files 454  455   +1 
 Lines   5460754650  +43 
   ==
   - Hits4502844990  -38 
   - Misses   9579 9660  +81 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <80.00%> (+0.23%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `89.98% <89.70%> (+1.17%)` | :arrow_up: |
   | 
[...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5)
 | `78.85% <93.75%> (-0.16%)` | :arrow_down: |
   | 
[...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==)
 | `89.45% <97.05%> (+0.02%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vX19pbml0X18ucHk=)
 | `85.71% <100.00%> (ø)` | |
   | 
[...ache\_beam/runners/interactive/recording\_manager.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9yZWNvcmRpbmdfbWFuYWdlci5weQ==)
 | `98.88% <100.00%> (-0.20%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5)
 | `83.75% <0.00%> (-0.58%)` | :arrow_down: |
   | ... and [25 
more](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=footer). Last 
update 
[1a09a75...28b604c](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495205565



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[
+with self._lock:
+  try:
+# pop() is threadsafe
+processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
+self.active_bundle_processors[
+  instruction_id] = bundle_descriptor_id, processor
+try:
+  del self.known_not_running_instruction_ids[instruction_id]

Review comment:
   Aren't all instruction ids unique for a given SDK harness/runner combo 
anyway? If this is the first time we've seen this instruction, why would the 
instruction be repeated?  I thought only the bundle descriptor ids were re-used?





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.

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

[GitHub] [beam] y1chi commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

y1chi commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495205645



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[

Review comment:
   isn't it the same if we do
   ```
   try:
 # pop() is threadsafe
 processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
   except IndexError:
 processor = bundle_processor.BundleProcessor(
 self.fns[bundle_descriptor_id],
 self.state_handler_factory.create_state_handler(
 self.fns[bundle_descriptor_id].state_api_service_descriptor),
 self.data_channel_factory)
   
   with self._lock:
 self.active_bundle_processors[
   instruction_id] = bundle_descriptor_id, processor
 try:
   del self.known_not_running_instruction_ids[instruction_id]
 except KeyError:
   # The instruction may have not been pre-registered before execution
   # since activate() may have never been invoked
   pass
   return processor
   ```





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.

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

[GitHub] [beam] kileys commented on pull request #12899: [BEAM-8024] Add JPMS E2E test

[GitBox]

kileys commented on pull request #12899:
URL: https://github.com/apache/beam/pull/12899#issuecomment-699127694


   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.

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

[GitHub] [beam] kileys commented on pull request #12899: [BEAM-8024] Add JPMS E2E test

[GitBox]

kileys commented on pull request #12899:
URL: https://github.com/apache/beam/pull/12899#issuecomment-699127479


   https://scans.gradle.com/s/kf3lwdc5cvidm/tests



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

codecov[bot] edited a comment on pull request #12934:
URL: https://github.com/apache/beam/pull/12934#issuecomment-698657357


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=h1) Report
   > Merging 
[#12934](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/967d7288cd5069f21617c98a9156dd4076bb9fb9?el=desc)
 will **decrease** coverage by `0.13%`.
   > The diff coverage is `92.36%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12934/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12934  +/-   ##
   ==
   - Coverage   82.45%   82.32%   -0.14% 
   ==
 Files 454  455   +1 
 Lines   5460754650  +43 
   ==
   - Hits4502844990  -38 
   - Misses   9579 9660  +81 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <80.00%> (+0.23%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `89.98% <89.70%> (+1.17%)` | :arrow_up: |
   | 
[...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5)
 | `78.85% <93.75%> (-0.16%)` | :arrow_down: |
   | 
[...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==)
 | `89.45% <97.05%> (+0.02%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vX19pbml0X18ucHk=)
 | `85.71% <100.00%> (ø)` | |
   | 
[...ache\_beam/runners/interactive/recording\_manager.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9yZWNvcmRpbmdfbWFuYWdlci5weQ==)
 | `98.88% <100.00%> (-0.20%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5)
 | `83.75% <0.00%> (-0.58%)` | :arrow_down: |
   | ... and [25 
more](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=footer). Last 
update 
[1a09a75...28b604c](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

codecov[bot] edited a comment on pull request #12942:
URL: https://github.com/apache/beam/pull/12942#issuecomment-699116630


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=h1) Report
   > Merging 
[#12942](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **increase** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12942/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master   #12942   +/-   ##
   ===
 Coverage   82.31%   82.31%   
   ===
 Files 455  455   
 Lines   5460354603   
   ===
   + Hits4494544948+3 
   + Misses   9658 9655-3 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <0.00%> (ø)` | |
   | 
[.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5)
 | `88.63% <0.00%> (-4.55%)` | :arrow_down: |
   | 
[...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=)
 | `90.90% <0.00%> (-1.82%)` | :arrow_down: |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=)
 | `89.20% <0.00%> (+0.44%)` | :arrow_up: |
   | 
[...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=)
 | `79.34% <0.00%> (+1.81%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=footer). Last 
update 
[806e67d...7691022](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

codecov[bot] edited a comment on pull request #12942:
URL: https://github.com/apache/beam/pull/12942#issuecomment-699116630


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=h1) Report
   > Merging 
[#12942](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **increase** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12942/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree)
   
   ```diff
   @@   Coverage Diff   @@
   ##   master   #12942   +/-   ##
   ===
 Coverage   82.31%   82.31%   
   ===
 Files 455  455   
 Lines   5460354603   
   ===
   + Hits4494544948+3 
   + Misses   9658 9655-3 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <0.00%> (ø)` | |
   | 
[.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5)
 | `88.63% <0.00%> (-4.55%)` | :arrow_down: |
   | 
[...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=)
 | `90.90% <0.00%> (-1.82%)` | :arrow_down: |
   | 
[...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==)
 | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   | 
[sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=)
 | `89.20% <0.00%> (+0.44%)` | :arrow_up: |
   | 
[...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=)
 | `79.34% <0.00%> (+1.81%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=footer). Last 
update 
[806e67d...7691022](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

codecov[bot] edited a comment on pull request #12934:
URL: https://github.com/apache/beam/pull/12934#issuecomment-698657357


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=h1) Report
   > Merging 
[#12934](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/967d7288cd5069f21617c98a9156dd4076bb9fb9?el=desc)
 will **decrease** coverage by `0.12%`.
   > The diff coverage is `92.36%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12934/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12934  +/-   ##
   ==
   - Coverage   82.45%   82.33%   -0.13% 
   ==
 Files 454  455   +1 
 Lines   5460754650  +43 
   ==
   - Hits4502844996  -32 
   - Misses   9579 9654  +75 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==)
 | `89.97% <80.00%> (+0.23%)` | :arrow_up: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `90.14% <89.70%> (+1.33%)` | :arrow_up: |
   | 
[...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5)
 | `78.85% <93.75%> (-0.16%)` | :arrow_down: |
   | 
[...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==)
 | `89.45% <97.05%> (+0.02%)` | :arrow_up: |
   | 
[sdks/python/apache\_beam/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vX19pbml0X18ucHk=)
 | `85.71% <100.00%> (ø)` | |
   | 
[...ache\_beam/runners/interactive/recording\_manager.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9yZWNvcmRpbmdfbWFuYWdlci5weQ==)
 | `98.88% <100.00%> (-0.20%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==)
 | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | ... and [25 
more](https://codecov.io/gh/apache/beam/pull/12934/diff?src=pr&el=tree-more) | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=footer). Last 
update 
[1a09a75...28b604c](https://codecov.io/gh/apache/beam/pull/12934?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] lukecwik commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495196469



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -415,10 +465,19 @@ def release(self, instruction_id):
 Resets the ``BundleProcessor`` and moves it from the active to the
 inactive cache.
 """
-descriptor_id, processor = 
self.active_bundle_processors.pop(instruction_id)
+with self._lock:
+  self.known_not_running_instruction_ids[instruction_id] = True
+  while len(self.known_not_running_instruction_ids
+) > MAX_KNOWN_NOT_RUNNING_INSTRUCTIONS:
+self.known_not_running_instruction_ids.popitem()
+  descriptor_id, processor = (
+  self.active_bundle_processors.pop(instruction_id))
+
+# Make sure that we reset the processor while not holding the lock.
 processor.reset()
-self.last_access_times[descriptor_id] = time.time()
-self.cached_bundle_processors[descriptor_id].append(processor)
+with self._lock:
+  self.last_access_times[descriptor_id] = time.time()
+  self.cached_bundle_processors[descriptor_id].append(processor)
 
   def shutdown(self):
 """

Review comment:
   From my understanding I also think it should be fine since it is on 
shutdown.





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.

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

[GitHub] [beam] lukecwik commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495196209



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[
+with self._lock:
+  try:
+# pop() is threadsafe
+processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
+self.active_bundle_processors[
+  instruction_id] = bundle_descriptor_id, processor
+try:
+  del self.known_not_running_instruction_ids[instruction_id]
+except KeyError:
+  # The instruction may have not been pre-registered before execution
+  # since activate() may have never been invoked
+  pass
+return processor
+  except IndexError:
+pass
+
+# Make sure we instantiate the processor while not holding the lock.
+processor = bundle_processor.BundleProcessor(
+self.fns[bundle_descriptor_id],
+self.state_handler_factory.create_state_handler(
+self.fns[bundle_descriptor_id].state_api_service_descriptor),
+self.data_channel_factory)
+with self._lock:
+  self.active_bundle_processors[
 instruction_id] = bundle_descriptor_id, processor
+  try:
+del self.known_not_running_instruction_ids[instruction_id]
+  except KeyError:
+# The instruction may have not been pre-registered before execution
+# since activate() may have never been invoked
+pass
 return processor
 
   def lookup(self, instruction_id):
 # type: (str) -> Optional[bundle_processor.BundleProcessor]
 
 """
 Return the requested ``BundleProcessor`` from the cache.
+
+Will return ``None`` if the BundleProcessor is known but not yet ready. 
Will
+raise an error if the ``instruction_id`` is not known or has been 
discarded.
 """
-return self.active_bundle_processors.get(instruction_id, (None, None))[-1]
+with self._lock:
+  if instruction_id in self.failed_instruction_ids:
+raise RuntimeError(
+'Bundle processing associated with %s has failed. '
+'Check prior failing response for details.' % instruction_id)
+  processor = self.active_bundle_processors.get(
+  instruction_id, (None, None))[-1]
+  if processor:
+return processor
+  if instruction_id in self.known_not_running_instruction_ids:
+return None
+  raise RuntimeError('Unknown process bundle id %s.' % instruction_id)
 
   def discard(self, instruction_id):
 # type: (str) -> None
 
 """
-Remove the ``BundleProcessor`` from the cache.
+Marks the instruction id as failed shutting down the ``BundleProcessor``.
 """
-self.active_bundle_processors[instruction_id][1].shutdown()
-del self.active_bundle_processors[instruction_id]
+with self._lock:
+  self.failed_instruction_ids[instruction_id] = True
+  while len(self.failed_instruction_ids) > MAX_FAILED_INSTRUCTIONS:
+self.failed_instruction_ids.popitem()

Review comment:
   Fixed.





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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12935: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

[GitBox]

codecov[bot] edited a comment on pull request #12935:
URL: https://github.com/apache/beam/pull/12935#issuecomment-698663460


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base 
(`release-2.25.0@821fdb6`). [Click here to learn what that 
means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12935/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff@@
   ## release-2.25.0   #12935   +/-   ##
   =
 Coverage  ?   82.32%   
   =
 Files ?  455   
 Lines ?54589   
 Branches  ?0   
   =
 Hits  ?44943   
 Misses? 9646   
 Partials  ?0   
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=footer). Last 
update 
[821fdb6...10e9545](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] lukecwik commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

lukecwik commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495195485



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[

Review comment:
   I'm not sure what change your suggesting.
   
   Note, it is important to not hold the lock while we instantiate the 
BundleProcessor or access `self.fns`





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.

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

[GitHub] [beam] tvalentyn commented on a change in pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

tvalentyn commented on a change in pull request #12942:
URL: https://github.com/apache/beam/pull/12942#discussion_r495189388



##
File path: sdks/python/apache_beam/runners/portability/sdk_container_builder.py
##
@@ -149,7 +149,6 @@ def invoke_docker_build_and_push(self, 
container_image_name):
   _LOGGER.info("Building sdk container, this may take a few minutes...")
   now = time.time()
   subprocess.run(['docker', 'build', '.', '-t', container_image_name],
- capture_output=True,

Review comment:
   How about 
   `subprocess.run([ ], stdout=subprocess.PIPE)`
   Woud this capture stdout and print STDERR?





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.

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

[GitHub] [beam] robinyqiu commented on pull request #12935: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

[GitBox]

robinyqiu commented on pull request #12935:
URL: https://github.com/apache/beam/pull/12935#issuecomment-699118127


   Run Samza ValidatesRunner



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

codecov[bot] edited a comment on pull request #12942:
URL: https://github.com/apache/beam/pull/12942#issuecomment-699116630


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=h1) Report
   > Merging 
[#12942](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **decrease** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12942/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12942  +/-   ##
   ==
   - Coverage   82.31%   82.30%   -0.01% 
   ==
 Files 455  455  
 Lines   5460354603  
   ==
   - Hits4494544943   -2 
   - Misses   9658 9660   +2 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <ø> (ø)` | |
   | 
[sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5)
 | `80.26% <0.00%> (-0.45%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `88.98% <0.00%> (+0.17%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=footer). Last 
update 
[806e67d...7691022](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] codecov[bot] commented on pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

codecov[bot] commented on pull request #12942:
URL: https://github.com/apache/beam/pull/12942#issuecomment-699116630


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=h1) Report
   > Merging 
[#12942](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/beam/commit/806e67d77b7e5d59b00d5ad0872923fae29cce9a?el=desc)
 will **decrease** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12942/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #12942  +/-   ##
   ==
   - Coverage   82.31%   82.30%   -0.01% 
   ==
 Files 455  455  
 Lines   5460354603  
   ==
   - Hits4494544943   -2 
   - Misses   9658 9660   +2 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=tree) | Coverage 
Δ | |
   |---|---|---|
   | 
[...\_beam/runners/portability/sdk\_container\_builder.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zZGtfY29udGFpbmVyX2J1aWxkZXIucHk=)
 | `31.21% <ø> (ø)` | |
   | 
[sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5)
 | `80.26% <0.00%> (-0.45%)` | :arrow_down: |
   | 
[...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12942/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==)
 | `88.98% <0.00%> (+0.17%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=footer). Last 
update 
[806e67d...4afea05](https://codecov.io/gh/apache/beam/pull/12942?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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

[GitHub] [beam] y1chi opened a new pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

y1chi opened a new pull request #12942:
URL: https://github.com/apache/beam/pull/12942


   **Please** add a meaningful description for your change here
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] 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://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/i
 
con)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 Status](htt
 
ps://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[![Build
 
Status](https://

[GitHub] [beam] y1chi commented on pull request #12942: [BEAM-10975] Remove capture_output argument in sdk_container_builder

[GitBox]

y1chi commented on pull request #12942:
URL: https://github.com/apache/beam/pull/12942#issuecomment-699110401


   R: @robinyqiu @tvalentyn 



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.

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

[GitHub] [beam] udim commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

udim commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495185774



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[
+with self._lock:
+  try:
+# pop() is threadsafe
+processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
+self.active_bundle_processors[
+  instruction_id] = bundle_descriptor_id, processor
+try:
+  del self.known_not_running_instruction_ids[instruction_id]
+except KeyError:
+  # The instruction may have not been pre-registered before execution
+  # since activate() may have never been invoked
+  pass
+return processor
+  except IndexError:
+pass
+
+# Make sure we instantiate the processor while not holding the lock.
+processor = bundle_processor.BundleProcessor(
+self.fns[bundle_descriptor_id],
+self.state_handler_factory.create_state_handler(
+self.fns[bundle_descriptor_id].state_api_service_descriptor),
+self.data_channel_factory)
+with self._lock:
+  self.active_bundle_processors[
 instruction_id] = bundle_descriptor_id, processor
+  try:
+del self.known_not_running_instruction_ids[instruction_id]
+  except KeyError:
+# The instruction may have not been pre-registered before execution
+# since activate() may have never been invoked
+pass
 return processor
 
   def lookup(self, instruction_id):
 # type: (str) -> Optional[bundle_processor.BundleProcessor]
 
 """
 Return the requested ``BundleProcessor`` from the cache.
+
+Will return ``None`` if the BundleProcessor is known but not yet ready. 
Will
+raise an error if the ``instruction_id`` is not known or has been 
discarded.
 """
-return self.active_bundle_processors.get(instruction_id, (None, None))[-1]
+with self._lock:
+  if instruction_id in self.failed_instruction_ids:
+raise RuntimeError(
+'Bundle processing associated with %s has failed. '
+'Check prior failing response for details.' % instruction_id)
+  processor = self.active_bundle_processors.get(
+  instruction_id, (None, None))[-1]
+  if processor:
+return processor
+  if instruction_id in self.known_not_running_instruction_ids:
+return None
+  raise RuntimeError('Unknown process bundle id %s.' % instruction_id)
 
   def discard(self, instruction_id):
 # type: (str) -> None
 
 """
-Remove the ``BundleProcessor`` from the cache.
+Marks the instruction id as failed shutting down the ``BundleProcessor``.
 """
-self.active_bundle_processors[instruction_id][1].shutdown()
-del self.active_bundle_processors[instruction_id]
+with self._lock:
+  self.failed_instruction_ids[instruction_id] = True
+  while len(self.failed_instruction_ids) > MAX_FAILED_INSTRUCTIONS:
+self.failed_instruction_ids.popitem()

Review comment:
   Good catch, we probably want FIFO





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.

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

[GitHub] [beam] udim commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

udim commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495178326



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -415,10 +465,19 @@ def release(self, instruction_id):
 Resets the ``BundleProcessor`` and moves it from the active to the
 inactive cache.
 """
-descriptor_id, processor = 
self.active_bundle_processors.pop(instruction_id)
+with self._lock:
+  self.known_not_running_instruction_ids[instruction_id] = True
+  while len(self.known_not_running_instruction_ids
+) > MAX_KNOWN_NOT_RUNNING_INSTRUCTIONS:
+self.known_not_running_instruction_ids.popitem()
+  descriptor_id, processor = (
+  self.active_bundle_processors.pop(instruction_id))
+
+# Make sure that we reset the processor while not holding the lock.
 processor.reset()
-self.last_access_times[descriptor_id] = time.time()
-self.cached_bundle_processors[descriptor_id].append(processor)
+with self._lock:
+  self.last_access_times[descriptor_id] = time.time()
+  self.cached_bundle_processors[descriptor_id].append(processor)
 
   def shutdown(self):
 """

Review comment:
   Not sure if any locking needs to be done in this method. Perhaps 
`known_not_running_instruction_ids` should be cleared?
   
   It seems that this is called when no more requests will be processed (end of 
SdkHarness.run()), so this should be fine.

##
File path: 
sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py
##
@@ -894,7 +897,10 @@ def process_bundle(self,
   finalize_request = beam_fn_api_pb2.InstructionRequest(
   finalize_bundle=beam_fn_api_pb2.FinalizeBundleRequest(
   instruction_id=process_bundle_id))
-  self._worker_handler.control_conn.push(finalize_request)
+  finalize_response = self._worker_handler.control_conn.push(
+  finalize_request).get()

Review comment:
   This this bug is unrelated to SDF, right? Would this be erroneously 
reporting successful bundle processing, or is it a minor issue since the 
pipeline is being shut down?





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.

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

[GitHub] [beam] TheNeuralBit commented on a change in pull request #12837: [BEAM-10891] Dockerfile for development container

[GitBox]

TheNeuralBit commented on a change in pull request #12837:
URL: https://github.com/apache/beam/pull/12837#discussion_r495155895



##
File path: website/www/site/content/en/contribute/_index.md
##
@@ -94,6 +94,11 @@ $ go get github.com/linkedin/goavro
 
 gLinux users should configure their machines for sudoless Docker.
 
+Alternatively, you can use the 
[Dockerfile](https://github.com/apache/beam/blob/master/Dockerfile) to setup a 
container meeting the requirements above, and mounting the directory that 
contains the Beam repo to the container. e.g.:

Review comment:
   ```suggestion
   Alternatively, you can use the 
[Dockerfile](https://github.com/apache/beam/blob/master/Dockerfile) to setup a 
container meeting the requirements above, and mount your clone of the Beam repo 
to the container. e.g.:
   ```

##
File path: website/www/site/content/en/contribute/_index.md
##
@@ -94,6 +94,11 @@ $ go get github.com/linkedin/goavro
 
 gLinux users should configure their machines for sudoless Docker.
 
+Alternatively, you can use the 
[Dockerfile](https://github.com/apache/beam/blob/master/Dockerfile) to setup a 
container meeting the requirements above, and mounting the directory that 
contains the Beam repo to the container. e.g.:
+```shell script
+docker run -it --mount 
type=bind,source=/Users/jsmith/Desktop/beam,target=/workspaces/beam 
 bash
+```

Review comment:
   It would be nice if this also had directions for building the container 
and getting the id, something like:
   ```sh
   docker build --rm -t beam-dev ${BEAM_DIR}
   set image_id=$(docker images -q beam-dev)
   docker run -it --mount type=bind,source=${BEAM_DIR},target=/workspaces/beam 
${image_id} bash
   ```





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.

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

[GitHub] [beam] y1chi commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

[GitBox]

y1chi commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495165024



##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[
+with self._lock:
+  try:
+# pop() is threadsafe
+processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
+self.active_bundle_processors[
+  instruction_id] = bundle_descriptor_id, processor
+try:
+  del self.known_not_running_instruction_ids[instruction_id]
+except KeyError:
+  # The instruction may have not been pre-registered before execution
+  # since activate() may have never been invoked
+  pass
+return processor
+  except IndexError:
+pass
+
+# Make sure we instantiate the processor while not holding the lock.
+processor = bundle_processor.BundleProcessor(
+self.fns[bundle_descriptor_id],
+self.state_handler_factory.create_state_handler(
+self.fns[bundle_descriptor_id].state_api_service_descriptor),
+self.data_channel_factory)
+with self._lock:
+  self.active_bundle_processors[
 instruction_id] = bundle_descriptor_id, processor
+  try:
+del self.known_not_running_instruction_ids[instruction_id]
+  except KeyError:
+# The instruction may have not been pre-registered before execution
+# since activate() may have never been invoked
+pass
 return processor
 
   def lookup(self, instruction_id):
 # type: (str) -> Optional[bundle_processor.BundleProcessor]
 
 """
 Return the requested ``BundleProcessor`` from the cache.
+
+Will return ``None`` if the BundleProcessor is known but not yet ready. 
Will
+raise an error if the ``instruction_id`` is not known or has been 
discarded.
 """
-return self.active_bundle_processors.get(instruction_id, (None, None))[-1]
+with self._lock:
+  if instruction_id in self.failed_instruction_ids:
+raise RuntimeError(
+'Bundle processing associated with %s has failed. '
+'Check prior failing response for details.' % instruction_id)
+  processor = self.active_bundle_processors.get(
+  instruction_id, (None, None))[-1]
+  if processor:
+return processor
+  if instruction_id in self.known_not_running_instruction_ids:
+return None
+  raise RuntimeError('Unknown process bundle id %s.' % instruction_id)
 
   def discard(self, instruction_id):
 # type: (str) -> None
 
 """
-Remove the ``BundleProcessor`` from the cache.
+Marks the instruction id as failed shutting down the ``BundleProcessor``.
 """
-self.active_bundle_processors[instruction_id][1].shutdown()
-del self.active_bundle_processors[instruction_id]
+with self._lock:
+  self.failed_instruction_ids[instruction_id] = True
+  while len(self.failed_instruction_ids) > MAX_FAILED_INSTRUCTIONS:
+self.failed_instruction_ids.popitem()

Review comment:
   Is it by design that we pop the items in LIFO order? 

##
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
 Moves the ``BundleProcessor`` from the inactive to the active cache.
 """
-try:
-  # pop() is threadsafe
-  processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-except IndexError:
-  processor = bundle_processor.BundleProcessor(
-  self.fns[bundle_descriptor_id],
-  self.state_handler_factory.create_state_handler(
-  self.fns[bundle_descriptor_id].state_api_service_descriptor),
-  self.data_channel_factory)
-self.active_bundle_processors[

Review comment:
   Seems that if we lock here we won't have to duplicate code
   





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.

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

[GitHub] [beam] rakeshcusat commented on pull request #12913: Minor fixes to the get-started/wordcount-example webpage.

[GitBox]

rakeshcusat commented on pull request #12913:
URL: https://github.com/apache/beam/pull/12913#issuecomment-699087674


   LGTM



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.

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

[GitHub] [beam] codecov[bot] edited a comment on pull request #12935: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

[GitBox]

codecov[bot] edited a comment on pull request #12935:
URL: https://github.com/apache/beam/pull/12935#issuecomment-698663460


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base 
(`release-2.25.0@821fdb6`). [Click here to learn what that 
means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/beam/pull/12935/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff@@
   ## release-2.25.0   #12935   +/-   ##
   =
 Coverage  ?   82.32%   
   =
 Files ?  455   
 Lines ?54589   
 Branches  ?0   
   =
 Hits  ?44943   
 Misses? 9646   
 Partials  ?0   
   ```
   
   
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=footer). Last 
update 
[821fdb6...10e9545](https://codecov.io/gh/apache/beam/pull/12935?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   



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.

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