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

Allow exporting of results for non-alert queries #929

Merged
merged 3 commits into from
Aug 23, 2021

Conversation

edoardopirovano
Copy link
Contributor

@edoardopirovano edoardopirovano commented Aug 19, 2021

Closes #832

This PR adds a new right click option in the Query History view called Export Results (CSV) which will export the results of a non-alert query to a CSV file (as chosen in a save dialog displayed to the user).

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • @github/docs-content-codeql has been cc'd in all issues for UI or other user-facing changes made by this pull request.

@edoardopirovano
Copy link
Contributor Author

Rebased to fix a conflict in the CHANGELOG.

Copy link
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

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

I'll let someone else review/approve who's more comfortable with the code changes (especially the changes to run-queries), but I tried this locally and it seems to work 😊

extensions/ql-vscode/package.json Outdated Show resolved Hide resolved
extensions/ql-vscode/package.json Show resolved Hide resolved
@edoardopirovano
Copy link
Contributor Author

Thanks for the helpful comments @aeisenberg and @shati-patel! I've pushed an additional commit addressing them.

@@ -216,6 +218,29 @@ export class QueryInfo {
return this.dilPath;
}

async exportCsvResults(qs: qsClient.QueryServerClient, csvPath: string, onFinish: () => void): Promise<void> {
let stopDecoding = false;
const out = fs.createWriteStream(csvPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

If the csv file already exists, then maybe no need to recreate it? Though if the operation is fast, not a big deal either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is already not being called if the file already exists (see this line)

Copy link
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

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

Looks like all comments are addressed :)

@edoardopirovano edoardopirovano merged commit e119218 into github:main Aug 23, 2021
@Manouchehri
Copy link

Super excited to see this in the next release, thanks!

@edoardopirovano edoardopirovano deleted the export-csv branch August 24, 2021 09:10
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.

Add csv export for non-alert data
4 participants