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.

Sign up for GitHub

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

Jump to bottom

[Improvement]: Remove AmsClient in UnkeyedTable and KeyedTable #1904

Merged
merged 38 commits into from
Sep 5, 2023
Merged

[Improvement]: Remove AmsClient in UnkeyedTable and KeyedTable #1904

merged 38 commits into from
Sep 5, 2023

Conversation

baiyangtx
Copy link
Contributor

Why are the changes needed?

Code refactor, remove AmsClient in UnkeyedTable and KeyedTable

Brief change log

  • Remove AmsClient in UnkeyedTable and KeyedTable

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run tests locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (NO)

baiyangtx and others added 29 commits August 23, 2023 20:18
@baiyangtx
mixed iceberg
3434905
@baiyangtx
Core Unit Tests Passed
ab1dc08
@baiyangtx
Fix checkstyle problems
581993b
@baiyangtx
Fix hive unit tests
924eda3
@baiyangtx
fix flink unit tests
39526f8
@baiyangtx
fix unit test in AMS
fe8f4ed
@baiyangtx
fix unit test in Flink
e239b85
@baiyangtx
trino unit tests
17e33b7
@baiyangtx
Merge branch 'master' into mixed-iceberg-based-on-iceberg-catalog
4edd638 
# Conflicts:
#	trino/src/test/java/com/netease/arctic/trino/arctic/TestHiveTableBaseForTrino.java
@baiyangtx
Merge branch 'master' into mixed-iceberg-based-on-iceberg-catalog
120041b
@baiyangtx
trino unit tests
4aa6468
@baiyangtx
Merge branch 'master' into mixed-iceberg-based-on-iceberg-catalog
8e70b62
@baiyangtx
trino unit tests
8fe8c1d
@baiyangtx
Merge remote-tracking branch 'origin/mixed-iceberg-based-on-iceberg-c…
28c2ffa 
…atalog' into mixed-iceberg-based-on-iceberg-catalog
@baiyangtx
add mixed-iceberg to dashboard
840e4bc
@baiyangtx
add doAs for new mixed-iceberg
0f7a1a7
@HuangFru
do not check resource
6c77e29
@baiyangtx
Merge pull request #12 from HuangFru/trino-ut: do not check resource
2d59ce8 
do not check resource
@HuangFru
do not check resource
629ded7
@baiyangtx
Merge pull request #13 from HuangFru/trino-ut: do not close AMS
ea218f0 
do not check resource
@baiyangtx
Merge branch 'master' into mixed-iceberg-based-on-iceberg-catalog
8787f4e
@baiyangtx
remove AmsClient in Tables.
babd4f0
@baiyangtx
Merge branch 'master' into mixed-iceberg-based-on-iceberg-catalog
321bb67
@baiyangtx
fix the review comments.
c0b3aae
@baiyangtx
Merge branch 'master' into mixed-iceberg-based-on-iceberg-catalog
5831750
@baiyangtx
remove duplicate class
5c85797
@baiyangtx
Merge remote-tracking branch 'origin/mixed-iceberg-based-on-iceberg-c…
9008895 
…atalog' into mixed-iceberg-based-on-iceberg-catalog
@baiyangtx
Merge branch 'mixed-iceberg-based-on-iceberg-catalog' into remove-ams…
5ed644a 
…-client-in-tables

# Conflicts:
#	core/src/main/java/com/netease/arctic/mixed/MixedTables.java
@baiyangtx
add comment
07bdf27
@github-actions github-actions bot added the module:mixed-flink Flink moduel for Mixed Format label Aug 31, 2023
@github-actions github-actions bot added module:core Core module module:ams-server Ams server module module:mixed-hive Hive moduel for Mixed Format module:mixed-trino trino module for Mixed Format module:ams-dashboard Ams dashboard module labels Aug 31, 2023
@baiyangtx
Merge branch 'master' into remove-ams-client-in-tables
6cd38c9 
# Conflicts:
#	core/src/main/java/com/netease/arctic/catalog/MixedTables.java
#	core/src/main/java/com/netease/arctic/mixed/MixedTables.java
@github-actions github-actions bot removed module:mixed-flink Flink moduel for Mixed Format module:ams-server Ams server module module:ams-dashboard Ams dashboard module labels Sep 4, 2023
@github-actions github-actions bot added the module:ams-dashboard Ams dashboard module label Sep 4, 2023
@baiyangtx baiyangtx marked this pull request as ready for review September 4, 2023 02:55
@codecov
Copy link

codecov bot commented Sep 4, 2023 •
edited
Loading

Codecov Report

Patch coverage is 36.29% of modified lines.

Files Changed Coverage
...arctic/server/dashboard/ServerTableDescriptor.java 0.00%
...ase/arctic/server/optimizing/KeyedTableCommit.java ø
...e/arctic/server/optimizing/UnKeyedTableCommit.java ø
...n/java/com/netease/arctic/catalog/MixedTables.java 0.00%
...netease/arctic/mixed/BasicMixedIcebergCatalog.java 0.00%
...java/com/netease/arctic/op/ArcticRewriteFiles.java 0.00%
.../com/netease/arctic/op/CreateTableTransaction.java 0.00%
...in/java/com/netease/arctic/op/SnapshotSummary.java ø
...va/com/netease/arctic/table/BasicTableBuilder.java ø
...com/netease/arctic/catalog/BasicArcticCatalog.java 9.09%
... and 15 more

📢 Thoughts on this report? Let us know!.

majin1102
majin1102 approved these changes Sep 4, 2023
View reviewed changes
Copy link
Contributor

@majin1102 majin1102 left a comment

Choose a reason for hiding this comment

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

LGTM

zhoujinsong
zhoujinsong approved these changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

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

LGTM

@baiyangtx baiyangtx merged commit dd2a9ae into apache:master Sep 5, 2023
5 of 6 checks passed
@baiyangtx baiyangtx deleted the remove-ams-client-in-tables branch September 5, 2023 04:59
@wangtaohz wangtaohz mentioned this pull request Sep 7, 2023
Closed
56 tasks
ShawHee pushed a commit to ShawHee/arctic that referenced this pull request Dec 29, 2023
@baiyangtx @ShawHee
[Improvement]: Remove AmsClient in UnkeyedTable and KeyedTable (apach…
89bfc2e 
…e#1904)

Code refactor, remove AmsClient in UnkeyedTable and KeyedTable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhoujinsong zhoujinsong zhoujinsong approved these changes

@majin1102 majin1102 majin1102 approved these changes

Assignees
No one assigned
Labels
module:ams-dashboard Ams dashboard module module:core Core module module:mixed-hive Hive moduel for Mixed Format module:mixed-trino trino module for Mixed Format
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@baiyangtx @majin1102 @zhoujinsong @HuangFru