Repository navigation
Retake-photo confirmation, CI on dev and a cached API build #61
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
Changes from all commits
82a91ce
f7112ae
ecffc9d
9477477
55fc89a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ exec docker buildx build \ | |
| --file apps/api/Dockerfile \ | ||
| --target prebuilt \ | ||
| --platform linux/amd64,linux/arm64 \ | ||
| --cache-from "type=registry,ref=$image:buildcache" \ | ||
| --cache-to "type=registry,ref=$image:buildcache,mode=max" \ | ||
|
Comment on lines
+18
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Technical details# Registry buildcache is unreachable for `--target prebuilt`
## Affected sites
- `apps/api/scripts/publish.sh:18-19` — `--cache-from` / `--cache-to type=registry,ref=$image:buildcache,mode=max`, applied to a `docker buildx build --target prebuilt`.
- `apps/api/Dockerfile` — `prebuilt` is `FROM runtime`, which is `FROM gcr.io/distroless/base-debian13:nonroot`. BuildKit only builds stages reachable from `--target`, so `deps` (`bun ci`) and `build` (`compile.sh`) are never instantiated.
- `apps/api/project.json` — `publish` `dependsOn` `build:linux-amd64` and `build:linux-arm64`, both nx-cached, both running `bun build --compile` on the host.
## Required outcome
- CD does not carry cache flags that cannot yield a hit, and a cache-export failure cannot fail a publish whose image push already succeeded.
## Suggested approach (optional)
- Simplest: drop both flags. The expensive work is already memoised by the nx cache on `build:linux-*`, and the real Docker build cost lives in the `hermetic` target, which `publish.sh` does not use.
- If you want to keep them as a hedge, at minimum set `ignore-error=true` on `--cache-to` and drop `mode=max` to `mode=min` — with no intermediate stages in the graph, `max` buys nothing over `min`.
## Open questions for the human
- Was the intent to speed up `hermetic` (which does run `bun ci` + `compile.sh` in-image) rather than `prebuilt`? If CI is meant to move to `hermetic`, the cache flags make sense there and this comment becomes a sequencing note instead. |
||
| "$@" \ | ||
| --label "org.opencontainers.image.source=https://github.com/$GITHUB_REPOSITORY" \ | ||
| --label "org.opencontainers.image.revision=$GITHUB_SHA" \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,16 @@ | ||||||||||||||||||||||
| import { useNavigate } from '@tanstack/react-router'; | ||||||||||||||||||||||
| import { AlertTriangle, Camera } from 'lucide-react'; | ||||||||||||||||||||||
| import { | ||||||||||||||||||||||
| AlertDialog, | ||||||||||||||||||||||
| AlertDialogAction, | ||||||||||||||||||||||
| AlertDialogCancel, | ||||||||||||||||||||||
| AlertDialogContent, | ||||||||||||||||||||||
| AlertDialogDescription, | ||||||||||||||||||||||
| AlertDialogFooter, | ||||||||||||||||||||||
| AlertDialogHeader, | ||||||||||||||||||||||
| AlertDialogTitle, | ||||||||||||||||||||||
| AlertDialogTrigger, | ||||||||||||||||||||||
| } from 'shadcn-ui/alert-dialog'; | ||||||||||||||||||||||
| import { Button } from 'shadcn-ui/button'; | ||||||||||||||||||||||
| import { useDeleteSession } from '@/features/session/api/delete-session'; | ||||||||||||||||||||||
| import { useUpdateSession } from '@/features/session/api/update-session'; | ||||||||||||||||||||||
|
|
@@ -109,21 +120,43 @@ function RetakePhotoButton({ session }: { session: Session }) { | |||||||||||||||||||||
| const deleteSession = useDeleteSession({ sessionId: session.id }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return ( | ||||||||||||||||||||||
| <Button | ||||||||||||||||||||||
| variant="secondary" | ||||||||||||||||||||||
| size="xl" | ||||||||||||||||||||||
| className="w-[40%] flex-none" | ||||||||||||||||||||||
| disabled={deleteSession.isPending} | ||||||||||||||||||||||
| onClick={() => | ||||||||||||||||||||||
| deleteSession.mutate( | ||||||||||||||||||||||
| { sessionId: session.id }, | ||||||||||||||||||||||
| { onSettled: () => navigate({ to: '/' }) }, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| > | ||||||||||||||||||||||
| <Camera size={20} /> | ||||||||||||||||||||||
| Retake photo | ||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||
| <AlertDialog> | ||||||||||||||||||||||
| <AlertDialogTrigger | ||||||||||||||||||||||
| aria-label="Retake photo" | ||||||||||||||||||||||
| render={ | ||||||||||||||||||||||
| <Button variant="secondary" size="xl" className="w-11 flex-none px-0"> | ||||||||||||||||||||||
| <Camera size={20} /> | ||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| ></AlertDialogTrigger> | ||||||||||||||||||||||
| <AlertDialogContent> | ||||||||||||||||||||||
| <AlertDialogHeader> | ||||||||||||||||||||||
| <AlertDialogTitle>Retake photo?</AlertDialogTitle> | ||||||||||||||||||||||
| <AlertDialogDescription> | ||||||||||||||||||||||
| This receipt and its items will be discarded so you can take a new | ||||||||||||||||||||||
| photo. | ||||||||||||||||||||||
| </AlertDialogDescription> | ||||||||||||||||||||||
| </AlertDialogHeader> | ||||||||||||||||||||||
| <AlertDialogFooter> | ||||||||||||||||||||||
| <AlertDialogCancel variant="ghost" size="xl"> | ||||||||||||||||||||||
| Cancel | ||||||||||||||||||||||
| </AlertDialogCancel> | ||||||||||||||||||||||
|
Comment on lines
+141
to
+143
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Base UI's
Suggested change
|
||||||||||||||||||||||
| <AlertDialogAction | ||||||||||||||||||||||
| variant="destructive" | ||||||||||||||||||||||
| size="xl" | ||||||||||||||||||||||
| disabled={deleteSession.isPending} | ||||||||||||||||||||||
| onClick={() => | ||||||||||||||||||||||
| deleteSession.mutate( | ||||||||||||||||||||||
| { sessionId: session.id }, | ||||||||||||||||||||||
| { onSettled: () => navigate({ to: '/' }) }, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| > | ||||||||||||||||||||||
| Retake | ||||||||||||||||||||||
| </AlertDialogAction> | ||||||||||||||||||||||
| </AlertDialogFooter> | ||||||||||||||||||||||
| </AlertDialogContent> | ||||||||||||||||||||||
| </AlertDialog> | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
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.
Stripping the comments leaves this file undocumented anywhere in the repo —
docs/deployment.mdis gone,README.mdhas no deploy section, anddocs/now holds onlydcu.pumlandsrs.md. This is the file a human copies to/opt/apps/snapsplit/.envand fills in by hand, and the removed lines carried precisely the non-obvious parts: thatCORS_ORIGINis the Cloudflare Worker origin rather than the API's own domain, whyTRUST_PROXY=1is required (without it the rate limiter keys every caller by Caddy's address and throttles them as one), and theterraform outputcommands that source the fiveS3_*values. Noteapps/api/.env.examplekept its comments, so the two files are now inconsistent.