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

Setup S3 connection with integrations #1057

Merged
merged 97 commits into from
Oct 4, 2023

Conversation

Swiddis
Copy link
Collaborator

@Swiddis Swiddis commented Sep 29, 2023

Description

Followup to #1052, this PR adds actual S3 integration functionality.

Work in progress -- Main querying pipeline is good, but need to do some work for the UI.

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.

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
This test is unneeded after 12c4bcf

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
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 <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
if (!result.ok) {
console.error('Query failed', result.error);
setLoading(false);
setToast('Something went wrong.', 'danger');
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we change this toast to more appropriate wording?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What's more appropriate wording? We don't have any error information in the response that we can use

"name": "sample",
"type": "logs",
"version": "2.0.0",
"type": "",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you removing type here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Stopped filling a default value when the integration isn't loaded yet -- We only use the type when we go to integration adding so it's safe to keep empty until it's loaded.

},
})
).queryId;
while (true) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there better way than while true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure what kind of polling timeout to put on it, I've heard some queries can take up to 10 mins and don't have any reference for when I should just tell the user that it isn't working

Copy link
Collaborator

@derek-ho derek-ho left a comment

Choose a reason for hiding this comment

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

Approving to unblock, left a few comments

@derek-ho
Copy link
Collaborator

derek-ho commented Oct 4, 2023

@Swiddis can you fix DCO

}
} catch (err: any) {
console.error(err);
return { ok: false, error: err };
Copy link
Member

@joshuali925 joshuali925 Oct 4, 2023

Choose a reason for hiding this comment

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

i think this works but usually it's error, loading, data. If loading is true then display loading, else if error is not null then display error, otherwise data is useable. for example like https://swr.vercel.app/#overview

this way you don't need to return both ok and error, and loading and sleeping should be distinguished in case UI needs them

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Copy link
Collaborator Author

Swiddis commented Oct 4, 2023

Dealt with small-scale PR feedback -- More large-scale changes will need to happen later. Just waiting on tests before merging.

@Swiddis Swiddis merged commit eee56aa into opensearch-project:main Oct 4, 2023
8 of 9 checks passed
@Swiddis Swiddis deleted the setup-s3 branch October 4, 2023 18:28
derek-ho pushed a commit to derek-ho/dashboards-observability that referenced this pull request Oct 4, 2023
pjfitzgibbons added a commit that referenced this pull request Oct 5, 2023
* 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 <dxho@amazon.com>
Signed-off-by: sumukhswamy <sumukhhs@amazon.com>
Signed-off-by: Eric Wei <menwe@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Peter Fitzgibbons <peter.fitzgibbons@gmail.com>
Signed-off-by: Shenoy Pratik <sgguruda@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
Co-authored-by: sumukhswamy <sumukhhs@amazon.com>
Co-authored-by: Eric Wei <menwe@amazon.com>
Co-authored-by: Paul Sebastian <paul_sebastian@live.com>
Co-authored-by: Peter Fitzgibbons <peter.fitzgibbons@gmail.com>
Co-authored-by: Paul Sebastian <paulstn@amazon.com>
Co-authored-by: Shenoy Pratik <sgguruda@amazon.com>
Co-authored-by: Simeon Widdis <sawiddis@amazon.com>
pjfitzgibbons pushed a commit that referenced this pull request Oct 5, 2023
* Working on review comments

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>

* Replacing series and dimension with dynamic naming

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>

* Revert code for histogram and treemap after antlr merge because of crashes in app

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>

* Working on review comments, renaming metrics to series

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>

* Removing extra import after rebasing main

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>

* Working on review comments

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>

* Replacing series and dimension with dynamic naming

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>

* Fixed logs view visualizations issues.

Signed-off-by: Saisanju Sreevalsakumar <saisanju_s@persistent.com>

* Resolved review comments for log view fixes 1.

Signed-off-by: Saisanju Sreevalsakumar <saisanju_s@persistent.com>

Signed-off-by: ruchika-narang <ruchika_narang@persistent.com>
Signed-off-by: Saisanju Sreevalsakumar <saisanju_s@persistent.com>
Co-authored-by: ruchika-narang <ruchika_narang@persistent.com>
mengweieric pushed a commit to mengweieric/dashboards-observability that referenced this pull request Oct 15, 2023
amsiglan pushed a commit to amsiglan/dashboards-observability that referenced this pull request Jun 7, 2024
* 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 <dxho@amazon.com>
Signed-off-by: sumukhswamy <sumukhhs@amazon.com>
Signed-off-by: Eric Wei <menwe@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Peter Fitzgibbons <peter.fitzgibbons@gmail.com>
Signed-off-by: Shenoy Pratik <sgguruda@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
Co-authored-by: sumukhswamy <sumukhhs@amazon.com>
Co-authored-by: Eric Wei <menwe@amazon.com>
Co-authored-by: Paul Sebastian <paul_sebastian@live.com>
Co-authored-by: Peter Fitzgibbons <peter.fitzgibbons@gmail.com>
Co-authored-by: Paul Sebastian <paulstn@amazon.com>
Co-authored-by: Shenoy Pratik <sgguruda@amazon.com>
Co-authored-by: Simeon Widdis <sawiddis@amazon.com>
(cherry picked from commit 171acde)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants