-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(QueryStringManager): fix Could not subscribe to application.currentAppId$ #10943
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
Conversation
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
❌ Invalid PrefixInvalid description prefix. Found "fix(QueryStringManager)". Expected "breaking", "deprecate", "feat", "fix", "infra", "doc", "chore", "refactor", "security", "skip", or "test". |
…pId$ Partially revert opensearch-project#9466. Current implementation doesn't work because QueryStringManager is constructed during setup, and setApplication happens during start. In playground or any instance we see `Could not subscribe to application.currentAppId$` in console warning. Signed-off-by: Joshua Li <[email protected]>
89ff62e to
fac904e
Compare
(cherry picked from commit 89ff62e)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10943 +/- ##
=======================================
Coverage 60.78% 60.78%
=======================================
Files 4531 4531
Lines 122235 122232 -3
Branches 20491 20491
=======================================
- Hits 74301 74300 -1
+ Misses 42695 42693 -2
Partials 5239 5239
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
this revert is not a good fix, because |
Description
Partially revert #9466. Current implementation doesn't work because QueryStringManager is constructed during setup, and setApplication happens during start. In playground or any instance we see
Could not subscribe to application.currentAppId$in console warning.Issues Resolved
Screenshot
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integration