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

Make name of debugging artifact and DB within it configurable #868

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

edoardopirovano
Copy link
Contributor

@edoardopirovano edoardopirovano commented Jan 7, 2022

As requested by @esbena and @hvitved for use on DCA.

Adds two new options next to the debugMode one to control the name of the debugging artifact uploaded and the name of the database within that artifact. This is needed on DCA because it uses the Action multiple times within one workflow with different SHAs that it is comparing, so without this the debugging artifacts will overwrite each-other. It may also be useful to users who want their debugging artifacts to have more meaningful names.

@robertbrignull This changes the code that calls codeql database bundle which is also called by the code that uploads databases to Remote Queries. In the Remote Queries instance of this case, I have passed in just language as the name of the DB. This is what was (implicitly) being used before, so I believe this change to be behaviour-preserving from the point of view of Remote Queries. You may want to check it nonetheless.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@edoardopirovano edoardopirovano marked this pull request as ready for review January 7, 2022 14:20
@edoardopirovano edoardopirovano requested a review from a team as a code owner January 7, 2022 14:20
Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

Thanks for the ping. The remote queries changes LGTM.

init/action.yml Outdated Show resolved Hide resolved
@edoardopirovano edoardopirovano changed the title Make name of debugging artifact and DB within in configurable Make name of debugging artifact and DB within it configurable Jan 7, 2022
config.dbLocation,
`${databasePath}.zip`
);
const databaseBundlePath = path.resolve(config.dbLocation, `${dbName}.zip`);
Copy link
Contributor

Choose a reason for hiding this comment

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

So, this will now place the bundled db in a different location? I don't think that's a problem, but it is a change and just want to be sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, that is correct. I hope there's no code outside the codeql-action that is relying on where we happen to put the DB bundles before uploading them to an artifact. I don't think that's a use-case we've ever claimed to support, nor do I think we should be supporting it, so I think it is okay to break.

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.

4 participants