-
Notifications
You must be signed in to change notification settings - Fork 216
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
backup: add GCS proto #514
Conversation
proto/backup.proto
Outdated
// } | ||
// } | ||
// ]` | ||
string acl_json = 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is object ACL, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, with comments.
proto/backup.proto
Outdated
string bucket = 2; | ||
string prefix = 3; | ||
string storage_class = 4; | ||
// acl json: like `[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a serialized json string. It doesn't have to be the format below. So I suggest modifying the comment. And also it's better to add a documentation link to these two fields since many people not knowing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
part of pingcap/br#89
GCS Support (1800 points)
Extend backup protobuf to support GCS