[BEAM-7505] Add side input load test to Python SDK #11136
Conversation
|
Run Python PreCommit |
1 similar comment
|
Run Python PreCommit |
|
R: @ibzib Can I ask you for a review? |
|
Also a note: although this PR might look like a rework of the existing side input test, it's better to treat it as a completely different, new test (a lot has changed) |
|
A PR with Java's version (very similar to this one): #10949 |
|
cc: @Ardagan |
|
|
||
| self.side_input_type = self.pipeline.get_option('side_input_type') | ||
| if self.side_input_type is None: | ||
| raise ValueError('side_input_type is required') |
There was a problem hiding this comment.
It'd be helpful to list the types here, too.
| for i, _ in enumerate(side_input): | ||
| if self._first_n and i >= self._first_n: | ||
| return | ||
| # No-op. We only make sure that the element is accessed. |
There was a problem hiding this comment.
For first_n < len(side_input), wouldn't we access a total of first_n + 1 elements?
There was a problem hiding this comment.
Ohh that's right. Thanks.
I'll use a while loop and iter(), so that we'd be able to treat list and iterable in the same way.
| current Unix seconds-since-epoch timestamp.""" | ||
| def setup(self): | ||
| import time | ||
| self.current_time = int(time.time()) |
There was a problem hiding this comment.
Is there any particular reason we need to use the real time? If not, it would be easier to read timestamps if we start at 0.
There was a problem hiding this comment.
No particular reason. I'll change it to start at 0.
kamilwu
force-pushed
the
side-input-tests
branch
from
1641f32
to
933022a
Compare
* list available types if `side_input_type` is None * don't check if `self._first_n` is not None (will always be not None) * use while loop for iterating over a sequence * assign timestamps starting from 0
kamilwu
force-pushed
the
side-input-tests
branch
from
933022a
to
2ce6d54
Compare
|
Thanks! I agree, let's wait on that feedback. If everything is ok, I'll merge it. |
|
Retest this please |
|
Run Python PreCommit |
|
Thanks @ibzib! |
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.