From be804ab5e86984ea4cec2e0a2936ef6d141b8652 Mon Sep 17 00:00:00 2001 From: M Pacer Date: Thu, 27 Jun 2019 12:12:57 -0700 Subject: [PATCH] Update API response based on requested changes in code reviews --- bookstore/bookstore_config.py | 4 ++-- bookstore/handlers.py | 7 +++---- bookstore/tests/test_bookstore_config.py | 12 ++++++------ bookstore/tests/test_handlers.py | 9 ++++----- docs/source/bookstore_api.yaml | 8 +++----- 5 files changed, 18 insertions(+), 22 deletions(-) diff --git a/bookstore/bookstore_config.py b/bookstore/bookstore_config.py index d48be2a..0fa3c50 100644 --- a/bookstore/bookstore_config.py +++ b/bookstore/bookstore_config.py @@ -74,7 +74,7 @@ def validate_bookstore(settings: BookstoreSettings): Returns ------- validation_checks : dict - Existence of settings by category (general, archive, publish) + Statements about whether features are validly configured and available """ general_settings = [settings.s3_bucket != "", settings.s3_endpoint_url != ""] archive_settings = [*general_settings, settings.workspace_prefix != ""] @@ -85,6 +85,6 @@ def validate_bookstore(settings: BookstoreSettings): "bookstore_valid": all(general_settings), "archive_valid": all(archive_settings), "publish_valid": all(published_settings), - "cloning_valid": all(cloning_settings), + "clone_valid": all(cloning_settings), } return validation_checks diff --git a/bookstore/handlers.py b/bookstore/handlers.py index 51b373e..4dcb068 100644 --- a/bookstore/handlers.py +++ b/bookstore/handlers.py @@ -40,15 +40,14 @@ def get(self): def build_response_dict(self): """Helper for building the version handler's response before serialization.""" return { - "bookstore": True, # TODO: Should we remove this; isn't it equivalent to the endpoint responding? - "version": self.settings['bookstore']["version"], + "release": self.settings['bookstore']["release"], "validation": self.settings['bookstore']["validation"], } def build_settings_dict(validation): """Helper for building the settings info that will be assigned to the web_app.""" - return {"version": version, "validation": validation} + return {"release": version, "validation": validation} def load_jupyter_server_extension(nb_app): @@ -103,7 +102,7 @@ def collect_handlers(log, base_url, validation): else: log.info("[bookstore] Publishing disabled. s3_bucket or endpoint are not configured.") - if validation['cloning_valid']: + if validation['clone_valid']: log.info(f"[bookstore] Enabling bookstore cloning, version: {version}") handlers.append( (url_path_join(base_bookstore_api_pattern, r"/clone(?:/?)*"), BookstoreCloneAPIHandler) diff --git a/bookstore/tests/test_bookstore_config.py b/bookstore/tests/test_bookstore_config.py index 954959e..a844d98 100644 --- a/bookstore/tests/test_bookstore_config.py +++ b/bookstore/tests/test_bookstore_config.py @@ -10,7 +10,7 @@ def test_validate_bookstore_defaults(): "bookstore_valid": False, "publish_valid": False, "archive_valid": False, - "cloning_valid": True, + "clone_valid": True, } settings = BookstoreSettings() assert validate_bookstore(settings) == expected @@ -22,7 +22,7 @@ def test_validate_bookstore_published(): "bookstore_valid": True, "publish_valid": False, "archive_valid": True, - "cloning_valid": True, + "clone_valid": True, } settings = BookstoreSettings(s3_bucket="A_bucket", published_prefix="") assert validate_bookstore(settings) == expected @@ -34,7 +34,7 @@ def test_validate_bookstore_workspace(): "bookstore_valid": True, "publish_valid": True, "archive_valid": False, - "cloning_valid": True, + "clone_valid": True, } settings = BookstoreSettings(s3_bucket="A_bucket", workspace_prefix="") assert validate_bookstore(settings) == expected @@ -46,7 +46,7 @@ def test_validate_bookstore_endpoint(): "bookstore_valid": False, "publish_valid": False, "archive_valid": False, - "cloning_valid": True, + "clone_valid": True, } settings = BookstoreSettings(s3_endpoint_url="") assert validate_bookstore(settings) == expected @@ -58,7 +58,7 @@ def test_validate_bookstore_bucket(): "bookstore_valid": True, "publish_valid": True, "archive_valid": True, - "cloning_valid": True, + "clone_valid": True, } settings = BookstoreSettings(s3_bucket="A_bucket") assert validate_bookstore(settings) == expected @@ -70,7 +70,7 @@ def test_disable_cloning(): "bookstore_valid": True, "publish_valid": True, "archive_valid": True, - "cloning_valid": False, + "clone_valid": False, } settings = BookstoreSettings(s3_bucket="A_bucket", enable_cloning=False) assert validate_bookstore(settings) == expected diff --git a/bookstore/tests/test_handlers.py b/bookstore/tests/test_handlers.py index 2c37150..5c054e5 100644 --- a/bookstore/tests/test_handlers.py +++ b/bookstore/tests/test_handlers.py @@ -96,9 +96,9 @@ def test_build_settings_dict(bookstore_settings): 'archive_valid': True, 'bookstore_valid': True, 'publish_valid': True, - 'cloning_valid': True, + 'clone_valid': True, }, - 'version': version, + 'release': version, } validation = validate_bookstore(bookstore_settings) assert expected == build_settings_dict(validation) @@ -153,13 +153,12 @@ def test_get(self): def test_build_response(self): empty_handler = self.get_handler('/api/bookstore/') expected = { - 'bookstore': True, 'validation': { 'archive_valid': True, 'bookstore_valid': True, 'publish_valid': True, - 'cloning_valid': True, + 'clone_valid': True, }, - 'version': version, + 'release': version, } assert empty_handler.build_response_dict() == expected diff --git a/docs/source/bookstore_api.yaml b/docs/source/bookstore_api.yaml index 1f55602..ad873ef 100644 --- a/docs/source/bookstore_api.yaml +++ b/docs/source/bookstore_api.yaml @@ -200,7 +200,7 @@ components: - bookstore_valid - archive_valid - publish_valid - - cloning_valid + - clone_valid properties: bookstore_valid: type: boolean @@ -208,16 +208,14 @@ components: type: boolean publish_valid: type: boolean - cloning_valid: + clone_valid: type: boolean VersionInfo: type: object required: - s3path properties: - bookstore: - type: boolean - version: + release: type: string validation: $ref: '#/components/schemas/ValidationInfo'