refactor(logging): Add optional logging support throughout codebase
Add centralized logging utility and optional logger parameters to all core functions for better observability and debugging capabilities. New modules: - utils/logging.py: Centralized logger configuration with console and optional file handlers Enhanced features: - Added optional logger parameter to all extractor_core functions - Added logger support to extractor, excel_converter, and auth modules - Functions remain silent when logger=None (backward compatible) - Improved environment variable validation in test files Documentation: - Added discrete_material_plan_extractor_core.md with complete API reference and usage patterns Benefits: - Consistent logging format across all components - Optional debug output for troubleshooting - No breaking changes - fully backward compatible - Better error messages and validation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -9,7 +9,9 @@ Caller is responsible for browser/session lifecycle management.
|
||||
import pandas as pd
|
||||
from pathlib import Path
|
||||
from typing import List, Optional, Tuple
|
||||
from playwright.sync_api import Page, Frame
|
||||
from playwright.sync_api import Page, Frame, FrameLocator
|
||||
import logging
|
||||
from utils.logging import get_logger
|
||||
|
||||
|
||||
def chunk_order_ids(order_ids: List[str], batch_size: int) -> List[List[str]]:
|
||||
@@ -51,11 +53,12 @@ def get_login_url(base_url: str) -> str:
|
||||
|
||||
|
||||
def extract_batch(
|
||||
work_frame: Frame,
|
||||
work_frame: FrameLocator,
|
||||
page: Page,
|
||||
order_ids: List[str],
|
||||
batch_index: int,
|
||||
download_dir: str,
|
||||
logger: Optional[logging.Logger] = None,
|
||||
) -> str:
|
||||
"""
|
||||
Execute download workflow for a single batch of order IDs.
|
||||
@@ -69,6 +72,7 @@ def extract_batch(
|
||||
order_ids: List of order IDs for this batch
|
||||
batch_index: Zero-based batch index for naming the output file
|
||||
download_dir: Directory path to save the downloaded file
|
||||
logger: Optional logger for debug output (silent if None)
|
||||
|
||||
Returns:
|
||||
Full path to the downloaded Excel file
|
||||
@@ -81,15 +85,17 @@ def extract_batch(
|
||||
order_ids=order_ids,
|
||||
batch_index=batch_index,
|
||||
download_dir=download_dir,
|
||||
logger=logger,
|
||||
)
|
||||
|
||||
|
||||
def extract_batches(
|
||||
work_frame: Frame,
|
||||
work_frame: FrameLocator,
|
||||
page: Page,
|
||||
order_ids: List[str],
|
||||
download_dir: str,
|
||||
batch_size: int = 10,
|
||||
logger: Optional[logging.Logger] = None,
|
||||
) -> List[str]:
|
||||
"""
|
||||
Download data for multiple batches of order IDs.
|
||||
@@ -105,6 +111,7 @@ def extract_batches(
|
||||
order_ids: List of order IDs to download
|
||||
download_dir: Directory path to save downloaded files
|
||||
batch_size: Maximum number of order IDs per batch
|
||||
logger: Optional logger for debug output (silent if None)
|
||||
|
||||
Returns:
|
||||
List of paths to downloaded Excel files
|
||||
@@ -124,7 +131,7 @@ def extract_batches(
|
||||
chunks = chunk_order_ids(order_ids, batch_size)
|
||||
|
||||
# Setup query interface once
|
||||
setup_query_interface(work_frame)
|
||||
setup_query_interface(work_frame, logger)
|
||||
|
||||
# Process each batch
|
||||
for batch_index, batch in enumerate(chunks):
|
||||
@@ -134,6 +141,7 @@ def extract_batches(
|
||||
order_ids=batch,
|
||||
batch_index=batch_index,
|
||||
download_dir=download_dir,
|
||||
logger=logger,
|
||||
)
|
||||
downloaded_files.append(file_path)
|
||||
|
||||
@@ -145,6 +153,7 @@ def post_process_downloads(
|
||||
output_file: str,
|
||||
verbose: bool = True,
|
||||
cleanup_temp_files: bool = True,
|
||||
logger: Optional[logging.Logger] = None,
|
||||
) -> Tuple[str, pd.DataFrame]:
|
||||
"""
|
||||
Convert and merge downloaded Excel files into structured DataFrame.
|
||||
@@ -154,8 +163,9 @@ def post_process_downloads(
|
||||
Args:
|
||||
downloaded_files: List of paths to downloaded Excel files
|
||||
output_file: Path to save merged Excel result
|
||||
verbose: Whether to print progress messages
|
||||
verbose: Whether to print progress messages (deprecated, use logger instead)
|
||||
cleanup_temp_files: Whether to delete temporary downloaded files after processing (default: True)
|
||||
logger: Optional logger for progress output. If None and verbose=True, creates default logger.
|
||||
|
||||
Returns:
|
||||
Tuple of (output_file_path, merged_dataframe)
|
||||
@@ -170,13 +180,26 @@ def post_process_downloads(
|
||||
"""
|
||||
from .excel_converter import ExcelConverter
|
||||
|
||||
converter = ExcelConverter(verbose=verbose)
|
||||
# Create default logger if needed
|
||||
if logger is None and verbose:
|
||||
logger = logging.getLogger('bipauto.extractor.post_process')
|
||||
logger.setLevel(logging.INFO)
|
||||
if not logger.handlers:
|
||||
handler = logging.StreamHandler()
|
||||
handler.setFormatter(logging.Formatter("[%(levelname)s] %(name)s: %(message)s"))
|
||||
logger.addHandler(handler)
|
||||
logger.propagate = False
|
||||
elif logger is None:
|
||||
# Silent mode
|
||||
logger = logging.getLogger('bipauto.extractor.post_process.silent')
|
||||
logger.setLevel(logging.CRITICAL + 1)
|
||||
|
||||
converter = ExcelConverter(verbose=verbose, logger=logger)
|
||||
all_dfs = []
|
||||
|
||||
# Convert each file
|
||||
for i, file_path in enumerate(downloaded_files):
|
||||
if verbose:
|
||||
print(f"Converting file {i + 1}/{len(downloaded_files)}: {file_path}")
|
||||
logger.info(f"Converting file {i + 1}/{len(downloaded_files)}: {file_path}")
|
||||
|
||||
# Convert (do not save intermediate result)
|
||||
df = converter.convert(input_file=file_path)
|
||||
@@ -193,43 +216,55 @@ def post_process_downloads(
|
||||
output_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
merged_df.to_excel(output_path, index=False)
|
||||
|
||||
if verbose:
|
||||
print(f"Merged result saved to: {output_path}")
|
||||
print(f"Total rows: {len(merged_df)}")
|
||||
logger.info(f"Merged result saved to: {output_path}")
|
||||
logger.info(f"Total rows: {len(merged_df)}")
|
||||
|
||||
# Cleanup temporary downloaded files
|
||||
if cleanup_temp_files:
|
||||
_cleanup_temp_files(downloaded_files, verbose)
|
||||
_cleanup_temp_files(downloaded_files, logger=logger)
|
||||
|
||||
return str(output_path), merged_df
|
||||
|
||||
|
||||
def _cleanup_temp_files(downloaded_files: List[str], verbose: bool = True) -> int:
|
||||
def _cleanup_temp_files(downloaded_files: List[str], logger: Optional[logging.Logger] = None, verbose: bool = True) -> int:
|
||||
"""
|
||||
Remove temporary downloaded files.
|
||||
|
||||
Args:
|
||||
downloaded_files: List of file paths to delete
|
||||
verbose: Whether to print progress messages
|
||||
logger: Optional logger for progress output. If None and verbose=True, creates default logger.
|
||||
verbose: Whether to print progress messages (deprecated, use logger instead)
|
||||
|
||||
Returns:
|
||||
Number of files successfully deleted
|
||||
"""
|
||||
# Create default logger if needed
|
||||
if logger is None and verbose:
|
||||
logger = logging.getLogger('bipauto.extractor.cleanup')
|
||||
logger.setLevel(logging.INFO)
|
||||
if not logger.handlers:
|
||||
handler = logging.StreamHandler()
|
||||
handler.setFormatter(logging.Formatter("[%(levelname)s] %(name)s: %(message)s"))
|
||||
logger.addHandler(handler)
|
||||
logger.propagate = False
|
||||
elif logger is None:
|
||||
# Silent mode
|
||||
logger = logging.getLogger('bipauto.extractor.cleanup.silent')
|
||||
logger.setLevel(logging.CRITICAL + 1)
|
||||
|
||||
deleted_count = 0
|
||||
for file_path in downloaded_files:
|
||||
try:
|
||||
Path(file_path).unlink()
|
||||
deleted_count += 1
|
||||
if verbose:
|
||||
print(f"Deleted temp file: {file_path}")
|
||||
logger.debug(f"Deleted temp file: {file_path}")
|
||||
except Exception as e:
|
||||
if verbose:
|
||||
print(f"Warning: Could not delete {file_path}: {e}")
|
||||
logger.warning(f"Warning: Could not delete {file_path}: {e}")
|
||||
return deleted_count
|
||||
|
||||
|
||||
def extract_and_post_process(
|
||||
work_frame: Frame,
|
||||
work_frame: FrameLocator,
|
||||
page: Page,
|
||||
order_ids: List[str],
|
||||
download_dir: str,
|
||||
@@ -237,6 +272,7 @@ def extract_and_post_process(
|
||||
batch_size: int = 10,
|
||||
verbose: bool = True,
|
||||
cleanup_temp_files: bool = True,
|
||||
logger: Optional[logging.Logger] = None,
|
||||
) -> Tuple[str, pd.DataFrame]:
|
||||
"""
|
||||
Complete extraction workflow: download batches + post-process to merged Excel.
|
||||
@@ -251,8 +287,9 @@ def extract_and_post_process(
|
||||
download_dir: Directory for temporary batch files
|
||||
output_file: Path for final merged Excel output
|
||||
batch_size: Maximum order IDs per batch
|
||||
verbose: Whether to print progress messages
|
||||
verbose: Whether to print progress messages (deprecated, use logger instead)
|
||||
cleanup_temp_files: Whether to delete temporary downloaded files after processing (default: True)
|
||||
logger: Optional logger for debug output. If None and verbose=True, creates default logger.
|
||||
|
||||
Returns:
|
||||
Tuple of (output_file_path, merged_dataframe)
|
||||
@@ -268,9 +305,22 @@ def extract_and_post_process(
|
||||
>>> context.close()
|
||||
>>> browser.close()
|
||||
"""
|
||||
# Create default logger if needed
|
||||
if logger is None and verbose:
|
||||
logger = logging.getLogger('bipauto.extractor')
|
||||
logger.setLevel(logging.INFO)
|
||||
if not logger.handlers:
|
||||
handler = logging.StreamHandler()
|
||||
handler.setFormatter(logging.Formatter("[%(levelname)s] %(name)s: %(message)s"))
|
||||
logger.addHandler(handler)
|
||||
logger.propagate = False
|
||||
elif logger is None:
|
||||
# Silent mode
|
||||
logger = logging.getLogger('bipauto.extractor.silent')
|
||||
logger.setLevel(logging.CRITICAL + 1)
|
||||
|
||||
# Step 1: Download all batches
|
||||
if verbose:
|
||||
print(f"Downloading {len(order_ids)} orders in batches of {batch_size}...")
|
||||
logger.info(f"Downloading {len(order_ids)} orders in batches of {batch_size}...")
|
||||
|
||||
downloaded_files = extract_batches(
|
||||
work_frame=work_frame,
|
||||
@@ -278,10 +328,10 @@ def extract_and_post_process(
|
||||
order_ids=order_ids,
|
||||
download_dir=download_dir,
|
||||
batch_size=batch_size,
|
||||
logger=logger,
|
||||
)
|
||||
|
||||
if verbose:
|
||||
print(f"Downloaded {len(downloaded_files)} batch file(s)")
|
||||
logger.info(f"Downloaded {len(downloaded_files)} batch file(s)")
|
||||
|
||||
# Step 2: Post-process (convert + merge)
|
||||
output_path, merged_df = post_process_downloads(
|
||||
@@ -289,6 +339,7 @@ def extract_and_post_process(
|
||||
output_file=output_file,
|
||||
verbose=verbose,
|
||||
cleanup_temp_files=cleanup_temp_files,
|
||||
logger=logger,
|
||||
)
|
||||
|
||||
return output_path, merged_df
|
||||
@@ -321,13 +372,14 @@ def read_order_ids_from_file(id_file: str, encoding: str = "utf-8") -> List[str]
|
||||
|
||||
def extract_from_file(
|
||||
id_file: str,
|
||||
work_frame: Frame,
|
||||
work_frame: FrameLocator,
|
||||
page: Page,
|
||||
download_dir: str,
|
||||
output_file: str,
|
||||
batch_size: int = 10,
|
||||
verbose: bool = True,
|
||||
cleanup_temp_files: bool = True,
|
||||
logger: Optional[logging.Logger] = None,
|
||||
) -> Tuple[str, pd.DataFrame]:
|
||||
"""
|
||||
Extract data from order IDs in a file and post-process to merged Excel.
|
||||
@@ -341,8 +393,9 @@ def extract_from_file(
|
||||
download_dir: Directory for temporary batch files
|
||||
output_file: Path for final merged Excel output
|
||||
batch_size: Maximum order IDs per batch
|
||||
verbose: Whether to print progress messages
|
||||
verbose: Whether to print progress messages (deprecated, use logger instead)
|
||||
cleanup_temp_files: Whether to delete temporary downloaded files after processing (default: True)
|
||||
logger: Optional logger for debug output. If None and verbose=True, creates default logger.
|
||||
|
||||
Returns:
|
||||
Tuple of (output_file_path, merged_dataframe)
|
||||
@@ -358,8 +411,22 @@ def extract_from_file(
|
||||
"""
|
||||
order_ids = read_order_ids_from_file(id_file)
|
||||
|
||||
if verbose:
|
||||
print(f"Loaded {len(order_ids)} order IDs from {id_file}")
|
||||
# Create default logger if needed (for this function's own logging)
|
||||
func_logger = logger
|
||||
if func_logger is None and verbose:
|
||||
func_logger = logging.getLogger('bipauto.extractor.file')
|
||||
func_logger.setLevel(logging.INFO)
|
||||
if not func_logger.handlers:
|
||||
handler = logging.StreamHandler()
|
||||
handler.setFormatter(logging.Formatter("[%(levelname)s] %(name)s: %(message)s"))
|
||||
func_logger.addHandler(handler)
|
||||
func_logger.propagate = False
|
||||
elif func_logger is None:
|
||||
# Silent mode
|
||||
func_logger = logging.getLogger('bipauto.extractor.file.silent')
|
||||
func_logger.setLevel(logging.CRITICAL + 1)
|
||||
|
||||
func_logger.info(f"Loaded {len(order_ids)} order IDs from {id_file}")
|
||||
|
||||
return extract_and_post_process(
|
||||
work_frame=work_frame,
|
||||
@@ -370,4 +437,5 @@ def extract_from_file(
|
||||
batch_size=batch_size,
|
||||
verbose=verbose,
|
||||
cleanup_temp_files=cleanup_temp_files,
|
||||
logger=logger,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user