Add logging filename, level and format #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jesusbv
force-pushed
the
add-logging-file
branch
2 times, most recently
from
ca4f4cc
to
1a8a467
Compare
jesusbv
force-pushed
the
add-logging-file
branch
2 times, most recently
from
786caa8
to
afcc177
Compare
jesusbv
force-pushed
the
add-logging-file
branch
from
b8fa005
to
d00e716
Compare
jesusbv
commented
Aug 4, 2023
rjschwei
reviewed
Aug 4, 2023
rtamalin
reviewed
Aug 4, 2023
smarlowucf
reviewed
Aug 4, 2023
smarlowucf
reviewed
Aug 4, 2023
smarlowucf
reviewed
Aug 4, 2023
There was a problem hiding this comment.
No matter where the adapter is deployed we want logging. If there are config options for the logger I think those would be handled in core code where the logger is instantiated.
If there's an additional handler needed by a plugin beyond stream handler I think that's best setup in the plugin setup hook. For VM based adapters it means there's a stream handler and a file based handler. Which seems fine.
Set the logging on setup_adapter and tests
jesusbv
force-pushed
the
add-logging-file
branch
from
614a4ee
to
0d8fd73
Compare
smarlowucf
reviewed
Aug 7, 2023
smarlowucf
reviewed
Aug 8, 2023
jesusbv
force-pushed
the
add-logging-file
branch
from
058a9c4
to
fafef20
Compare
smarlowucf
approved these changes
Aug 9, 2023
rjschwei
requested changes
Aug 16, 2023
csp_billing_adapter_local/plugin.py
Outdated
| @csp_billing_adapter.hookimpl | ||
| def setup_adapter(config: Config): | ||
| log_to_file = logging.FileHandler(CSP_LOG_FILEPATH) | ||
| csp_log = logging.getLogger(LOGGER_NAME) |
There was a problem hiding this comment.
should either be csp_log = log or log = logging.getLogger(LOGGER_NAME) on line 37 should be removed. Why create the module variable at the top level when it is not being used?
Remove get logger call in favor of using log previously declared
rjschwei
approved these changes
Aug 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should be merged after #10