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

Alter log level for disabled services #1478

Merged
merged 4 commits into from
Aug 1, 2024
Merged

Conversation

Azanul
Copy link
Collaborator

@Azanul Azanul commented Jul 30, 2024

Problem

Issue #1418

Solution

Fixes #1418

Changes Made

  • GCP services that are disabled will show a warning instead of an error log
  • Fixed bucket size calculation

Checklist

  • Code follows the contributing guidelines
  • Changes have been thoroughly tested
  • Documentation has been updated, if necessary
  • Any dependencies have been added to the project, if necessary

Azanul added 3 commits July 27, 2024 11:54
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
@@ -34,12 +35,12 @@ RegionsLoop:
"projects/" + client.GCPClient.Credentials.ProjectID + "/locations/" + regionName,
).Do()
if err != nil {
if err.Error() == "googleapi: Error 403: Location "+regionName+" is not found or access is unauthorized., forbidden" {
if err.Error() == "googleapi: Error 403: Location "+regionName+" is not found or access is unauthorized., forbidden" || strings.Contains(err.Error(), "SERVICE_DISABLED") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this resource specific or general 403 for all the services  

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure, it was there already so I didn't change it

Copy link
Collaborator

@AvineshTripathi AvineshTripathi left a comment

Choose a reason for hiding this comment

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

I would prefer a generic error handler that checks if the error with provider exist in our list and ignore it(this could stay in provider util- a provider specific handler)

@Azanul
Copy link
Collaborator Author

Azanul commented Jul 31, 2024

I would prefer a generic error handler that checks if the error with provider exist in our list and ignore it(this could stay in provider util- a provider specific handler)

Good suggestion! I thought so too but currently we have only one such error. Will implement it once we have more

Copy link
Collaborator

@AvineshTripathi AvineshTripathi left a comment

Choose a reason for hiding this comment

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

we should create an issue for similar cases for different provider

@Azanul
Copy link
Collaborator Author

Azanul commented Aug 1, 2024

we should create an issue for similar cases for different provider

This service disabled errors are not actual errors was pointed out by a user. If you have knowledge of such cases for any providers please feel free to create respective issues

@Azanul Azanul merged commit 5655ca0 into tailwarden:develop Aug 1, 2024
6 checks passed
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.

Panic within GCP redis calculating costs
2 participants