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

[Polish] Code polish #2105

Merged
merged 9 commits into from
Oct 19, 2023
Merged

[Polish] Code polish #2105

merged 9 commits into from
Oct 19, 2023

Conversation

GOODBOY008
Copy link
Member

Polish code

@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: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 Oct 16, 2023
@GOODBOY008
Copy link
Member Author

@shidayang PTAL.

@shidayang
Copy link
Contributor

@shidayang PTAL.

Thank you for your contribution. There are some formatting issues in the code that caused the CI to fail. Could you please fix them?

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.

Thanks a lot for your contribution!

I noticed that you used the wrong indent configuration to format the code. You may need to reconfigure and reformat it.

.append('.')
.append(database)
.toString();
return name() +
Copy link
Contributor

Choose a reason for hiding this comment

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

The project use 2 spaces as indent and tab size, you may reconfigure it an reformat the codes here.

@baiyangtx
Copy link
Contributor

What is the standard for this code specification? Are there any relevant checking tools to detect whether the code meets the standard?

Is this standardization of the code a one-time event or can it be continuously enforced? It seems like a one-time optimization, and there is no guarantee that subsequent submissions will meet the relevant constraints. Can checks be added in Github CI?

@baiyangtx
Copy link
Contributor

The community is currently promoting another initiative to use the googleJavaFormatter + spotless plugin to format the code, please refer to this issue #1402. Is the goal of this PR consistent with that initiative?

@codecov
Copy link

codecov bot commented Oct 17, 2023

@GOODBOY008 GOODBOY008 force-pushed the master-polish branch 2 times, most recently from e8837c7 to 566de76 Compare October 18, 2023 07:40
@GOODBOY008
Copy link
Member Author

@zhoujinsong @shidayang @baiyangtx PTAL , Thank you~

@shidayang
Copy link
Contributor

LGTM

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.

Thanks a lot for your contribution!

Comment on lines 40 to +92
void alterPartition(String dbName, String tblName, Partition newPart, EnvironmentContext environmentContext)
throws InvalidOperationException, MetaException, TException, ClassNotFoundException,
throws TException, ClassNotFoundException,
NoSuchMethodException, InvocationTargetException, IllegalAccessException;

Partition getPartition(String dbName, String tblName,
List<String> partVals) throws NoSuchObjectException, MetaException, TException;
List<String> partVals) throws TException;

Partition getPartition(String dbName, String tblName,
String name) throws MetaException, UnknownTableException, NoSuchObjectException, TException;
String name) throws TException;

Table getTable(String dbName, String tableName) throws MetaException,
TException, NoSuchObjectException;
Table getTable(String dbName, String tableName) throws
TException;

void alterTable(String defaultDatabaseName, String tblName,
Table table) throws InvalidOperationException, MetaException, TException;
Table table) throws TException;

List<Partition> listPartitions(String dbName, String tblName,
short maxParts) throws NoSuchObjectException, MetaException, TException;
short maxParts) throws TException;

List<Partition> listPartitions(String dbName, String tblName,
List<String> partVals, short maxParts)
throws NoSuchObjectException, MetaException, TException;
throws TException;

List<String> listPartitionNames(String dbName, String tblName,
short maxParts) throws MetaException, TException;
short maxParts) throws TException;


void createDatabase(Database db)
throws InvalidObjectException, AlreadyExistsException, MetaException, TException;
throws TException;

void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownDb, boolean cascade)
throws NoSuchObjectException, InvalidOperationException, MetaException, TException;
throws TException;

void dropTable(String dbname, String tableName, boolean deleteData,
boolean ignoreUnknownTab) throws MetaException, TException,
NoSuchObjectException;
boolean ignoreUnknownTab) throws TException;

void createTable(Table tbl) throws AlreadyExistsException,
InvalidObjectException, MetaException, NoSuchObjectException, TException;
void createTable(Table tbl) throws
TException;

Database getDatabase(String databaseName)
throws NoSuchObjectException, MetaException, TException;
throws TException;

Partition addPartition(Partition partition)
throws InvalidObjectException, AlreadyExistsException, MetaException, TException;
throws TException;

boolean dropPartition(String dbName, String tblName, List<String> partVals,
PartitionDropOptions options) throws TException;

int addPartitions(List<Partition> partitions)
throws InvalidObjectException, AlreadyExistsException, MetaException, TException;
throws TException;


List<String> getAllTables(String dbName) throws MetaException, TException, UnknownDBException;
List<String> getAllTables(String dbName) throws TException;
Copy link
Contributor

Choose a reason for hiding this comment

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

It makes sense to throw more semantically clear exceptions in an interface.

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!
Thanks a lot for your great work!

@zhoujinsong zhoujinsong merged commit 86cf5c7 into apache:master Oct 19, 2023
@shidayang shidayang mentioned this pull request Oct 24, 2023
70 tasks
@GOODBOY008 GOODBOY008 deleted the master-polish branch October 24, 2023 06:21
ShawHee pushed a commit to ShawHee/arctic that referenced this pull request Dec 29, 2023
* exclude flink

* fix ci

* fix ci

* test ci

* test ci

* polish flink 1.15

* add polish code

* Address comment

* Address comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:ams-dashboard Ams dashboard module module:ams-server Ams server module 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants