feat(elt-pipelines): dbt project with electricity_sharepoint transform - #429
Conversation
This is the copied directly from the warehouses/facility_ops/transform directory, omitting the other transforms. They don't currently work because of schema changes between the warehouses and elt-pipelines ingest steps, so will be ported separately, but the electricity_sharepoint -> power_consumption transform works without changes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe pull request adds a Facility Operations dbt project with Trino profiles, SQLFluff rules, dependencies, reusable macros, electricity staging and accelerator models, documentation, and SQL fixtures. ChangesFacility Operations transformation
Possibly related PRs
Suggested labels: Mergeability Score: ⚪ Minimal · up to This PR adds the scoped electricity SharePoint transformation and supporting project files; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@elt-pipelines/facility_ops/transform/macros/parse_utc_timestamp.sql`:
- Around line 7-12: Update trino__parse_utc_timestamp so its precision contract
matches parse_datetime: either restrict time_format to at most millisecond
fractional precision before parsing, or replace parse_datetime with a parser
that preserves timestamp(6) precision. Ensure the cast does not imply recovery
of digits already discarded by parsing.
- Around line 7-15: Update trino__parse_utc_timestamp to parse the combined
date/time with an explicit src_timezone, or use a time-zone-free parser, so
DST-gap and ambiguous fall-back values are resolved in the source zone rather
than the Trino session zone. Preserve microsecond precision by avoiding
parse_datetime when fractional seconds are supported, and add coverage for
spring-forward and fall-back cases.
In `@elt-pipelines/facility_ops/transform/profiles.yml`:
- Around line 24-26: Update the remote LDAP profile’s http_scheme setting to
require https instead of reading DBT_TRINO_HTTP_SCHEME, while leaving the user
and password environment variables unchanged and keeping HTTP configuration
separate from LDAP credentials.
In `@elt-pipelines/facility_ops/transform/requirements/requirements.txt`:
- Around line 22-23: Regenerate both lockfiles,
elt-pipelines/facility_ops/transform/requirements/requirements.txt lines 22-23
and elt-pipelines/facility_ops/transform/requirements/developer.txt lines 24-27,
so Click resolves to 8.3.3 or later instead of the affected versions. Validate
compatibility with the dbt CLI and SQLFluff dependency sets.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 279b9e17-0311-4d44-aa22-13efe5b0c28c
📒 Files selected for processing (33)
elt-pipelines/README.mdelt-pipelines/facility_ops/transform/.gitignoreelt-pipelines/facility_ops/transform/.sqlfluffelt-pipelines/facility_ops/transform/.sqlfluffignoreelt-pipelines/facility_ops/transform/README.mdelt-pipelines/facility_ops/transform/analyses/.gitkeepelt-pipelines/facility_ops/transform/dbt_project.ymlelt-pipelines/facility_ops/transform/macros/create_equipment_category_key.sqlelt-pipelines/facility_ops/transform/macros/normalize_whitespace.sqlelt-pipelines/facility_ops/transform/macros/parse_utc_timestamp.sqlelt-pipelines/facility_ops/transform/models/marts/accelerator/power_consumption.sqlelt-pipelines/facility_ops/transform/models/marts/accelerator/power_consumption.ymlelt-pipelines/facility_ops/transform/models/staging/estates/_estates__sources.ymlelt-pipelines/facility_ops/transform/models/staging/estates/stg_electricity_sharepoint_rdm_data.sqlelt-pipelines/facility_ops/transform/models/staging/estates/stg_electricity_sharepoint_rdm_data.ymlelt-pipelines/facility_ops/transform/package-lock.ymlelt-pipelines/facility_ops/transform/packages.ymlelt-pipelines/facility_ops/transform/profiles.ymlelt-pipelines/facility_ops/transform/requirements/developer.inelt-pipelines/facility_ops/transform/requirements/developer.txtelt-pipelines/facility_ops/transform/requirements/requirements.inelt-pipelines/facility_ops/transform/requirements/requirements.txtelt-pipelines/facility_ops/transform/seeds/.gitkeepelt-pipelines/facility_ops/transform/snapshots/.gitkeepelt-pipelines/facility_ops/transform/tests/.gitkeepelt-pipelines/facility_ops/transform/tests/fixtures/base_opralogweb__additional_columns.sqlelt-pipelines/facility_ops/transform/tests/fixtures/base_opralogweb__chapter_entry.sqlelt-pipelines/facility_ops/transform/tests/fixtures/base_opralogweb__entries.sqlelt-pipelines/facility_ops/transform/tests/fixtures/base_opralogweb__logbook_chapter.sqlelt-pipelines/facility_ops/transform/tests/fixtures/base_opralogweb__logbooks.sqlelt-pipelines/facility_ops/transform/tests/fixtures/base_opralogweb__more_entry_columns.sqlelt-pipelines/facility_ops/transform/tests/fixtures/base_statusdisplay__cycles.sqlelt-pipelines/facility_ops/transform/tests/fixtures/base_statusdisplay__cycles__phases.sql
|
@martyngigg do you have thoughts on any of the rabbit comments? I don't know enough about this to understand the nuances, and they mostly seem somewhat borderline on how to address them. |
I'll take a look at the code related ones. For the dependencies can we remove the requirements/ directory and use the pyproject.toml in the |
These are included transitively from elt-common via elt-pipelines. The dbt-adapater dependency is provided transitively by dbt-trino. For the pinned dependency, it looks like it may have been fixed upstream. If not, I'll look at fixing it when it causes problems.
#429 (comment) It's unlikely we need to handle sub millisecond precision timestamps represented by strings, so this should be fine for now
#429 (comment) Without the tz information during parse_datetime, the tz of the trino session is used. This means that during daylight savings there would be an hour difference applied to any data collected as UTC This does now mean there's a problem in the other direction (if the data was collected during daylight savings), but I guess anyone storing daylight savings times without timezones is unlikely to miss an hour here or there.
|
Sorry for all the individual pushes, I kept forgetting things I hadn't addressed 😓 There's a kind of funny 'warning' at the moment |
No worries!
Happy to live with this for now. |
2 participants
ref #421
Puts in enough of the transform functionality to get
elt run facility_ops electricity_sharepoint --step transformworking.elt-pipelineselectricity_sharepointtransform - the others need changes to work with schema changes