Skip to content
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

Deploy option to enable continuous deployment #1745

Open
wants to merge 50 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
adaa867
checkpoint - kicking off cloud build from cli sorta working
tophtucker Oct 4, 2024
fa5e762
correct ellipsis
tophtucker Oct 8, 2024
6e614ca
continuousDeployment flag persisted in deploy.json
tophtucker Oct 11, 2024
a7efd4c
poll for repo access
tophtucker Oct 11, 2024
1077838
cleanup. remove "Do you wanna link GitHub" prompt, since you have to
tophtucker Oct 11, 2024
44b1e5f
various post-demo cleanup; maybeLinkGitHub even if we’re not enabling…
tophtucker Oct 15, 2024
b6c2a3f
better handling of the case where github is not connected at all; set…
tophtucker Oct 15, 2024
bb790ef
throw Error → throw new Error
tophtucker Oct 15, 2024
93203ca
Merge branch 'main' into toph/onramp
tophtucker Oct 15, 2024
8128173
fix existing tests
tophtucker Oct 15, 2024
56e4d14
start on docs
tophtucker Oct 22, 2024
6f7c852
Merge branch 'main' into toph/onramp
tophtucker Oct 25, 2024
9e234a2
use new singular endpoint to see if repo is authed
tophtucker Oct 25, 2024
c9effe0
clean up logic, more dry
tophtucker Oct 26, 2024
8c75f52
rm getProjectEnvironment calls
tophtucker Oct 29, 2024
75cbf50
link to internal interstitial screen instead of directly to github
tophtucker Oct 30, 2024
e80786b
slightly more human-readable error message
tophtucker Oct 30, 2024
ea676b1
dont change continuousDeployment setting as a side effect of github l…
tophtucker Oct 30, 2024
06b52f7
flatten structure of maybeLinkGitHub with early returns
tophtucker Oct 31, 2024
a7ba250
rename maybeLinkGitHub: boolean to validateGitHubLink: void, more err…
tophtucker Oct 31, 2024
5830afc
support SSH github remotes
tophtucker Nov 1, 2024
048fd88
move validateGitHubLink call to a better higher clearer more consolid…
tophtucker Nov 1, 2024
7c719ae
tweak cd prompt to clarify that you need a github repo
tophtucker Nov 1, 2024
a6ac45e
Merge branch 'main' into toph/onramp
tophtucker Nov 1, 2024
8407f88
minimize diff
tophtucker Nov 1, 2024
476ebec
minimize diff, again
tophtucker Nov 1, 2024
bc60124
first tests of validateGitHubLink
tophtucker Nov 2, 2024
14ce19b
whoohoo end-to-end test of kicking off cloud build
tophtucker Nov 2, 2024
27128bb
ALL TESTS PASSING incl coverage threshold, thanks to testing cloud bu…
tophtucker Nov 2, 2024
3a5de29
Merge branch 'main' into toph/onramp
tophtucker Nov 2, 2024
82f7b0d
move mockIsolatedDirectory to own file
tophtucker Nov 2, 2024
105f0ed
Merge branch 'main' into toph/onramp
tophtucker Nov 2, 2024
e511cc7
testing if git is installed on ubuntu
tophtucker Nov 2, 2024
bb9c311
testing deterministic default branch name
tophtucker Nov 2, 2024
ff60b80
more debugging...
tophtucker Nov 2, 2024
f1cd74d
setting name and email config for git, seeing if that helps
tophtucker Nov 2, 2024
e188a39
fix command separator from ; to && for windows; prettier for ubuntu
tophtucker Nov 2, 2024
fae07e1
use fs instead of touch for cross-platform (windows) compatibility; f…
tophtucker Nov 2, 2024
41f9682
force: true on rm dir for windows complaining ENOTEMPTY
tophtucker Nov 2, 2024
dd50d70
adopting rimraf in lieu of node fs rm for what i hope is better windo…
Nov 2, 2024
3f0bbb8
oops i wasnt actually closing the file i made. good call, windows. ro…
Nov 2, 2024
fede926
ah i had another case of touch
Nov 2, 2024
172bf50
new test for when repo doesnt match
Nov 2, 2024
87f707d
add test for polling for repo auth; deploy.ts test coverage is now ba…
Nov 2, 2024
159d708
WHEW i think everything should be passing now, removing debug console…
Nov 2, 2024
1a5c9f9
lmao last few commits are attributed to my test user bc the exec git …
tophtucker Nov 2, 2024
4a8e4e7
now that were not setting the git config options globally we have to …
Nov 2, 2024
a23c35c
set initial branch when initializing repo instead of setting the defa…
Nov 2, 2024
36e2153
Merge branch 'main' into toph/onramp
Fil Nov 8, 2024
3e5e669
Fil/onramp review (#1805)
Fil Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 44 additions & 32 deletions src/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ class Deployer {
}

private async cloudBuild(deployTarget: DeployTargetInfo) {
if (deployTarget.create) return false; // TODO
if (deployTarget.create) {
throw Error("Incorrect deployTarget state");
tophtucker marked this conversation as resolved.
Show resolved Hide resolved
}
const {deployPollInterval: pollInterval = DEPLOY_POLL_INTERVAL_MS} = this.deployOptions;
await this.apiClient.postProjectBuild(deployTarget.project.id);
const spinner = this.effects.clack.spinner();
Expand All @@ -225,20 +227,25 @@ class Deployer {
deployTarget.workspace.login
}/${deployTarget.project.slug}/deploys/${latestCreatedDeployId}`
);
return latestCreatedDeployId;
// latestCreatedDeployId is initially null for a new project, but once
// it changes to a string it can never change back; since we know it has
// changed, we assert here that it’s not null
return latestCreatedDeployId!;
}
await new Promise((resolve) => setTimeout(resolve, pollInterval));
}
}

private async maybeLinkGitHub(deployTarget: DeployTargetInfo): Promise<boolean> {
if (!this.effects.isTty || deployTarget.create) return false;
if (deployTarget.create) {
throw Error("Incorrect deployTarget state");
tophtucker marked this conversation as resolved.
Show resolved Hide resolved
}
if (!this.effects.isTty) return false;
tophtucker marked this conversation as resolved.
Show resolved Hide resolved
if (deployTarget.environment.build_environment_id && deployTarget.environment.source) {
// can do cloud build
return true;
} else {
// TODO Where should it look for .git? only supports projects at root rn…
// const isGit = existsSync(this.deployOptions.config.root + "/.git");
// We only support cloud builds from the root directory so this ignores this.deployOptions.config.root
const isGit = existsSync(".git");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: log cwd when running yarn deploy. you can run yarn deploy from a child directory like src, but i think it still runs in the context of the root directory, in which case this is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried logging cwd out of curiosity and indeed, as one would hope/expect, yarn deploy runs in the root directory no matter where you call it from.

if (isGit) {
const remotes = (await promisify(exec)("git remote -v", {cwd: this.deployOptions.config.root})).stdout
Copy link
Contributor Author

@tophtucker tophtucker Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that in loader.ts we make our promises "by hand" (and with spawn instead of exec), but in create.ts we already use promisify, so… seems fine!

Expand All @@ -264,7 +271,9 @@ class Deployer {
} else {
// repo not auth’ed; link to auth page and poll for auth
this.effects.clack.log.info(
`Authorize Observable to access the ${bold(repoName)} repository: ${link("https://github.com/apps/observable-data-apps-dev/installations/select_target")}`
`Authorize Observable to access the ${bold(repoName)} repository: ${link(
"https://github.com/apps/observable-data-apps-dev/installations/select_target"
)}`
);
const spinner = this.effects.clack.spinner();
spinner.start("Waiting for repository to be authorized");
Expand Down Expand Up @@ -292,23 +301,19 @@ class Deployer {
}
}
} else {
throw new CliError("No GitHub remote"); // TODO better error
this.effects.clack.log.error("No GitHub remote found");
}
} else {
throw new CliError("Not at root of a git repository"); // TODO better error
this.effects.clack.log.error("Not at root of a git repository");
}
}
return false;
}

private async startNewDeploy(): Promise<GetDeployResponse> {
const deployConfig = await this.getUpdatedDeployConfig();
const deployTarget = await this.getDeployTarget(deployConfig);
const deployConfig2 = await this.getUpdatedDeployConfig(); // TODO inelegant… move cd prompt to getUpdatedDeployConfig?
let deployId;
if (deployConfig2.continuousDeployment) {
// TODO move maybeLinkGitHub so that continuous deployment is only enabled if it succeeds
await this.maybeLinkGitHub(deployTarget);
const {deployConfig, deployTarget} = await this.getDeployTarget(await this.getUpdatedDeployConfig());
let deployId: string | null;
if (deployConfig.continuousDeployment) {
deployId = await this.cloudBuild(deployTarget);
} else {
const buildFilePaths = await this.getBuildFilePaths();
Expand Down Expand Up @@ -360,8 +365,6 @@ class Deployer {
);
}

// TODO validate continuousDeployment

if (deployConfig.projectId && (!deployConfig.projectSlug || !deployConfig.workspaceLogin)) {
const spinner = this.effects.clack.spinner();
this.effects.clack.log.warn("The `projectSlug` or `workspaceLogin` is missing from your deploy.json.");
Expand Down Expand Up @@ -394,7 +397,9 @@ class Deployer {
}

// Get the deploy target, prompting the user as needed.
private async getDeployTarget(deployConfig: DeployConfig): Promise<DeployTargetInfo> {
private async getDeployTarget(
deployConfig: DeployConfig
): Promise<{deployTarget: DeployTargetInfo; deployConfig: DeployConfig}> {
let deployTarget: DeployTargetInfo;
if (deployConfig.workspaceLogin && deployConfig.projectSlug) {
try {
Expand Down Expand Up @@ -481,11 +486,11 @@ class Deployer {
workspaceId: deployTarget.workspace.id,
accessLevel: deployTarget.accessLevel
});
// TODO(toph): initial env config
deployTarget = {
create: false,
workspace: deployTarget.workspace,
project,
// TODO: In the future we may have a default environment
environment: {
automatic_builds_enabled: null,
build_environment_id: null,
Expand Down Expand Up @@ -515,33 +520,40 @@ class Deployer {
}
}

let continuousDeployment = deployConfig.continuousDeployment;
let {continuousDeployment} = deployConfig;
if (continuousDeployment === null) {
continuousDeployment = !!(await this.effects.clack.confirm({
const enable = await this.effects.clack.confirm({
message: wrapAnsi(
`Do you want to enable continuous deployment? ${faint(
"This builds in the cloud instead of on this machine and redeploys whenever you push to this repository."
"This builds in the cloud and redeploys whenever you push to this repository."
)}`,
this.effects.outputColumns
),
active: "Yes",
inactive: "No"
}));
active: "Yes, enable and build in cloud",
inactive: "No, build locally"
});
if (this.effects.clack.isCancel(enable)) throw new CliError("User canceled deploy", {print: false, exitCode: 0});
continuousDeployment = enable;
}

// Disables continuous deployment if there’s no env/source & we can’t link GitHub
if (continuousDeployment) continuousDeployment = await this.maybeLinkGitHub(deployTarget);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fil says: if you enable continuous deployment, it should stay on, and if we can't connect to github for whatever reason (you're not in a repo, or no git remote, or no link in our database), the deploy should just fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, and it feels like a much better way to think about it


const newDeployConfig = {
projectId: deployTarget.project.id,
projectSlug: deployTarget.project.slug,
workspaceLogin: deployTarget.workspace.login,
continuousDeployment
};

await this.effects.setDeployConfig(
this.deployOptions.config.root,
this.deployOptions.deployConfigPath,
{
projectId: deployTarget.project.id,
projectSlug: deployTarget.project.slug,
workspaceLogin: deployTarget.workspace.login,
continuousDeployment
},
newDeployConfig,
this.effects
);

return deployTarget;
return {deployConfig: newDeployConfig, deployTarget};
}

// Create the new deploy on the server.
Expand Down
Loading