Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/run-precommit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:

# mainly needed so mypy will have the dependencies it needs
- name: Install elementary
run: pip install -e .
run: pip install -e ".[integrations]"

- name: Install dev requirements
run: pip install -r dev-requirements.txt
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test-warehouse.yml
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,9 @@ jobs:
# using ".[vertica]" would re-resolve dbt-vertica's deps and downgrade
# dbt-core to ~=1.8. Install elementary without the adapter extra.
if [ "${{ inputs.warehouse-type }}" = "vertica" ]; then
pip install "."
pip install ".[integrations]"
else
pip install ".[${{ (inputs.warehouse-type == 'databricks_catalog' && 'databricks') || inputs.warehouse-type }}]"
pip install ".[${{ (inputs.warehouse-type == 'databricks_catalog' && 'databricks') || inputs.warehouse-type }},integrations]"
fi

- name: Write dbt profiles
Expand Down
9 changes: 6 additions & 3 deletions elementary/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
from pathlib import Path
from typing import Optional

import google.auth # type: ignore[import]
from dateutil import tz
from google.auth.exceptions import DefaultCredentialsError # type: ignore[import]

from elementary.exceptions.exceptions import InvalidArgumentsError
from elementary.monitor.alerts.grouping_type import GroupingType
Expand Down Expand Up @@ -265,9 +263,14 @@ def has_gcloud(self):
if self.google_service_account_path:
return True
try:
import google.auth # type: ignore[import]

google.auth.default()
return True
except DefaultCredentialsError:
except ImportError:
return False
except Exception:
# google.auth.exceptions.DefaultCredentialsError or similar
return False
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 has_gcloud catches overly broad Exception, silently swallowing credential errors

The original has_gcloud only caught DefaultCredentialsError ("no credentials found"). The new code catches except Exception at line 272, which silently swallows legitimate errors like TransportError, RefreshError, or PermissionError during credential loading. When a user has configured GCS credentials but encounters a transient or fixable credential issue, has_gcloud will return False instead of propagating the error, causing GCS uploads to be silently skipped. The user will get no indication that their credential configuration is broken — the system will just behave as if GCS isn't configured.

Suggested change
except Exception:
# google.auth.exceptions.DefaultCredentialsError or similar
return False
except ImportError:
return False
except Exception:
# google.auth.exceptions.DefaultCredentialsError or similar
# Log the error so users can diagnose credential issues
logger.debug("Google Cloud credentials check failed", exc_info=True)
return False
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good observation about the broad except Exception. The original code caught only DefaultCredentialsError, but we broadened it to avoid importing the exception class at the top level. Adding a logger.debug here would be a reasonable improvement to avoid silently swallowing credential errors — will defer to the maintainer on whether to include that in this PR or handle separately.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just making sure - we are no longer shadowing exceptions we didn't before? I prefer not to change behavior that we don't have to

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — the original code only caught DefaultCredentialsError, so the except Exception broadens the behavior. I'll narrow it back to catch DefaultCredentialsError specifically (with an ImportError guard for when google.auth itself is missing). This way we don't change existing behavior.


@property
Expand Down
74 changes: 59 additions & 15 deletions elementary/monitor/data_monitoring/report/data_monitoring_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
import webbrowser
from typing import Optional, Tuple

from elementary.clients.azure.client import AzureClient
from elementary.clients.gcs.client import GCSClient
from elementary.clients.s3.client import S3Client
from elementary.clients.slack.client import SlackClient
from elementary.config.config import Config
from elementary.monitor.api.invocations.invocations import InvocationsAPI
from elementary.monitor.api.report.report import ReportAPI
Expand Down Expand Up @@ -43,14 +39,62 @@ def __init__(
config, tracking, force_update_dbt_package, disable_samples, selector_filter
)
self.report_api = ReportAPI(self.internal_dbt_runner)
self.s3_client = S3Client.create_client(self.config, tracking=self.tracking)
self.gcs_client = GCSClient.create_client(self.config, tracking=self.tracking)
self.azure_client = AzureClient.create_client(
self.config, tracking=self.tracking
)
self.slack_client = SlackClient.create_client(
self.config, tracking=self.tracking
)

self.s3_client = None
if self.config.has_s3:
try:
from elementary.clients.s3.client import S3Client

self.s3_client = S3Client.create_client(
self.config, tracking=self.tracking
)
except ImportError:
logger.warning(
"S3 dependencies are not installed. "
"Install them with: pip install 'elementary-data[s3]'"
)

self.gcs_client = None
if self.config.has_gcs:
try:
from elementary.clients.gcs.client import GCSClient

