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

Refactor Integrations Backend to abstract IO #947

Merged
merged 30 commits into from
Sep 18, 2023
Merged

Refactor Integrations Backend to abstract IO #947

merged 30 commits into from
Sep 18, 2023

Conversation

Swiddis
Copy link
Collaborator

@Swiddis Swiddis commented Aug 22, 2023 •
edited
Loading

Description

As part of continuing work on enabling remote catalogs, more refactoring for existing code is being done. In particular, we're introducing the concept of a CatalogReader which can be freely swapped out to enable new input sources, such as a URL or a zip file.

The main conversion for the Integrations class is done, but before this PR is ready, there is still some rippling changes to apply, and more tests need updating. In particular, with the introduction of a Result type in #943, the error handling code is being refactored considerably. This PR is just opened early to get an initial look.

Ready for review.

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.

Swiddis added 17 commits August 17, 2023 15:41
@Swiddis
Stub catalog reader interface
c5cea8d 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Add basic catalog functionality to new catalog reader
77ad89b 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Refactor validation logic with a deeper interface
676591d 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Refactor validation logic with a deeper interface
12c4bcf 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Remove redundant test.
412480e 
This test is unneeded after 12c4bcf

Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Add tests for new validators
6d2bad1 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Merge branch 'better-validation' into prototype-network-catalog
c3dd111
@Swiddis
Make better failure mode for invalid objects
55d6490 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Merge branch 'main' into prototype-network-catalog
c62d28b
@Swiddis
Generalize Result type
ba56fb8 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Convert backend to use catalog reader (unstable)
cb29208 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Repair tests for integrations class (unstable)
c9de36e 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Refactor repository for new integration interface
37c7980 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Fix outer repository and backend tests
b331090 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Add tests for sample data
4b29e17 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Merge remote-tracking branch 'upstream/main' into prototype-network-c…
b35dfa8 
…atalog
@Swiddis
Add CatalogReader JavaDocs
1bef29e 
Signed-off-by: Simeon Widdis <[email protected]>
@codecov
Copy link

codecov bot commented Aug 23, 2023 •
edited
Loading

Codecov Report

Merging #947 (ee33a51) into main (e7bdc67) will increase coverage by 0.32%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #947      +/-   ##
==========================================
+ Coverage   43.83%   44.16%   +0.32%     
==========================================
  Files         311      312       +1     
  Lines       18676    18742      +66     
  Branches     4555     4587      +32     
==========================================
+ Hits         8187     8277      +90     
+ Misses      10447     9922     -525     
- Partials       42      543     +501
Flag Coverage Δ
dashboards-observability 44.16% <ø> (+0.32%) ⬆️

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

see 84 files with indirect coverage changes

Swiddis added 6 commits August 23, 2023 11:07
@Swiddis
Repair integrations builder
a18cf47 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Remove extra commented test
047b14a 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Remove unnecessary log statement
e65fe8b 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Repair getSchemas behavior to return correct type
00ddb5b 
Let it be known at on this day, with this commit,
I have truly grokked why we don't use `any` in typescript.

Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Add tests for getSchemas
8af7f4a 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Add tests for asset and sample data backend methods
a31963d 
Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis Swiddis marked this pull request as ready for review August 23, 2023 20:39
@Swiddis Swiddis added enhancement New feature or request integrations Used to denote items related to the Integrations project labels Aug 23, 2023
pjfitzgibbons
pjfitzgibbons suggested changes Sep 9, 2023
View reviewed changes
Copy link
Contributor

@pjfitzgibbons pjfitzgibbons left a comment

Choose a reason for hiding this comment

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

I would like to discuss design/architecture of these changes.

server/adaptors/integrations/__test__/kibana_backend.test.ts Outdated Show resolved Hide resolved
server/adaptors/integrations/__test__/kibana_backend.test.ts Show resolved Hide resolved
server/adaptors/integrations/__test__/validators.test.ts Outdated Show resolved Hide resolved
server/adaptors/integrations/integrations_kibana_backend.ts Outdated Show resolved Hide resolved
server/adaptors/integrations/__test__/builder.test.ts Show resolved Hide resolved
server/adaptors/integrations/repository/catalog_reader.ts Outdated Show resolved Hide resolved
server/adaptors/integrations/repository/integration.ts Outdated Show resolved Hide resolved
@Swiddis
Copy link
Collaborator Author

Swiddis commented Sep 11, 2023

