Skip to content

Change Arctic to Mixed for class names in module core#2771

Merged
zhoujinsong merged 20 commits into
apache:masterfrom
zhoujinsong:change-core-class-name
Apr 27, 2024
Merged

Change Arctic to Mixed for class names in module core#2771
zhoujinsong merged 20 commits into
apache:masterfrom
zhoujinsong:change-core-class-name

Conversation

@zhoujinsong

Copy link
Copy Markdown
Contributor

Why are the changes needed?

Arctic is no longer the name of the project.
In some old classes, Arctic is used to specify the mixed format, and this PR corrects the class names in the core module.

Brief change log

  • Change Arctic to Mixed for class names for module core.
  • Change some comments.

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 test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@github-actions github-actions Bot added module:mixed-flink Flink moduel for Mixed Format module:mixed-spark Spark module for Mixed Format module:core Core module labels Apr 19, 2024
@zhoujinsong zhoujinsong changed the title Change Arctic to Mixed for class names for module core Change Arctic to Mixed for class names in module core Apr 19, 2024
@github-actions github-actions Bot added module:ams-server Ams server module module:mixed-hive Hive moduel for Mixed Format module:mixed-trino trino module for Mixed Format labels Apr 19, 2024
@Aireed

Aireed commented Apr 19, 2024

Copy link
Copy Markdown
Contributor

Some configuration items are named with "arctic." at the beginning. should we change them too?

  public static final String ARCTIC_TABLE_FLAG = "arctic.enabled";
  @Deprecated public static final String ARCTIC_TABLE_FLAG_LEGACY = "arctic.enable";

  public static final String ARCTIC_TABLE_PRIMARY_KEYS = "arctic.table.primary-keys";
  // save the root location of arctic table.
  public static final String ARCTIC_TABLE_ROOT_LOCATION = "arctic.table.root-location";

@zhoujinsong

Copy link
Copy Markdown
Contributor Author

This PR does not change the thrift-related classes and table property keys, so it still maintains compatibility with older versions.

@zhongqishang

Copy link
Copy Markdown
Contributor

@zhoujinsong Could you resolve the conflict if you had time?

@github-actions github-actions Bot added type:infra and removed module:mixed-flink Flink moduel for Mixed Format module:mixed-spark Spark module for Mixed Format 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 labels Apr 22, 2024
@zhoujinsong

Copy link
Copy Markdown
Contributor Author

@zhoujinsong Could you resolve the conflict if you had time?

I have merged master branch and fixed the conflicts, please task a look.

@czy006 czy006 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think some of the key-value configurations should also be amoro and internally compatible. In the future v1.0 we will completely remove this compatibility.WHDY? @zhoujinsong

@zhoujinsong

Copy link
Copy Markdown
Contributor Author

I think some of the key-value configurations should also be amoro and internally compatible. In the future v1.0 we will completely remove this compatibility.WHDY? @zhoujinsong

Yes, we may push forward this in another PR after discussion in the dev mail list.

Comment thread amoro-core/src/main/thrift/amoro_commons.thrift
@zhongqishang

Copy link
Copy Markdown
Contributor

All class name were changed from Arctic to Mixed in the core module? IMO, MixedFileIO looks weird, what does mixed represent?

@zhoujinsong

Copy link
Copy Markdown
Contributor Author

All class name were changed from Arctic to Mixed in the core module? IMO, MixedFileIO looks weird, what does mixed represent?

ArcticFileIO was initially used to implement mixed format tables, but as it evolved, it is now also employed to provide authenticated FileIO implementations for the Iceberg format. So how about renaming ArcticFileIO to AuthenticatedFileIO

@github-actions github-actions Bot added module:mixed-flink Flink moduel for Mixed Format module:mixed-spark Spark module for Mixed Format module:core Core module module:mixed-hive Hive moduel for Mixed Format module:mixed-trino trino module for Mixed Format labels Apr 25, 2024

@zhongqishang zhongqishang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution.
Approve first and merge after resolving conflicts.

@github-actions github-actions Bot added type:docs Improvements or additions to documentation module:ams-dashboard Ams dashboard module labels Apr 27, 2024
@github-actions github-actions Bot removed type:docs Improvements or additions to documentation module:ams-dashboard Ams dashboard module labels Apr 27, 2024
@zhoujinsong zhoujinsong merged commit 74fcba4 into apache:master Apr 27, 2024
@zhoujinsong zhoujinsong deleted the change-core-class-name branch April 27, 2024 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:core Core module module:mixed-flink Flink moduel for Mixed Format module:mixed-hive Hive moduel for Mixed Format module:mixed-spark Spark module for Mixed Format module:mixed-trino trino module for Mixed Format type:build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants