Skip to content

Conversation

@juankx-bodo
Copy link
Contributor

No description provided.

@juankx-bodo juankx-bodo force-pushed the jkx/Fix_log_level_with_file_handlers branch from 2df5197 to cd5172f Compare January 20, 2026 05:24
Copy link
Contributor

@hadia206 hadia206 left a comment

Choose a reason for hiding this comment

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

The code looks correct to me.
I'm not familiar with logging so will let Kian or John give approval.

@juankx-bodo juankx-bodo force-pushed the jkx/Fix_log_level_with_file_handlers branch from 1c7fb29 to 18e3acd Compare January 26, 2026 15:58
Copy link
Contributor

@john-sanchez31 john-sanchez31 left a comment

Choose a reason for hiding this comment

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

Looks good. Just one comment below. I'll let the final approval to Kian

@juankx-bodo juankx-bodo marked this pull request as ready for review January 27, 2026 13:13
Copy link
Contributor

@knassre-bodo knassre-bodo left a comment

Choose a reason for hiding this comment

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

One comment, otherwise LGTM

Comment on lines +2246 to +2247
reset_logger("pydough")
reset_logger("pydough.mask_server")
Copy link
Contributor

Choose a reason for hiding this comment

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

If these are the values to be used, then replace __name__ when get_logger is used with these strings in all the places it is currently used (execute_relational.py and mask_server.py)

@juankx-bodo juankx-bodo force-pushed the jkx/Fix_log_level_with_file_handlers branch from d004fb6 to 27e7d70 Compare February 2, 2026 15:52
@juankx-bodo juankx-bodo merged commit 1813cde into main Feb 2, 2026
12 checks passed
@juankx-bodo juankx-bodo deleted the jkx/Fix_log_level_with_file_handlers branch February 2, 2026 18:06
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.

4 participants