Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Iceberg failure recovery test to separate job #11142

Merged

Conversation

losipiuk
Copy link
Member

Description

Move Iceberg failure recovery test to separate job

Is this change a fix, improvement, new feature, refactoring, or other?

test refactoring

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

test code, iceberg connector

How would you describe this change to a non-technical end user or system administrator?

N/A

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Feb 22, 2022
@losipiuk losipiuk force-pushed the lo/extract-iceberg-failure-recovery-tests branch from a2e8e33 to 7630734 Compare February 22, 2022 09:02
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
Copy link
Member

@aczajkowski aczajkowski Feb 22, 2022

Choose a reason for hiding this comment

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

Shouldn't we leave them in plugin management section ? Or you just consider it as duplicate.
Which seems it might be 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

IDK. I copied approach form pom.xml for Hive connector.

Copy link
Member Author

Choose a reason for hiding this comment

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

Damn - it looks like excluded tests still executed for default profile ...

Copy link
Member Author

Choose a reason for hiding this comment

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

typo

Copy link
Member

@aczajkowski aczajkowski left a comment

Choose a reason for hiding this comment

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

LGTM % one question

@losipiuk losipiuk force-pushed the lo/extract-iceberg-failure-recovery-tests branch from 7630734 to f618dcb Compare February 22, 2022 13:18
<rules>
<requireUpperBoundDeps>
<excludes combine.children="append">
<exclude>com.google.guava:guava</exclude>
Copy link
Member

Choose a reason for hiding this comment

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

Why this appears as a change here?
did you mean a prep commit?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is verbatim move from -> to default profile

Copy link
Member Author

Choose a reason for hiding this comment

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

I extracted separate commit. But it still shows in diff as it is different level of indentation.

@losipiuk losipiuk force-pushed the lo/extract-iceberg-failure-recovery-tests branch from f618dcb to d9f3939 Compare February 23, 2022 10:43
@losipiuk losipiuk force-pushed the lo/extract-iceberg-failure-recovery-tests branch from d9f3939 to f40b7ea Compare February 23, 2022 10:45
@losipiuk losipiuk merged commit 0086df8 into trinodb:master Feb 23, 2022
@github-actions github-actions bot added this to the 372 milestone Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants