Move ExportContainer out of WSLASession into WSLAContainer#14238
Move ExportContainer out of WSLASession into WSLAContainer#14238ptrivedi wants to merge 3 commits intofeature/wsl-for-appsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the ExportContainer functionality by moving it from the WSLASession class to the WSLAContainer class. This change aligns the API design with container-centric operations, making Export a method on the container instance rather than a session-level operation that requires a container ID parameter.
Changes:
- Moved
ExportContainerfrom session-level to container-level API - Updated the IDL to reflect the new interface structure
- Modified tests to use the new container-based Export method
- Changed error handling for deleted containers from
WSLA_E_CONTAINER_NOT_FOUNDtoRPC_E_DISCONNECTED
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/windows/wslaservice/inc/wslaservice.idl | Removed ExportContainer from IWSLASession interface and added Export to IWSLAContainer interface |
| src/windows/wslasession/WSLASession.h | Removed ExportContainer and ExportContainerImpl method declarations |
| src/windows/wslasession/WSLASession.cpp | Removed ExportContainer and ExportContainerImpl implementations |
| src/windows/wslasession/WSLAContainer.h | Added Export method declaration to WSLAContainerImpl and WSLAContainer |
| src/windows/wslasession/WSLAContainer.cpp | Implemented Export method in WSLAContainerImpl and COM wrapper; added unused variable in Delete method |
| test/windows/WSLATests.cpp | Updated ExportContainer test to use new container-based API and test deleted container scenario |
| m_name.c_str(), | ||
| m_state); | ||
|
|
||
| std::pair<uint32_t, wil::unique_socket> SocketCodePair; |
There was a problem hiding this comment.
This variable declaration appears to be unused and should be removed. It's declared but never assigned or referenced anywhere in the Delete method.
| std::pair<uint32_t, wil::unique_socket> SocketCodePair; |
| else | ||
| { | ||
| io.AddHandle(std::make_unique<RelayHandle<HTTPChunkBasedReadHandle>>( | ||
| HandleWrapper{std::move(SocketCodePair.second)}, HandleWrapper{std::move(containerFileHandle)})); |
There was a problem hiding this comment.
The onCompleted callback is defined but never passed to the HandleWrapper. In the original WSLASession implementation, this callback was passed as the second argument to the containerFileHandle HandleWrapper. The callback should be passed to ensure proper cancellation handling when the export completes.
| HandleWrapper{std::move(SocketCodePair.second)}, HandleWrapper{std::move(containerFileHandle)})); | |
| HandleWrapper{std::move(SocketCodePair.second)}, HandleWrapper{std::move(containerFileHandle), onCompleted})); |
1 participant
No description provided.