Skip to content

Commit

Permalink
Update API response based on requested changes in code reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
mpacer committed Jun 29, 2019
1 parent 7f98456 commit be804ab
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 22 deletions.
4 changes: 2 additions & 2 deletions bookstore/bookstore_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 != ""]
Expand All @@ -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
7 changes: 3 additions & 4 deletions bookstore/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions bookstore/tests/test_bookstore_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
9 changes: 4 additions & 5 deletions bookstore/tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
8 changes: 3 additions & 5 deletions docs/source/bookstore_api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,24 +200,22 @@ components:
- bookstore_valid
- archive_valid
- publish_valid
- cloning_valid
- clone_valid
properties:
bookstore_valid:
type: boolean
archive_valid:
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'

0 comments on commit be804ab

Please sign in to comment.