Read from s3 #16
Read from s3 #16
Conversation
ditto_web_api/.gitignore
Outdated
| @@ -3,4 +3,4 @@ | |||
| .idea/ | |||
| .pytest_cache/ | |||
| deployment/ | |||
| venv/ | |||
| venv/ | |||
There was a problem hiding this comment.
Please have files end with a new line
|
|
||
| class ListPresentHandler(tornado.web.RequestHandler): | ||
| def initialize(self, data_replication_service): | ||
| self.data_replication_service = data_replication_service |
There was a problem hiding this comment.
Please change to self._data_replication_service (i.e. add underscore)
| @@ -7,3 +9,14 @@ def __init__(self, configuration): | |||
| configuration.s3_access_key, | |||
| configuration.s3_secret_key, | |||
| configuration.s3_use_secure) | |||
| self._buckets = self.s3_client.list_buckets() | |||
There was a problem hiding this comment.
This is probably fine, but in case the buckets change while the service is running please call this inside get_buckets() instead.
Also, could you please add an underscore at the start of s3_client? My mistake!
| self.test_service = DataReplicationService(self.mock_external_data_service, self.mock_internal_data_service, | ||
| self.mock_logger) | ||
|
|
||
| def test_retrieve_objects_returns_all_json_objects(self): |
There was a problem hiding this comment.
Is the name of this method appropriate? I don't see any JSON here.
Also, as in list_present_handler_test.py, please test more scenarios (no objects, multiple objects, etc)
ditto_web_api/DittoWebApi/main.py
Outdated
| @@ -48,5 +48,6 @@ def setup_logger(log_file_location, level): | |||
| app = tornado.web.Application([ | |||
| (r"/", MainHandler, dict(data_replication_service=data_replication_service)), | |||
There was a problem hiding this comment.
This line should change to
(r"/listpresent/", ListPresentHandler, dict(data_replication_service=data_replication_service)),
(you'll need to update the imports too)
We can delete the main_handler.py file at that stage, as it's no longer needed.
| buckets = self._external_data_service.get_buckets() | ||
| objects = self._external_data_service.get_objects(buckets) | ||
| object_dicts = [obj.to_dict for obj in objects] | ||
| self._logger.info("Found {} objects".format(len(objects))) |
There was a problem hiding this comment.
Since we now have the buckets here, could we add the number of them to the log message?
self._logger.info("Found {} objects in {} buckets".format(len(objects), len(buckets))
#3