[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jenoOvchi
To complete the pull request process, please assign mattdelac after the PR has been reviewed.
You can assign the PR to them by writing /assign @mattdelac in a comment when ready.

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

/assign @MattDelac

Codecov Report

Merging #2572 (2f79430) into master (6589f15) will decrease coverage by 9.49%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2572      +/-   ##
==========================================
- Coverage   80.67%   71.18%   -9.50%     
==========================================
  Files         165      140      -25     
  Lines       13759    12170    -1589     
==========================================
- Hits        11100     8663    -2437     
- Misses       2659     3507     +848     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6589f15...2f79430. Read the comment docs.

Mind signing your commits?

Mind signing your commits?

Yes. Do I need to recreate pull request for this?

nope you can resign in this PR and force push

@jenoOvchi Could the base image be tagged too with the version number?

nope you can resign in this PR and force push

Done

@jenoOvchi Could the base image be tagged too with the version number?

Done

What's the intention behind this btw? Since for local development, yarn start is still definitely the fastest path.

If you want to be deploying this as a service, you'll have to also be regularly syncing with the registry (as is).

Additionally, presumably if you want to use it you also would want to run e.g. minify, etc. e.g. something like

FROM node:17.9.0-slim

WORKDIR /app
# add `/app/node_modules/.bin` to $PATH
ENV PATH /app/node_modules/.bin:$PATH

# Install UI dependencies
COPY ui/package.json .
RUN npm install

# Copy over app
COPY ui/tsconfig.json .
COPY ui/public ./public
COPY ui/src ./src

# Build for production.
RUN npm run build --production

# Serve the UI from a port
RUN npm install -g serve
EXPOSE 3000
CMD ["serve", "-s", "build"]

@adchia I think this would be useful when deploying the Feast UI as a service. I assume that you can configure Feast UI to point to a remote repository.

@adchia I think this would be useful when deploying the Feast UI as a service. I assume that you can configure Feast UI to point to a remote repository.

Yes, I think so too. Now we ingegrates Feast in our platform, as Feature Registry use an S3 Bucket and as workaround make a sync procedure from S3 to Feast UI files directory. But in the future it will be greate to have an option to configure on the UI side connection to the remote repository and use it as a service.

hey sorry just checking in here. looks like commits aren't signed anymore? Otherwise, happy to merge

Hi @jenoOvchi, would you mind signing your commits?

Hi @jenoOvchi, would you mind signing your commits?

Done.

What's the intention behind this btw? Since for local development, yarn start is still definitely the fastest path.

If you want to be deploying this as a service, you'll have to also be regularly syncing with the registry (as is).

Additionally, presumably if you want to use it you also would want to run e.g. minify, etc. e.g. something like

FROM node:17.9.0-slim

WORKDIR /app
# add `/app/node_modules/.bin` to $PATH
ENV PATH /app/node_modules/.bin:$PATH

# Install UI dependencies
COPY ui/package.json .
RUN npm install

# Copy over app
COPY ui/tsconfig.json .
COPY ui/public ./public
COPY ui/src ./src

# Build for production.
RUN npm run build --production

# Serve the UI from a port
RUN npm install -g serve
EXPOSE 3000
CMD ["serve", "-s", "build"]

If it is actual i can change Dockerfile in this PR to something like that.

Looks like you need to sign commits too.

If you want to deploy as a service, might be better to have this Dockerfile instead call into the feast ui command instead then that I just added?

https://docs.feast.dev/v/master/reference/alpha-web-ui#feast-cli

Having this, in combination with a feature_store.yaml that points at a remote registry which is being updated should have a UI that continuously updates against that remote registry