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

[junit] Use uuidV4 for filename #987

Merged
merged 4 commits into from
Jul 18, 2023

Conversation

rodrigo-roca
Copy link
Contributor

@rodrigo-roca rodrigo-roca commented Jul 14, 2023

What and why?

If we send the normal filename it can contain some characters that would cause 400 errors at the intake. Now we create a uuidv4 for the filename and send the original filename somewhere else.

How?

Use uuid package and send filename somewhere else as _dd.report_name

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)

@rodrigo-roca rodrigo-roca requested a review from a team as a code owner July 14, 2023 08:35
@rodrigo-roca rodrigo-roca self-assigned this Jul 14, 2023
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Jul 14, 2023

Datadog Report

Branch report: rodrigo.roca/junit-safe-filename
Commit report: 381c5f5

datadog-ci-tests: 0 Failed, 0 New Flaky, 4614 Passed, 0 Skipped, 1m 41.51s Wall Time

@rodrigo-roca rodrigo-roca requested a review from a team as a code owner July 14, 2023 09:20
@rodrigo-roca rodrigo-roca changed the title [junit] Add back safe filename [junit] Use uuidV4 for filename Jul 14, 2023
Copy link
Contributor

@AdrianLC AdrianLC left a comment

Choose a reason for hiding this comment

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

LGTM but let's wait until the backend change is deployed

Copy link
Contributor

@juan-fernandez juan-fernandez left a comment

Choose a reason for hiding this comment

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

LGTM, but we need to add the new dependencies to https:/DataDog/datadog-ci/blob/master/LICENSE-3rdparty.csv.

Also, let's wait for the approval from someone else from datadog-ci team, since this is adding a new dependency and it affects other commands

Copy link
Contributor

@duncanista duncanista left a comment

Choose a reason for hiding this comment

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

LGTM – on the dependencies side!

@rodrigo-roca rodrigo-roca merged commit c5d9f2f into master Jul 18, 2023
10 checks passed
@rodrigo-roca rodrigo-roca deleted the rodrigo.roca/junit-safe-filename branch July 18, 2023 09:45
@lefebvree lefebvree mentioned this pull request Jul 18, 2023
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