Timx 530 prep work and s3 client #156
Conversation
Pull Request Test Coverage Report for Build 16684341675Details
💛 - Coveralls |
Why these changes are being introduced: With the addition of dataset/metadata assets in S3, we will need to perform actions like downloading the static DB file, uploading a new one, and deleting append deltas. How this addresses that need: Creates new utility class S3Client that performs these actions. Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-530
ghukill
force-pushed
the
TIMX-530-prep-work-and-s3-client
branch
from
e6eeb72 to
c212b50
Compare
jonavellecuerdo
left a comment
jonavellecuerdo
left a comment
There was a problem hiding this comment.
Just a couple of questions before I take another pass at review!
| logging.basicConfig(level=logging.DEBUG) | ||
| return configure_logger(__name__) | ||
| if not logging.getLogger().handlers: | ||
| logging.basicConfig(level=logging.WARNING) |
There was a problem hiding this comment.
Why does line 37 set the logging level to logging.WARNING only to be set to logging.DEBUG a few lines after? 🤔
There was a problem hiding this comment.
Line 37 sets the WARNING for logging.basicConfig() which is kind of global. But then we set the actual timdex_dataset_api logger instance to DEBUG.
I'll admit, there are probably other ways to do this... but... it's works nicely locally for dev work like so, quite literally from something I was just working on:
import os
from timdex_dataset_api import TIMDEXDatasetMetadata
from timdex_dataset_api.config import configure_dev_logger
configure_dev_logger() # <---------------
tdm = TIMDEXDatasetMetadata(os.environ["TIMDEX_DATASET_LOCATION"])This ensures DEBUG logging from this library, but everything else is pretty quiet.
There was a problem hiding this comment.
Oh! Got it. There's a part of me that thinks logger -> tda_logger but 'tis a non-blocking suggestion!
Purpose and background context
This PR is some small prep work and updates for the upcoming "real" changes. And, the addition of an
S3Clientclass that will be used to download, upload, and delete files in S3.How can a reviewer manually see the effects of these changes?
Nothing very exciting to see at this time!
Includes new or updated dependencies?
YES
Changes expectations for external applications?
NO
What are the relevant tickets?