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

Removing condition in setup mounting #1115

Merged
merged 1 commit into from May 16, 2023

Conversation

Tulsishah
Copy link
Collaborator

@Tulsishah Tulsishah commented May 15, 2023

This will be pull request template

Description

Before if mounting output is throwing more then one line it was throwing an error.
but if I pass --key-file as parameter, in the first line it is fetching absolute path of key file and then throw success message.
but because of this condition it was failing before.

Remove the condition to fix the failure

Link to the issue in case of a bug fix.

Testing details

  1. Manual
  • All the integration tests are working fine for both the flags testBucket and mountedDirectoy.
  • Tested for both foreground and background
    - No permission from key file to mount the bucket (mounting is failing)
    - Without key file and gcloud credentials
    - Panic (mounting is failing)
    - Crash during mounting
    - Normal error while mounting
  1. Unit tests - N/A
  2. Integration tests- All the integration tests are working fine together.

@Tulsishah Tulsishah requested a review from raj-prince May 15, 2023 05:17
@Tulsishah Tulsishah assigned sethiay and unassigned sethiay May 15, 2023
@Tulsishah Tulsishah requested a review from sethiay May 15, 2023 05:17
@Tulsishah Tulsishah changed the title [Don't Review] Removing condition in setup mounting Removing condition in setup mounting May 15, 2023
@raj-prince
Copy link
Collaborator

Can you please check the below behaviour for both foreground and background:

  1. Crash while mounting?
  2. Normal error while mounting?
    Also, could you please go through the mounting workflow to ensure we are handling the error appropriately?

@raj-prince
Copy link
Collaborator

We can also see the history of this change? Why was this condition added?

@Tulsishah
Copy link
Collaborator Author

We can also see the history of this change? Why was this condition added?

Yeah, I already checked that, @lezh added it, and there is no description on why to add that.

@Tulsishah
Copy link
Collaborator Author

Can you please check the below behaviour for both foreground and background:

  1. Crash while mounting?
  2. Normal error while mounting?
    Also, could you please go through the mounting workflow to ensure we are handling the error appropriately?

Tested

@Tulsishah Tulsishah merged commit 91d46bf into GoogleCloudPlatform:master May 16, 2023
4 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.

None yet

3 participants