-
Notifications
You must be signed in to change notification settings - Fork 3.3k
internal: rework studio handshake to allow better caching #31599
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
cypress
|
Project |
cypress
|
Branch Review |
ryanm/fix/make-caching-possible
|
Run status |
|
Run duration | 14m 31s |
Commit |
|
Committer | Ryan Manuel |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
10
|
|
0
|
|
589
|
View all changes introduced in this branch ↗︎ |
UI Coverage
0%
|
|
---|---|
|
4
|
|
0
|
Accessibility
97.09%
|
|
---|---|
|
0 critical
1 serious
0 moderate
0 minor
|
|
6
|
packages/server/test/unit/cloud/api/studio/post_studio_session_spec.ts
Outdated
Show resolved
Hide resolved
packages/server/test/unit/cloud/api/studio/post_studio_session_spec.ts
Outdated
Show resolved
Hide resolved
packages/server/test/unit/cloud/api/studio/post_studio_session_spec.ts
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
This PR reworks the studio handshake mechanism to allow better caching by introducing a dedicated studio session endpoint and refactoring studio manager initialization.
- Introduces a postStudioSession API to retrieve studio/protocol URLs before downloading the studio bundle.
- Adapts tests and project-base logic to integrate the new handshake flow.
- Updates studio manager creation and studio-app types to support the new caching mechanism.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
scripts/gulp/tasks/gulpCloudDeliveredTypes.ts | Updated studio bundle download to use studioSession URL from postStudioSession. |
packages/server/test/unit/project_spec.js | Modified test stubs and asynchronous callbacks for protocol management. |
packages/server/test/unit/cloud/api/studio/post_studio_session_spec.ts | Added tests for the new postStudioSession API. |
packages/server/test/unit/cloud/api/studio/get_and_initialize_studio_manager_spec.ts | Updated tests to supply studioUrl to the studio manager initializer. |
packages/server/test/unit/StudioLifecycleManager_spec.ts | Adjusted studio lifecycle tests to use the postStudioSession stub. |
packages/server/lib/project-base.ts | Refactored studio destruction logic for improved flow. |
packages/server/lib/cloud/routes.ts | Updated API endpoints to include a studioSession route. |
packages/server/lib/cloud/api/studio/post_studio_session.ts | Introduced a new API that posts to the studio session endpoint with retry logic. |
packages/server/lib/cloud/api/studio/get_and_initialize_studio_manager.ts | Modified API to accept studioUrl and download the appropriate studio bundle. |
packages/server/lib/StudioLifecycleManager.ts | Refactored studio manager creation logic to use the new postStudioSession and establish protocol setup. |
packages/app/src/studio/studio-app-types.ts | Updated studio types to mark certain callbacks as optional and add new hooks. |
Comments suppressed due to low confidence (2)
packages/app/src/studio/studio-app-types.ts:3
- [nitpick] Since onStudioPanelClose has been changed to optional and new hooks have been added, consider updating the documentation or inline comments to clarify how consumers should handle the absence of these properties.
export interface StudioPanelProps {
packages/server/test/unit/project_spec.js:885
- [nitpick] The updated async handling using a promise to capture the onStudioDestroy callback improves clarity. Ensure that this pattern is applied consistently across similar asynchronous test scenarios to avoid potential race conditions.
this.project.server.startWebsockets.callsFake(async (automation, config, callbacks) => {
packages/server/lib/project-base.ts
Outdated
await studio?.destroy() | ||
|
||
if (studio?.protocolManager) { |
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.
Seems odd to destroy studio and then check if there is a protocolManager.
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.
Yeah good point. I think this is less confusing: 4e326ff
(#31599)
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.
Looks good 👍🏻
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Add a "handshake" mechanism to the studio workflow to get exact studio/protocol URLs up front. This allows for better caching.
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?