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

rolling deploy #664

Merged
merged 9 commits into from
May 13, 2024
Merged

rolling deploy #664

merged 9 commits into from
May 13, 2024

Conversation

tharvik
Copy link
Collaborator

@tharvik tharvik commented May 1, 2024

  • publish NPM packages & container image
  • deploy server & website
  • rework packages names
  • split container generation in builder & runner
  • bundle all models depending on Google APIs (killed inside Google network)
  • increase some GPT's tests timeouts

live @ https://discolab.ai

@tharvik tharvik force-pushed the 637-rolling-deploy-tharvik branch 6 times, most recently from 8687fb8 to cd4fc7a Compare May 6, 2024 08:49
@tharvik tharvik force-pushed the 637-rolling-deploy-tharvik branch from 11d5ba5 to 4877869 Compare May 6, 2024 09:59
@JulienVig
Copy link
Collaborator

Closes #666

@JulienVig
Copy link
Collaborator

Could you document how things are working and how to maintain them when you're done? 🙏

@tharvik tharvik self-assigned this May 7, 2024
@tharvik tharvik force-pushed the 637-rolling-deploy-tharvik branch from a1f2718 to 0622a84 Compare May 7, 2024 15:11
@tharvik tharvik marked this pull request as ready for review May 7, 2024 15:15
@tharvik tharvik requested a review from JulienVig May 7, 2024 15:15
This was linked to issues May 8, 2024
Copy link
Collaborator

@JulienVig JulienVig left a comment

Choose a reason for hiding this comment

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

Amazing work!!
I pushed a few commits to update the docs and added non-blocking comments and questions.

Regarding publishing to npm

  • Could you add a quick readme to the npm packages? (@epfml/discojs, @epfml/discojs-web and @epfml/discojs-node)?
  • I saw that discojs, discojs-node, discojs-web have been published, should we also publish the server (last published 1y ago) and the web-client (which used to be @epfml/discojs)?

Regarding Docker running ./with_server fails with error

=> ERROR [builder 12/14] RUN npm --workspace=discojs --workspace=discojs  0.2s
------
 > [builder 12/14] RUN npm --workspace=discojs --workspace=discojs-node run build:
0.155 npm ERR! No workspaces found:
0.155 npm ERR!   --workspace=discojs --workspace=discojs-node
0.156
0.156 npm ERR! A complete log of this run can be found in: /root/.npm/_logs/2024-05-08T09_21_38_612Z-debug-0.log
------
Dockerfile:14
--------------------
  12 |     COPY discojs-node/ discojs-node/
  13 |     COPY tsconfig.base.json .
  14 | >>> RUN npm --workspace=discojs --workspace=discojs-node run build
  15 |
  16 |     COPY server/ server/
--------------------
ERROR: failed to solve: process "/bin/sh -c npm --workspace=discojs --workspace=discojs-node run build" did not complete successfully: exit code: 1

It seems the same error occurred in the github actions I don't know what changed after your last commit

DEV.md Outdated Show resolved Hide resolved
DEV.md Outdated Show resolved Hide resolved
cli/package.json Show resolved Hide resolved
discojs-web/package.json Show resolved Hide resolved
docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/DEPLOYMENT.md Outdated Show resolved Hide resolved
docs/DEPLOYMENT.md Outdated Show resolved Hide resolved
@tharvik tharvik force-pushed the 637-rolling-deploy-tharvik branch 2 times, most recently from 573dda3 to 379cf00 Compare May 13, 2024 08:57
@tharvik
Copy link
Collaborator Author

tharvik commented May 13, 2024

good idea, added in latest commit.

  • I saw that discojs, discojs-node, discojs-web have been published, should we also publish the server (last published 1y ago) and the web-client (which used to be @epfml/discojs)?

NPM is more for dependencies than end products, the server itself is published on github packages. I don't really see the need to publish the webapp itself.

Regarding Docker running ./with_server fails with error
0.155 npm ERR! No workspaces found:

it's now worked around with #672

@tharvik tharvik force-pushed the 637-rolling-deploy-tharvik branch from 429956d to 01a05aa Compare May 13, 2024 12:11
@JulienVig
Copy link
Collaborator

NPM is more for dependencies than end products, the server itself is published on github packages. I don't really see the need to publish the webapp itself.

I think there was a point where the goal was to let users deploy their own server and webapp (and the github pages were just to showcase Disco rather than actually be used), we should probably discuss this at the weekly to clarify how we want Disco to be used.

@tharvik tharvik force-pushed the 637-rolling-deploy-tharvik branch from 8a433d5 to 6e816dc Compare May 13, 2024 12:53
@tharvik tharvik requested a review from JulienVig May 13, 2024 12:53
@JulienVig
Copy link
Collaborator

JulienVig commented May 13, 2024

Conclusion from the weekly, Disco users wanting to deploy their own Disco instance are not the priority because it is a minority of experts. We assume some technical knowledge and let them deploy their own server and webapp by cloning the repository and following the DEV.md instructions

Copy link
Collaborator

@JulienVig JulienVig left a comment

Choose a reason for hiding this comment

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

All good!! Only non-blocking doc comments
It seems that some rebasing reverted some of the changes I pushed (like renaming deployement.md to deplyoment.md, renaming disc), could you check what got lost on the way if you have the time?

DEV.md Outdated Show resolved Hide resolved
discojs-node/README.md Show resolved Hide resolved
discojs-web/README.md Show resolved Hide resolved
docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
@tharvik tharvik force-pushed the 637-rolling-deploy-tharvik branch from d3bbd0e to 58b6260 Compare May 13, 2024 13:58
@tharvik tharvik merged commit cc18768 into develop May 13, 2024
23 checks passed
@tharvik tharvik deleted the 637-rolling-deploy-tharvik branch May 13, 2024 14:06
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.

Link new URL reworks packages names
2 participants