You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Bug: UserWarning when passing FeatureService object to both apply() and get_online_features() - Caused by difference in FeatureViewProjection.from_proto() and FeatureViewProjection.from_definition() #4030
If a FeatureService object is created and is being passed to both the apply() and the get_online_features() method, the following user warning is thrown:
UserWarning: The FeatureService object that has been passed in as an argument is inconsistent with the version from the registry. Potentially a newer version of the FeatureService has been applied to the registry.
This is caused by a bug in the creation/retrieval of feature_view_projections, which is an attribute of FeatureService. An empty string is set to the name_alias value of FeatureViewProjection when calling from_proto. However, when creating the FeatureViewProjection by creating a FeatureService object (using the default value) name_alias is set to None, because it is created with FeatureViewProjection.from_definition().
The warning is raised here, because the feature_service_from_registry has the empty string value for the underlying FeatureViewProjection.name_alias object, while the for _features the underlying FeatureViewProjection.name_alias is valued None.
Expected Behavior
I would expect that if a FeatureViewProjection encapsulated within a FeatureService has a None value for the name_alias attribute when being stored in the feature store, that it would also load with the same None value when calling FeatureViewProjection.from_proto().
Then the comparison should also not fail, and the warning should not be thrown.
Current Behavior
The FeatureViewProjection is loaded with an empty string for the value name_alias when FeatureViewProjection.from_proto() is called.
This causes the. comparison to fail, and the warning to be thrown.
I have little to no experience with proto, so I'm not sure whether it would be possible to load the None value directly from the proto definition.
One potential solution would be to check whether an empty string is loaded for this field when loading from proto, and then set it to None in the from_proto function.
changed the title [-]Bug: UserWarning when passing `FeatureService` object to both `apply()` and `get_online_features() - Caused by difference in `FeatureViewProjection.from_proto()` and `FeatureViewProjection.from_definition()`[/-][+]Bug: UserWarning when passing `FeatureService` object to both `apply()` and `get_online_features()` - Caused by difference in `FeatureViewProjection.from_proto()` and `FeatureViewProjection.from_definition()`[/+]on Mar 22, 2024
Tracked down the PR from where the mysterious or "" originates #2321 Still no idea why that was added, though. Normally, round-trip to and from proto should not change the object (at least as far as == operator is concerned), so it does look like a bug.
I see there might be some changes required for the proto definition as well. I'm however not very familiar with this, and I'm unfortunately not able to look into this right now.. If someone else has the knowledge and time to pick this up soon that would be highly appreciated 🙏
@job-almekinders my initial though was that simply removing or "" from to_proto method would be sufficient, but I may be wrong... I'll take a stab at it later if no one picks it up
Activity
[-]Bug: UserWarning when passing `FeatureService` object to both `apply()` and `get_online_features() - Caused by difference in `FeatureViewProjection.from_proto()` and `FeatureViewProjection.from_definition()`[/-][+]Bug: UserWarning when passing `FeatureService` object to both `apply()` and `get_online_features()` - Caused by difference in `FeatureViewProjection.from_proto()` and `FeatureViewProjection.from_definition()`[/+]tokoko commentedon Mar 22, 2024
Tracked down the PR from where the mysterious
or ""originates #2321 Still no idea why that was added, though. Normally, round-trip to and from proto should not change the object (at least as far as==operator is concerned), so it does look like a bug.job-almekinders commentedon Mar 25, 2024
Thanks for your reply.
I'll try to open a PR for this :)
job-almekinders commentedon Mar 25, 2024
I see there might be some changes required for the
protodefinition as well. I'm however not very familiar with this, and I'm unfortunately not able to look into this right now.. If someone else has the knowledge and time to pick this up soon that would be highly appreciated 🙏tokoko commentedon Mar 25, 2024
@job-almekinders my initial though was that simply removing
or ""from to_proto method would be sufficient, but I may be wrong... I'll take a stab at it later if no one picks it up