Skip to content

Fix/exporter path - #67

Merged
kaya70875 merged 5 commits into
mainfrom
fix/exporter-path
Jun 21, 2026
Merged

Fix/exporter path#67
kaya70875 merged 5 commits into
mainfrom
fix/exporter-path

Conversation

@kaya70875

Copy link
Copy Markdown
Owner

No description provided.

@kaya70875 kaya70875 self-assigned this Jun 21, 2026
@kaya70875 kaya70875 added bug Something isn't working enhancement New feature or request labels Jun 21, 2026
@what-the-diff

what-the-diff Bot commented Jun 21, 2026

Copy link
Copy Markdown

PR Summary

  • Improved Documentation of Changes
    A record of the newly added features and corrections has been updated in the 'ChangeLog.md'. For instance, the 'BaseExporter' feature has been enhanced to create a directory path for exporters if it doesn't exist, rather than raising an error. Also, a bug that was causing the 'Exporter' class to stumble upon non-locatable output directories has been fixed.

  • More Descriptive Exception Class Name
    The name of the exception class 'OutputDirectoryNotFoundError' has been changed to 'OutputDirectoryCannotBeCreated' within 'ytfetcher/exceptions.py'. This revision more accurately reflects its functionality.

  • Updated Exception Handling
    The handling of exceptions in 'ytfetcher/services/exports.py' has been updated. It now employs the newly named 'OutputDirectoryCannotBeCreated' exception. In addition, unnecessary checks for the existence of the output directory have been removed to streamline the process.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Create missing exporter output directory and rename creation error
🐞 Bug fix 📝 Documentation 🕐 10-20 Minutes

Grey Divider

Description

• Create the exporter output directory on demand instead of raising when it’s missing.
• Rename the exporter path error to reflect directory creation failures.
• Document the behavior change and fix in the changelog.
Diagram

graph TD
  A["CLI / Library call"] --> B["TXT/JSON/CSV Exporter"] --> C["BaseExporter"] --> D[("Filesystem")]
  C --> E["OutputDirectoryCannotBeCreated"]
  D --> E
Loading
High-Level Assessment

The approach is appropriate: creating the output directory at write-time (via mkdir(parents=True, exist_ok=True)) matches user expectations and limits failures to true OS/path issues. A custom pre-check in init was considered but would reintroduce the original failure mode and duplicate filesystem logic.

Files changed (3) +7 / -7

Bug fix (1) +2 / -5
exports.pyCreate missing output directory and raise a clearer creation error +2/-5

Create missing output directory and raise a clearer creation error

• Stops raising an error when the output directory does not already exist. Ensures the directory is created via mkdir(parents=True, exist_ok=True) and raises OutputDirectoryCannotBeCreated when directory initialization fails due to an OSError.

ytfetcher/services/exports.py

Refactor (1) +2 / -2
exceptions.pyRename exporter output-directory exception for clarity +2/-2

Rename exporter output-directory exception for clarity

• Renames the exporter exception from OutputDirectoryNotFoundError to OutputDirectoryCannotBeCreated and updates its docstring to reflect the real failure condition.

ytfetcher/exceptions.py

Documentation (1) +3 / -0
CHANGELOG.mdDocument exporter output-dir auto-creation and related fix +3/-0

Document exporter output-dir auto-creation and related fix

• Adds a changelog entry noting that BaseExporter now creates the output directory instead of raising. Records the fix for errors raised when an output directory was missing.

CHANGELOG.md

@qodo-code-review

qodo-code-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Exception rename breaks imports 🐞 Bug ≡ Correctness
Description
The PR removes/renames the public exception OutputDirectoryNotFoundError to
OutputDirectoryCannotBeCreated without a compatibility alias, but existing code still imports the
old name (tests currently do), causing ImportError and a failing test suite. This is a breaking
API change for any downstream callers that catch/import the old exception name.
Code

ytfetcher/exceptions.py[R11-14]

+class OutputDirectoryCannotBeCreated(ExporterError):
    """
-    Raised when the specified output directory does not exist.
+    Raised when the specified output directory cannot be created.
    """
Evidence
OutputDirectoryNotFoundError no longer exists (renamed in exceptions module), while tests still
import and expect it, which will fail at import time before tests even run.

