Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds validation methods to the driver #493

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Adds validation methods to the driver #493

merged 1 commit into from
Oct 30, 2023

Conversation

elijahbenizzy
Copy link
Collaborator

Changes

  • validate_materialization
  • validate_execution

How I tested this

  • Unit tests

Notes

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

@elijahbenizzy elijahbenizzy requested a review from skrawcz October 23, 2023 21:44
@sweep-ai
Copy link
Contributor

sweep-ai bot commented Oct 23, 2023

Apply Sweep Rules to your PR?

  • Apply: Leftover TODOs in the code should be handled.
  • Apply: All new business logic should have corresponding unit tests in the tests/ directory.
  • Apply: Any clearly inefficient or repeated code should be optimized or refactored.

@elijahbenizzy
Copy link
Collaborator Author

See #484 for more motivation

skrawcz
skrawcz reviewed Oct 25, 2023
View reviewed changes
hamilton/driver.py Outdated Show resolved Hide resolved
skrawcz
skrawcz reviewed Oct 25, 2023
View reviewed changes
hamilton/driver.py Outdated Show resolved Hide resolved
@elijahbenizzy
Adds validation methods to the driver
4daa3b4 
This is so you can test it out before you run it. We have
validate_materialization, and validate_execution.

See #484 for more.

There could probably be a slight refactor to drop repeated code, but for
now this is OK.

Included is a validation fix so that it catches the edge-case
effectively of a referenced node not existing in the DAG. This is
grouped together as it is a logically similar change.
@elijahbenizzy elijahbenizzy merged commit 045104f into main Oct 30, 2023
@elijahbenizzy elijahbenizzy deleted the validate-execution branch October 30, 2023 17:48
@elijahbenizzy elijahbenizzy mentioned this pull request Nov 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skrawcz skrawcz skrawcz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@elijahbenizzy @skrawcz