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

added offline server remote deployment config and Dockerfile #7

Conversation

redhatHameed
Copy link

What this PR does / why we need it:

Added offline server remote deployment config and Dockerfile to generate the offline server image

Which issue(s) this PR fixes:

Fixes

Signed-off-by: Abdul Hameed <ahameed@redhat.com>
@@ -419,6 +419,14 @@ build-feature-server-java-docker:
-t $(REGISTRY)/feature-server-java:$(VERSION) \
-f java/infra/docker/feature-server/Dockerfile --load .

build-offline-server-docker:
Copy link

@dmartinol dmartinol May 22, 2024

Choose a reason for hiding this comment

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

please invoke this target from build-docker target as well
Note Maybe this target is not needed at all, please see next comment

Choose a reason for hiding this comment

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

IIUC, the python feature server already has a Dockerfile that installs the latest feast from pip and a Dockerfile.dev that instead copies the source code in the docker build context and tries to use the latest codebase.

Both run feast serve as the application command through the Helm chart.
Can't we use the same Docker image(s) and just replace the command to feast serve_offline instead? We propably don't need an app.py launcher if the feast application already includes the same with the serve_offline option (and sorry if I gave you wrong pointers before!).

BTW: I just noticed that the dev docker image is affected by this issue #4135, we must involve @jeremyary to discuss a common solution

Copy link
Author

@redhatHameed redhatHameed May 22, 2024

Choose a reason for hiding this comment

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

we can do that.
if I understand with this idea deploying both servers(online/offline) in a single image within a single pod ? my thoughts was to keep both sever in different pods with it's own image. and Also the offline and online port configuration is different. How the user can configure the port or expose both services externally ?

Copy link

@jeremyary jeremyary May 22, 2024

Choose a reason for hiding this comment

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

In theory we have a fix for the docker version issue, the PR just got stuck in conflict/review limbo.

Copy link
Author

Choose a reason for hiding this comment

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

@jeremyary what is your thoughts on single image/pod for both offline or online store ?

Choose a reason for hiding this comment

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

@tchughesiv what do you think about having a separate docker image for this server that anyway includes the same application as the python server and runs from a different command like feast serve_offline?

Choose a reason for hiding this comment

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

i was just calling that out that config in case it wasn't known. if you also need an offline deployment config, then helm changes are still the way to go.

Choose a reason for hiding this comment

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

So, a single image is sufficient but we need to fix the issue about dev image
And the offline probably needs a helm condition to run the serve_offline instead of the serve command.
Thanks!

Copy link

@tchughesiv tchughesiv May 22, 2024

Choose a reason for hiding this comment

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

in helm, you could also do something like feast_mode: offline instead ... which open the door to future ui support?
e.g.

  # supported modes - online (default), offline, or ui
  feast_mode: offline

Choose a reason for hiding this comment

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

cool, so we can also deploy for free the (undocumented ) registry server that runs as feast serve_registry 😁
Let’s mention this option to Tornike

Copy link
Author

Choose a reason for hiding this comment

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

PR for this change

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

Successfully merging this pull request may close these issues.

4 participants