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

Change DataSourceType from enum to class #2730

Merged
merged 9 commits into from
Jun 12, 2024
Merged

Change DataSourceType from enum to class #2730

merged 9 commits into from
Jun 12, 2024

Conversation

ykmr1224
Copy link
Collaborator

@ykmr1224 ykmr1224 commented Jun 7, 2024 •
edited
Loading

Description

  • Change DataSourceType from enum to class, so that new type can be defined outside.
  • Fixed serialization logics since the class was serialized like {"name": "S3GLUE"}. The custom Gson object serialize/deserialize it to/from S3GLUE.

Issues Resolved

n/a

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • [] New functionality has javadoc added
    • [] New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@ykmr1224 ykmr1224 added backport 2.x maintenance Improves code quality, but not the product labels Jun 7, 2024
@ykmr1224 ykmr1224 marked this pull request as ready for review June 7, 2024 23:27
ykmr1224 added 5 commits June 7, 2024 17:31
@ykmr1224
Fix test failure
3cf958f 
Signed-off-by: Tomoyuki Morita <[email protected]>
@ykmr1224
Fix serialization issue
8f0b204 
Signed-off-by: Tomoyuki Morita <[email protected]>
@ykmr1224
Fix format
d90a91f 
Signed-off-by: Tomoyuki Morita <[email protected]>
@ykmr1224
Fix integTest
a0aba15 
Signed-off-by: Tomoyuki Morita <[email protected]>
@ykmr1224
Fix style
34efe47 
Signed-off-by: Tomoyuki Morita <[email protected]>
@codecov Codecov
Copy link

codecov bot commented Jun 10, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.47%. Comparing base (3ab7851) to head (a0aba15).

❗ Current head a0aba15 differs from pull request most recent head 4059fcf

Please upload reports for the commit 4059fcf to get more accurate results.

Files Patch % Lines
...earch/sql/datasources/exceptions/ErrorMessage.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff            @@
##               main    #2730   +/-   ##
=========================================
  Coverage     95.47%   95.47%           
- Complexity     5196     5198    +2     
=========================================
  Files           509      510    +1     
  Lines         14637    14646    +9     
  Branches        982      982           
=========================================
+ Hits          13974    13983    +9     
  Misses          641      641           
  Partials         22       22
Flag Coverage Δ
sql-engine 95.47% <91.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ykmr1224
Fix failing test
9a97d81 
Signed-off-by: Tomoyuki Morita <[email protected]>
seankao-az
seankao-az reviewed Jun 11, 2024
View reviewed changes
core/src/main/java/org/opensearch/sql/datasource/model/DataSourceType.java Outdated Show resolved Hide resolved
core/src/test/java/org/opensearch/sql/utils/SerializeUtilsTest.java Outdated

Gson gson = SerializeUtils.buildGson();
String json = gson.toJson(dataSourceMetadata);
System.out.println(json);
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching. removed.

ykmr1224 added 2 commits June 11, 2024 08:40
@ykmr1224
Address comment
4beb6c2 
Signed-off-by: Tomoyuki Morita <[email protected]>
@ykmr1224
Fix DataSourceType to allow registering new type
4059fcf 
Signed-off-by: Tomoyuki Morita <[email protected]>
@ykmr1224 ykmr1224 merged commit 1d703e8 into opensearch-project:main Jun 12, 2024
14 of 20 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 12, 2024
@github-actions
Change DataSourceType from enum to class (#2730)
373a9bf 
* Change DataSourceType from enum to class

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix test failure

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix serialization issue

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix format

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix integTest

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix style

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix failing test

Signed-off-by: Tomoyuki Morita <[email protected]>

* Address comment

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix DataSourceType to allow registering new type

Signed-off-by: Tomoyuki Morita <[email protected]>

---------

Signed-off-by: Tomoyuki Morita <[email protected]>
(cherry picked from commit 1d703e8)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Jun 12, 2024
Closed
@ykmr1224 ykmr1224 deleted the dqs/datasource-type branch June 12, 2024 14:59
ykmr1224 added a commit to ykmr1224/sql that referenced this pull request Jun 12, 2024
@ykmr1224
Change DataSourceType from enum to class (opensearch-project#2730)
f425714 
* Change DataSourceType from enum to class

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix test failure

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix serialization issue

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix format

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix integTest

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix style

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix failing test

Signed-off-by: Tomoyuki Morita <[email protected]>

* Address comment

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix DataSourceType to allow registering new type

Signed-off-by: Tomoyuki Morita <[email protected]>

---------

Signed-off-by: Tomoyuki Morita <[email protected]>
(cherry picked from commit 1d703e8)
ykmr1224 added a commit that referenced this pull request Jun 12, 2024
@ykmr1224
Change DataSourceType from enum to class (#2730)
2854240 
* Change DataSourceType from enum to class

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix test failure

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix serialization issue

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix format

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix integTest

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix style

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix failing test

Signed-off-by: Tomoyuki Morita <[email protected]>

* Address comment

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix DataSourceType to allow registering new type

Signed-off-by: Tomoyuki Morita <[email protected]>

---------

Signed-off-by: Tomoyuki Morita <[email protected]>
(cherry picked from commit 1d703e8)
jzonthemtn pushed a commit to jzonthemtn/sql that referenced this pull request Aug 28, 2024
@ykmr1224 @jzonthemtn
Change DataSourceType from enum to class (opensearch-project#2730)
c86fd87 
* Change DataSourceType from enum to class

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix test failure

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix serialization issue

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix format

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix integTest

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix style

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix failing test

Signed-off-by: Tomoyuki Morita <[email protected]>

* Address comment

Signed-off-by: Tomoyuki Morita <[email protected]>

* Fix DataSourceType to allow registering new type

Signed-off-by: Tomoyuki Morita <[email protected]>

---------

Signed-off-by: Tomoyuki Morita <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seankao-az seankao-az seankao-az approved these changes

@vmmusings vmmusings vmmusings approved these changes

@ps48 ps48 Awaiting requested review from ps48 ps48 is a code owner

@kavithacm kavithacm Awaiting requested review from kavithacm kavithacm is a code owner

@derek-ho derek-ho Awaiting requested review from derek-ho derek-ho is a code owner

@joshuali925 joshuali925 Awaiting requested review from joshuali925 joshuali925 is a code owner

@dai-chen dai-chen Awaiting requested review from dai-chen dai-chen is a code owner

@YANG-DB YANG-DB Awaiting requested review from YANG-DB YANG-DB is a code owner

@rupal-bq rupal-bq Awaiting requested review from rupal-bq

@mengweieric mengweieric Awaiting requested review from mengweieric mengweieric is a code owner

@Swiddis Swiddis Awaiting requested review from Swiddis Swiddis is a code owner

@penghuo penghuo Awaiting requested review from penghuo penghuo is a code owner

@MaxKsyunz MaxKsyunz Awaiting requested review from MaxKsyunz MaxKsyunz is a code owner

@Yury-Fridlyand Yury-Fridlyand Awaiting requested review from Yury-Fridlyand Yury-Fridlyand is a code owner

@anirudha anirudha Awaiting requested review from anirudha anirudha is a code owner

@forestmvey forestmvey Awaiting requested review from forestmvey forestmvey is a code owner

@acarbonetto acarbonetto Awaiting requested review from acarbonetto acarbonetto is a code owner

@GumpacG GumpacG Awaiting requested review from GumpacG GumpacG is a code owner

Assignees
No one assigned
Labels
backport 2.x maintenance Improves code quality, but not the product
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ykmr1224 @vmmusings @seankao-az