Skip to content

Conversation

@ttngu207
Copy link
Contributor

No description provided.

@ttngu207 ttngu207 changed the title add more logging for populate() [DRAFT] add more logging for populate() May 27, 2022
@guzman-raphael guzman-raphael marked this pull request as draft May 27, 2022 20:07
jobs.complete(self.target.table_name, self._job_key(key))
else:
logger.info("Populating: " + str(key))
logger.info("Start populating TABLE: {} - KEY: {}".format(self.target.table_name, key))
Copy link
Member

@dimitri-yatsenko dimitri-yatsenko May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.info("Start populating TABLE: {} - KEY: {}".format(self.target.table_name, key))
logger.info(f"Making {self.target.table_name}: {key}")

exception=error.__class__.__name__,
msg=": " + str(error) if str(error) else "",
)
logger.info("Error in populating TABLE: {} - KEY: {}\n\t Error Message: {}".format(self.target.table_name, key, error_message))
Copy link
Member

@dimitri-yatsenko dimitri-yatsenko May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.info("Error in populating TABLE: {} - KEY: {}\n\t Error Message: {}".format(self.target.table_name, key, error_message))
logger.info(f"Error making {self.target.table_name}: {key} - {error_message}")

return key, error if return_exception_objects else error_message
else:
self.connection.commit_transaction()
logger.info("Successful in populating TABLE: {} - KEY: {}".format(self.target.table_name, key))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.info("Successful in populating TABLE: {} - KEY: {}".format(self.target.table_name, key))
logger.info(f"Success populating {self.target.table_name}: {key}")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to log so much?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to facilitate the "notification" mechanism, e.g. having clear and distinct log messages so that it can be parsed easily by some custom loghandler and then fed into some notification scheme (e.g. send emails, slack messages, etc.)

Hence I purposely have the log message being a bit wordy but hopefully distinctive - Start populating TABLE, Error in populating TABLE, etc. - so that it is easy to parse the log later.

Making... or Error making... may just be too generic.

@guzman-raphael
Copy link
Collaborator

guzman-raphael commented Jun 13, 2022

These logs should be debug so that that the default behavior is not too verbose. Additionally, agree with @dimitri-yatsenko's point on making them a bit more concise. However, to help with parsing/automation, we can use the making [key] -> [table] - [error] syntax to help us indicate these events; it is currently not being used anywhere else. So in summary:

Making {key} -> {self.target.table_name} 
Error making {key} -> {self.target.table_name} - {error_message}
Success making {key} -> {self.target.table_name}

Also, since this is superseded by #1031, closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants