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

Rename SENTRY__DSN to Sentry__Dsn #1424

Merged
merged 1 commit into from
May 21, 2024

Conversation

martyn-w
Copy link
Contributor

It looks to be case-sensitive

@martyn-w martyn-w temporarily deployed to development_aks May 21, 2024 08:44 — with GitHub Actions Inactive
@github-actions github-actions bot added the DevOps DevOps label May 21, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@@ -5,4 +5,4 @@
SLACK_ICON: https://raw.githubusercontent.com/DFE-Digital/get-into-teaching-api/master/.github/image.png?size=48
SLACK_USERNAME: GiT Workflows
SLACK_FOOTER: Get Into Teaching API Service
SENTRY__DSN: ""
Sentry__Dsn: ""

Choose a reason for hiding this comment

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

really?!

also, does it definitely want the double underscore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep 🤯 in my local tests, they pass if the variable is named Sentry__Dsn but fail if it is named SENTRY__DSN. And yep, a double-underscore is required.

Choose a reason for hiding this comment

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

how obscure. seems to relate to this. are we confident this isn't going to cause problems in production?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @carlosmartinez so it shouldn't affect Production as that value is read from AKS keyvaults; this value is used just in the github build pipeline. It is currently unset in the build pipeline, but a new version of the Sentry library requires it to be set to a blank string ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To add, in local tests where I have run:

export SENTRY__DSN=""
dotnet test --no-restore --verbosity normal 
...
Test Run Failed.
Total tests: 1051
     Passed: 1017
     Failed: 34
 Total time: 41.7688 Seconds

vs

export Sentry__Dsn="" 
dotnet test --no-restore --verbosity normal 
...
Test Run Successful.
Total tests: 1051
     Passed: 1051
 Total time: 5.5251 Minutes
     3>Done Building Project "/Users/martyn/development/dfe/get-into-teaching-api/GetIntoTeachingApiTests/GetIntoTeachingApiTests.csproj" (VSTest target(s)).
     1>Done Building Project "/Users/martyn/development/dfe/get-into-teaching-api/GetIntoTeachingApi.sln" (VSTest target(s)).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Choose a reason for hiding this comment

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

huh! life, what a journey.

@martyn-w martyn-w merged commit 86662db into master May 21, 2024
10 checks passed
@martyn-w martyn-w deleted the add-blank-sentry-dsn-for-github-build branch May 21, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DevOps DevOps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants