Uh oh!
There was an error while loading. Please reload this page.
[BEAM-8204][BEAM-8205] Exclude AvroSchemaTest for Apex and Flink runner for now - #9541
Merged
Conversation
robinyqiu
commented
Sep 11, 2019
ContributorAuthor
Run Flink ValidatesRunner |
robinyqiu
commented
Sep 11, 2019
ContributorAuthor
Run Apex ValidatesRunner |
robinyqiu
commented
Sep 11, 2019
ContributorAuthor
Run Flink ValidatesRunner |
robinyqiu
commented
Sep 11, 2019
ContributorAuthor
Run Apex ValidatesRunner |
reuvenlax
commented
Sep 11, 2019
via email
Contributor
How are they breaking those runners? This test should be working fine on
those runners AFAIK. …On Tue, Sep 10, 2019 at 5:53 PM Yueyang Qiu ***@***.***> wrote:
Run Apex ValidatesRunner
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9541?email_source=notifications&email_token=AFAYJVLD2EEFCDERKGEZLJTQJA6PZA5CNFSM4IVN4VMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6M5DLY#issuecomment-530174383>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFAYJVJ6IMOT3OEHV6QOIRDQJA6PZANCNFSM4IVN4VMA>
.
|
TheNeuralBit
commented
Sep 11, 2019
Member
I'm not really up to speed on what's going on here, but both Apex and Flink seem upset about coders of two different types, if that helps. From the Flink run: and from the Apex run: |
robinyqiu
commented
Sep 11, 2019
ContributorAuthor
markflyhigh
approved these changes
Sep 11, 2019
markflyhigh
left a comment
Contributor
There was a problem hiding this comment.
Thank you Robin to keep ValidatesRunner test green. This PR LGTM.
TheNeuralBit
commented
Sep 11, 2019
Member
+1 LGTM. I think @reuvenlax is right that #9446's addition of SchemaCoder.equals may fix this, but it's not fully baked yet. I think we should just skip until we can get the equality check right and un-skip these. |
reuvenlax
commented
Sep 11, 2019
via email
Contributor
Technically I think Flink should not be relying on comparing coders. Beam
has never required Coder authors to implement a Java equals method, and
other runners don't do such Coder comparisons on flatten. However for now
we can disable the test on Flink. …On Wed, Sep 11, 2019 at 1:07 PM Brian Hulette ***@***.***> wrote:
+1 LGTM. I think @reuvenlax <https://github.com/reuvenlax> is right that
#9446 <#9446>'s addition of
SchemaCoder.equals may fix this, but it's not fully baked yet. I think we
should just skip until we can get the equality check right and un-skip
these.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9541?email_source=notifications&email_token=AFAYJVJDFAZZIPKEDUFJP5DQJFFX7A5CNFSM4IVN4VMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6PXNZA#issuecomment-530544356>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFAYJVORR3UE3KAPFPZHBWDQJFFX7ANCNFSM4IVN4VMA>
.
|
markflyhigh
commented
Sep 11, 2019
Contributor
Got enough lgtm. I'll merge this. |
pl04351820 pushed a commit
to pl04351820/beam
that referenced
this pull request
Dec 20, 2023
This file contains hidden or 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
Sign up for freeto 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.
Exclude the test because they are currently breaking post-commit tests.
The test was added in #9454
r: @TheNeuralBit@reuvenlax
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.