Skip to content

Handle blank database URLs#28

Merged
mixxorz merged 1 commit into
mainfrom
fix/handle-blank-urls
Jul 9, 2025
Merged

Handle blank database URLs#28
mixxorz merged 1 commit into
mainfrom
fix/handle-blank-urls

Conversation

@mixxorz

@mixxorz mixxorz commented Jul 9, 2025

Copy link
Copy Markdown
Owner

Fixes #27

@mixxorz mixxorz requested a review from Copilot July 9, 2025 01:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that invoking the CLI without specifying a database URL fails fast, surfaces a clear error, and exits with a non-zero status.

  • Adds validation in config.initialize to raise an error if url is blank
  • Updates the CLI entrypoint to catch that error, print a message, and sys.exit(1)
  • Introduces test_no_url and adjusts the existing preference-order test in tests/test_cli.py

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/test_cli.py Added test_no_url and updated call count assertion in the settings-preference test
dslr/config.py Added a blank-URL check that raises ValueError
dslr/cli.py Catches the blank-URL error, prints it in red, and exits 1
Comments suppressed due to low confidence (2)

dslr/config.py:26

  • Define and raise a more specific exception (e.g. MissingDatabaseUrlError) instead of the generic ValueError, so consumers can distinguish missing-URL errors from other configuration issues.
        if not self.url:

tests/test_cli.py:261

  • [nitpick] Consider adding a unit test directly against config.initialize(url="", debug=False) to verify it raises the expected exception, improving coverage of the validation logic.
    def test_no_url(self, mock_get_snapshots):

Comment thread dslr/cli.py
@mixxorz mixxorz merged commit 6b690a9 into main Jul 9, 2025
5 checks passed
@mixxorz mixxorz deleted the fix/handle-blank-urls branch July 9, 2025 01:21
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.

Zero Length Delimeter Error

2 participants