GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix: Bad feature_servers/multicloud Dockerfile by bushwhackr · Pull Request #4138 · feast-dev/feast

Conversation

@bushwhackr

@bushwhackr bushwhackr commented Apr 23, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Multicloud dockerfile is not consistent with the rest of the infra/feature_servers in that it pulls pypi feast package. Whilst others uses the repository code to build feast.

This PR seeks to standardise it so that we are not reliant on the pypi feast package.

NOTE: I am not a user of multicloud feast and do not understand its use case. The Dockerfile I created is based on my observation on what the incumbent was doing.

Which I understood as the following:

  1. Install feast into docker image
  2. Install runtime dependencies like libarrow-dev

I would need clarification on what is the purpose of this Docker image.

Which issue(s) this PR fixes:

#4135, Related: #3953

Fixes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bushwhackr bushwhackr changed the title Fixing building multicloud feature_servers for feast fix: building feature_servers/multicloud Dockerfile Apr 23, 2024
@bushwhackr bushwhackr changed the title fix: building feature_servers/multicloud Dockerfile fix: feature_servers/multicloud Dockerfile Apr 23, 2024
@bushwhackr bushwhackr changed the title fix: feature_servers/multicloud Dockerfile fix: Bad feature_servers/multicloud Dockerfile Apr 23, 2024
@bushwhackr
bushwhackr force-pushed the chester/multicloud-fix branch from 820ef16 to 0e22da4 Compare April 23, 2024 16:13
@bushwhackr

bushwhackr commented Apr 23, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author
$ export VERSION=0.37.1
$ export REGISTRY=docker.io
$ make build-feature-server-docker

[+] Building 95.1s (14/14) FINISHED                                                                                                                                                                                                                                                                          docker:default
 => [internal] load build definition from Dockerfile                                                                                                                                                                                                                                                                   0.0s
 => => transferring dockerfile: 1.44kB                                                                                                                                                                                                                                                                                 0.0s
 => [internal] load metadata for docker.io/library/python:3.9-slim                                                                                                                                                                                                                                                     0.9s
    ...
    ...
 => exporting to image                                                                                                                                                                                                                                                                                                 3.6s
 => => exporting layers                                                                                                                                                                                                                                                                                                3.6s
 => => writing image sha256:7cd5076b1023823d01b98297b06544f2ab52964aa6c690ba6b2e8492a841decf                                                                                                                                                                                                                           0.0s
 => => naming to docker.io/library/feature-server:0.37.1
 
$ docker run -it docker.io/library/feature-server:0.37.1 pip show feast | grep Version
Version: 0.37.1.dev16+g0e22da42.d20240423
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bushwhackr

bushwhackr commented Apr 23, 2024

Copy link
Copy Markdown
Contributor Author

cc @jeremyary

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jeremyary jeremyary added the ok-to-test label Apr 23, 2024
@jeremyary

jeremyary commented Apr 23, 2024

Copy link
Copy Markdown
Collaborator

Thanks! We'll take a look. As far as use case goes (as far as I understand), it's a stretch to imagine an end user utilizing every cloud provider we enable, but this checks the box for ensuring whatever variant of subsets a user plugs in are good to go together.

bushwhackr reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bushwhackr

bushwhackr commented Apr 25, 2024

Copy link
Copy Markdown
Contributor Author

any blockers?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jeremyary

jeremyary commented Apr 30, 2024

Copy link
Copy Markdown
Collaborator

@bushwhackr apologies for taking a bit of time. We've had a backlog to work through after resolving our release process issues, so I'm slowly catching us up on PR's. If you wouldn't mind TAL at currrent conflicts, I'll work to bring in your change for our next release. Thanks again for your time spent here & contributions!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@franciscojavierarceo

franciscojavierarceo commented May 17, 2024

Copy link
Copy Markdown
Member

i believe this is used during our deployment to verify the build

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jeremyary jeremyary mentioned this pull request May 22, 2024
added offline server remote deployment config and Dockerfile RHEcosystemAppEng/feast#7
Closed
@shuchu

shuchu commented May 29, 2024

Copy link
Copy Markdown
Collaborator

I believe the original Dockerfile (with pypi installation) still useful while the users want to test official Feast releases.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

franciscojavierarceo
franciscojavierarceo reviewed May 29, 2024
View reviewed changes
Comment thread Makefile
build-feature-server-dev:
docker buildx build --build-arg VERSION=dev \
-t feastdev/feature-server:dev \
-f sdk/python/feast/infra/feature_servers/multicloud/Dockerfile.dev --load .

@franciscojavierarceo franciscojavierarceo May 29, 2024

Copy link
Copy Markdown
Member

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.

was this used at all for deploying any images? Do you want to ask achal or any of the old maintainers?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bushwhackr bushwhackr May 29, 2024

Copy link
Copy Markdown
Contributor Author

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.

The only difference in the previous Dockerfile vs Dockerfile.dev was the COPY . . instruction.

The old Dockerfile installs only the pypi available feast package, whereas the old Dockerfile.dev installs the pypi + copies the current codebase into the image.

With the changes I made to the Dockerfile, it does not rely on pypi for the feast package and we would only use the current codebase to install feast. Essentially the same as what old Dockerfile.dev was doing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@shuchu

shuchu commented May 29, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

I believe the original Dockerfile (with pypi installation) still useful while the users want to test official Feast releases.

ah, I know the problem. In the document, Development Guide part. The last step of Environment setup is:
docker build -t docker-whale -f ./sdk/python/feast/infra/feature_servers/multicloud/Dockerfile .

As a result, it will call the pypi version....

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bushwhackr added 2 commits May 29, 2024 17:56
@bushwhackr
building feast-dev locally into docker image
68780c5
Signed-off-by: Chester Ong <[email protected]>
@bushwhackr
make build-feature-server-dev can use the base Dockerfile
015da38
Signed-off-by: Chester Ong <[email protected]>
@bushwhackr
bushwhackr force-pushed the chester/multicloud-fix branch from 0e22da4 to 015da38 Compare May 29, 2024 09:57
@bushwhackr
retest
4e4e59c
Signed-off-by: Chester Ong <[email protected]>
@tokoko tokoko closed this Aug 22, 2024
@bushwhackr
bushwhackr deleted the chester/multicloud-fix branch September 3, 2024 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@franciscojavierarceo franciscojavierarceo franciscojavierarceo left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@bushwhackr @jeremyary @franciscojavierarceo @shuchu @tokoko
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.