After talking with Peter a bit about potential directions for building on the architecture of this, I've worked out some next steps to maybe work out both comments, will see about getting this PR ready in the next couple days with some corresponding issues for further architecture work.

@Swiddis Swiddis self-assigned this Sep 11, 2023
@Swiddis Swiddis changed the title Refactor Integrations class to rely on CatalogReader Refactor Integrations Backend to abstract IO and simplify logic Sep 14, 2023
@Swiddis Swiddis changed the title Refactor Integrations Backend to abstract IO and simplify logic Refactor Integrations Backend to abstract IO Sep 14, 2023
@Swiddis Swiddis mentioned this pull request Sep 15, 2023
Merged
6 tasks
@Swiddis Swiddis merged commit 35a4d28 into opensearch-project:main Sep 18, 2023
@Swiddis Swiddis deleted the prototype-network-catalog branch September 18, 2023 16:11
pjfitzgibbons pushed a commit that referenced this pull request Sep 26, 2023
@Swiddis @pjfitzgibbons
Refactor Integrations Backend to abstract IO (#947)
67476d6
derek-ho pushed a commit to derek-ho/dashboards-observability that referenced this pull request Oct 4, 2023
@Swiddis @derek-ho
Refactor Integrations Backend to abstract IO (opensearch-project#947)
cf1ddf8
pjfitzgibbons added a commit that referenced this pull request Oct 5, 2023
@derek-ho @sumukhswamy
@mengweieric @paulstn @pjfitzgibbons @ps48 @Swiddis
Update 2.x with latest commits on main (#1070)
171acde 
* Update mocha (#960)
* added code for the change of design of the sidebar, fixed overriding … (#928)
* Refactor add_integration_flyout to break out nested methods (#956)
* [Log Explorer] Remove top level tabs (#970)
* Explorer's timeheader & color change (#959)
* Update caching (#965)
* Move query_utils to /public (#983)
* Remove Zeppelin Parser from Notebook (#985)
* Explorer empty bucket (#990)
* refactored classnames for sidebar (#933)
* Refactor Integrations Backend to abstract IO (#947)
* Create React component for Integrations Setup UI (#1009)
* Create Data sources plugin with Manage Datasources Flow  (#1035)
* Manage datasources (#967)
* Add acceleration management UI  (#989)
* Rename data sources to data connections (#1004)
* Add fallback to show if user does not have datasource API permissions (#1008)
* Add access control tab content (#992)
* Delete datasource and Connection Configuration Tab (#1024)
* adjust explorer chart color and spacing (#1051)
* Configure S3 datasource flow (#1049)
* Manage datasources (#967)
* Add acceleration management UI  (#989)
* Rename data sources to data connections (#1004)
* Add fallback to show if user does not have datasource API permissions (#1008)
* Add access control tab content (#992)
* Delete datasource and Connection Configuration Tab (#1024)
* modified explorer data grid to follow discover look and feel (#1041)
* Create prometheus datasource flow (#1054)
* Manage datasources (#967)
* Add acceleration management UI  (#989)
* Rename data sources to data connections (#1004)
* Add fallback to show if user does not have datasource API permissions (#1008)
* Add access control tab content (#992)
* Delete datasource and Connection Configuration Tab (#1024)
* added design changes for sidebar (#1061)
* updated snapshots (#1062)
* Update UI for integrations setup (#1052)
* Data sources UI improvements (#1059)
* adding redirection to datasources (#1063)
* Move convertDateTime, refactor query_utils (#1064)
* Setup S3 connection with integrations (#1057)
* Use approved svg from UX (#1066)
* Fix missing import 'moment' on query_utils. (#1067)

---------

Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: Eric Wei <[email protected]>
Signed-off-by: Paul Sebastian <[email protected]>
Signed-off-by: Peter Fitzgibbons <[email protected]>
Signed-off-by: Shenoy Pratik <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Co-authored-by: sumukhswamy <[email protected]>
Co-authored-by: Eric Wei <[email protected]>
Co-authored-by: Paul Sebastian <[email protected]>
Co-authored-by: Peter Fitzgibbons <[email protected]>
Co-authored-by: Paul Sebastian <[email protected]>
Co-authored-by: Shenoy Pratik <[email protected]>
Co-authored-by: Simeon Widdis <[email protected]>
@Swiddis Swiddis mentioned this pull request Jan 10, 2024
Merged
6 tasks
amsiglan pushed a commit to amsiglan/dashboards-observability that referenced this pull request Jun 7, 2024
@derek-ho
Update 2.x with latest commits on main (opensearch-project#1070)
c236f33 
* Update mocha (opensearch-project#960)
* added code for the change of design of the sidebar, fixed overriding … (opensearch-project#928)
* Refactor add_integration_flyout to break out nested methods (opensearch-project#956)
* [Log Explorer] Remove top level tabs (opensearch-project#970)
* Explorer's timeheader & color change (opensearch-project#959)
* Update caching (opensearch-project#965)
* Move query_utils to /public (opensearch-project#983)
* Remove Zeppelin Parser from Notebook (opensearch-project#985)
* Explorer empty bucket (opensearch-project#990)
* refactored classnames for sidebar (opensearch-project#933)
* Refactor Integrations Backend to abstract IO (opensearch-project#947)
* Create React component for Integrations Setup UI (opensearch-project#1009)
* Create Data sources plugin with Manage Datasources Flow  (opensearch-project#1035)
* Manage datasources (opensearch-project#967)
* Add acceleration management UI  (opensearch-project#989)
* Rename data sources to data connections (opensearch-project#1004)
* Add fallback to show if user does not have datasource API permissions (opensearch-project#1008)
* Add access control tab content (opensearch-project#992)
* Delete datasource and Connection Configuration Tab (opensearch-project#1024)
* adjust explorer chart color and spacing (opensearch-project#1051)
* Configure S3 datasource flow (opensearch-project#1049)
* Manage datasources (opensearch-project#967)
* Add acceleration management UI  (opensearch-project#989)
* Rename data sources to data connections (opensearch-project#1004)
* Add fallback to show if user does not have datasource API permissions (opensearch-project#1008)
* Add access control tab content (opensearch-project#992)
* Delete datasource and Connection Configuration Tab (opensearch-project#1024)
* modified explorer data grid to follow discover look and feel (opensearch-project#1041)
* Create prometheus datasource flow (opensearch-project#1054)
* Manage datasources (opensearch-project#967)
* Add acceleration management UI  (opensearch-project#989)
* Rename data sources to data connections (opensearch-project#1004)
* Add fallback to show if user does not have datasource API permissions (opensearch-project#1008)
* Add access control tab content (opensearch-project#992)
* Delete datasource and Connection Configuration Tab (opensearch-project#1024)
* added design changes for sidebar (opensearch-project#1061)
* updated snapshots (opensearch-project#1062)
* Update UI for integrations setup (opensearch-project#1052)
* Data sources UI improvements (opensearch-project#1059)
* adding redirection to datasources (opensearch-project#1063)
* Move convertDateTime, refactor query_utils (opensearch-project#1064)
* Setup S3 connection with integrations (opensearch-project#1057)
* Use approved svg from UX (opensearch-project#1066)
* Fix missing import 'moment' on query_utils. (opensearch-project#1067)

---------

Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: Eric Wei <[email protected]>
Signed-off-by: Paul Sebastian <[email protected]>
Signed-off-by: Peter Fitzgibbons <[email protected]>
Signed-off-by: Shenoy Pratik <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Co-authored-by: sumukhswamy <[email protected]>
Co-authored-by: Eric Wei <[email protected]>
Co-authored-by: Paul Sebastian <[email protected]>
Co-authored-by: Peter Fitzgibbons <[email protected]>
Co-authored-by: Paul Sebastian <[email protected]>
Co-authored-by: Shenoy Pratik <[email protected]>
Co-authored-by: Simeon Widdis <[email protected]>
(cherry picked from commit 171acde)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YANG-DB YANG-DB YANG-DB approved these changes

@pjfitzgibbons pjfitzgibbons pjfitzgibbons approved these changes

@ps48 ps48 Awaiting requested review from ps48

@kavithacm kavithacm Awaiting requested review from kavithacm

@derek-ho derek-ho Awaiting requested review from derek-ho

@joshuali925 joshuali925 Awaiting requested review from joshuali925

@dai-chen dai-chen Awaiting requested review from dai-chen

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

@mengweieric mengweieric Awaiting requested review from mengweieric

@vmmusings vmmusings Awaiting requested review from vmmusings

@penghuo penghuo Awaiting requested review from penghuo

@seankao-az seankao-az Awaiting requested review from seankao-az

@anirudha anirudha Awaiting requested review from anirudha

Assignees

@Swiddis Swiddis

Labels
enhancement New feature or request integrations Used to denote items related to the Integrations project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Swiddis @pjfitzgibbons @YANG-DB