-
Notifications
You must be signed in to change notification settings - Fork 85
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: allow multiple KMS keys to create CMEK database/backup #1191
base: main
Are you sure you want to change the base?
Conversation
Here is the summary of changes. You are about to add 4 region tags.
This comment is generated by snippet-bot.
|
I am a googler following self-serve guide. |
@hwin16 The handwritten changes are not needed since it is a admin feature. |
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
Thank you @harshachinta! PTAL. |
@hwin16 Can you please fix the lint by running |
@harshachinta , I have fixed the linting issue. I fixed some issues with integration tests but wasn't able to run them. Our feature is adding repeated field to proto and similar to existing unit test "create_database_with_encryption_key". When I run existing unit test, that is failing without any changes. Looks like instance set up is incorrect in integration test? |
@hwin16
|
@harshachinta , done fixing integration tests and they are passing now. Currently, skipped them before feature becomes public. |
@pytest.mark.dependency(name="create_backup_with_multiple_kms_keys") | ||
def test_create_backup_with_multiple_kms_keys( | ||
capsys, | ||
multi_region_instance, |
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.
Do we have a fixture for this multi_region_instance?
Where is this instance getting create? I am not able to find in this code
LGTM with couple of questions. Once the backend is ready we can merge this PR. |
Add
kms_key_names
field to create database/create backup code snippets.