nft.storage icon indicating copy to clipboard operation
nft.storage copied to clipboard

feat: Allow developers to run API in docker, and select between mutable and immutable modes

Open redaphid opened this issue 2 years ago • 8 comments

This PR intends to simplify the development ergonomics of nft.storage. It does so by simplifying the environment via docker-compose and uses the environment variables passed into docker-compose as the only source of truth for configuration.

  • Any developer on the project can simply copy .env.tpl to .env, then run yarn dev:api to start the api system up in an immutable state.
  • For development work, the .env file defines the constants for playwright, the cloudflare worker, and Github Actions CI.
  • Github Actions now runs the same command as a developer starting the system. (e.g. copying .env.tpl to .env and running yarn test)
  • yarn test can be run at the same time as the development cluster without port conflicts or data cross-contamination.
  • yarn test:watch in api watches for code changes, then re-runs the playwright tests in an isolated environment on save.
  • yarn dev watches the code, and on save rebuilds
  • yarn dev:persist persists the database in a docker volume between runs.
  • yarn dev:clean will clean the database state for yarn dev:persist
  • yarn test will run playwright locally against the docker-compose cluster
  • yarn test:docker will run playwright itself inside a docker container`

redaphid avatar May 03 '22 20:05 redaphid

Codecov Report

Merging #1888 (0970904) into main (377b045) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##              main     #1888   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines         1259      1259           
=========================================
  Hits          1259      1259           

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 4f88c21...0970904. Read the comment docs.

codecov-commenter avatar May 03 '22 20:05 codecov-commenter

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9b0584c
Status: ✅  Deploy successful!
Preview URL: https://b069fd75.nft-storage-1at.pages.dev
Branch Preview URL: https://feat-make-api-running-immuta.nft-storage-1at.pages.dev

View logs

@redaphid I think I fixed the thing where playwright wants to download chromium every time - there was just a version mismatch between the docker image and our code.

I also got the test to sort of run by ripping out parts of the pw-test.config.cjs file, and copying api/scripts into the container, since the playwright config was trying to load things from there. The playwright config also tries to load the env file from ../../.env, which I think doesn't exist in the container, so the tests aren't finding the right vars... That's as far as I got, so I didn't bother pushing up the mess :)

Anyway, it seems like it will be kind of a hassle to write a playwright config that could run both in the container and on the host... maybe we could get away with always running in docker? That might simplify things a bit.

yusefnapora avatar May 13 '22 16:05 yusefnapora

When we talked about this, we discussed improving the current docker compose situation and the database persistence for dev and/or tests. This PR goes away beyond that by running miniflare and playwright-test inside docker i would like to not do that, its way easier to run, debug and test miniflare/pw-test outside docker. This would make pw-test --debug impossible.

On future PRs like this, its probably better to split into smaller scopes and smaller PRs so we can all review step by step, smaller improvements.

hugomrdias avatar May 26 '22 10:05 hugomrdias

Hey @hugomrdias sorry that this crept past the scope - that's totally on me for not keeping you looped in as we kept iterating on this.

How would you feel about yarn dev and yarn test still defaulting to running the API and playwright on the host machine, and we just add additional yarn dev:docker, yarn test:docker, etc. commands to run in the container?

yusefnapora avatar May 26 '22 15:05 yusefnapora

Hey @hugomrdias sorry that this crept past the scope - that's totally on me for not keeping you looped in as we kept iterating on this.

How would you feel about yarn dev and yarn test still defaulting to running the API and playwright on the host machine, and we just add additional yarn dev:docker, yarn test:docker, etc. commands to run in the container?

IMO having dev and test under a different --project name with different configs for db persistence would be ideal for this PR. Further improvements can come after.

On it :)

hugomrdias avatar May 31 '22 13:05 hugomrdias

@hugomrdias We incorporated your feedback :). Mind looking this over again when you have the chance?

redaphid avatar Jun 02 '22 23:06 redaphid

@vasco-santos I've incorporated all your suggestions! Lmk if this works for you.

redaphid avatar Jun 14 '22 21:06 redaphid