Skip to content

fix: update entity_id handling for custom SQL tests in dbt_logs.py an… - #128

Open
Oluwajuwon-O wants to merge 1 commit into
datakind:mainfrom
Oluwajuwon-O:fix/clean-custom-sql-entity-id-parsing
Open

fix: update entity_id handling for custom SQL tests in dbt_logs.py an…#128
Oluwajuwon-O wants to merge 1 commit into
datakind:mainfrom
Oluwajuwon-O:fix/clean-custom-sql-entity-id-parsing

Conversation

@Oluwajuwon-O

@Oluwajuwon-O Oluwajuwon-O commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Description

When DOT generates physical .sql files for custom_sql tests, it appends a suffix like _id1, _id2, etc., to the filename to prevent collisions. When parsing the DBT logs later, DOT used .split("_id")[0] to strip this suffix and recover the original entity name.
However,

  • It was being applied to all tests, not just custom_sql tests.
  • It truncated any entity name that contained _id anywhere in the string. For example, an entity named person_identifier was truncated to person, leading to a relation does not exist DatabaseError when DOT attempted to query the non-existent person table.

The Solution:

  • Introduced if test_type == "custom_sql": check so it only executes for the relevant tests.
  • Replaced .split("_id")[0] with a regular expression (re.sub(r"_id\d+$", "", ...)) that only strips _id if it is followed by digits at the very end of the string.

Asana Task

Deployment Readiness*

Testing

Describe or check:

  • Created or updated unit, feature, and/or integration tests
  • Typical manual testing in the local env browser, dev pipeline, etc.

Deployment Notes

Describe or check:

  • No special deployment steps required

Rollback Plan

Describe or check:

  • Standard revert is sufficient (git revert)

Reviewer Guidance / Questions*

Screenshots / Testing Evidence*

SOC 2 Change Management Checklist

  • None of the below are true in this code
  • New roles/permissions are introduced without review and approval by the product manager
  • Hardcoded credentials, secrets, or API keys are present in this code
  • Secrets are being managed outside of the approved secrets management process (e.g., GitHub Secrets, environment variables)
  • PII or sensitive data handling is introduced or changed without being reviewed against our data classification policy
  • Sensitive data is written to logs
  • Input validation and sanitization is missing
  • An unnecessary attack surface has been introduced (e.g., unused endpoints, open ports, debug modes left enabled)
  • Common vulnerabilities have been introduced in the code (inc. any dependencies added or updated)
  • No review for common vulnerabilities has been conducted
  • Not tested in a non-production environment
  • Breaking changes to existing APIs or integrations with downstream consumers being notified
  • Performance impact has not been considered or acceptable
  • Appropriate audit logging is missing for any security-relevant actions introduced by this change
  • Log entries contain sensitive or PII data
  • All existing tests do not pass locally (./vendor/bin/pest)

Provide justification if you are submitting a PR with any boxes checked other than the first.


Reminder for Reviewers: By approving this PR you are confirming that you have reviewed the code for correctness, security, and compliance with our engineering and SOC 2 standards. Do not approve PRs where SOC 2 checklist items are checked without documented justification.

*Optional


…d utils.py

- Refactored entity_id processing to use regex for stripping "_idXX" suffix in custom SQL tests.
- Ensured consistent handling of entity_id across relevant functions in both dbt_logs.py and utils.py.
@Oluwajuwon-O
Oluwajuwon-O marked this pull request as ready for review August 26, 2026 15:11
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.

1 participant