search

pulumi / pulumi-azure-native

Azure Native Provider
Apache License 2.0
125 stars 33 forks source link

Improve auth CLI missing error #3349

Closed danielrbradley closed 3 months ago

danielrbradley commented 3 months ago

Add context for why the CLI is expected and what operation is being attempted.

Fixes https://github.com/pulumi/pulumi-azure-native/issues/3347

This will now print a message like:

Diagnostics:
  azure-native:features:SubscriptionFeatureRegistration (subscriptionFeatureRegistration):
    error: checking az cli version: could not find `az`: exec: "az": executable file not found in $PATH: MSI, OIDC, 
      client secret and client certificate authentication are not configured so we require the AZ CLI for authentication
github-actions[bot] commented 3 months ago

Does the PR have any schema changes?

Looking good! No breaking changes found. No new resources/functions.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 56.82%. Comparing base (2c7da0a) to head (944884d). Report is 1 commits behind head on master.

Files Patch % Lines
provider/pkg/provider/auth.go 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3349 +/- ## ========================================== - Coverage 56.82% 56.82% -0.01% ========================================== Files 66 66 Lines 8082 8083 +1 ========================================== Hits 4593 4593 - Misses 3054 3055 +1 Partials 435 435 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

thomas11 commented 3 months ago

The message is missing a period, semicolon or the like after $PATH. I know the linter says not to use full sentences in errors, but maybe we should here?

danielrbradley commented 3 months ago

The message is missing a period, semicolon or the like after $PATH. I know the linter says not to use full sentences in errors, but maybe we should here?

Oh that was just me mocking up the diagnostics. It actually has a colon between. Updated the example.