From c27888abf32a3e098fb650407afb7a2677220821 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Sat, 13 Feb 2016 00:14:04 -0800 Subject: [PATCH] Removing custom dataset ID environment variable. This is no longer needed since the project ID is used by the datastore v1beta3 API. --- CONTRIBUTING.rst | 4 +- gcloud/datastore/client.py | 23 ++----- gcloud/datastore/demo/__init__.py | 4 +- gcloud/datastore/test_client.py | 90 +++++++--------------------- gcloud/environment_vars.py | 6 -- scripts/pylintrc_default | 5 -- system_tests/clear_datastore.py | 4 +- system_tests/datastore.py | 6 +- system_tests/local_test_setup.sample | 1 - system_tests/populate_datastore.py | 4 +- system_tests/run_system_test.py | 11 +--- system_tests/system_test_utils.py | 19 ++---- 12 files changed, 45 insertions(+), 132 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 66fc46d3fb98..c2b509bfc2ff 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -162,8 +162,6 @@ Running System Tests - ``GCLOUD_TESTS_PROJECT_ID``: Developers Console project ID (e.g. bamboo-shift-455). - - ``GCLOUD_TESTS_DATASET_ID``: The name of the dataset your tests connect to. - This is typically the same as ``GCLOUD_TESTS_PROJECT_ID``. - ``GOOGLE_APPLICATION_CREDENTIALS``: The path to a JSON key file; see ``system_tests/app_credentials.json.sample`` as an example. Such a file can be downloaded directly from the developer's console by clicking @@ -195,7 +193,7 @@ Running System Tests # Create the indexes $ gcloud preview datastore create-indexes system_tests/data/index.yaml \ - > --project=$GCLOUD_TESTS_DATASET_ID + > --project=$GCLOUD_TESTS_PROJECT_ID # Restore your environment to its previous state. $ unset CLOUDSDK_PYTHON_SITEPACKAGES diff --git a/gcloud/datastore/client.py b/gcloud/datastore/client.py index 1d97f5c494ff..aefcbafbd3b8 100644 --- a/gcloud/datastore/client.py +++ b/gcloud/datastore/client.py @@ -16,8 +16,7 @@ import os from gcloud._helpers import _LocalStack -from gcloud._helpers import _app_engine_id -from gcloud._helpers import _compute_engine_id +from gcloud._helpers import _determine_default_project as _base_default_project from gcloud.client import Client as _BaseClient from gcloud.datastore import helpers from gcloud.datastore.connection import Connection @@ -27,7 +26,6 @@ from gcloud.datastore.key import Key from gcloud.datastore.query import Query from gcloud.datastore.transaction import Transaction -from gcloud.environment_vars import DATASET from gcloud.environment_vars import GCD_DATASET @@ -35,11 +33,6 @@ """Maximum number of iterations to wait for deferred keys.""" -def _get_production_project(): - """Gets the production application ID if it can be inferred.""" - return os.getenv(DATASET) - - def _get_gcd_project(): """Gets the GCD application ID if it can be inferred.""" return os.getenv(GCD_DATASET) @@ -51,8 +44,8 @@ def _determine_default_project(project=None): In implicit case, supports four environments. In order of precedence, the implicit environments are: - * GCLOUD_DATASET_ID environment variable - * DATASTORE_DATASET environment variable (for ``gcd`` testing) + * DATASTORE_DATASET environment variable (for ``gcd`` / emulator testing) + * GCLOUD_PROJECT environment variable * Google App Engine application ID * Google Compute Engine project ID (from metadata server) @@ -62,17 +55,11 @@ def _determine_default_project(project=None): :rtype: string or ``NoneType`` :returns: Default project if it can be determined. """ - if project is None: - project = _get_production_project() - if project is None: project = _get_gcd_project() if project is None: - project = _app_engine_id() - - if project is None: - project = _compute_engine_id() + project = _base_default_project(project=project) return project @@ -180,7 +167,7 @@ class Client(_BaseClient): def __init__(self, project=None, namespace=None, credentials=None, http=None): - project = _determine_default_project(project) + project = _determine_default_project(project=project) if project is None: raise EnvironmentError('Project could not be inferred.') self.project = project diff --git a/gcloud/datastore/demo/__init__.py b/gcloud/datastore/demo/__init__.py index cf0a84004efc..a49240c57180 100644 --- a/gcloud/datastore/demo/__init__.py +++ b/gcloud/datastore/demo/__init__.py @@ -13,8 +13,8 @@ # limitations under the License. import os -from gcloud.environment_vars import TESTS_DATASET +from gcloud.environment_vars import TESTS_PROJECT __all__ = ['PROJECT'] -PROJECT = os.getenv(TESTS_DATASET) +PROJECT = os.getenv(TESTS_PROJECT) diff --git a/gcloud/datastore/test_client.py b/gcloud/datastore/test_client.py index 35547574c996..02098f284e51 100644 --- a/gcloud/datastore/test_client.py +++ b/gcloud/datastore/test_client.py @@ -31,33 +31,6 @@ def _make_entity_pb(project, kind, integer_id, name=None, str_val=None): return entity_pb -class Test__get_production_project(unittest2.TestCase): - - def _callFUT(self): - from gcloud.datastore.client import _get_production_project - return _get_production_project() - - def test_no_value(self): - import os - from gcloud._testing import _Monkey - - environ = {} - with _Monkey(os, getenv=environ.get): - project = self._callFUT() - self.assertEqual(project, None) - - def test_value_set(self): - import os - from gcloud._testing import _Monkey - from gcloud.datastore.client import DATASET - - MOCK_PROJECT = object() - environ = {DATASET: MOCK_PROJECT} - with _Monkey(os, getenv=environ.get): - project = self._callFUT() - self.assertEqual(project, MOCK_PROJECT) - - class Test__get_gcd_project(unittest2.TestCase): def _callFUT(self): @@ -88,81 +61,58 @@ def test_value_set(self): class Test__determine_default_project(unittest2.TestCase): def _callFUT(self, project=None): - from gcloud.datastore.client import _determine_default_project + from gcloud.datastore.client import ( + _determine_default_project) return _determine_default_project(project=project) - def _determine_default_helper(self, prod=None, gcd=None, gae=None, - gce=None, project=None): + def _determine_default_helper(self, gcd=None, fallback=None, + project_called=None): from gcloud._testing import _Monkey from gcloud.datastore import client _callers = [] - def prod_mock(): - _callers.append('prod_mock') - return prod - def gcd_mock(): _callers.append('gcd_mock') return gcd - def gae_mock(): - _callers.append('gae_mock') - return gae - - def gce_mock(): - _callers.append('gce_mock') - return gce + def fallback_mock(project=None): + _callers.append(('fallback_mock', project)) + return fallback patched_methods = { - '_get_production_project': prod_mock, '_get_gcd_project': gcd_mock, - '_app_engine_id': gae_mock, - '_compute_engine_id': gce_mock, + '_base_default_project': fallback_mock, } with _Monkey(client, **patched_methods): - returned_project = self._callFUT(project) + returned_project = self._callFUT(project_called) return returned_project, _callers def test_no_value(self): project, callers = self._determine_default_helper() self.assertEqual(project, None) - self.assertEqual(callers, - ['prod_mock', 'gcd_mock', 'gae_mock', 'gce_mock']) + self.assertEqual(callers, ['gcd_mock', ('fallback_mock', None)]) def test_explicit(self): PROJECT = object() project, callers = self._determine_default_helper( - project=PROJECT) + project_called=PROJECT) self.assertEqual(project, PROJECT) self.assertEqual(callers, []) - def test_prod(self): - PROJECT = object() - project, callers = self._determine_default_helper(prod=PROJECT) - self.assertEqual(project, PROJECT) - self.assertEqual(callers, ['prod_mock']) - def test_gcd(self): PROJECT = object() project, callers = self._determine_default_helper(gcd=PROJECT) self.assertEqual(project, PROJECT) - self.assertEqual(callers, ['prod_mock', 'gcd_mock']) + self.assertEqual(callers, ['gcd_mock']) - def test_gae(self): + def test_fallback(self): PROJECT = object() - project, callers = self._determine_default_helper(gae=PROJECT) + project, callers = self._determine_default_helper(fallback=PROJECT) self.assertEqual(project, PROJECT) - self.assertEqual(callers, ['prod_mock', 'gcd_mock', 'gae_mock']) - - def test_gce(self): - PROJECT = object() - project, callers = self._determine_default_helper(gce=PROJECT) - self.assertEqual(project, PROJECT) - self.assertEqual(callers, - ['prod_mock', 'gcd_mock', 'gae_mock', 'gce_mock']) + self.assertEqual(callers, ['gcd_mock', ('fallback_mock', None)]) class TestClient(unittest2.TestCase): @@ -195,7 +145,7 @@ def test_ctor_w_project_no_environ(self): # Some environments (e.g. AppVeyor CI) run in GCE, so # this test would fail artificially. - with _Monkey(_MUT, _compute_engine_id=lambda: None): + with _Monkey(_MUT, _base_default_project=lambda project: None): self.assertRaises(EnvironmentError, self._makeOne, None) def test_ctor_w_implicit_inputs(self): @@ -205,10 +155,15 @@ def test_ctor_w_implicit_inputs(self): OTHER = 'other' creds = object() + default_called = [] + + def fallback_mock(project): + default_called.append(project) + return project or OTHER klass = self._getTargetClass() with _Monkey(_MUT, - _determine_default_project=lambda x: x or OTHER): + _determine_default_project=fallback_mock): with _Monkey(_base_client, get_credentials=lambda: creds): client = klass() @@ -219,6 +174,7 @@ def test_ctor_w_implicit_inputs(self): self.assertTrue(client.connection.http is None) self.assertTrue(client.current_batch is None) self.assertTrue(client.current_transaction is None) + self.assertEqual(default_called, [None]) def test_ctor_w_explicit_inputs(self): OTHER = 'other' diff --git a/gcloud/environment_vars.py b/gcloud/environment_vars.py index 7a4b6aaff28a..344ffd3c8785 100644 --- a/gcloud/environment_vars.py +++ b/gcloud/environment_vars.py @@ -24,9 +24,6 @@ TESTS_PROJECT = 'GCLOUD_TESTS_PROJECT_ID' """Environment variable defining project for tests.""" -DATASET = 'GCLOUD_DATASET_ID' -"""Environment variable defining default dataset ID.""" - GCD_DATASET = 'DATASTORE_DATASET' """Environment variable defining default dataset ID under GCD.""" @@ -36,8 +33,5 @@ PUBSUB_EMULATOR = 'PUBSUB_EMULATOR_HOST' """Environment variable defining host for Pub/Sub emulator.""" -TESTS_DATASET = 'GCLOUD_TESTS_DATASET_ID' -"""Environment variable defining dataset ID for tests.""" - CREDENTIALS = 'GOOGLE_APPLICATION_CREDENTIALS' """Environment variable defining location of Google credentials.""" diff --git a/scripts/pylintrc_default b/scripts/pylintrc_default index 79586b614d56..2d47e0b8d8fc 100644 --- a/scripts/pylintrc_default +++ b/scripts/pylintrc_default @@ -74,10 +74,6 @@ load-plugins=pylint.extensions.check_docs # identical implementation but different docstrings. # - star-args: standard Python idioms for varargs: # ancestor = Query().filter(*order_props) -# - method-hidden: Decorating a method in a class (e.g. in _DefaultsContainer) -# @_lazy_property_deco -# def dataset_id(): -# ... # - redefined-variable-type: This error is overzealous and complains at e.g. # if some_prop: # return int(value) @@ -100,7 +96,6 @@ disable = redefined-builtin, similarities, star-args, - method-hidden, redefined-variable-type, wrong-import-position, diff --git a/system_tests/clear_datastore.py b/system_tests/clear_datastore.py index f544dd484cbb..96d214bc464d 100644 --- a/system_tests/clear_datastore.py +++ b/system_tests/clear_datastore.py @@ -21,7 +21,7 @@ from six.moves import input from gcloud import datastore -from gcloud.environment_vars import TESTS_DATASET +from gcloud.environment_vars import TESTS_PROJECT FETCH_MAX = 20 @@ -90,7 +90,7 @@ def remove_kind(kind, client): def remove_all_entities(client=None): if client is None: # Get a client that uses the test dataset. - client = datastore.Client(project=os.getenv(TESTS_DATASET)) + client = datastore.Client(project=os.getenv(TESTS_PROJECT)) for kind in ALL_KINDS: remove_kind(kind, client) diff --git a/system_tests/datastore.py b/system_tests/datastore.py index 34b30dc0f5da..34298adeb76a 100644 --- a/system_tests/datastore.py +++ b/system_tests/datastore.py @@ -19,12 +19,12 @@ import httplib2 import unittest2 +from gcloud import _helpers from gcloud._helpers import UTC from gcloud import datastore -from gcloud.datastore import client as client_mod from gcloud.datastore.helpers import GeoPoint from gcloud.environment_vars import GCD_DATASET -from gcloud.environment_vars import TESTS_DATASET +from gcloud.environment_vars import TESTS_PROJECT from gcloud.exceptions import Conflict # This assumes the command is being run via tox hence the # repository root is the current directory. @@ -56,7 +56,7 @@ def setUpModule(): # Isolated namespace so concurrent test runs don't collide. test_namespace = 'ns%d' % (1000 * time.time(),) if emulator_dataset is None: - client_mod.DATASET = TESTS_DATASET + _helpers.PROJECT = TESTS_PROJECT Config.CLIENT = datastore.Client(namespace=test_namespace) else: credentials = EmulatorCreds() diff --git a/system_tests/local_test_setup.sample b/system_tests/local_test_setup.sample index 4219c962570b..771638c56985 100644 --- a/system_tests/local_test_setup.sample +++ b/system_tests/local_test_setup.sample @@ -1,5 +1,4 @@ export GOOGLE_APPLICATION_CREDENTIALS="app_credentials.json.sample" export GCLOUD_TESTS_PROJECT_ID="my-project" -export GCLOUD_TESTS_DATASET_ID=${GCLOUD_TESTS_PROJECT_ID} export GCLOUD_REMOTE_FOR_LINT="upstream" export GCLOUD_BRANCH_FOR_LINT="master" diff --git a/system_tests/populate_datastore.py b/system_tests/populate_datastore.py index e4be6cfbadad..041471bd0a0c 100644 --- a/system_tests/populate_datastore.py +++ b/system_tests/populate_datastore.py @@ -22,7 +22,7 @@ from six.moves import zip from gcloud import datastore -from gcloud.environment_vars import TESTS_DATASET +from gcloud.environment_vars import TESTS_PROJECT ANCESTOR = ('Book', 'GoT') @@ -91,7 +91,7 @@ def print_func(message): def add_characters(client=None): if client is None: # Get a client that uses the test dataset. - client = datastore.Client(project=os.getenv(TESTS_DATASET)) + client = datastore.Client(project=os.getenv(TESTS_PROJECT)) with client.transaction() as xact: for key_path, character in zip(KEY_PATHS, CHARACTERS): if key_path[-1] != character['name']: diff --git a/system_tests/run_system_test.py b/system_tests/run_system_test.py index 171a0d1c85f2..1b63a078ce08 100644 --- a/system_tests/run_system_test.py +++ b/system_tests/run_system_test.py @@ -25,12 +25,6 @@ from system_tests import system_test_utils -REQUIREMENTS = { - 'datastore': ['dataset_id', 'credentials'], - 'storage': ['project', 'credentials'], - 'pubsub': ['project', 'credentials'], - 'bigquery': ['project', 'credentials'], -} TEST_MODULES = { 'datastore': datastore, 'storage': storage, @@ -43,7 +37,7 @@ def get_parser(): parser = argparse.ArgumentParser( description='GCloud test runner against actual project.') parser.add_argument('--package', dest='package', - choices=REQUIREMENTS.keys(), + choices=TEST_MODULES.keys(), default='datastore', help='Package to be tested.') parser.add_argument( '--ignore-requirements', @@ -55,8 +49,7 @@ def get_parser(): def run_module_tests(module_name, ignore_requirements=False): if not ignore_requirements: # Make sure environ is set before running test. - requirements = REQUIREMENTS[module_name] - system_test_utils.check_environ(*requirements) + system_test_utils.check_environ() suite = unittest2.TestSuite() test_mod = TEST_MODULES[module_name] diff --git a/system_tests/system_test_utils.py b/system_tests/system_test_utils.py index eb7b3b5517bb..d936bd05cd74 100644 --- a/system_tests/system_test_utils.py +++ b/system_tests/system_test_utils.py @@ -17,13 +17,11 @@ import sys from gcloud.environment_vars import CREDENTIALS as TEST_CREDENTIALS -from gcloud.environment_vars import TESTS_DATASET from gcloud.environment_vars import TESTS_PROJECT # From shell environ. May be None. PROJECT_ID = os.getenv(TESTS_PROJECT) -DATASET_ID = os.getenv(TESTS_DATASET) CREDENTIALS = os.getenv(TEST_CREDENTIALS) ENVIRON_ERROR_MSG = """\ @@ -46,21 +44,14 @@ def create_scoped_required(): return False -def check_environ(*requirements): - +def check_environ(): missing = [] - if 'dataset_id' in requirements: - if DATASET_ID is None: - missing.append(TESTS_DATASET) - - if 'project' in requirements: - if PROJECT_ID is None: - missing.append(TESTS_PROJECT) + if PROJECT_ID is None: + missing.append(TESTS_PROJECT) - if 'credentials' in requirements: - if CREDENTIALS is None or not os.path.isfile(CREDENTIALS): - missing.append(TEST_CREDENTIALS) + if CREDENTIALS is None or not os.path.isfile(CREDENTIALS): + missing.append(TEST_CREDENTIALS) if missing: print(ENVIRON_ERROR_MSG % ', '.join(missing), file=sys.stderr)