chore: Use Go-based implementation as embedded extension to Python SDK #2429
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2429 +/- ##
==========================================
+ Coverage 83.66% 84.73% +1.06%
==========================================
Files 125 126 +1
Lines 10838 10811 -27
==========================================
+ Hits 9068 9161 +93
+ Misses 1770 1650 -120
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
pyalex
force-pushed
the
embedded-go
branch
2 times, most recently
from
1231226
to
301abe6
Compare
go/embedded/online_features.go
Outdated
| if err != nil { | ||
| log.Fatalln(err) | ||
| } | ||
| defer fs.DestructOnlineStore() |
There was a problem hiding this comment.
Isn't this called immediately since we're returning in the next line? What's the value of deferring it?
| schema = pa.schema(entity_fields) | ||
| batch = pa.RecordBatch.from_arrays(entity_arrays, schema=schema) | ||
|
|
||
| out_c_schema = ffi.new("struct ArrowSchema*") | ||
| out_ptr_schema = int(ffi.cast("uintptr_t", out_c_schema)) | ||
|
|
||
| out_c_array = ffi.new("struct ArrowArray*") | ||
| out_ptr_array = int(ffi.cast("uintptr_t", out_c_array)) | ||
|
|
||
| in_c_schema = ffi.new("struct ArrowSchema*") | ||
| in_ptr_schema = int(ffi.cast("uintptr_t", in_c_schema)) | ||
|
|
||
| in_c_array = ffi.new("struct ArrowArray*") | ||
| in_ptr_array = int(ffi.cast("uintptr_t", in_c_array)) | ||
|
|
||
| schema._export_to_c(in_ptr_schema) | ||
| batch._export_to_c(in_ptr_array) |
There was a problem hiding this comment.
Some comments here would be nice
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
|
@achals @felixwang9817 Addressed your comments. |
Makefile
Outdated
| go mod tidy | ||
| go build -o ${ROOT_DIR}/sdk/python/feast/binaries/server github.com/feast-dev/feast/go/cmd/server | ||
| compile-go-lib: install-go-proto-dependencies install-go-ci-dependencies | ||
| python -m install pybindgen |
There was a problem hiding this comment.
should this be python -m pip install pybindgen? And should we pin the version of pybindgen?
There was a problem hiding this comment.
this is embarrassing) fixed
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
go/internal/feast/featurestore.go
Outdated
| currentVector.Values = builder.NewArray() | ||
| } else { | ||
| currentVector.Values = array.NewNull(numRows) | ||
| arrowValues, err := utils.ProtoValuesToArrowArray(protoValues, arrowAllocator, numRows) |
There was a problem hiding this comment.
nit, can we have a different module name instead of utils? Seems like it'll become a kitchen sink..
There was a problem hiding this comment.
renamed into types since it's type utils
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, pyalex The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
|
/lgtm |
What this PR does / why we need it:
Converts Go-based implementation of get online features functionality into library which will be embedded into Python code and called directly from Python SDK (w/o GRPC in the middle). This allows us to also use Python functions as callbacks to call some implementations (like online/offline store connectors) from Go feature server.
Which issue(s) this PR fixes:
Fixes #