Skip to content

Conversation

wiedld
Copy link
Contributor

@wiedld wiedld commented Nov 1, 2022

Part of https://github.com/influxdata/idpe/issues/16210

Per feedback from the automation team (see this PR https://github.com/influxdata/idpe/pull/16212), and with approval of @nathanielc , making changes to the csv Dialect to add download headers.

Background:

  • we are doing direct filesystem download of data in the UI, bypassing browser memory.
  • With this approach, the download filename is only configured with content-disposition headers.
  • Made changes in queryrouterd to add the headers.
  • Per automation team's request, pushing the header change into the csv Dialect.

Checklist

  • ✏️ Write a PR description, regardless of triviality, to include the value of this PR
  • 🔗 Reference related issues
  • 🏃 Test cases are included to exercise the new code --> test cases are in idpe.
    * did not add Headers test to this repo, since I didn't see other headers tested.
    * Did add an idpe test case, which has already been successfully run with local changes in this package. Confirmed works.

@wiedld wiedld marked this pull request as ready for review November 2, 2022 02:23
@wiedld wiedld requested a review from a team as a code owner November 2, 2022 02:23
@wiedld wiedld requested review from jsternberg and removed request for a team November 2, 2022 02:23
@wiedld wiedld force-pushed the idpe-16210/set-download-headers-in-dialect branch from 9cb54f7 to cf0ddb5 Compare November 3, 2022 18:22
@wiedld wiedld changed the title chore(idpe-16210): add download headers within csv dialect feat(csv): add download headers within csv dialect Nov 3, 2022
…atures, such as the general structure of a Content-Disposition download header
@wiedld wiedld force-pushed the idpe-16210/set-download-headers-in-dialect branch from cf0ddb5 to 257e36c Compare November 3, 2022 19:48
@wiedld wiedld merged commit bf5d087 into master Nov 7, 2022
@jacobmarble jacobmarble deleted the idpe-16210/set-download-headers-in-dialect branch January 4, 2024 17:05
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.

3 participants