Skip to content
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

feat: Added docling and pytorch as add on #5089

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

ntkathole
Copy link
Contributor

What this PR does / why we need it:

Adds docling and torch as add on to feast.
Integrating Docling would allow users to efficiently chunk text data within Online Feature Views (ODFVs) during the write process and torch will allow conversion of feature vectors into pytorch tensors.

Which issue(s) this PR fixes:

#5037 #4890

@ntkathole ntkathole requested a review from a team as a code owner February 25, 2025 11:53
@ntkathole ntkathole force-pushed the docling_extra branch 2 times, most recently from a71074f to cbe4e99 Compare February 25, 2025 14:59
@ntkathole ntkathole force-pushed the docling_extra branch 4 times, most recently from d98454b to c6176b1 Compare February 25, 2025 16:48
@ntkathole
Copy link
Contributor Author

It seems this is causing increase in an image size. Do we have large runners available under our github org?

@franciscojavierarceo
Copy link
Member

@ntkathole this is what I see available. Anything here work?

Screenshot 2025-02-25 at 12 28 14 PM

@ntkathole ntkathole force-pushed the docling_extra branch 3 times, most recently from 5239ef1 to 4be9a30 Compare February 26, 2025 05:51
@@ -162,7 +162,7 @@ docker-build: ## Build docker image with the manager.
## Build feast docker image.
.PHONY: feast-ci-dev-docker-img
feast-ci-dev-docker-img:
cd ./../.. && make build-feature-server-dev
cd ./../.. && make build-feature-server-dev-minimal
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@franciscojavierarceo Since we do not have larger runner to run kind + feast with all dependencies, either we need self-hosted runners or we need to use image with minimal dependencies installed in CI.
Currently moving from multicloud/Dockerfile.dev to multicloud/Dockerfile works in CI to run operator e2e test.

Choose a reason for hiding this comment

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

My concern is that we miss potential breaks by using this minimal image. That's the risk here, right?

Cc @lokeshrangineni @tchughesiv @redhatHameed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will be using this image just for operator e2e test, feast functionality will be tested via integration and unit tests with all dependencies installed.
@tchughesiv or someone more familiar with operator work can confirm if it's a big risk in operator testing or better way to handle this.

Copy link
Contributor

Choose a reason for hiding this comment

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

this change needs to be reverted

Copy link
Contributor

@tchughesiv tchughesiv Mar 3, 2025

Choose a reason for hiding this comment

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

@ntkathole @franciscojavierarceo this must remain make build-feature-server-dev

Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
@ntkathole ntkathole force-pushed the docling_extra branch 2 times, most recently from 01d60df to b2e1769 Compare February 27, 2025 06:12
Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
@franciscojavierarceo
Copy link
Member

looks like that worked!

@franciscojavierarceo franciscojavierarceo merged commit 135342b into feast-dev:master Feb 27, 2025
24 checks passed
Comment on lines +530 to +535
build-feature-server-dev-minimal:
docker buildx build \
-t feastdev/feature-server:dev \
-f sdk/python/feast/infra/feature_servers/multicloud/Dockerfile \
--load sdk/python/feast/infra/feature_servers/multicloud

Copy link
Contributor

Choose a reason for hiding this comment

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

this is just a release build... its not a dev image build

Copy link
Contributor

Choose a reason for hiding this comment

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

we'll need to remove this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants