Skip to content
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

Enabled download of dot files from Download icon #279

Merged
merged 25 commits into from
Sep 28, 2022
Merged

Enabled download of dot files from Download icon #279

merged 25 commits into from
Sep 28, 2022

Conversation

KenSuenobu
Copy link
Contributor

@KenSuenobu KenSuenobu commented Sep 26, 2022

Which issue does this PR close?

Closes #276.

Builds on #242 so we need to merge that first.

Rationale for this change

Adds an anchor link around the GrDocumentDownload link from the ActionsCell to enable downloading of the DOT file generated from a job run.

What changes are included in this PR?

Modifies the ActionsCell code in QueriesList.tsx by passing in the job ID to the ActionsCell. An id field had to be assigned along side to avoid duplicate keys. Additionally, added a snippet of documentation to the README.md file to indicate solutions to common problems when trying to run the server via yarn start.

Are there any user-facing changes?

Only the noted change to README.md

@andygrove
Copy link
Member

Thanks, @KenSuenobu. I merged this into my branch locally and tried it out. The href link is correct, and it downloads a file with the correct filename, but unfortunately, it is downloading an HTML page and not hitting the API endpoint. This seems to be because the accept header is not set to application/json.

@andygrove
Copy link
Member

I'm open to adding another endpoint that doesn't require application/json but I don't know how to do that in warp yet.

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @KenSuenobu! I tested this locally, and it worked perfectly. I will merge this once my PR is approved and merged.

@andygrove
Copy link
Member

@KenSuenobu CI is failing on a formatting issue

@andygrove andygrove merged commit 253ff46 into apache:master Sep 28, 2022
@KenSuenobu KenSuenobu deleted the PR-242 branch September 28, 2022 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UI] Add ability to download query plans in dot format
2 participants