Skip to content

Refactor: Migrate Solver Execution Logging to Python logging #107

Closed
parthdagia05 wants to merge 2 commits intoEAPD-DRB:mainfrom
parthdagia05:pr-15
Closed

Refactor: Migrate Solver Execution Logging to Python logging #107
parthdagia05 wants to merge 2 commits intoEAPD-DRB:mainfrom
parthdagia05:pr-15

Conversation

@parthdagia05
Copy link
Copy Markdown

@parthdagia05 parthdagia05 commented Feb 28, 2026

Closes #14

Overview

This PR replaces legacy print() statements within the DataFileClass.run() method with Python’s native logging module.

As the project evolves toward cross-platform support and more complex solver workflows, transitioning from unformatted stdout to structured logging is essential for professional debugging, maintainability, and future-proofing.


Changes

1. Structured Logging in DataFileClass

  • Informational Logs: Replaced timing and progress print() calls with logger.info().
  • Enhanced Error Handling: Replaced standard print(ex) with logger.error(..., exc_info=True). This captures the full stack trace in the server logs while maintaining a clean output for the end-user.
  • Contextual Metadata: Console logs now automatically include:
    • ISO-8601 Timestamps
    • Module/Logger Name (DataFileClass)
    • Log Severity Level (INFO, ERROR, etc.)

2. Centralized Configuration

Introduced a standardized logging configuration in API/app.py:

logging.basicConfig(
    level=logging.INFO,
    format="%(asctime)s [%(name)s] %(levelname)s: %(message)s"
)

---

@autibet
Copy link
Copy Markdown
Collaborator

autibet commented Mar 2, 2026

You cannot write:

**Closes:** #14 (with a colon and markdown formatting)

That pattern often fails keyword parsing and GitHub is not be able to recognize it's linked to Issue #14.

You must write it simply, no markdown, in its own line:

Closes #14

Check on the right sidebar, under Development that the issue is correctly referenced.

Copy link
Copy Markdown
Collaborator

@autibet autibet left a comment

Choose a reason for hiding this comment

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

🔴 [P1] PR is not mergeable right now, and the conflict zone is safety-critical

GitHub reports PR #107 as:

  • mergeStateStatus: DIRTY
  • mergeable: CONFLICTING

The conflict occurs in DataFileClass.run() around the CBC invocation.

In the PR snapshot, it uses:

subprocess.run('cbc ' + lpfile + ..., shell=True)

(API/Classes/Case/DataFileClass.py, around line 2165 in the PR branch).

On current main, this area has already been updated to use a resolved binary with an argv list (cbc_exec, no shell=True), which is safer and more portable.

Why this matters

Resolving this conflict incorrectly could reintroduce command-invocation regressions or security risks while attempting to address what is otherwise a logging-only change.

Suggested fix

  1. Rebase onto current main first.
  2. Keep the current main solver invocation logic unchanged.
  3. Apply only the logging substitutions on top of that updated implementation.

…to logging

- Replace informational print() calls with logger.info()
- Replace exception print with logger.error(..., exc_info=True)
- Configure logging in app.py using logging.basicConfig()
- Preserve solver execution behavior and UI response

No functional changes introduced.

Refs EAPD-DRB#14
@parthdagia05
Copy link
Copy Markdown
Author

@autibet i have rebased on current main branch

@parthdagia05
Copy link
Copy Markdown
Author

Closing this the v5.5 sync (#422) introduced the new logging setup, which supersedes most of what this PR proposed. Happy to follow up with smaller targeted improvements if any gaps remain.

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.

Add pytest test suite and GitHub Actions CI pipeline

2 participants