-
Notifications
You must be signed in to change notification settings - Fork 236
refactor: use @heroku/sdk for apps:info command #3749
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
d61397b
0b2a965
f8633ed
0fcf0dd
990f8a1
7a58cbe
29d65dd
04ea66b
8a4fcb2
c2467a4
44eec86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| #!/usr/bin/env bash | ||
| # Print SDK route metadata for a given (verb, path). | ||
| # | ||
| # Use to diagnose "body silently dropped at runtime" symptoms: if | ||
| # `hasRequestBody` is false but the endpoint logically requires a body, the | ||
| # fix is upstream — bump @heroku/types to a version where the route metadata | ||
| # is correct. Don't escape-hatch around it. | ||
| # | ||
| # Usage: check-route.sh <VERB> <PATH> | ||
| # e.g. check-route.sh PATCH /apps/example/config-vars | ||
| set -euo pipefail | ||
|
|
||
| VERB="${1:-}" | ||
| ROUTE_PATH="${2:-}" | ||
|
|
||
| if [ -z "$VERB" ] || [ -z "$ROUTE_PATH" ]; then | ||
| echo "usage: $0 <VERB> <PATH>" >&2 | ||
| echo " e.g. $0 PATCH /apps/example/config-vars" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| npx tsx -e " | ||
| import {RouteIndex} from './scripts/codemods/sdk-migration/routes-index.ts'; | ||
| const r = RouteIndex.load().lookup('$VERB', '$ROUTE_PATH'); | ||
| if (!r) { console.log('no match for $VERB $ROUTE_PATH'); process.exit(0); } | ||
| console.log('hasRequestBody:', r.entry.hasRequestBody, 'method:', r.entry.resource + '.' + r.entry.method); | ||
| " |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| #!/usr/bin/env bash | ||
| # Pre-flight checks for sdk-command-migration: P1 (clean working tree, SDK on | ||
| # disk) and P2 (capture baselines). | ||
| # | ||
| # Why the SDK probe looks the way it does: the SDK's `exports` map doesn't | ||
| # expose `./package.json` (so `require('@heroku/sdk/package.json')` fails with | ||
| # ERR_PACKAGE_PATH_NOT_EXPORTED), and the SDK's entry uses top-level `await` | ||
| # (so `require('@heroku/sdk')` fails with ERR_REQUIRE_ASYNC_MODULE). Reading | ||
| # `node_modules/@heroku/sdk/package.json` directly and using | ||
| # `--input-type=module` for the import sidesteps both. | ||
| # | ||
| # Usage: preflight.sh <command-test-path> | ||
| # e.g. preflight.sh test/unit/commands/apps/info.unit.test.ts | ||
| set -euo pipefail | ||
|
|
||
| TEST_PATH="${1:-}" | ||
| if [ -z "$TEST_PATH" ]; then | ||
| echo "usage: $0 <command-test-path>" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| BASELINE="${TSC_BASELINE:-/tmp/tsc-baseline.txt}" | ||
|
|
||
| echo "=== P1: working tree ===" | ||
| git status -sb | ||
|
|
||
| echo | ||
| echo "=== P1: SDK on disk ===" | ||
| grep -E '"(name|version)"' node_modules/@heroku/sdk/package.json | ||
| node --input-type=module -e "import {HerokuSDK} from '@heroku/sdk'; console.log('HerokuSDK is', typeof HerokuSDK)" | ||
|
|
||
| echo | ||
| echo "=== P2: tsc baseline -> $BASELINE ===" | ||
| npx tsc --noEmit -p tsconfig.json 2>&1 | tee "$BASELINE" | tail -20 | ||
|
|
||
| echo | ||
| echo "=== P2: target test file ===" | ||
| npx mocha "$TEST_PATH" --reporter min 2>&1 | tail -5 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #!/usr/bin/env bash | ||
| # Run tsc and filter pre-existing errors against /tmp/tsc-baseline.txt. | ||
| # | ||
| # The `[ -s "$BASELINE" ]` guard is load-bearing: when the baseline file is | ||
| # empty (clean repo at pre-flight time), `grep -v -F -f` matches *nothing* | ||
| # because the empty pattern file has no patterns — yielding a false "no | ||
| # errors" signal in exactly the case where strict checking matters most. | ||
| # | ||
| # Usage: tsc-delta.sh [tail-lines] | ||
| # tail-lines: optional, pipes through `tail -N` (use "all" for unfiltered) | ||
| set -euo pipefail | ||
|
|
||
| BASELINE="${TSC_BASELINE:-/tmp/tsc-baseline.txt}" | ||
| TAIL="${1:-all}" | ||
|
|
||
| run_tsc() { | ||
| if [ -s "$BASELINE" ]; then | ||
| npx tsc --noEmit -p tsconfig.json 2>&1 | grep -v -F -f "$BASELINE" || true | ||
| else | ||
| npx tsc --noEmit -p tsconfig.json 2>&1 || true | ||
| fi | ||
| } | ||
|
|
||
| if [ "$TAIL" = "all" ]; then | ||
| run_tsc | ||
| else | ||
| run_tsc | tail -"$TAIL" | ||
| fi |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,21 @@ | ||
| import type {AppInfo} from '@heroku/sdk/resources/platform/app/info' | ||
|
|
||
| import {Command, flags} from '@heroku-cli/command' | ||
| import * as Heroku from '@heroku-cli/schema' | ||
| import {color, hux} from '@heroku/heroku-cli-util' | ||
| import {HerokuApiClient} from '@heroku/heroku-fetch' | ||
| import {HerokuSDK} from '@heroku/sdk' | ||
| import {appExtensions} from '@heroku/sdk/extensions/platform' | ||
| import {Args, ux} from '@oclif/core' | ||
| import {filesize} from 'filesize' | ||
| import {inspect} from 'node:util' | ||
|
|
||
| import {getGeneration} from '../../lib/apps/generation.js' | ||
| import {lazyModuleLoader} from '../../lib/lazy-module-loader.js' | ||
| import {ExtendedApp} from '../../lib/types/app.js' | ||
|
|
||
| type Platform = HerokuSDK<readonly [typeof appExtensions]>['platform'] | ||
|
|
||
| type Info = Omit<AppInfo, 'app'> & {app: ExtendedApp} | ||
|
|
||
| export default class AppsInfo extends Command { | ||
| static args = { | ||
|
|
@@ -45,14 +54,16 @@ repo_size=5000000 | |
| const app = args.app || flags.app | ||
| if (!app) throw new Error('No app specified.\nUSAGE: heroku apps:info --app my-app') | ||
|
|
||
| const info = await getInfo(app, this, flags.extended) | ||
| const addons = info.addons.map((a: Heroku.AddOn) => a.plan?.name).sort() | ||
| const collaborators = info.collaborators.map((c: Heroku.Collaborator) => c.user.email) | ||
| .filter((c: Heroku.Collaborator) => c !== info.app.owner.email) | ||
| const {platform} = new HerokuSDK({extensions: [appExtensions]}) | ||
|
|
||
| const info = await getInfo(app, platform, flags.extended) | ||
| const addons = info.addons.map(a => a.plan?.name).sort() | ||
| const collaborators = info.collaborators.map(c => c.user.email) | ||
| .filter(c => c !== info.app.owner.email) | ||
| .sort() | ||
|
|
||
| function shell() { | ||
| function print(k: string, v: string) { | ||
| function print(k: string, v: unknown) { | ||
| ux.stdout(`${_.snakeCase(k)}=${v}`) | ||
| } | ||
|
|
||
|
|
@@ -65,12 +76,12 @@ repo_size=5000000 | |
| if (info.app.cron_next_run) print('cron_next_run', formatDate(new Date(info.app.cron_next_run))) | ||
| if (info.app.database_size) print('database_size', filesize(info.app.database_size, {round: 0, standard: 'jedec'})) | ||
| if (info.app.create_status !== 'complete') print('create_status', info.app.create_status) | ||
| if (info.pipeline_coupling) print('pipeline', `${info.pipeline_coupling.pipeline.name}:${info.pipeline_coupling.stage}`) | ||
| if (info.pipelineCoupling) print('pipeline', `${info.pipelineCoupling.pipeline.name}:${info.pipelineCoupling.stage}`) | ||
|
|
||
| print('git_url', info.app.git_url) | ||
| print('web_url', info.app.web_url) | ||
| print('repo_size', filesize(info.app.repo_size, {round: 0, standard: 'jedec'})) | ||
| if (getGeneration(info.app) !== 'fir') print('slug_size', filesize(info.app.slug_size, {round: 0, standard: 'jedec'})) | ||
| print('web_url', info.app.web_url as string) | ||
| print('repo_size', filesize(info.app.repo_size as number, {round: 0, standard: 'jedec'})) | ||
| if (getGeneration(info.app) !== 'fir') print('slug_size', filesize(info.app.slug_size as number, {round: 0, standard: 'jedec'})) | ||
| print('owner', info.app.owner.email) | ||
| print('region', info.app.region.name) | ||
| print('dynos', inspect(_.countBy(info.dynos, 'type'))) | ||
|
|
@@ -80,7 +91,8 @@ repo_size=5000000 | |
| if (flags.shell) { | ||
| shell() | ||
| } else if (flags.json) { | ||
| hux.styledJSON(info) | ||
| const {pipelineCoupling, ...rest} = info | ||
| hux.styledJSON({...rest, pipeline_coupling: pipelineCoupling}) | ||
| } else { | ||
| print(info, addons, collaborators, flags.extended, _) | ||
| } | ||
|
|
@@ -91,51 +103,22 @@ function formatDate(date: Date) { | |
| return date.toISOString() | ||
| } | ||
|
|
||
| async function getInfo(app: string, client: Command, extended: boolean) { | ||
| const promises = [ | ||
| client.heroku.get<Heroku.AddOn[]>(`/apps/${app}/addons`), | ||
| client.heroku.request<Heroku.App>(`/apps/${app}`), | ||
| client.heroku.get<Heroku.Dyno[]>(`/apps/${app}/dynos`).catch(() => ({body: []})), | ||
| client.heroku.get<Heroku.Collaborator[]>(`/apps/${app}/collaborators`).catch(() => ({body: []})), | ||
| client.heroku.get<Heroku.PipelineCoupling[]>(`/apps/${app}/pipeline-couplings`).catch(() => ({body: null})), | ||
| ] | ||
|
|
||
| if (extended) { | ||
| promises.push(client.heroku.get<Heroku.App>(`/apps/${app}?extended=true`)) | ||
| } | ||
|
|
||
| const [ | ||
| {body: addons}, | ||
| {body: appWithMoreInfo}, | ||
| {body: dynos}, | ||
| {body: collaborators}, | ||
| {body: pipelineCouplings}, | ||
| appExtendedResponse, | ||
| ] = await Promise.all(promises) | ||
|
|
||
| const data: Heroku.App = { | ||
| addons, | ||
| app: appWithMoreInfo, | ||
| collaborators, | ||
| dynos, | ||
| pipeline_coupling: pipelineCouplings, | ||
| } | ||
|
|
||
| if (appExtendedResponse) { | ||
| data.appExtended = appExtendedResponse.body | ||
| } | ||
| async function getInfo(app: string, platform: Platform, extended: boolean): Promise<Info> { | ||
| const data: Info = await platform.app.describe(app) | ||
|
|
||
| if (extended) { | ||
|
Contributor
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. We're using HerokuApiClient directly for the extended endpoint. Since the SDK already owns the composite describe, should describe accept an extended?: boolean option, fetch ?extended=true internally, and merge the result — pushing that logic out of the CLI?
Contributor
Author
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. I went back and forth about this, and landed on this being uniquely a CLI concern, as I understand it. The other thought is that SDK will be customer-facing and this extended flag pertains only to sudo users. |
||
| data.appExtended.acm = data.app.acm | ||
| data.app = data.appExtended | ||
| delete data.appExtended | ||
| const client = new HerokuApiClient() | ||
| const response = await client.get(`/apps/${app}?extended=true`) | ||
| const appExtended = await response.json() as ExtendedApp | ||
| appExtended.acm = data.app.acm | ||
| data.app = appExtended | ||
| } | ||
|
|
||
| return data | ||
| } | ||
|
|
||
| function print(info: Heroku.App, addons: Heroku.AddOn[], collaborators: Heroku.Collaborator[], extended: boolean, _: any) { | ||
| const data: Heroku.App = {} | ||
| function print(info: Info, addons: (string | undefined)[], collaborators: (string | undefined)[], extended: boolean, _: any) { | ||
| const data: Record<string, unknown> = {} | ||
| data.Addons = addons | ||
| data.Collaborators = collaborators | ||
|
|
||
|
|
@@ -144,15 +127,15 @@ function print(info: Heroku.App, addons: Heroku.AddOn[], collaborators: Heroku.C | |
| if (info.app.cron_next_run) data['Cron Next Run'] = formatDate(new Date(info.app.cron_next_run)) | ||
| if (info.app.database_size) data['Database Size'] = filesize(info.app.database_size, {round: 0, standard: 'jedec'}) | ||
| if (info.app.create_status !== 'complete') data['Create Status'] = info.app.create_status | ||
| if (info.app.space) data.Space = color.space(info.app.space.name) | ||
| if (info.app.space) data.Space = color.space(info.app.space.name as string) | ||
| if (info.app.space && info.app.internal_routing) data['Internal Routing'] = info.app.internal_routing | ||
| if (info.pipeline_coupling) data.Pipeline = `${color.pipeline(info.pipeline_coupling.pipeline.name)} - ${info.pipeline_coupling.stage}` | ||
| if (info.pipelineCoupling) data.Pipeline = `${color.pipeline(info.pipelineCoupling.pipeline.name)} - ${info.pipelineCoupling.stage}` | ||
|
|
||
| data['Auto Cert Mgmt'] = info.app.acm | ||
| data['Git URL'] = info.app.git_url | ||
| data['Web URL'] = color.info(info.app.web_url) | ||
| data['Repo Size'] = filesize(info.app.repo_size, {round: 0, standard: 'jedec'}) | ||
| if (getGeneration(info.app) !== 'fir') data['Slug Size'] = filesize(info.app.slug_size, {round: 0, standard: 'jedec'}) | ||
| data['Web URL'] = color.info(info.app.web_url as string) | ||
| data['Repo Size'] = filesize(info.app.repo_size as number, {round: 0, standard: 'jedec'}) | ||
| if (getGeneration(info.app) !== 'fir') data['Slug Size'] = filesize(info.app.slug_size as number, {round: 0, standard: 'jedec'}) | ||
| data.Owner = color.user(info.app.owner.email) | ||
| data.Region = info.app.region.name | ||
| data.Dynos = _.countBy(info.dynos, 'type') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,3 +2,11 @@ import type {App as BaseApp, TeamApp} from '@heroku/types/3.sdk' | |
|
|
||
| export type App = BaseApp & Pick<TeamApp, 'locked' | 'joined'> | ||
| export type Apps = App[] | ||
|
|
||
| export type ExtendedApp = App & { | ||
|
Contributor
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. We're defining ExtendedApp locally with create_status, cron_*, database_size, extended fields. If the SDK's describe gains the extended option, these fields belong on the SDK's return type. Should we move this type into @heroku/sdk or @heroku/types so it's the SDK's contract, not a CLI-local workaround?
Contributor
Author
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. See my previous note on the extended arg to the describe function. |
||
| create_status?: string | ||
| cron_finished_at?: null | string | ||
| cron_next_run?: null | string | ||
| database_size?: null | number | ||
| extended?: unknown | ||
| } | ||
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.
We're casting web_url as string, repo_size as number, slug_size as number because @heroku/security-starter-projects defines them as T | null. The command assumes they're always populated for an existing app. Should we add a DescribedApp narrowed type in the SDK's info.ts that asserts these fields are non-null post-fetch (since describe only returns real apps, not create-in-progress shells), so CLI commands don't need as casts?