Skip to content

Conversation

@tgroh
Copy link
Member

@tgroh tgroh commented May 10, 2016

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

Not ready for review. Publishing PR to hook into Travis and Jenkins.

Update runners/direct-java/pom.xml to enable the RunnableOnService
tests phase.

@tgroh tgroh force-pushed the enable_ros_tests branch 9 times, most recently from e8255cf to e4fc8b4 Compare May 11, 2016 23:52
@tgroh
Copy link
Member Author

tgroh commented May 12, 2016

Takes ~1.5m to build the direct runner + run the additional RunnableOnService Tests on Jenkins; about 2-3 on the slower travis targets.

@tgroh
Copy link
Member Author

tgroh commented May 12, 2016

R: @kennknowles
After merge of #326

@kennknowles
Copy link
Member

Nice. Since we will, afterwards, be removing the test runs of the current DirectPipelineRunner, the change in build time will be reduced. I think we can merge this straight away. If we encounter any problems it will be easy to revert and/or disable selected tests.

@kennknowles
Copy link
Member

It does seem that the cost on travis is much greater.

@kennknowles
Copy link
Member

You can actually do the disabling of redundant tests in this PR by adding <excludedGroups>org.apache.beam.testing.RunnableOnService</excludedGroups> to the sdks/java/ test configuration. Then we will have an even better idea of the test timing.

tgroh added 2 commits May 12, 2016 09:59
Reduces the number of active threads, which should improve scheduling
behavior.

Additionally improves caching behavior of ParDo evaluators due to
increased reuse of DoFns.
Explicitly remove beamTestPipelineOptions in examples. Otherwise,
running the examples after a PipelineRunner with
@tgroh tgroh force-pushed the enable_ros_tests branch from e4fc8b4 to 113e257 Compare May 12, 2016 16:59
With the direct runner executing all of this category (in
runners/direct-java), we maintain this test coverage without running
these tests while building the Core SDK.

Required to remove the legacy direct runner.
@kennknowles
Copy link
Member

LGTM. I think it is time to merge this.

@asfgit asfgit merged commit e406949 into apache:master May 12, 2016
asfgit pushed a commit that referenced this pull request May 12, 2016
@kennknowles kennknowles mentioned this pull request May 13, 2016
4 tasks
dhalperi added a commit to dhalperi/beam that referenced this pull request Aug 23, 2016
iemejia pushed a commit to iemejia/beam that referenced this pull request Jan 12, 2018
pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
* chore: manual synth

* ran synthtool to add bundle proto definitions

* beginning of bundle implementation

added methods to assemble bundles, but not yet serialize them into length-prefixed json strings

with tests for bundle assembly

* linting

* Added bundle build method

* fixed incomplete document id

* fixed git merge error

* Added first draft of docstrings

* Added FirestoreBundle deserialization

* Fixed import desync

* Improved test coverage for bundles

* linting

* test coverage

* CI happiness

* converted redundant exception to assertion

* removed todo

* Updated comments

* linted

* Moved query limit type into bundle code

* Added typed response for parsing reference values

* refactored document reference parsing

* removed auto import of bundles from firestore

* small tweaks

* added tests for document iters

* Updated FirestoreBundle imports and synthtool gen

* linting

* extra test coverage

* responses to code review

* linting

* Fixed stale docstring

* camelCased bundle output

* updated stale comments

* Added test for binary data

* linting

Co-authored-by: Craig Labenz <craiglabenz@google.com>
Co-authored-by: Christopher Wilcox <crwilcox@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants