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

[Feature] Acceleration components' data implementation #1521

Merged
merged 37 commits into from
Mar 14, 2024

Conversation

RyanL1997
Copy link
Collaborator

@RyanL1997 RyanL1997 commented Mar 11, 2024

Description

This PR contains 2 major parts of acceleration's components work:

  • The cache fetching implementation on the acceleration table level
  • The flyout data connection
    • mappings
    • settings
    • cat index status
    • single acceleration cache

Issues Resolved

[List any issues this PR will resolve]

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.

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Attention: Patch coverage is 78.72340% with 40 lines in your changes are missing coverage. Please review.

Project coverage is 57.53%. Comparing base (f21939d) to head (ac69e52).

Files Patch % Lines
.../manage/accelerations/utils/acceleration_utils.tsx 66.66% 15 Missing ⚠️
...nage/accelerations/acceleration_details_flyout.tsx 80.76% 10 Missing ⚠️
...onents/manage/accelerations/acceleration_table.tsx 83.63% 9 Missing ⚠️
...ations/flyout_modules/acceleration_details_tab.tsx 85.71% 3 Missing ⚠️
public/services/requests/dsl.ts 50.00% 2 Missing ⚠️
...ations/flyout_modules/accelerations_schema_tab.tsx 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1521      +/-   ##
==========================================
+ Coverage   56.98%   57.53%   +0.55%     
==========================================
  Files         348      352       +4     
  Lines       12705    12882     +177     
  Branches     3275     3317      +42     
==========================================
+ Hits         7240     7412     +172     
- Misses       5411     5416       +5     
  Partials       54       54              
Flag Coverage Δ
dashboards-observability 57.53% <78.72%> (+0.55%) ⬆️

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.

@RyanL1997 RyanL1997 changed the title [WIP] Acceleration flyout data [WIP] Acceleration data implementation Mar 12, 2024
@RyanL1997 RyanL1997 added enhancement New feature or request backport 2.x labels Mar 13, 2024
@RyanL1997 RyanL1997 changed the title [WIP] Acceleration data implementation [WIP] Acceleration's data implementation Mar 13, 2024
@RyanL1997 RyanL1997 changed the title [WIP] Acceleration's data implementation [Feature] Acceleration's data implementation Mar 13, 2024
@RyanL1997 RyanL1997 changed the title [Feature] Acceleration's data implementation [Feature] Acceleration components' data implementation Mar 13, 2024
@RyanL1997 RyanL1997 marked this pull request as ready for review March 13, 2024 23:41
} from '../accelerations/helpers/utils';
} from './utils/acceleration_utils';
import { coreRefs } from '../../../../../framework/core_refs';
import { OpenSearchDashboardsResponse } from '../../../../../../../../src/core/server/http/router';

export interface AccelerationDetailsFlyoutProps {
acceleration: any;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will fix this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

coming with the follow up fix on this issue: #1534

const [selectedTab, setSelectedTab] = useState('details');
const tabsMap: { [key: string]: any } = {
details: AccelerationDetailsTab,
schema: AccelerationSchemaTab,
sql_definition: AccelerationSqlTab,
};
const [settings, setSettings] = useState<object>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please define your settings, mappings and indexInfo types/interfaces instead of

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will open up a follow up to address all the types.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

follow up issue has been created: #1534


useEffect(() => {
getAccDetail(flintIndexName);
}, [flintIndexName]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing dependency 'getAccDetail', and question: do we need to handle cases where flintIndexName can be '' or undefined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we don't need to handle that case, since the field flintIndexName field should be the actual 'index name' in OS, so that it should be always present as long as the index existed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add a check anyways. However, transferring some of the conversation between me and @mengweieric also into the issue: #1534

Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
@mengweieric
Copy link
Collaborator

/home/runner/work/dashboards-observability/dashboards-observability/OpenSearch-Dashboards/plugins/dashboards-observability/public/services/requests/dsl.ts
Error:   15:8   error    Prefer named exports                      import/no-default-export

linting error is irrelevant from this change

you could do a quick resolution

/* eslint-disable import/no-default-export */

and mark it in your issue as to do

@RyanL1997
Copy link
Collaborator Author

/home/runner/work/dashboards-observability/dashboards-observability/OpenSearch-Dashboards/plugins/dashboards-observability/public/services/requests/dsl.ts
Error:   15:8   error    Prefer named exports                      import/no-default-export

linting error is irrelevant from this change

you could do a quick resolution

/* eslint-disable import/no-default-export */

and mark it in your issue as to do

sounds good, also updated the issue: #1534

@sejli
Copy link
Member

sejli commented Mar 14, 2024

Still see a couple console.log(), will we be removing them before this PR gets merged in?

@RyanL1997
Copy link
Collaborator Author

RyanL1997 commented Mar 14, 2024

Still see a couple console.log(), will we be removing them before this PR gets merged in?

adding into the issue #1534
Here we go: #1537

@mengweieric mengweieric merged commit 71dccc3 into opensearch-project:main Mar 14, 2024
16 of 20 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 14, 2024
* 1st commit of acc details connection

Signed-off-by: Ryan Liang <[email protected]>

* Update the snapshot

Signed-off-by: Ryan Liang <[email protected]>

* Fix the interface naming

Signed-off-by: Ryan Liang <[email protected]>

* Fix the status

Signed-off-by: Ryan Liang <[email protected]>

* Add the index health

Signed-off-by: Ryan Liang <[email protected]>

* Add change the field name into action

Signed-off-by: Ryan Liang <[email protected]>

* Wired up schema tab

Signed-off-by: Ryan Liang <[email protected]>

* Cache is working 0 with max depth exceeding issue

Signed-off-by: Ryan Liang <[email protected]>

* update snapshots

Signed-off-by: Ryan Liang <[email protected]>

* Fix the infinite loop and apply the status check correctly

Signed-off-by: Ryan Liang <[email protected]>

* Implement the refresh button

Signed-off-by: Ryan Liang <[email protected]>

* Rebase after apply new interface 1

Signed-off-by: Ryan Liang <[email protected]>

* Rebase after apply new interface 2 + finalize the design of refreshing button

Signed-off-by: Ryan Liang <[email protected]>

* refactor some comments

Signed-off-by: Ryan Liang <[email protected]>

* Fix table type column

Signed-off-by: Ryan Liang <[email protected]>

* Fix empty item with replacement of unredered -

Signed-off-by: Ryan Liang <[email protected]>

* Fix the destination index column

Signed-off-by: Ryan Liang <[email protected]>

* Fix status

Signed-off-by: Ryan Liang <[email protected]>

* Fix the skip index name

Signed-off-by: Ryan Liang <[email protected]>

* Fix the destination index column behavior when it is skip index

Signed-off-by: Ryan Liang <[email protected]>

* Correct the render behavior for skip index flyout

Signed-off-by: Ryan Liang <[email protected]>

* Fix the table loading infinite loop

Signed-off-by: Ryan Liang <[email protected]>

* Modify the behavior of getting this refreh interval and type for skipping

Signed-off-by: Ryan Liang <[email protected]>

* Fix the data source at the flyout details tab

Signed-off-by: Ryan Liang <[email protected]>

* Swtich the data connection tabs back to default order and update snapshots

Signed-off-by: Ryan Liang <[email protected]>

* Add refresh time for refreshing

Signed-off-by: Ryan Liang <[email protected]>

* Add loading panel 0

Signed-off-by: Ryan Liang <[email protected]>

* Fix loading state for table

Signed-off-by: Ryan Liang <[email protected]>

* Add the refresh type column to acceleration table

Signed-off-by: Ryan Liang <[email protected]>

* Add acceleration table test

Signed-off-by: Ryan Liang <[email protected]>

* Add acceleration table test 2

Signed-off-by: Ryan Liang <[email protected]>

* Add refresh field to flyout

Signed-off-by: Ryan Liang <[email protected]>

* Fix some comments

Signed-off-by: Ryan Liang <[email protected]>

* Fix some comments 2

Signed-off-by: Ryan Liang <[email protected]>

* Add null/undefined check for flintIndexName

Signed-off-by: Ryan Liang <[email protected]>

* remove console logs

Signed-off-by: Ryan Liang <[email protected]>

* Add eslint-dsiable for export in dsl

Signed-off-by: Ryan Liang <[email protected]>

---------

Signed-off-by: Ryan Liang <[email protected]>
(cherry picked from commit 71dccc3)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
paulstn pushed a commit to paulstn/dashboards-observability that referenced this pull request Mar 15, 2024
…oject#1521)

* 1st commit of acc details connection

Signed-off-by: Ryan Liang <[email protected]>

* Update the snapshot

Signed-off-by: Ryan Liang <[email protected]>

* Fix the interface naming

Signed-off-by: Ryan Liang <[email protected]>

* Fix the status

Signed-off-by: Ryan Liang <[email protected]>

* Add the index health

Signed-off-by: Ryan Liang <[email protected]>

* Add change the field name into action

Signed-off-by: Ryan Liang <[email protected]>

* Wired up schema tab

Signed-off-by: Ryan Liang <[email protected]>

* Cache is working 0 with max depth exceeding issue

Signed-off-by: Ryan Liang <[email protected]>

* update snapshots

Signed-off-by: Ryan Liang <[email protected]>

* Fix the infinite loop and apply the status check correctly

Signed-off-by: Ryan Liang <[email protected]>

* Implement the refresh button

Signed-off-by: Ryan Liang <[email protected]>

* Rebase after apply new interface 1

Signed-off-by: Ryan Liang <[email protected]>

* Rebase after apply new interface 2 + finalize the design of refreshing button

Signed-off-by: Ryan Liang <[email protected]>

* refactor some comments

Signed-off-by: Ryan Liang <[email protected]>

* Fix table type column

Signed-off-by: Ryan Liang <[email protected]>

* Fix empty item with replacement of unredered -

Signed-off-by: Ryan Liang <[email protected]>

* Fix the destination index column

Signed-off-by: Ryan Liang <[email protected]>

* Fix status

Signed-off-by: Ryan Liang <[email protected]>

* Fix the skip index name

Signed-off-by: Ryan Liang <[email protected]>

* Fix the destination index column behavior when it is skip index

Signed-off-by: Ryan Liang <[email protected]>

* Correct the render behavior for skip index flyout

Signed-off-by: Ryan Liang <[email protected]>

* Fix the table loading infinite loop

Signed-off-by: Ryan Liang <[email protected]>

* Modify the behavior of getting this refreh interval and type for skipping

Signed-off-by: Ryan Liang <[email protected]>

* Fix the data source at the flyout details tab

Signed-off-by: Ryan Liang <[email protected]>

* Swtich the data connection tabs back to default order and update snapshots

Signed-off-by: Ryan Liang <[email protected]>

* Add refresh time for refreshing

Signed-off-by: Ryan Liang <[email protected]>

* Add loading panel 0

Signed-off-by: Ryan Liang <[email protected]>

* Fix loading state for table

Signed-off-by: Ryan Liang <[email protected]>

* Add the refresh type column to acceleration table

Signed-off-by: Ryan Liang <[email protected]>

* Add acceleration table test

Signed-off-by: Ryan Liang <[email protected]>

* Add acceleration table test 2

Signed-off-by: Ryan Liang <[email protected]>

* Add refresh field to flyout

Signed-off-by: Ryan Liang <[email protected]>

* Fix some comments

Signed-off-by: Ryan Liang <[email protected]>

* Fix some comments 2

Signed-off-by: Ryan Liang <[email protected]>

* Add null/undefined check for flintIndexName

Signed-off-by: Ryan Liang <[email protected]>

* remove console logs

Signed-off-by: Ryan Liang <[email protected]>

* Add eslint-dsiable for export in dsl

Signed-off-by: Ryan Liang <[email protected]>

---------

Signed-off-by: Ryan Liang <[email protected]>
ps48 pushed a commit that referenced this pull request Mar 15, 2024
* 1st commit of acc details connection



* Update the snapshot



* Fix the interface naming



* Fix the status



* Add the index health



* Add change the field name into action



* Wired up schema tab



* Cache is working 0 with max depth exceeding issue



* update snapshots



* Fix the infinite loop and apply the status check correctly



* Implement the refresh button



* Rebase after apply new interface 1



* Rebase after apply new interface 2 + finalize the design of refreshing button



* refactor some comments



* Fix table type column



* Fix empty item with replacement of unredered -



* Fix the destination index column



* Fix status



* Fix the skip index name



* Fix the destination index column behavior when it is skip index



* Correct the render behavior for skip index flyout



* Fix the table loading infinite loop



* Modify the behavior of getting this refreh interval and type for skipping



* Fix the data source at the flyout details tab



* Swtich the data connection tabs back to default order and update snapshots



* Add refresh time for refreshing



* Add loading panel 0



* Fix loading state for table



* Add the refresh type column to acceleration table



* Add acceleration table test



* Add acceleration table test 2



* Add refresh field to flyout



* Fix some comments



* Fix some comments 2



* Add null/undefined check for flintIndexName



* remove console logs



* Add eslint-dsiable for export in dsl



---------


(cherry picked from commit 71dccc3)

Signed-off-by: Ryan Liang <[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>
amsiglan pushed a commit to amsiglan/dashboards-observability that referenced this pull request Jun 7, 2024
…oject#1521) (opensearch-project#1536)

* 1st commit of acc details connection

* Update the snapshot

* Fix the interface naming

* Fix the status

* Add the index health

* Add change the field name into action

* Wired up schema tab

* Cache is working 0 with max depth exceeding issue

* update snapshots

* Fix the infinite loop and apply the status check correctly

* Implement the refresh button

* Rebase after apply new interface 1

* Rebase after apply new interface 2 + finalize the design of refreshing button

* refactor some comments

* Fix table type column

* Fix empty item with replacement of unredered -

* Fix the destination index column

* Fix status

* Fix the skip index name

* Fix the destination index column behavior when it is skip index

* Correct the render behavior for skip index flyout

* Fix the table loading infinite loop

* Modify the behavior of getting this refreh interval and type for skipping

* Fix the data source at the flyout details tab

* Swtich the data connection tabs back to default order and update snapshots

* Add refresh time for refreshing

* Add loading panel 0

* Fix loading state for table

* Add the refresh type column to acceleration table

* Add acceleration table test

* Add acceleration table test 2

* Add refresh field to flyout

* Fix some comments

* Fix some comments 2

* Add null/undefined check for flintIndexName

* remove console logs

* Add eslint-dsiable for export in dsl

---------

(cherry picked from commit 71dccc3)

Signed-off-by: Ryan Liang <[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>
(cherry picked from commit 0a03c16)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants