-
Notifications
You must be signed in to change notification settings - Fork 57
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
Conversation
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
This test is unneeded after 12c4bcf Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
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]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
...c/components/integrations/components/__tests__/__snapshots__/setup_integration.test.tsx.snap
Outdated
Show resolved
Hide resolved
Signed-off-by: Simeon Widdis <[email protected]>
public/components/integrations/components/setup_integration.tsx
Outdated
Show resolved
Hide resolved
if (!result.ok) { | ||
console.error('Query failed', result.error); | ||
setLoading(false); | ||
setToast('Something went wrong.', 'danger'); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
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": "", |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
public/components/integrations/components/setup_integration.tsx
Outdated
Show resolved
Hide resolved
public/components/integrations/components/setup_integration.tsx
Outdated
Show resolved
Hide resolved
}, | ||
}) | ||
).queryId; | ||
while (true) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this 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
@Swiddis can you fix DCO |
public/components/integrations/components/setup_integration.tsx
Outdated
Show resolved
Hide resolved
} | ||
} catch (err: any) { | ||
console.error(err); | ||
return { ok: false, error: err }; |
There was a problem hiding this comment.
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
public/components/integrations/components/setup_integration.tsx
Outdated
Show resolved
Hide resolved
public/components/integrations/components/setup_integration.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
Dealt with small-scale PR feedback -- More large-scale changes will need to happen later. Just waiting on tests before merging. |
* 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]>
* Working on review comments Signed-off-by: ruchika-narang <[email protected]> * Replacing series and dimension with dynamic naming Signed-off-by: ruchika-narang <[email protected]> * Revert code for histogram and treemap after antlr merge because of crashes in app Signed-off-by: ruchika-narang <[email protected]> * Working on review comments, renaming metrics to series Signed-off-by: ruchika-narang <[email protected]> * Removing extra import after rebasing main Signed-off-by: ruchika-narang <[email protected]> * Working on review comments Signed-off-by: ruchika-narang <[email protected]> * Replacing series and dimension with dynamic naming Signed-off-by: ruchika-narang <[email protected]> * Fixed logs view visualizations issues. Signed-off-by: Saisanju Sreevalsakumar <[email protected]> * Resolved review comments for log view fixes 1. Signed-off-by: Saisanju Sreevalsakumar <[email protected]> Signed-off-by: ruchika-narang <[email protected]> Signed-off-by: Saisanju Sreevalsakumar <[email protected]> Co-authored-by: ruchika-narang <[email protected]>
* 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)
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
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.