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(Spanner): enable drop db protection #6008

Merged

Conversation

vishwarajanand
Copy link
Contributor

@vishwarajanand vishwarajanand commented Mar 28, 2023

Note: This can only be merged when the gapic changes are added. Sample gapic changes can be viewed here: #6009. Till owlbot PR is merge, tests will fail.

Enables drop db feature in Spanner.
Design doc: go/drop-db-protection-spanner-php

Changes

  1. Adds a update Database functionality
  2. Added UT and integration test for the same

Tests

  1. Integration tests pass on PROD:
➜  Spanner git:(drop-database-protection-changes) ✗ echo $SPANNER_EMULATOR_HOST

➜  Spanner git:(drop-database-protection-changes) ✗ vendor/bin/phpunit -c phpunit-system.xml.dist --filter="testDatabaseDropProtection"
PHPUnit 8.5.33 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 2.85 minutes, Memory: 12.00 MB

OK (1 test, 9 assertions)
➜  Spanner git:(drop-database-protection) ✗
  1. Other tests (UT and Snippet tests pass)

  2. Test script returns expected results:

Code to evaluate Database::updateDatabase(..)

require_once __DIR__ . '/vendor/autoload.php';

// get a db

use Google\Cloud\Core\Exception\FailedPreconditionException;
use Google\Cloud\Spanner\SpannerClient;

$INSTANCE_NAME = 'google-cloud-php-system-tests';
// $dbName = 'gcloud_testing_640a1bb0a4fef';

$keyFilePath = getenv('GOOGLE_CLOUD_PHP_TESTS_KEY_PATH');

$clientConfig = [
  'keyFilePath' => $keyFilePath
];
$client = new SpannerClient($clientConfig);
$instance = $client->instance($INSTANCE_NAME);

// create a db with drop protection
$dbName = uniqid('gcloud_testing');
$op = $instance->createDatabase($dbName, ['enableDropProtection' => true]);
$op->pollUntilComplete();

$database = $instance->database($dbName);

// get value for enableDropProtection
$info = $database->reload();
echo 'enableDropProtection is ' . (string)(int)$info['enableDropProtection'] . ' expecting true' . PHP_EOL;

// update drop protection (enable)
$op = $database->updateDatabase(['enableDropProtection' => true]);
$op->pollUntilComplete();

$info = $database->reload();
echo 'enableDropProtection is ' . (string)(int)$info['enableDropProtection'] . ' expecting true' . PHP_EOL;

// delete should fail
$isException = false;
try{
    $database->drop();
} catch (FailedPreconditionException $ex){
    $isException = true;
    echo "Exception raised while dropping DB as expected.".PHP_EOL;
}
if (!$isException) {
  echo "NO Exception raised while dropping DB, expected an exception.".PHP_EOL;
}
echo 'Database exists check returns ' . (string)(int)$database->exists() . ' expecting true' . PHP_EOL;

// update drop protection (disable)
$op = $database->updateDatabase(['enableDropProtection' => false]);
$op->pollUntilComplete();

$info = $database->reload();
echo 'enableDropProtection is ' . (string)(int)$info['enableDropProtection'] . ' expecting false' . PHP_EOL;

// delete should pass
$database->drop();
echo 'Database exists check returns ' . (string)(int)$database->exists() . ' expecting false' . PHP_EOL;

echo "Test script completed successfully, compare printed vs expected values." . PHP_EOL;

BREAKING_CHANGE_REASON="New api added"

@vishwarajanand vishwarajanand added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 28, 2023
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Mar 28, 2023
@vishwarajanand
Copy link
Contributor Author

By mistake, I pushed the gapic changes too. I will fix it when its time to merge this PR.

@vishwarajanand vishwarajanand force-pushed the drop-database-protection branch from 27f84cc to 3be24cf Compare June 17, 2023 09:37
@vishwarajanand vishwarajanand removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 17, 2023
@vishwarajanand vishwarajanand force-pushed the drop-database-protection branch from 841931b to 2fbff7c Compare June 17, 2023 11:24
@vishwarajanand vishwarajanand marked this pull request as ready for review June 17, 2023 11:27
@vishwarajanand vishwarajanand requested review from a team as code owners June 17, 2023 11:27
@vishwarajanand
Copy link
Contributor Author

I added a BREAKING_CHANGE_REASON still the breaking changes check is failing.

@bshaffer
Copy link
Contributor

bshaffer commented Jun 17, 2023 via email

Spanner/src/Connection/Grpc.php Show resolved Hide resolved
Spanner/src/Database.php Outdated Show resolved Hide resolved
Spanner/src/Database.php Outdated Show resolved Hide resolved
@vishwarajanand vishwarajanand added the release blocking Required feature/issue must be fixed prior to next release. label Jun 19, 2023
Copy link
Contributor

@yash30201 yash30201 left a comment

Choose a reason for hiding this comment

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

LGTM

@vishwarajanand vishwarajanand merged commit d97c721 into googleapis:main Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. breaking change allowed release blocking Required feature/issue must be fixed prior to next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants