Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove noisy log from SQL table check #31037

Merged
merged 3 commits into from
May 8, 2023
Merged

Conversation

jon-evergreen
Copy link
Contributor

@jon-evergreen jon-evergreen commented May 3, 2023

A logging statement was added to the _generate_sql_query() function which gets called during __init__() and thus creates logging output every DAG parse. This removes that logging line

A logging statement was added to the `_generate_sql_query()` function which gets called during `__init__()` and thus creates logging output every DAG parse
@@ -622,8 +622,6 @@ def execute(self, context: Context):
self.log.info("All tests have passed")

def _generate_sql_query(self):
self.log.info("Partition clause: %s", self.partition_clause)
Copy link
Member

Choose a reason for hiding this comment

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

logging might be important for someone so I think we should move the statement calling _generate_sql_query outside init if we can or keep it but make it debug, wdyt?

Copy link
Contributor Author

@jon-evergreen jon-evergreen May 3, 2023

Choose a reason for hiding this comment

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

I know I added that logging line while I was debugging some templating issues so it hasn't been around long. The function call itself needs to be in __init__() so the results gets templated correctly.

This is also why I think the log line is redundant and can be safely removed: the templated partition_clause is available via the airflow UI as a rendered field so people can see it (or the lack of it). I could see an argument for logging out the full query in the execute function for a "this is what ran" vs "this is what you can see through the UI for rendered templates", but the line this PR removes doesn't seem worthwhile.

EDIT: clarified where in the airflow UI you would see the partition_clause

Copy link
Member

Choose a reason for hiding this comment

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

Maybe change the level to debug instead? Since you added it for debugging it could be useful again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine, I've amended the log level to debug. I still don't think the line is needed, but at least it won't spam logs this way.

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

Successfully merging this pull request may close these issues.

4 participants