Implement patch API for datasources #2273

derek-ho · 2023-10-10T13:34:51Z

Description

Implements PATCH api for data sources. This enables edit of data sources with partial fields, instead of requiring the whole data source metadata.

Issues Resolved

#2168

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.

Signed-off-by: Derek Ho <[email protected]>

codecov · 2023-10-10T13:53:49Z

Codecov Report

Merging #2273 (87f0605) into main (501cf91) will increase coverage by 0.42%.
Report is 1 commits behind head on main.
The diff coverage is 72.81%.

@@             Coverage Diff              @@
##               main    #2273      +/-   ##
============================================
+ Coverage     96.61%   97.03%   +0.42%     
+ Complexity     4777     4538     -239     
============================================
  Files           449      401      -48     
  Lines         12907    11779    -1128     
  Branches        882      792      -90     
============================================
- Hits          12470    11430    -1040     
+ Misses          429      343      -86     
+ Partials          8        6       -2

Flag	Coverage Δ
sql-engine	`97.03% <72.81%> (+0.42%)`	⬆️

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

Files	Coverage Δ
...sql/datasources/service/DataSourceServiceImpl.java	`100.00% <100.00%> (ø)`
...rces/transport/TransportPatchDataSourceAction.java	`100.00% <100.00%> (ø)`
...rch/sql/datasources/utils/XContentParserUtils.java	`98.90% <97.50%> (-1.10%)`	⬇️
...model/transport/PatchDataSourceActionResponse.java	`0.00% <0.00%> (ø)`
.../model/transport/PatchDataSourceActionRequest.java	`21.42% <21.42%> (ø)`
...ql/datasources/rest/RestDataSourceQueryAction.java	`0.00% <0.00%> (ø)`

... and 51 files with indirect coverage changes

Signed-off-by: Derek Ho <[email protected]>

datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java

...c/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java

...rc/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java

datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java

Signed-off-by: Derek Ho <[email protected]>

datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java

YANG-DB · 2023-10-12T00:04:43Z

@derek-ho can u please elaborate on the use case for which the patch is needed ?
can u plz add an issue describing this and the surrounding flow ?

Signed-off-by: Derek Ho <[email protected]>

datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java

...s/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java

vmmusings · 2023-10-16T16:47:47Z

https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/admin/datasources.rst
Can you also update this document. Add new API below existing APIs. Also Mention what are the required fields.

Signed-off-by: Derek Ho <[email protected]>

derek-ho · 2023-10-17T15:50:07Z

https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/admin/datasources.rst Can you also update this document. Add new API below existing APIs. Also Mention what are the required fields.

Done in: 87f0605

* Implement patch API for datasources Signed-off-by: Derek Ho <[email protected]> * Change patch implementation to Map Signed-off-by: Derek Ho <[email protected]> * Fix up, everything complete except unit test Signed-off-by: Derek Ho <[email protected]> * Revise PR to use existing functions Signed-off-by: Derek Ho <[email protected]> * Remove unused utility function Signed-off-by: Derek Ho <[email protected]> * Add tests Signed-off-by: Derek Ho <[email protected]> * Add back line Signed-off-by: Derek Ho <[email protected]> * fix build issue Signed-off-by: Derek Ho <[email protected]> * Fix tests and add in rst Signed-off-by: Derek Ho <[email protected]> * Register patch Signed-off-by: Derek Ho <[email protected]> * Add imports Signed-off-by: Derek Ho <[email protected]> * Patch Signed-off-by: Derek Ho <[email protected]> * Fix integration test Signed-off-by: Derek Ho <[email protected]> * Update IT Signed-off-by: Derek Ho <[email protected]> * Add tests Signed-off-by: Derek Ho <[email protected]> * Fix test Signed-off-by: Derek Ho <[email protected]> * Fix tests and increase code cov Signed-off-by: Derek Ho <[email protected]> * Add more coverage to impl Signed-off-by: Derek Ho <[email protected]> * Fix test and jacoco passing Signed-off-by: Derek Ho <[email protected]> * Test fix Signed-off-by: Derek Ho <[email protected]> * Add docs Signed-off-by: Derek Ho <[email protected]> --------- Signed-off-by: Derek Ho <[email protected]> (cherry picked from commit f835112) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Implement patch API for datasources * Change patch implementation to Map * Fix up, everything complete except unit test * Revise PR to use existing functions * Remove unused utility function * Add tests * Add back line * fix build issue * Fix tests and add in rst * Register patch * Add imports * Patch * Fix integration test * Update IT * Add tests * Fix test * Fix tests and increase code cov * Add more coverage to impl * Fix test and jacoco passing * Test fix * Add docs --------- (cherry picked from commit f835112) Signed-off-by: Derek Ho <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…opensearch-project#2329)" This reverts commit 4c151fe. Signed-off-by: Eric <[email protected]>

