Skip to content

Make error on retrieval of nonexistent feature humanly readable - #625

Merged
feast-ci-bot merged 8 commits into
feast-dev:masterfrom
mrzzy:invalid-feature-error
Apr 16, 2020
Merged

Make error on retrieval of nonexistent feature humanly readable#625
feast-ci-bot merged 8 commits into
feast-dev:masterfrom
mrzzy:invalid-feature-error

Conversation

@mrzzy

@mrzzy mrzzy commented Apr 15, 2020

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:
Notifies the user with a human readable error when the user attempts to retrieve
(both online/batch) feature data from feast serving with an feature reference does not match any feature stored in feast.

  • currently the error that the user will see is:
InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
	status = StatusCode.UNKNOWN
	details = ""
	debug_error_string = "{"created":"@1580607090.708289999","description":"Error received from peer ipv4:172.28.0.8:6566","file":"src/core/lib/surface/call.cc","file_line":1056,"grpc_message":"","grpc_status":2}"
>
  • this PR replaces the the error with something more user friendly:
grpc.RpcError: Unable to find feature set for feature ref: (project: s, name: s, version: 1, max_age: )

Which issue(s) this PR fixes:

Fixes #454

Does this PR introduce a user-facing change?:

- Make error on retrieval of nonexistent feature humanly readable.

@woop

woop commented Apr 15, 2020

Copy link
Copy Markdown
Member

project: s, name: s, version: 1, max_age:

Max age isn't a part of the feature reference, so I think that should be removed.

woop
woop reviewed Apr 15, 2020
View reviewed changes
Comment thread serving/src/main/java/feast/serving/specs/CachedSpecService.java Outdated
@woop

woop commented Apr 15, 2020

Copy link
Copy Markdown
Member

/ok-to-test

@mrzzy

mrzzy commented Apr 16, 2020

Copy link
Copy Markdown
Collaborator Author

/test test-serving

@mrzzy

mrzzy commented Apr 16, 2020

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end

@mrzzy

mrzzy commented Apr 16, 2020

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end-batch

@mrzzy

mrzzy commented Apr 16, 2020

Copy link
Copy Markdown
Collaborator Author

@woop I have removed the max age from the error message.

@woop

woop commented Apr 16, 2020

Copy link
Copy Markdown
Member

/lgtm
/approve

@feast-ci-bot

feast-ci-bot commented Apr 16, 2020

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrzzy, woop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot
feast-ci-bot merged commit e461cde into feast-dev:master Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@woop woop woop left review comments
@davidheryanto davidheryanto Awaiting requested review from davidheryanto
@khorshuheng khorshuheng Awaiting requested review from khorshuheng
@pradithya pradithya Awaiting requested review from pradithya
@zhilingc zhilingc Awaiting requested review from zhilingc

Assignees

@woop woop

Labels

approved lgtm ok-to-test size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feast Serving should notify users of invalid feature names

3 participants

@mrzzy @woop @feast-ci-bot