[BEAM-136] Look up a runner if it is not registered #61
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a fully qualified runner is passed as the value of --runner, and it is not present within the map of registered runners, attempts to look up the runner using Class#forName, and uses the result class if the result class is an instance of PipelineRunner. This brings the behavior in line with the described behavior in PipelineOptions.
|
R: @lukecwik |
| "Unknown 'runner' specified '%s', supported pipeline runners %s", | ||
| runner, Sets.newTreeSet(SUPPORTED_PIPELINE_RUNNERS.keySet())); | ||
| convertedOptions.put("runner", SUPPORTED_PIPELINE_RUNNERS.get(runner)); | ||
| if (SUPPORTED_PIPELINE_RUNNERS.containsKey(runner)) { |
There was a problem hiding this comment.
Please update the method comment.
In PipelineOptions it is clear what we are supposed to support but from this method description I take it as we only support registered runners and we can select it either by the simple or fully qualified names.
There was a problem hiding this comment.
Done.
We only supported registered runners, and it looks like we only ever supported the simple names; this fixes that as well.
|
I guess this is aimed at what I just filed as https://issues.apache.org/jira/browse/BEAM-136? I'll assign you ;) I am somewhat confused though, because I don't see a hardcoded list in that file. I thought the problem was likely to be fixing the reflection. Can you point out the difference? |
|
There is no hard coded list. We rely on autoservice which will register all On Fri, Mar 18, 2016, 4:46 PM Daniel Halperin notifications@github.com
|
tgroh
changed the title
Update comment of parseObjects for the behavior of resolving the appropriate runner Add a test to demonstrate fully-qualified runner names are handle appropriately
|
Should we also register Flink and Spark runners? Or maybe file issues against them to add the relevant annotations? |
|
Yes, Flink and Spark should use AutoService as well to support automatic registration. If they don't already, creating an issue so that they do seems reasonable. |
|
LGTM |
davorbonaci
added a commit
to GoogleCloudPlatform/DataflowJavaSDK
that referenced
this pull request
Mar 25, 2016
mareksimunek
pushed a commit
to mareksimunek/beam
that referenced
this pull request
May 9, 2018
…riptors apache#61 [euphoria-flink] Do not hold states in memory
mareksimunek
pushed a commit
to mareksimunek/beam
that referenced
this pull request
May 9, 2018
…h allowed lateness
mareksimunek
pushed a commit
to mareksimunek/beam
that referenced
this pull request
May 9, 2018
apache#61 Allowed lateness unit test
dmvk
pushed a commit
to dmvk/beam
that referenced
this pull request
May 15, 2018
…h allowed lateness
mareksimunek
pushed a commit
to seznam/beam
that referenced
this pull request
Jul 9, 2018
…h allowed lateness
dmvk
pushed a commit
to seznam/beam
that referenced
this pull request
Aug 17, 2018
…h allowed lateness
dmvk
pushed a commit
to seznam/beam
that referenced
this pull request
Oct 5, 2018
…h allowed lateness
hengfengli
referenced
this pull request
in hengfengli/beam
Mar 21, 2022
* fix: fix split checker The previous logic was comparing the last claimed position timestamp against the timestamp of the current claimed position. The problem with this is that if the write rate is high, Spanner will have multiple updates at the same timestamp. Because of this, the split checker will, more often than not, return false, preventing splitting from happening. In this PR we compare the claimed position timestamp against the restriction start timestamp. In this sense, we allow for splits if at least 1 microsecond has passed from the start timestamp. This fixes the issue of having multiple records with the same timestamp. * fix: use watermark estimator in bundle finalizer Instead of registering the callback with the current watermark, gives the watermark estimator to the bundle finalizer callback so that the current watermark is retrieved within the callback. * feat: add heartbeat record count metric Adds optional metric to check for heartbeat count. * feat: add debug logging to restriction tracker
ryucc
pushed a commit
to ryucc/beam
that referenced
this pull request
Jan 23, 2023
Abacn
pushed a commit
to Abacn/beam
that referenced
this pull request
Jan 31, 2023
PiperOrigin-RevId: 278792316
pl04351820
pushed a commit
to pl04351820/beam
that referenced
this pull request
Dec 20, 2023
* feat: add async tests for AsyncClient * feat: add AsyncClient implementation * feat: add AsyncDocument implementation * feat: add AsyncDocument support to AsyncClient * feat: add AsyncDocument tests Note: tests relying on Collection will fail in this commit * feat: add AsyncCollectionReference class * feat: integrate AsyncCollectionReference * feat: add async_collection tests * fix: swap coroutine/function declaration in async_collection * feat: add async_batch implementation * feat: integrate async_batch * feat: add async_batch tests * feat: add async_query implementation * feat: add async_query integration * feat: add async_query tests * fix: AsyncQuery.get async_generator nesting * feat: add async_transaction integration and tests * fix: linter errors * feat: refactor async tests to use aiounittest and pytest-asyncio * feat: remove duplicate code from async_client * feat: remove duplicate code from async_batch * feat: remove duplicate code from async_collection * feat: remove duplicate code from async_document * fix: remove unused imports * fix: remove duplicate test * feat: remove duplicate code from async_transaction * fix: remove unused Python2 compatibility * fix: resolve async generator tests * fix: create mock async generator to get full coverage * fix: copyright date * feat: create Client/AsyncClient superclass * fix: base client test class * feat: create WriteBatch/AsyncWriteBatch superclass * feat: create CollectionReference/AsyncCollectionReference superclass * feat: create DocumentReference/AsyncDocumentReference superclass * fix: base document test class name * feat: create Query/AsyncQuery superclass * refactor: generalize collection tests with mocks * feat: create Transaction/AsyncTransaction superclass * feat: add microgen support to async interface * fix: async client copyright date * fix: standardize assert syntax * fix: incorrect copyright date * fix: incorrect copyright date * fix: clarify _sleep assertions in transaction * fix: clarify error in context manager tests * fix: clarify error in context manager tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a fully qualified runner is passed as the value of --runner, and it
is not present within the map of registered runners, attempts to look
up the runner using Class#forName, and uses the result class if the
result class is an instance of PipelineRunner. This brings the behavior
in line with the described behavior in PipelineOptions.