…opensearch-project#2329)" This reverts commit 4c151fe.

* Revert "Add more metrics and handle emr exception message (#2422) (#2426)" This reverts commit b57f7cc. * Revert "Block settings in sql query settings API and add more unit tests (#2407) (#2412)" This reverts commit 3024737. * Revert "Added session, statement, emrjob metrics to sql stats api (#2398) (#2400)" This reverts commit 6e17ae6. * Revert "Redefine Drop Index as logical delete (#2386) (#2397)" This reverts commit e939bb6. * Revert "add concurrent limit on datasource and sessions (#2390) (#2395)" This reverts commit deb3ccf. * Revert "Add Flint Index Purging Logic (#2372) (#2389)" This reverts commit dd48b9b. * Revert "Refactoring for tags usage in test files and also added explicit denly list setting. (#2383) (#2385)" This reverts commit 37e010f. * Revert "Enable session by default (#2373) (#2375)" This reverts commit 7d95e4c. * Revert "Create new session if client provided session is invalid (#2368) (#2371)" This reverts commit 5ab7858. * Revert "Add where clause support in create statement (#2366) (#2370)" This reverts commit b620a56. * Revert "create new session if current session not ready (#2363) (#2365)" This reverts commit 5d07281. * Revert "Handle Describe,Refresh and Show Queries Properly (#2357) (#2362)" This reverts commit 16e2f30. * Revert "Add Session limitation (#2354) (#2359)" This reverts commit 0f334f8. * Revert "Bug Fix, support cancel query in running state (#2351) (#2353)" This reverts commit 9a40591. * Revert "Fix bug, using basic instead of basicauth (#2342) (#2355)" This reverts commit e4827a5. * Revert "Add missing tags and MV support (#2336) (#2346)" This reverts commit 8791bb0. * Revert "[Backport 2.x] deprecated job-metadata-index (#2340) (#2343)" This reverts commit bea432c. * Revert "Integration with REPL Spark job (#2327) (#2338)" This reverts commit 58a5ae5. * Revert "Implement patch API for datasources (#2273) (#2329)" This reverts commit 4c151fe. * Revert "Add sessionId parameters for create async query API (#2312) (#2324)" This reverts commit 3d1a376. * Revert "Add Statement (#2294) (#2318) (#2319)" This reverts commit b3c2e94. * Revert "Upgrade json (#2307) (#2314)" This reverts commit 6c65bb4. * Revert "Minor Refactoring (#2308) (#2317)" This reverts commit 051cc4f. * Revert "add InteractiveSession and SessionManager (#2290) (#2293) (#2315)" This reverts commit 6ac197b. --------- Co-authored-by: Vamsi Manohar <[email protected]>

Implement patch API for datasources

32f9577

Signed-off-by: Derek Ho <[email protected]>

derek-ho added 2 commits October 10, 2023 10:55

Change patch implementation to Map

9ed692f

Signed-off-by: Derek Ho <[email protected]>

Fix up, everything complete except unit test

9f4506a

Signed-off-by: Derek Ho <[email protected]>

derek-ho commented Oct 10, 2023

View reviewed changes

datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java Outdated Show resolved Hide resolved

vmmusings requested changes Oct 11, 2023

View reviewed changes

derek-ho added 2 commits October 11, 2023 11:24

Revise PR to use existing functions

84522f1

Signed-off-by: Derek Ho <[email protected]>

Remove unused utility function

9382ff3

Signed-off-by: Derek Ho <[email protected]>

vmmusings reviewed Oct 11, 2023

View reviewed changes

datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java Outdated Show resolved Hide resolved

vmmusings reviewed Oct 11, 2023

View reviewed changes

datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java Outdated Show resolved Hide resolved

derek-ho added 13 commits October 12, 2023 14:54

Add tests

91899fe

Signed-off-by: Derek Ho <[email protected]>

Add back line

e246a23

Signed-off-by: Derek Ho <[email protected]>

fix build issue

e86794a

Signed-off-by: Derek Ho <[email protected]>

