gouline / dbt-metabase

dbt + Metabase integration
https://pypi.org/project/dbt-metabase/
MIT License
442 stars 63 forks source link

Fix the docstring of DBTInterface because schema seems not ignored by the manifest parser #173

Closed moreaupascal56 closed 10 months ago

moreaupascal56 commented 10 months ago

Hi @gouline,

Just a small issue because we were misleading by these doctrings:

class DbtInterface:
    """Interface for interacting with dbt and preparing a validated parser object."""

    _parser: Optional[Union[DbtManifestReader, DbtFolderReader]] = None

    def __init__(
        self,
        database: str,
        manifest_path: Optional[str] = None,
        path: Optional[str] = None,
        schema: Optional[str] = None,
        schema_excludes: Optional[Iterable] = None,
        includes: Optional[Iterable] = None,
        excludes: Optional[Iterable] = None,
    ):
        """Constructor.

        Args:
            database (str): Target database name as specified in dbt models to be actioned.
            manifest_path (Optional[str], optional): Path to dbt manifest.json file (typically located in the /target/ directory of the dbt project). Defaults to None.
            path (Optional[str], optional): Path to dbt project. If specified with manifest_path, then the manifest is prioritized. Defaults to None.
            schema (Optional[str], optional): Target schema. Should be passed if using folder parser. Defaults to None.
            schema_excludes (Optional[Iterable], optional): Target schemas to exclude. Ignored in folder parser. Defaults to None.
            includes (Optional[Iterable], optional): Model names to limit processing to. Defaults to None.
            excludes (Optional[Iterable], optional): Model names to exclude. Defaults to None.
        """

(from here)

Especially the documentation of schema and schema_excludes:

            schema (Optional[str], optional): Target schema. Should be passed if using folder parser. Defaults to None.
            schema_excludes (Optional[Iterable], optional): Target schemas to exclude. Ignored in folder parser. Defaults to None.

Because it says that the manifest parser ignores the schema attribute, but it seems from the dbt_manifest.py file that it is not ignored:

            if self.schema and model_schema != self.schema:
                logger().debug(
                    "Skipping %s in schema %s not in target schema %s",
                    model_name,
                    model_schema,
                    self.schema,
                )
                continue

I think just a fix in the docstring is enough or using only one of schema/schema_excludes in both parsers.

We can do a PR.

have a great day :)

gouline commented 10 months ago

Docstring says excluded from the folder parser, you're referencing the manifest parser. I'm not seeing the issue here?