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

feat(storage): Soft deleted Bucket Restore #28102

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

shubhangi-google
Copy link
Contributor

Add support for restoring soft deleted bucket.

Operation Supported:

  • Get Bucket Generation
  • Get a Soft Deleted Bucket (Also soft-delete time and hard-delete time)
  • List Soft Deleted Buckets
  • Restore a Soft Deleted Bucket
@example
require "google/cloud/storage"
storage = Google::Cloud::Storage.new
bucket = storage.bucket "my-bucket"
bucket.delete
##fetch bucket generation
 generation= bucket.generation
## list soft_deleted buckets
deleted_buckets= storage.buckets soft_deleted: true
## restore bucket
bucket = storage.restore_bucket "my-bucket", generation, soft_deleted: true

@shubhangi-google shubhangi-google marked this pull request as ready for review December 16, 2024 05:11
@generation = @gapi.generation
end

def soft_delete_time
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need these instance variables?

@@ -3144,7 +3158,7 @@ def lazy?

##
# @private New Bucket from a Google API Client object.
def self.from_gapi gapi, service, user_project: nil
def self.from_gapi gapi, service, user_project: nil, generation: nil, soft_deleted: nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering why we need these parameters if we're not using them.

buckets = new(Array(gapi_list.items).map do |gapi_object|
Bucket.from_gapi gapi_object, service, user_project: user_project
Bucket.from_gapi gapi_object, service, user_project: user_project, soft_deleted: soft_deleted
Copy link
Contributor

Choose a reason for hiding this comment

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

qq: Do we not need to set soft_deleted instance variable?

@@ -193,11 +193,20 @@ def add_custom_header header_name, header_value
# puts bucket.name
# end
#
def buckets prefix: nil, token: nil, max: nil, user_project: nil
# @example Retrieve soft deleted
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# @example Retrieve soft deleted
# @example Retrieve soft deleted buckets

#
# storage = Google::Cloud::Storage.new
#
# user_buckets = storage.buckets soft_deleted: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# user_buckets = storage.buckets soft_deleted: true
# soft_deleted_buckets = storage.buckets soft_deleted: true


_(bucket.name).must_equal bucket_name
_(bucket.generation).must_equal generation
_(bucket.gapi.hard_delete_time).wont_be_nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use gapi here? We added new getter methods for soft and hard delete time.

end

def find_bucket_gapi name = nil
Google::Apis::StorageV1::Bucket.from_json random_bucket_hash(name: name).to_json
end

def find_deleted_bucket_gapi name = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can we please make it keyword argument?

def list_buckets_gapi count = 2, token = nil
buckets = count.times.map { Google::Apis::StorageV1::Bucket.from_json random_bucket_hash.to_json }
Google::Apis::StorageV1::Buckets.new(
kind: "storage#buckets", items: buckets, next_page_token: token
)
end

def list_deleted_buckets_gapi count = 2, token = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can we please make these keyword argument?

)
end

def restored_bucket_gapi name, _generation
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need _generation if we're not using it?

def === other
return(to_h === other.to_h) if other.respond_to? :to_h
super
module Google
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are making changes to this block?

Copy link

snippet-bot bot commented Dec 16, 2024

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

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