self.gcs_client = GCSClient.create_client(
self.config, tracking=self.tracking
)
except ImportError:
logger.warning(
"GCS dependencies are not installed. "
"Install them with: pip install 'elementary-data[gcs]'"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

self.azure_client = None
if self.config.has_blob:
try:
from elementary.clients.azure.client import AzureClient

self.azure_client = AzureClient.create_client(
self.config, tracking=self.tracking
)
except ImportError:
logger.warning(
"Azure dependencies are not installed. "
"Install them with: pip install 'elementary-data[azure]'"
)

self.slack_client = None
if self.config.has_slack:
try:
from elementary.clients.slack.client import SlackClient

self.slack_client = SlackClient.create_client(
self.config, tracking=self.tracking
)
except ImportError:
logger.warning(
"Slack dependencies are not installed. "
"Install them with: pip install 'elementary-data[slack]'"
)

def generate_report(
self,
Expand Down Expand Up @@ -165,9 +209,9 @@ def _add_report_tracking(
report_data.tracking = dict(
posthog_api_key=self.tracking.POSTHOG_PROJECT_API_KEY,
report_generator_anonymous_user_id=self.tracking.anonymous_user_id,
anonymous_warehouse_id=self.warehouse_info.id
if self.warehouse_info
else None,
anonymous_warehouse_id=(
self.warehouse_info.id if self.warehouse_info else None
),
)

def send_report(
Expand Down
22 changes: 16 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,22 @@ requests = ">=2.28.1,<3.0.0"
beautifulsoup4 = "<5.0.0"
ratelimit = "*"
posthog = "<3.0.0"
boto3 = "<2.0.0"
google-cloud-storage = ">=2.4,<3.2"
"ruamel.yaml" = "<1.0.0"
alive-progress = "<=2.3.1"
slack-sdk = ">=3.20.1,<4.0.0"

pydantic = "<3.0"
networkx = ">=2.3,<3"
packaging = ">=20.9"
azure-storage-blob = ">=12.11.0"
pymsteams = ">=0.2.2,<1.0.0"
tabulate = ">= 0.9.0"
tenacity = ">=8.0,<10.0"
pytz = ">= 2025.1"

boto3 = {version = "<2.0.0", optional = true}
google-cloud-storage = {version = ">=2.4,<3.2", optional = true}
slack-sdk = {version = ">=3.20.1,<4.0.0", optional = true}
azure-storage-blob = {version = ">=12.11.0", optional = true}
pymsteams = {version = ">=0.2.2,<1.0.0", optional = true}

dbt-snowflake = {version = ">=1.8,<2.0.0", optional = true}
dbt-bigquery = {version = ">=1.8,<2.0.0", optional = true}
dbt-redshift = {version = ">=1.8,<2.0.0", optional = true}
Expand All @@ -60,6 +61,7 @@ dbt-fabric = {version = ">=1.8,<2.0.0", optional = true}
dbt-fabricspark = {version = ">=1.8,<2.0.0", optional = true}
dbt-sqlserver = {version = ">=1.8,<2.0.0", optional = true}
dbt-vertica = {version = ">=1.8,<2.0.0", optional = true}

[tool.poetry.extras]
snowflake = ["dbt-snowflake"]
bigquery = ["dbt-bigquery"]
Expand All @@ -76,10 +78,18 @@ fabric = ["dbt-fabric"]
fabricspark = ["dbt-fabricspark"]
sqlserver = ["dbt-sqlserver"]
vertica = ["dbt-vertica"]

s3 = ["boto3"]
Comment thread
haritamar marked this conversation as resolved.
Outdated
gcs = ["google-cloud-storage"]
azure = ["azure-storage-blob"]
slack = ["slack-sdk"]
teams = ["pymsteams"]
integrations = ["boto3", "google-cloud-storage", "azure-storage-blob", "slack-sdk", "pymsteams"]

# dbt-fabricspark is excluded due to broken upstream dependencies (azure-cli pre-release pins).
# dbt-vertica is excluded because it pins dbt-core==1.8.5, forcing the entire resolution to dbt 1.8.
# Both are still available as individual extras (e.g. pip install elementary-data[vertica]).
all = ["dbt-snowflake", "dbt-bigquery", "dbt-redshift", "dbt-postgres", "dbt-databricks", "dbt-spark", "dbt-athena-community", "dbt-trino", "dbt-clickhouse", "dbt-duckdb", "dbt-dremio", "dbt-fabric", "dbt-sqlserver"]
all = ["dbt-snowflake", "dbt-bigquery", "dbt-redshift", "dbt-postgres", "dbt-databricks", "dbt-spark", "dbt-athena-community", "dbt-trino", "dbt-clickhouse", "dbt-duckdb", "dbt-dremio", "dbt-fabric", "dbt-sqlserver", "boto3", "google-cloud-storage", "azure-storage-blob", "slack-sdk", "pymsteams"]

[build-system]
requires = ["poetry-core>=1.0.0"]
Expand Down
Loading