Skip to content

[BEAM-7437] Add streaming flag to BQ streaming inserts IT test#8934

Merged
udim merged 2 commits intoapache:masterfrom
ttanay:beam-7437
Jul 9, 2019
Merged

[BEAM-7437] Add streaming flag to BQ streaming inserts IT test#8934
udim merged 2 commits intoapache:masterfrom
ttanay:beam-7437

Conversation

@ttanay
Copy link
Copy Markdown
Contributor

@ttanay ttanay commented Jun 24, 2019 •
edited
Loading

I missed adding the streaming=True option to the PipelineOptions earlier.
Since TestStream ends once watermark advances to infinity,
this test passed.
But, after adding the streaming flag, it failed,
because the BigqueryFullResultMatcher did not poll for changes in a streaming test pipeline.

Added BigqueryFullResultStreamingMatcher to poll BigQuery before matching.

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) 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.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status		 Build Status Build Status Build Status
Build Status
Python Build Status
Build Status		 --- Build Status
Build Status		 Build Status --- --- Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 24, 2019

Run Portable_Python PreCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 24, 2019

Run Portable_Python PreCommit

@ttanay
[BEAM-7437] Add streaming flag to BQ streaming inserts IT test
3fc4478 
I missed adding the streaming=True option to the PipelineOptions
earlier. Since TestStream ends once watermark advances to infinity,
this test passed. But, after adding the streaming flag, it failed
because the BigqueryFullResultMatcher did not poll for changes for
a streaming test pipeline.
Added BigqueryFullResultStreamingMatcher to match while polling
for changes.
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 24, 2019

Run Python PostCommit

1 similar comment
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 25, 2019

Run Python PostCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 25, 2019

R: @pabloem

@pabloem
Copy link
Copy Markdown
Member

pabloem commented Jun 25, 2019

r: @udim

udim
udim requested changes Jun 26, 2019
View reviewed changes
sdks/python/apache_beam/io/gcp/tests/bigquery_matcher.py Outdated
A timeout can be specified.
"""

def __init__(self, project, query, data, timeout=MAX_TIMESTAMP):
Copy link
Copy Markdown
Member

@udim udim Jun 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make the default timeout to be finite. Would 5/10/15 minutes work? (the less the better - having post-commit tests hang until their 2 hour deadline is not fun. It easier to debug where the test timed out if it raises a TimeoutError and the stacktrace is printed)

Copy link
Copy Markdown
Contributor Author

@ttanay ttanay Jun 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 5 minutes should be more than enough. I'll make the change.

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 27, 2019

Run Python PreCommit

1 similar comment
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 27, 2019

Run Python PreCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 27, 2019

Run Python PostCommit

1 similar comment
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 27, 2019

Run Python PostCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 28, 2019

Run Python PreCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jun 28, 2019

Hi @udim
Made the changes. PTAL.

@ttanay ttanay mentioned this pull request Jul 2, 2019
Merged
3 tasks
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 3, 2019

Run Python PostCommit

3 similar comments
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 3, 2019

Run Python PostCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 3, 2019

Run Python PostCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 3, 2019

Run Python PostCommit

@udim
Copy link
Copy Markdown
Member

udim commented Jul 3, 2019

I'm sorry, postcommits seem to be broken ATM: https://issues.apache.org/jira/browse/BEAM-7687

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 4, 2019

Thanks @udim!
I thought they were flaky. I'll try again later.

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 6, 2019

Run Python PostCommit

1 similar comment
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 9, 2019

Run Python PostCommit

@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 9, 2019

Tests pass.
PTAL @udim

@udim udim merged commit db913ef into apache:master Jul 9, 2019
@ttanay
Copy link
Copy Markdown
Contributor Author

ttanay commented Jul 9, 2019

Thank You @udim :)

@ttanay ttanay mentioned this pull request Jul 11, 2019
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@udim udim udim approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ttanay @pabloem @udim