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

chore: include region value in output #5604

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

thisislawatts
Copy link
Member

@thisislawatts thisislawatts commented Nov 27, 2024

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)

What does this PR do?

Introduces a Region key to our debug output to make it quicker to troubleshoot the region we are currently configured to run against.
Associated documentation: https://docs.snyk.io/working-with-snyk/regional-hosting-and-data-residency#available-snyk-regions

How should this be manually tested?

  • snyk config environment default
  • snyk help -d The output will include Region: snyk-us-01
  • snyk config environment dev
  • snyk help -d will not include a Region property as we are running against a dev instance.

Expected Output

2024-11-27T13:57:14Z main - Using a preview feature!
2024-11-27T13:57:14Z main - Version:               1.1295.0-dev
2024-11-27T13:57:14Z main - Platform:              darwin amd64
2024-11-27T13:57:14Z main - API:                   https://api.snyk.io
2024-11-27T13:57:14Z main - Region:                snyk-us-01
2024-11-27T13:57:14Z main - Cache:                 /Users/path/tocache
2024-11-27T13:57:14Z main - Organization:
2024-11-27T13:57:14Z main - Insecure HTTPS:        false
2024-11-27T13:57:14Z main - Analytics:             enabled
2024-11-27T13:57:14Z main - Authorization:
2024-11-27T13:57:14Z main - Interaction:           d1548136-5367-452a-ae7a-d8d1746833c2
2024-11-27T13:57:14Z main - Features:
2024-11-27T13:57:14Z main -   preview:             enabled
2024-11-27T13:57:14Z main -   fips:                Not available
2024-11-27T13:57:14Z main - Checks:
2024-11-27T13:57:14Z main -   Configuration:       all good

@thisislawatts thisislawatts force-pushed the CLI-568/include-region-alias-in-output branch from 927ee05 to af90306 Compare November 27, 2024 13:59
@thisislawatts thisislawatts marked this pull request as ready for review November 27, 2024 14:00
@thisislawatts thisislawatts requested a review from a team as a code owner November 27, 2024 14:00
@thisislawatts thisislawatts force-pushed the CLI-568/include-region-alias-in-output branch from af90306 to 0cd9208 Compare November 27, 2024 14:05
@@ -109,6 +110,12 @@ func writeLogHeader(config configuration.Configuration, networkAccess networking
tablePrint("Version", cliv2.GetFullVersion()+" "+buildType)
tablePrint("Platform", userAgent)
tablePrint("API", config.GetString(configuration.API_URL))

region, err := localworkflows.DetermineRegionFromUrl(config.GetString(configuration.API_URL))
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: I know we handle the error in GAF, but in general I'm not sure I'm convinced of this pattern of assuming errors are handled in the dependencies.

I would suggest:

if err != nil {
...
} else {
...
}

but it's a minor point

@j-luong j-luong force-pushed the CLI-568/include-region-alias-in-output branch from 0cd9208 to 402a8cf Compare November 28, 2024 11:24
@j-luong j-luong merged commit d079554 into main Nov 28, 2024
7 checks passed
@j-luong j-luong deleted the CLI-568/include-region-alias-in-output branch November 28, 2024 12:06
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.

2 participants