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

Mounting succeeds gcsbucket doesnot exist #779

Closed
wants to merge 4 commits into from
Closed

Mounting succeeds gcsbucket doesnot exist #779

wants to merge 4 commits into from

Conversation

Tulsishah
Copy link
Collaborator

Tried to solve an error Mounting succeeds even if gcs call fails.

Copy link
Collaborator

@vadlakondaswetha vadlakondaswetha left a comment

Choose a reason for hiding this comment

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

Please make changes in jacobsa/gcloud repo

if typed, ok := err.(*googleapi.Error); ok {
switch typed.Code {
case http.StatusForbidden:
//if typed, ok := err.(*googleapi.Error); ok {
Copy link
Collaborator

Choose a reason for hiding this comment

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

please dont raise cls with commented code.

//}

if err != nil {
if strings.Contains(err.Error(), "Error 403") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

as discussed please dont use string comparision for error. cast it appropriately like this and compare the status code.
(err.(*fmt.wrapError).err).(*googleapi.Error).Code

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also check if we can cast error as mentioned in the link Prince shared: jacobsa/gcloud#15

@@ -20,15 +20,16 @@ import (
"log"
"net/http"
"net/url"
"strings"
Copy link
Collaborator

Choose a reason for hiding this comment

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

add unit tests

@raj-prince
Copy link
Collaborator

Just for the information, this bug is similar to the issue open in the gcloud codebase: jacobsa/gcloud#15

@Tulsishah
Copy link
Collaborator Author

raise the same PR on jacobsa/gcloud repo.

@Tulsishah Tulsishah closed this Sep 5, 2022
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.

3 participants