-
Notifications
You must be signed in to change notification settings - Fork 44
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
[FSTORE-1008] enable interacting with java client to hopsworks #1110
base: master
Are you sure you want to change the base?
Conversation
@@ -79,35 +79,35 @@ public static List<TrainingDatasetFeature> makeLabelFeatures(QueryBase query, Li | |||
for (Feature feat : (List<Feature>) query.getLeftFeatures()) { | |||
labelWithPrefixToFeature.put(feat.getName(), feat.getName()); | |||
labelWithPrefixToFeatureGroup.put(feat.getName(), | |||
(new FeatureGroupBaseForApi(null, feat.getFeatureGroupId()))); | |||
(new StreamFeatureGroup(null, feat.getFeatureGroupId()))); |
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.
why change it to StreamFeatureGroup
?
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.
FeatureGroupBaseForApi was used as a dummy class for API calls. In hsfs only abstract classes were present and it is not initializable. Since we added now support for StreamFeatureGroup, it can do the job
private FeatureGroupEngine featureGroupEngine; | ||
private FeatureViewEngine featureViewEngine; | ||
|
||
public FeatureStore() { |
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.
General question: what methods should be included in the java client?
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.
Only thing in java client can be done is fetch FG, FV metadata and get feature vectors. I don't see any other use cases
* @throws FeatureStoreException | ||
* @throws IOException | ||
*/ | ||
public abstract void addTag(String name, Object value) throws FeatureStoreException, IOException; |
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.
why removing all the abstract method? Are they not needed for spark/flink etc? same for featurestorebase
and featuregroupbase
.
I think you can implement the abstract method as "not available" in the FeatureView
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.
My point was that who will use addTag
from Flink or Beam? It doesn't make sense. But it is possible do add since only api classes are used
When I was testing it, I found a bug. Can you change line 215 in
Basically, the problem is that when there are multiple primary key, they need to be sorted according to the index of |
This PR adds/fixes/changes...
JIRA Issue: -
Priority for Review: -
Related PRs: -
How Has This Been Tested?
Checklist For The Assigned Reviewer: