-
Notifications
You must be signed in to change notification settings - Fork 33
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
Integrate sidecar service #164
Conversation
Depending on opensearch-project/OpenSearch-Dashboards#5920, so CI would fail for now. |
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Signed-off-by: tygao <tygao@amazon.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #164 +/- ##
==========================================
+ Coverage 89.82% 90.07% +0.24%
==========================================
Files 52 54 +2
Lines 1396 1441 +45
Branches 340 347 +7
==========================================
+ Hits 1254 1298 +44
- Misses 140 141 +1
Partials 2 2 ☔ View full report in Codecov by Sentry. |
// mock sidecar open,hide and show | ||
jest.spyOn(coreContextExports, 'useCore').mockReturnValue({ | ||
overlays: { | ||
// @ts-ignore |
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.
Shall we add some comments to show why we ignore 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.
I think most test cases are similar, we don't need to mock all values returned in useCore
, we just mock something needed in the test case.
@@ -119,12 +122,36 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { | |||
} | |||
}; | |||
|
|||
useEffect(() => { |
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.
Shall we move useEffect
to the bottom of useCallback
?
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.
Do you mean setMountPoint
? I dont think declaration order will have any difference because SetMountPoint
will always excute after sidecar().open()
.
Signed-off-by: tygao <tygao@amazon.com>
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.
LGTM
* integrate sidecar poc Signed-off-by: tygao <tygao@amazon.com> * update sidecar menu Signed-off-by: tygao <tygao@amazon.com> * add portal log Signed-off-by: tygao <tygao@amazon.com> * update portal Signed-off-by: tygao <tygao@amazon.com> * keep sidecar size consistent when switching between right and left Signed-off-by: tygao <tygao@amazon.com> * update Signed-off-by: tygao <tygao@amazon.com> * add test subj for icon menu Signed-off-by: tygao <tygao@amazon.com> * remove extra change Signed-off-by: tygao <tygao@amazon.com> * remove core provider Signed-off-by: tygao <tygao@amazon.com> * update tests and add tests for sidecar icon menu Signed-off-by: tygao <tygao@amazon.com> * doc: update changelog and release note Signed-off-by: tygao <tygao@amazon.com> * doc: update release not Signed-off-by: tygao <tygao@amazon.com> * update Signed-off-by: tygao <tygao@amazon.com> * fix wrong enum value Signed-off-by: tygao <tygao@amazon.com> --------- Signed-off-by: tygao <tygao@amazon.com> (cherry picked from commit a4f210c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
* integrate sidecar poc Signed-off-by: tygao <tygao@amazon.com> * update sidecar menu Signed-off-by: tygao <tygao@amazon.com> * add portal log Signed-off-by: tygao <tygao@amazon.com> * update portal Signed-off-by: tygao <tygao@amazon.com> * keep sidecar size consistent when switching between right and left Signed-off-by: tygao <tygao@amazon.com> * update Signed-off-by: tygao <tygao@amazon.com> * add test subj for icon menu Signed-off-by: tygao <tygao@amazon.com> * remove extra change Signed-off-by: tygao <tygao@amazon.com> * remove core provider Signed-off-by: tygao <tygao@amazon.com> * update tests and add tests for sidecar icon menu Signed-off-by: tygao <tygao@amazon.com> * doc: update changelog and release note Signed-off-by: tygao <tygao@amazon.com> * doc: update release not Signed-off-by: tygao <tygao@amazon.com> * update Signed-off-by: tygao <tygao@amazon.com> * fix wrong enum value Signed-off-by: tygao <tygao@amazon.com> --------- Signed-off-by: tygao <tygao@amazon.com> (cherry picked from commit a4f210c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Integrate sidecar service in OSD
Issues Resolved
#156
opensearch-project/OpenSearch-Dashboards#5620
Depending on opensearch-project/OpenSearch-Dashboards#5920
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.