-
Notifications
You must be signed in to change notification settings - Fork 25
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
Infer consume operation if not present in dataset interface #859
Infer consume operation if not present in dataset interface #859
Conversation
@property | ||
def consumes_additional_properties(self) -> bool: | ||
"""Returns a boolean indicating whether the component consumes additional properties.""" | ||
return self._specification.get("consumes", {}).get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be nice to implement on the spec (same for the produces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean in the OperationSpec
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a better look and never mind. I was lost in the many specs 😛 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mrchtr!
Left some comments, but they're mostly nits. Looks a lot cleaner now!
logger.info(msg) | ||
return None | ||
|
||
# Component has consumes and additionalProperties, we will load all dataset columns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering about the case here where there's both a schema and additionalProperties
. But I guess this is fine for now. We might want to have more logic here once we add support for additionalProperties
schemas instead of just the boolean.
Basic implementation, I still have to add tests. I wanted to get some feedback first.
ComponentSpec
based on the attributes.ComponentSpec
.In cases where a user hasn't specified a
consume
in the pipeline operations, we now infer this. If a component spec contains aconsumes
section andadditionalProperties
are set to true, we load all columns. IfadditionalProperties
is set to false, we limit the columns defined in the component spec.Fix #836