Sentry integration #22
Conversation
cyrossignol
left a comment
cyrossignol
left a comment
There was a problem hiding this comment.
Looks cool. Unfortunately, I don't think this is plug-and-play with our current setup. Did you get it to work locally?
Dockerfile
Outdated
| ENV VITE_IMAGERY_SCHEMA=https://raw.githubusercontent.com/TaskarCenterAtUW/tdei-tools/main/docs/imagery-layer/schema.json | ||
| ENV VITE_IMAGERY_EXAMPLE_URL=https://raw.githubusercontent.com/TaskarCenterAtUW/tdei-tools/main/docs/imagery-layer/example.json | ||
| ENV VITE_LONG_FORM_QUEST_SCHEMA=https://raw.githubusercontent.com/TaskarCenterAtUW/tdei-tools/refs/heads/main/docs/quest-definition/schema.json | ||
| ENV VITE_LONG_FORM_QUEST_EXAMPLE_URL=https://raw.githubusercontent.com/TaskarCenterAtUW/tdei-tools/refs/heads/main/docs/quest-definition/example.json |
There was a problem hiding this comment.
Can we please drop the defaults so there's only one place to maintain these? I'd prefer to update workspaces-stack/example.env. We also want the build to fail when any of these are missing, and Docker will bail when there are no defaults set.
There was a problem hiding this comment.
Sure thing--these were the values in the readme though, so allowed one to more easily setup a dev instance. I removed.
There was a problem hiding this comment.
Thanks! These need to be ARGs, though (with no default empty string). ENVs don't work at build time.
There was a problem hiding this comment.
Got it. Added a comment since that's not intuitive.
There was a problem hiding this comment.
Good call, thanks. Sorry, can you please remove the empty string defaults? Those will short-circuit Docker's missing arg checks.
It did work locally yes. |
|
One more thing, this isn't a full npm package upgrade, right? I see a lot of changes in the lock file. IIRC, there's some dependency in there that broke something, and I didn't have time before the demo to figure it out. |
I did run npm audit yes. What was broken? |
|
@cyrossignol Can you approve this if you're okay with these changes now please? |
|
Please don't merge that |
Integration of the Sentry SDK for workspaces JS errors