-
Notifications
You must be signed in to change notification settings - Fork 76
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
fix: download operators result #3241
Conversation
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.
Left some comments
...mber/src/main/scala/edu/uci/ics/texera/web/model/websocket/request/ResultExportRequest.scala
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/texera/web/resource/ResultResource.scala
Outdated
Show resolved
Hide resolved
core/gui/src/app/dashboard/service/user/download/download.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/dashboard/service/user/download/download.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # core/amber/src/main/scala/edu/uci/ics/texera/web/service/ResultExportService.scala
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.
left some comments
core/gui/src/app/dashboard/service/user/download/download.interface.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/dashboard/service/user/download/download.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/component/result-exportation/result-exportation.component.ts
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/texera/web/resource/ResultResource.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/texera/web/service/ResultExportService.scala
Outdated
Show resolved
Hide resolved
core/amber/src/main/scala/edu/uci/ics/texera/web/service/ResultExportService.scala
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.
LGTM! left some minor comments
This PR is in continuous of PR 3221 to implement download logic in backend.
Export request structure updates
In this PR, we update
ResultExportRequest
to include a new variable nameddestination
. If its set tolocal
, means the user wants to download operators result. Also,operatorIds
is now plural so with one request, multiple operators can be exported (locally or to dataset).Export endpoint updates
In this PR, we update
ResultResource
to supportdestination=local
. In this regard, we have two cases:The export to dataset follows the old logic.
Export service updates
In this PR, we update
ResultExportService
to have two core more new functions:exportOperatorResultAsStream
: For local download of a single operator. Streams the data directly.exportOperatorsAsZip
: For local download of multiple operators. Zip the output and stream back.Demo video
2025-02-02.14-00-34.mp4