pyproject.toml (1)

81-81: LGTM: Good approach for handling optional dependencies.

The new manyio dependency group and its inclusion in the all group is a clean way to support the new URL loading functionality without forcing all users to install unnecessary dependencies.

Consider using a variable or reference for the pueblo version to avoid having to update it in multiple places (line 77 and 95).

- "pueblo==0.0.11",
+ "pueblo=={PUEBLO_VERSION}",
...
- "pueblo[fileio]==0.0.11",
+ "pueblo[fileio]=={PUEBLO_VERSION}",

Also applies to: 94-96

README.md (1)

58-58: LGTM: CLI documentation is clear and comprehensive.

The documentation clearly explains the default behavior and new URL loading capabilities, along with dependency requirements and filesystem support.

Consider adding a brief note about error handling or behavior when a URL is invalid or unreachable.

Also applies to: 62-69

src/cratedb_about/outline/core.py (2)

33-40: Consider adding more robust error handling.

While the current implementation works and error handling is tested, you might want to add more specific error handling in the read() method to provide user-friendly error messages for different failure scenarios (invalid URLs, network errors, etc.).

def read(self) -> str:
    if self.url is None:
        return self.BUILTIN.read_text()
    from pueblo.io import to_io

+    try:
        with to_io(self.url) as f:
            return f.read()
+    except FileNotFoundError:
+        raise FileNotFoundError(f"Outline file not found: {self.url}")
+    except (ValueError, IOError) as e:
+        raise ValueError(f"Failed to read outline from {self.url}: {str(e)}")

---

41-43: Consider documenting the url parameter.

The load method takes a url parameter, but there's no docstring explaining what formats are supported. It would be helpful to add a docstring explaining that it supports both local file paths and HTTP URLs.

@classmethod
def load(cls, url=None) -> "OutlineDocument":
+    """
+    Load the outline document from the specified URL or the built-in resource.
+    
+    Args:
+        url: Optional URL to load the outline from. Can be a local file path,
+             an HTTP/HTTPS URL, or None to use the built-in resource.
+             
+    Returns:
+        Parsed OutlineDocument instance.
+        
+    Raises:
+        FileNotFoundError: If the URL points to a non-existent file.
+        ValueError: If the URL is invalid or the content cannot be parsed.
+    """
    return OutlineDocument.from_yaml(cls(url=url).read())

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

docs/backlog.md (1)

17-21: Fix bare URLs in markdown

The URLs should be properly formatted according to markdown best practices.

- Outline: Unlock Discourse, auto-select https://community.cratedb.com/raw/1015
- Outline: Unlock HTML resources, auto-convert using the best standalone program.
  => https://www.urltoany.com/url-to-markdown
- Outline: Unlock GitHub projects: https://github.com/mattduck/gh2md
+ Outline: Unlock Discourse, auto-select [https://community.cratedb.com/raw/1015](https://community.cratedb.com/raw/1015)
+ Outline: Unlock HTML resources, auto-convert using the best standalone program.
  => [https://www.urltoany.com/url-to-markdown](https://www.urltoany.com/url-to-markdown)
+ Outline: Unlock GitHub projects: [https://github.com/mattduck/gh2md](https://github.com/mattduck/gh2md)

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

18-18: Bare URL used
null

(MD034, no-bare-urls)

---

20-20: Bare URL used
null

(MD034, no-bare-urls)

---

21-21: Bare URL used
null

(MD034, no-bare-urls)

src/cratedb_about/outline/core.py (1)

33-43: Fix docstring return type for read() method

The docstring incorrectly states the return type for the read() method.

    """
    Load the file from an external URL or the built-in resource.

    Returns:
-        Parsed OutlineDocument instance.
+        String containing the raw YAML content.

    Raises:
        FileNotFoundError: If the URL points to a non-existent file.
        ValueError: If the URL is invalid or the content cannot be parsed.
    """

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/cratedb_about/outline/core.py (4)

18-19: Update example to match implementation: read() is an instance method, not a class method.

In the docstring, the example shows yaml_content = CrateDbKnowledgeOutline.read() which suggests read() is a class method, but it's actually an instance method. This could mislead users.

-        # Get raw YAML content
-        yaml_content = CrateDbKnowledgeOutline.read()
+        # Get raw YAML content
+        yaml_content = CrateDbKnowledgeOutline().read()

---

33-43: Returns type description is accurate but could be more consistent.

The docstring for read() correctly states it returns a string, but the language used differs from load() method's docstring. Consider standardizing the "Returns:" section format across methods.

---

46-47: Consider moving import to module level.

While late imports are a common pattern to avoid import overhead for rarely used modules, the pueblo.io import is a core dependency for this functionality and would be used whenever a URL is provided. Moving it to the top of the file would improve readability.

 import typing as t
 from importlib import resources
+from pueblo.io import to_io
 
 from cratedb_about.outline.model import OutlineDocument
 
 
@dataclasses.dataclass
...
    def read(self) -> str:
        ...
        if self.url is None:
            return self.BUILTIN.read_text()
-        from pueblo.io import to_io

---

57-57: Add type hint for url parameter.

The url parameter in the load() method lacks a type hint, while the class attribute on line 29 has one. For consistency and better type checking, consider adding the same type hint.

-    def load(cls, url=None) -> "OutlineDocument":
+    def load(cls, url: t.Optional[str] = None) -> "OutlineDocument":

tests/test_outline.py (2)

93-95: Use a more stable URL or mock for testing.

The test uses a specific version tag (v0.0.3) in the GitHub URL. This could break if that tag is removed or if the repository structure changes. Consider using a more stable URL or better yet, mock this HTTP request for more reliable testing.

-    outline = CrateDbKnowledgeOutline.load(
-        "https://github.com/crate/about/raw/refs/tags/v0.0.3/src/cratedb_about/outline/cratedb-outline.yaml"
-    )
+    # Option 1: Use a mocked HTTP response
+    with patch('pueblo.io.to_io') as mock_to_io:
+        mock_file = MagicMock()
+        mock_file.read.return_value = (Path("src/cratedb_about/outline/cratedb-outline.yaml")
+                                       .read_text())
+        mock_to_io.return_value.__enter__.return_value = mock_file
+        outline = CrateDbKnowledgeOutline.load("https://example.com/outline.yaml")
+    
+    # Option 2: Use a more stable URL that's less likely to change
+    # outline = CrateDbKnowledgeOutline.load(
+    #     "https://raw.githubusercontent.com/crate/about/main/src/cratedb_about/outline/cratedb-outline.yaml"
+    # )

---

136-138: Include assertion message for clarity on test failure.

When a test fails, a custom assertion message helps identify what went wrong. Add descriptive messages to your assertions to make debugging easier.

-    assert ex.match("Outline file not found")
+    assert ex.match("Outline file not found"), "Expected error message mentioning 'Outline file not found'"

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro