add logging info #73
add logging info #73
Conversation
| try: | ||
| meshify_constructor.add_model_contract() | ||
| logger.success(f"Successfully added contract to model: {model_unique_id}") | ||
| except Exception as e: |
There was a problem hiding this comment.
I am OK with this for now to add debugging but I have 2 comments:
- what do we want the tool to do when there is an error? stop at the first error or skip the error and continue until the end (which is what we are doing with the
try/except) - if possible, we should be specific about the exceptions raised so we can add specific logic/logging:
Exception as eis the generic one but we could try to be more specific. This is something that can be revisited later.
| except Exception as e: | ||
| logger.error(f"Error adding model version to model yml: {e}") |
There was a problem hiding this comment.
Currently, this try/except eats the exception. Am I correct in believe that we want to re-raise the exception after logging?
|
@nicholasyager @b-per made a couple updates yesterday that I think address the question of try/except -- in the except branch, i now include @b-per -- re: more detailed excpetions, i totally agree, but wasn't positive which class was more representative of what the error might be at the level I placed the logs. |
|
@nicholasyager @b-per i am going to merge this as is so I can add logging to PR #63 before I leave to go OOO. I opened #92 to reflect the outstanding discussion points from above! |
resolves #68
@nicholasyager tried my hand at using
logurufor some basic logging messages here. very open to changing how we structure this and whether we use this package at all -- it's pretty straightforward to use, but I have never added logs to a cli app before, so I don't know what best practices to consider.