[cli] implements vc deploy --logs and vc inspect --logs #11672
Conversation
🦋 Changeset detectedLatest commit: 3f3b503 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const eventsRes = await client.fetch(eventsUrl, { | ||
| json: false, | ||
| signal: abortController?.signal, | ||
| }); |
There was a problem hiding this comment.
client.fetch() supports node-retry options, so this code may be refactored and not being wrapped in a retry call.
| client, | ||
| deployment.id, | ||
| { | ||
| mode: 'logs', |
There was a problem hiding this comment.
We don't use 'deploy' mode here (which triggers its own polling), because it's actually not working.
The API does not return the special "events" which are meant to controller polling.
I highly doubt it's working, and I would suggest we remove this mode parameter and the related code.
There was a problem hiding this comment.
Do we use the deploy anywhere?
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
fbf7f1b
to
f2687f4
Compare
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
f2687f4
to
b778a32
Compare
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
b778a32
to
b812a90
Compare
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
b812a90
to
0e6fd6e
Compare
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
0e6fd6e
to
a09198b
Compare
feugy
requested review from
TooTallNate,
EndangeredMassa,
trek,
onsclom,
jeffsee55 and
erikareads
as code owners
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
a09198b
to
912badb
Compare
| client, | ||
| deployment.id, | ||
| { | ||
| mode: 'logs', |
There was a problem hiding this comment.
Do we use the deploy anywhere?
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
0621a61
to
222df32
Compare
feugy
force-pushed
the
damien/log-389-improve-vc-logs-build
branch
from
222df32
to
3f3b503
Compare
| function exitCode(state: Deployment['readyState']) { | ||
| if (state === 'ERROR' || state === 'CANCELED') { | ||
| return 1; | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Shouldn't this be returning 0 all the time if you don't want to change how status code is returned?
| return chalk.gray('UNKNOWN'); | ||
| function exitCode(state: Deployment['readyState']) { | ||
| if (state === 'ERROR' || state === 'CANCELED') { | ||
| return 1; |
There was a problem hiding this comment.
This is a change, but I think it makes sense.
🧐 What's in there?
This is the first part of distinguishing build logs from runtime logs.
The proposal is to provide:
vc deploy --logs: in addition to deploying the application, it should tail and display the build logs until the build finishes.vc inspect --logs: instead of retuning a summary of the builds, it reads the build logsvc inspect --logs --wait: when the build is in progress, tails the build logs until the configured timeout (3m by default), or display all available logs when the build is finished.vc logs: read the runtime logs (not part of this PR, breaking change)🧪 How to test?
Given a local application location in
xyzfolder, already linked to a vercel project:pnpm -F vercel dev deploy --cwd xyz --logsto deploy and see build logspnpm -F vercel dev deploy --cwd xyzto deploy and see spinners insteadpnpm -F vercel dev inspect --cwd xyz dpl_ID/url --logsto see build logs of an existing deploymentpnpm -F vercel dev inspect --cwd xyz dpl_ID/urlto see an existing deployment detailspnpm -F vercel dev inspect --cwd xyz dpl_ID/url --logs --waitto see build logs of an existing deployment (better if in progress)pnpm -F vercel dev inspect --cwd xyz dpl_ID/url --logs --wait --timeout 5sto see build logs of an existing deployment (better if in progress) and bail after 5 secondsUnit tests added:
pnpm -F vercel exec vitest run /inspect.testpnpm -F vercel exec vitest run /deploy.test❗ Notes to reviewers
It's better to omit whitespace changes when reviewing
packages/cli/src/util/events.tssince most of the code was enclosed in a try-catch.