Fix tests and add in rst

9384f8c

Signed-off-by: Derek Ho <[email protected]>

Register patch

fa130d7

Signed-off-by: Derek Ho <[email protected]>

Add imports

aa673a2

Signed-off-by: Derek Ho <[email protected]>

Patch

b179c49

Signed-off-by: Derek Ho <[email protected]>

Fix integration test

f336064

Signed-off-by: Derek Ho <[email protected]>

Update IT

8ca5e03

Signed-off-by: Derek Ho <[email protected]>

Add tests

0ac557a

Signed-off-by: Derek Ho <[email protected]>

Fix test

1051d50

Signed-off-by: Derek Ho <[email protected]>

Fix tests and increase code cov

af4c924

Signed-off-by: Derek Ho <[email protected]>

Add more coverage to impl

970b5bf

Signed-off-by: Derek Ho <[email protected]>

derek-ho marked this pull request as ready for review October 13, 2023 15:28

derek-ho requested review from pjfitzgibbons, ps48, kavithacm, joshuali925 and dai-chen as code owners October 13, 2023 15:28

derek-ho added 2 commits October 13, 2023 12:00

Fix test and jacoco passing

721c64f

Signed-off-by: Derek Ho <[email protected]>

Test fix

b98f0b3

Signed-off-by: Derek Ho <[email protected]>

vmmusings reviewed Oct 13, 2023

View reviewed changes

datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java Show resolved Hide resolved

vmmusings reviewed Oct 16, 2023

View reviewed changes

...s/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java Show resolved Hide resolved

vmmusings previously approved these changes Oct 16, 2023

View reviewed changes

derek-ho dismissed vmmusings’s stale review via 0954647 October 16, 2023 21:37

derek-ho force-pushed the patch branch from 7c43b35 to b98f0b3 Compare October 17, 2023 15:36

derek-ho added 2 commits October 17, 2023 11:36

Merge branch 'main' of github.com:opensearch-project/sql into patch

ad97a7a

Add docs

87f0605

Signed-off-by: Derek Ho <[email protected]>

vmmusings added the enhancement New feature or request label Oct 17, 2023

vmmusings assigned vmmusings and derek-ho and unassigned vmmusings Oct 17, 2023

vmmusings approved these changes Oct 18, 2023

View reviewed changes

derek-ho added the backport 2.11 label Oct 18, 2023

penghuo approved these changes Oct 18, 2023

View reviewed changes

vmmusings approved these changes Oct 18, 2023

View reviewed changes

vmmusings merged commit f835112 into opensearch-project:main Oct 18, 2023
19 of 23 checks passed

This was referenced Oct 18, 2023

[Backport 2.x] Implement patch API for datasources #2328

Merged

[Backport 2.11] Implement patch API for datasources #2329

Merged

mengweieric added a commit to mengweieric/sql that referenced this pull request Nov 8, 2023

Revert "Implement patch API for datasources (opensearch-project#2273) (…

55409eb

…opensearch-project#2329)" This reverts commit 4c151fe. Signed-off-by: Eric <[email protected]>

mengweieric added a commit to mengweieric/sql that referenced this pull request Nov 8, 2023

Revert "Implement patch API for datasources (opensearch-project#2273) (…

8420495

…opensearch-project#2329)" This reverts commit 4c151fe. Signed-off-by: Eric <[email protected]>

vmmusings added a commit to mengweieric/sql that referenced this pull request Nov 13, 2023

Revert "Implement patch API for datasources (opensearch-project#2273) (…

ab12221

…opensearch-project#2329)" This reverts commit 4c151fe.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement patch API for datasources #2273

Implement patch API for datasources #2273

derek-ho commented Oct 10, 2023 •

edited

Loading

codecov bot commented Oct 10, 2023 •

edited

Loading

YANG-DB commented Oct 12, 2023

vmmusings commented Oct 16, 2023

derek-ho commented Oct 17, 2023

Implement patch API for datasources #2273

Implement patch API for datasources #2273

Conversation

derek-ho commented Oct 10, 2023 • edited Loading

Description

Issues Resolved

Check List

codecov bot commented Oct 10, 2023 • edited Loading

Codecov Report

YANG-DB commented Oct 12, 2023

vmmusings commented Oct 16, 2023

derek-ho commented Oct 17, 2023

derek-ho commented Oct 10, 2023 •

edited

Loading

codecov bot commented Oct 10, 2023 •

edited

Loading