ytfetcher/exceptions.py[6-15]
tests/exports/test_exporter.py[1-34]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`OutputDirectoryNotFoundError` was renamed/removed, but internal tests (and potentially external users) still import it, causing `ImportError` and breaking callers.

## Issue Context
Exporter behavior changed to create directories, so tests expecting an exception for a non-existent relative directory should be updated to assert directory creation; a separate test should cover the failure case (e.g., output_dir points to an existing file or a non-writable location).

## Fix Focus Areas
- ytfetcher/exceptions.py[6-16]
- tests/exports/test_exporter.py[1-40]
- ytfetcher/services/exports.py[1-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Exporter docs mismatch exception ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
BaseExporter documentation still claims it raises OutputDirectoryNotFoundError, but the
implementation now raises OutputDirectoryCannotBeCreated, making the documented API contract
incorrect. This can lead to callers catching the wrong exception type and missing the new failure
mode.
Code

ytfetcher/services/exports.py[R55-58]

            return output_path
        except OSError:
            logger.exception("Failed to initialize output directory %s", self.output_dir)
-            raise
+            raise OutputDirectoryCannotBeCreated(f"Output directory {self.output_dir} could not be created")
Evidence
The docstring still references the removed exception name, while _initialize_output_path now
raises the new exception type.

ytfetcher/services/exports.py[18-35]
ytfetcher/services/exports.py[49-59]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `BaseExporter` docstring documents an exception that no longer exists (`OutputDirectoryNotFoundError`) and does not mention the new one (`OutputDirectoryCannotBeCreated`).

## Issue Context
The exporter now creates directories during `_initialize_output_path()` (called by `write()`), and failures are surfaced via `OutputDirectoryCannotBeCreated`.

## Fix Focus Areas
- ytfetcher/services/exports.py[18-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Missing exception chaining ✓ Resolved 🐞 Bug ◔ Observability
Description
_initialize_output_path catches OSError and raises OutputDirectoryCannotBeCreated without
chaining (raise ... from e), which removes the original exception cause from the propagated
exception object. While logger.exception logs the traceback, callers/tests lose structured access
to the underlying error via __cause__.
Code

ytfetcher/services/exports.py[R56-58]

        except OSError:
            logger.exception("Failed to initialize output directory %s", self.output_dir)
-            raise
+            raise OutputDirectoryCannotBeCreated(f"Output directory {self.output_dir} could not be created")
Evidence
The code catches OSError but re-raises a new exception without from e, so the new exception will
not carry the original OSError as its cause.

ytfetcher/services/exports.py[49-59]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The raised `OutputDirectoryCannotBeCreated` does not preserve the original `OSError` as the chained cause.

## Issue Context
Keeping exception chaining improves debuggability for API consumers and test assertions while retaining the nicer domain-specific exception.

## Fix Focus Areas
- ytfetcher/services/exports.py[49-59]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread ytfetcher/exceptions.py
Comment on lines +11 to 14
class OutputDirectoryCannotBeCreated(ExporterError):
"""
Raised when the specified output directory does not exist.
Raised when the specified output directory cannot be created.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Exception rename breaks imports 🐞 Bug ≡ Correctness

The PR removes/renames the public exception OutputDirectoryNotFoundError to
OutputDirectoryCannotBeCreated without a compatibility alias, but existing code still imports the
old name (tests currently do), causing ImportError and a failing test suite. This is a breaking
API change for any downstream callers that catch/import the old exception name.
Agent Prompt
## Issue description
`OutputDirectoryNotFoundError` was renamed/removed, but internal tests (and potentially external users) still import it, causing `ImportError` and breaking callers.

## Issue Context
Exporter behavior changed to create directories, so tests expecting an exception for a non-existent relative directory should be updated to assert directory creation; a separate test should cover the failure case (e.g., output_dir points to an existing file or a non-writable location).

## Fix Focus Areas
- ytfetcher/exceptions.py[6-16]
- tests/exports/test_exporter.py[1-40]
- ytfetcher/services/exports.py[1-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@kaya70875
kaya70875 merged commit 5a43ba6 into main Jun 21, 2026
2 checks passed
@kaya70875
kaya70875 deleted the fix/exporter-path branch June 21, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant