Skip to content

feat: Updating protos to separate transformation #4018

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

Merged
franciscojavierarceo merged 18 commits into from
Mar 24, 2024
Merged

feat: Updating protos to separate transformation #4018

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8f97cf1
feat: updating protos to separate transformation
franciscojavierarceo Mar 17, 2024
bc981c7
fixed stuff...i think
franciscojavierarceo Mar 17, 2024
326ca4a
updated tests and registry diff function
franciscojavierarceo Mar 18, 2024
0ea16a3
updated base registry
franciscojavierarceo Mar 18, 2024
5121d8d
updated react component
franciscojavierarceo Mar 18, 2024
cbbfcf8
formatted
franciscojavierarceo Mar 18, 2024
01b82c8
updated stream feature view proto
franciscojavierarceo Mar 18, 2024
8712b2e
making the proto changes backwards compatable
franciscojavierarceo Mar 18, 2024
f1e3764
trying to make this backwards compatible
franciscojavierarceo Mar 20, 2024
21c1c35
caught a bug and fixed the linter
franciscojavierarceo Mar 20, 2024
ff57b45
actually linted
franciscojavierarceo Mar 20, 2024
6de6fcc
updated ui component
franciscojavierarceo Mar 20, 2024
1212c12
accidentally commented out fixtures
franciscojavierarceo Mar 20, 2024
2883d1b
Updated
franciscojavierarceo Mar 22, 2024
ea5e559
incrementing protos
franciscojavierarceo Mar 22, 2024
ef0795b
updated tests
franciscojavierarceo Mar 22, 2024
529acac
fixed linting issue and made backwards compatible
franciscojavierarceo Mar 22, 2024
6748fe7
added more tests
franciscojavierarceo Mar 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
incrementing protos
  • Loading branch information
@franciscojavierarceo
franciscojavierarceo committed Mar 22, 2024
commit ea5e559703f499acf68e86e917126894749f11d1
4 changes: 2 additions & 2 deletions protos/feast/core/Transformation.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ message UserDefinedFunctionV2 {
message FeatureTransformationV2 {
// Note this Transformation starts at 5 for backwards compatibility
oneof transformation {
UserDefinedFunctionV2 user_defined_function = 5;
OnDemandSubstraitTransformationV2 on_demand_substrait_transformation = 6;
UserDefinedFunctionV2 user_defined_function = 1;
OnDemandSubstraitTransformationV2 on_demand_substrait_transformation = 2;
}
Copy link
Copy Markdown
Collaborator

@tokoko tokoko Mar 22, 2024

Choose a reason for hiding this comment

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

I think it's a good time to rethink the naming here. My first suggestion was to rename the field (not message type) to on_demand_pandas_transformation instead of user_defined_function. But on second thought, since we are also aiming to reuse this in StreamFeatureViews, I think protos should no longer be called OnDemand... What do you think? I'm thinking of something like this:

UserDefinedFunctionV2 pandas_transformation = 1;
SubstraitTransformationV2 substrait_transformation = 2;

Copy link
Copy Markdown
Member Author

@franciscojavierarceo franciscojavierarceo Mar 22, 2024

Choose a reason for hiding this comment

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

I was planning on doing that in a follow up PR to not add too much complexity here.

}

Expand Down