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.

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

Misc todos #183

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Misc todos #183

wants to merge 3 commits into from

Conversation

skrawcz
Copy link
Collaborator

@skrawcz skrawcz commented May 25, 2023

@elijahbenizzy this needs more testing. See #182 .

Previously, a data validator specified the types it was allowed to
process by a single function that accepted a type and outputted a
boolean. Now it specifies those types by returning a list of types. This
is consistent with how we do the same mechanism in data adapters.
Previously we had hardcoded rules that could occasionally clash. This
anonymizes it every time. While this isn't the prettiest way to do it,
its better than implementing state. Next up, we'll be building out
tooling to mark these with tags as related to a cluster of nodes, and
use that to group the DAG.
We had the order switched of a subclass call. This caused it to break in
certain cases. We switched it back and added tests.
@elijahbenizzy elijahbenizzy self-requested a review May 26, 2023 07:33
Copy link
Collaborator Author

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

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

LGTM -- @elijahbenizzy you'll need to approve.

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.

None yet

2 participants