Skip to content

Comments

feat: implement centralized logging configuration and integrate into …#37

Closed
kassyray wants to merge 1 commit intomainfrom
feat/log-file-organization
Closed

feat: implement centralized logging configuration and integrate into …#37
kassyray wants to merge 1 commit intomainfrom
feat/log-file-organization

Conversation

@kassyray
Copy link
Contributor

This pull request introduces a centralized logging configuration for the pipeline and refactors the logging throughout scripts/preprocess.py to use the new logger. The update replaces scattered and inconsistent logging and print statements with a uniform logger, improving maintainability and log management.

Centralized logging setup:

  • Added a new module logger_config.py that provides a get_logger function for consistent logger configuration, including log formatting, log level, and rotating file handlers.

Refactoring and standardization in preprocess.py:

  • Replaced all previous logging and print statements with calls to the centralized logger, ensuring consistent log output for info, warning, and error messages. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

Code cleanup:

  • Removed direct imports and configuration of the logging module from preprocess.py, relying instead on the shared logger Log file organization #5

@kassyray
Copy link
Contributor Author

Keeping this as a draft until python orchestration is done.

Thinking we can implement logging across pipeline and add to this PR as shell is refactored into python? Reviewing as we go?

file_handler = RotatingFileHandler(log_file, maxBytes=5*1024*1024, backupCount=5)
console_handler = logging.StreamHandler()

# Create formatters and add them to handlers
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we have more context for debugging

ex: '%(asctime)s - %(levelname)s - %(name)s - %(funcName)s:%(lineno)d - %(message)s'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry @eswarchandravidyasagar I had this as a draft PR because I am not done yet!

logger.setLevel(level)

# Create handlers - stream handler for stdout and rotating file handler for file logging
file_handler = RotatingFileHandler(log_file, maxBytes=5*1024*1024, backupCount=5)
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you think it would be helpful to define encoding format ?like UTF-8 to prevent encoding errors

Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if multiple logger instances called at the same time will it create multiple tiny log files or one continued log file?
is it possible that Timestamped log files might break log continuity

@kassyray
Copy link
Contributor Author

Closing this PR. Irrelevant with refactor.

@kassyray kassyray closed this Oct 30, 2025
@kassyray kassyray deleted the feat/log-file-organization branch November 6, 2025 18:53
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.

2 participants