chore: replace print statements with logging #422
Conversation
chore: replace print with logging for app chore: replace print with logging for config_manager chore: replace print with logging for state_manager chore: replace print with logging for cli_tools chore: replace print with logging for validation_builder chore: replace print with logging for test chore: replace print with logging for clients and combiner chore: replace print with logging
|
/gcbrun |
| @@ -479,9 +480,9 @@ def main(): | |||
| elif args.command == "configs": | |||
| run_validation_configs(args) | |||
| elif args.command == "find-tables": | |||
| print(find_tables_using_string_matching(args)) | |||
| logging.info(find_tables_using_string_matching(args)) | |||
There was a problem hiding this comment.
We will need to set level for logging to 'INFO' for it to display the logging.info messages too. The format is to get the timestamps and the logging level along with the messages.
logging.basicConfig(level=logging.INFO, format='%(asctime)s-%(levelname)s: %(message)s')
@nehanene15 - Could you please validate the format.
There was a problem hiding this comment.
Also, we could add a variable coming out of a config file to for level in - logging.basicConfig(level=logging.INFO, format='%(asctime)s-%(levelname)s: %(message)s'). This will help us change it to different Level without any code changes.
There was a problem hiding this comment.
I'm worried about this change. This is not actually a log, it is the output of the command.
Personally, I use this response to build validations for a whole schema when i know the names will be very similar.
In reality, my usage should be a feature which gets pulled into the validation command. For now, changing this print is actually an API change and we should hold off until the feature for this is built.
There was a problem hiding this comment.
Makes sense! Will hold on for these!
| @@ -181,7 +182,7 @@ def get_all_tables(client, allowed_schemas=None): | |||
| try: | |||
| tables = list_tables(client, schema_name) | |||
| except Exception as e: | |||
| print(f"List Tables Error: {schema_name} -> {e}") | |||
| logging.exception(f"List Tables Error: {schema_name} -> {e}") | |||
| continue | |||
There was a problem hiding this comment.
My understanding is this shouldn't be an exception level. We could use warning or info
There was a problem hiding this comment.
Warning would make sense to me
|
@Sync271 Following up on this PR - We'll need to pull the latest code and make the requested code changes to merge this |
|
@nehanene15 you should probably take this away from me, won't be able to contribute. |
|
This has been merged with PR #543. Thanks! |
closes #419