Batch Download of traces #1274
Batch Download of traces #1274
Conversation
Katarzyna-B
requested review from
everett980,
rubenvp8510 and
tiffon
as code owners
|
Thanks for the PR. I have a couple of comments, but I posted them in the ticket to solicit opinions from people who voted for this feature: #663 (comment) |
| </Button> | ||
| ); | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
what does <> represent? Is it needed?
There was a problem hiding this comment.
<> </>
This is called the Empty tag in React. This is a shorter syntax for React Fragments.
Documentation: https://react.dev/reference/react/Fragment
It is needed. If you remove this tag you will get exception like
[AltViewOptions.tsx](react-dom.development.js:14887 Uncaught Error: Objects are not valid as a React child (found: object with keys {downloadBtn})).....
It is used already in this project. See for example AltViewOptions.tsx
There was a problem hiding this comment.
I was wrong. I got the exception which I described above, but the lint complained about this part of code:
.../SearchResults/DownloadResults.tsx
[eslint ] 29:10 error Fragments should contain more than one child - otherwise, there’s no need for a Fragment at all react/jsx-no-useless-fragment
I changed the code in a different way and it works locally without the above described error.
| document.body.appendChild(element); | ||
| element.click(); | ||
| URL.revokeObjectURL(element.href) | ||
| element.remove() |
There was a problem hiding this comment.
Reading this as not a web developer, is this the preferred way of executing downloads from a page? Do you have some pointer to documentation describing it?
There was a problem hiding this comment.
I am not primarily a web developer. But when I was looking for the best way of executing downloads from a page I found bunch of similar solution how you can do it. Here some references:
- https://www.techiediaries.com/javascript-blobs-object-urls/ (Date: 02 Oct 2022)
- https://theroadtoenterprise.com/blog/how-to-download-csv-and-json-files-in-react (27 Aug, 2021)
element.click() --> it's simulate a click
URL.revokeObjectURL(element.href) --> "it is recommended to release object URLs to optimize performance and reduce memory consumption." In the first article it is good explained.
| @@ -114,7 +115,7 @@ function searchDone(state, { meta, payload }) { | |||
| } | |||
| const traces = { ...state.traces, ...resultTraces }; | |||
| const search = { ...state.search, results, state: fetchedState.DONE }; | |||
| return { ...state, search, traces }; | |||
| return { ...state, search, traces, tracesToDownload: payloadData}; | |||
There was a problem hiding this comment.
nit: I don't think this reducer knows why it's returning the full payload, so naming the output tracesToDownload is not accurate. Can we name it something like rawTraces?
There was a problem hiding this comment.
Good point. I will do it.
Signed-off-by: katarzyna <[email protected]>
## Which problem is this PR solving? - Unbreak the UI in the production build (closes jaegertracing#1275) ## Short description of the changes It seems that the antd v3 upgrade in cda1746 causes an error in the production build, apparently due to a bad interplay with vite/esbuild bundling. As a workaround, provide an import alias for the antd icon sprite file that's causing the issue.[1] --- [1] ant-design/ant-design#19002 (comment) Signed-off-by: Máté Szabó <[email protected]>
- Resolves jaegertracing#663 ## Short description of the changes - Add a button with download functionality Signed-off-by: katarzyna <[email protected]>
Katarzyna-B
force-pushed
the
batch_download_of_traces
branch
from
54e8463 to
ec0a790
Compare
- Resolves jaegertracing#663 ## Short description of the changes - Add a button with download functionality Signed-off-by: katarzyna <[email protected]>
|
please fix lint errors |
- Resolves jaegertracing#663 ## Short description of the changes - Add a button with download functionality Signed-off-by: katarzyna <[email protected]>
- Resolves jaegertracing#663 ## Short description of the changes - Add a button with download functionality Signed-off-by: katarzyna <[email protected]>
Katarzyna-B
requested review from
yurishkuro
and removed request for
tiffon,
everett980 and
rubenvp8510
Batch Download of traces ## Which problem is this PR solving? - Resolves jaegertracing#663 ## Short description of the changes - Add a button with download functionality --------- Signed-off-by: katarzyna <[email protected]> Signed-off-by: Máté Szabó <[email protected]> Co-authored-by: Máté Szabó <[email protected]>
Batch Download of traces
Which problem is this PR solving?
Short description of the changes