feat(scaffolder): add scaffolder command #277

tavsec · 2024-09-19T16:22:39Z

`neonctl scaffold`

This PR will add scaffold command to the neonctl tool. This command consists of two subcommands list and start.

`list`

The list subcommand will list all of the available templates from templates repository, and display the via the default output. It utilises GitHub API to list the folders from the repository.

Note

The "template" repository has to be created and populated before this functionality can be tested. For development purposes, I created temporary repository https://github.com/neon-scaffolder/templates

`start <template-name>`

This subcommand will initialize the scaffolding process. If the project-id flag is set, then the newly scaffolded directory will be templated using the data from the given project. Otherwise, new Neon project will be created. This subcommand accepts the same flags as neonctl projects create.

Templating

One of the biggest functionality that this commands provide is for community users to declare their own scaffolding templates, that can be used and selected by end users. This is done by using [mustache.js] (https://github.com/janl/mustache.js) templating engine. Available templating variables are project API object, and connection strings, but this can be extended if needed.

TODO:

Create templates repository
Add templating documentation, so new templates can be added by contributors

closes neondatabase#196

davidgomes · 2024-10-01T21:05:29Z

@tavsec we'll get to trying this out soon!

clarkbw · 2024-10-05T14:11:28Z

src/commands/scaffold.ts

+      {
+        connection_uri: connectionString,
+        connection_parameters: {
+          database: projectData.project.name,


The project name is not the database name, project names could have spaces and emoji etc. I guess you could try to parse out the path from the URL but there might be a better way.

clarkbw · 2024-10-07T13:17:56Z

👋🏼 this is very cool. here's some feedback i wrote up:

I ended up with an .env.example file that still has templating (this is feedback for astro actually)

{{#connection_uris}}
DATABASE_URL="postgresql://{{connection_parameters.role}}:{{connection_parameters.password}}@{{connection_parameters.host}}:5432/{{connection_parameters.database}}?sslmode=require"
{{/connection_uris}}

I like how it creates the .env file automatically for me, that avoids an annoying step.

scaffold start could have a much better error state, perhaps show the list of scaffolds available instead or at least remind me of the command.

Part of this issue will be fixed when we allow free users more than 1 project but I don't think it should create a new project automatically. Would be better to ask if I want to reuse a project or at least list out the project names and ids along with the parameter so i could choose 1 without jumping to the console. I ended up going into the console, finding the project id and then running scaffold start --help to find the parameter required to pass in a project.

…database#297) Move action repo from personal account to org, and bump to last version.

tavsec · 2024-10-14T12:06:32Z

Hey @clarkbw thank you so much for taking the time and reviewing this PR, and also sorry for the late response from my side.

Regarding the first point (.env.example), do you think we should add new property to config.neon.json file in the template directory (https://github.com/neon-scaffolder/templates/blob/main/astro/config.neon.json) to include array of "cleanup_files", which will delete the listed files after the processing? This way, we could have file .env.template, which would get templated into .env, and at the end, .env.template would get deleted. What do you think about that?

As for the second point (error handling and project selection), I will update the code with better validation and error handling (check if project exists, check if enough parameters has been passed, ...), and change the behavior in case that no arguments have been passed (ask the user if he wants to reuse the project or create new one).

Thanks again, will push the updates soon 😃

clarkbw · 2024-10-16T19:27:38Z

This way, we could have file .env.template, which would get templated into .env, and at the end, .env.template would get deleted. What do you think about that?

that sounds like a good pattern to me.

tavsec · 2024-10-17T06:03:04Z

@clarkbw In the latest commit, I added the following:

If user does not pass --project-id flag, the command will ask him if he wants to create New project or select from the list of user's projects
If --template-id flag is not present, the command will prompt user to select the template from the template list
Added new property delete_files in the template directory, which contains a list of files that get deleted after templating

davidgomes · 2024-10-17T22:29:46Z

@tavsec since the UX/DevEx is now addressed, we'll do a code review soon

pffigueiredo · 2024-10-18T08:57:14Z

@tavsec since the UX/DevEx is now addressed, we'll do a code review soon

@tavsec the PR is marked as "Draft", is there any work left to do "logic-wise", or it's mostly because of these?

 - Create templates repository
 - Add templating documentation, so new templates can be added by contributors

tavsec · 2024-10-18T09:10:00Z

@tavsec since the UX/DevEx is now addressed, we'll do a code review soon

@tavsec the PR is marked as "Draft", is there any work left to do "logic-wise", or it's mostly because of these?
 - Create templates repository
 - Add templating documentation, so new templates can be added by contributors

There is nothing left logic-wise, mostly just the tasks on the list you posted

pffigueiredo · 2024-11-05T15:30:10Z

package.json

@@ -58,13 +59,17 @@
    "@segment/analytics-node": "^1.0.0-beta.26",
    "axios": "^1.4.0",
    "axios-debug-log": "^1.0.0",
+    "buffer": "^6.0.3",


Where is this dependency being used in?

pffigueiredo · 2024-11-05T15:37:11Z

src/utils/github.ts

+
+const MULTITHREADING_LIMIT = 10;
+
+export async function getContent(owner: string, repository: string) {


nit: can we use an object to pass the params instead of 2 string args? It just makes it a bit less error-prone:

e.g.

export async function getContent({owner, repository}: {owner: string, repository: string}) { ... }

pffigueiredo · 2024-11-05T15:38:33Z

src/commands/scaffold.ts

+
+const GENERATED_FOLDER_PREFIX = 'neon-';
+
+// TODO: Maybe move to constants file?


Let's remove the TODO, colocation is more than fine here 👍

pffigueiredo · 2024-11-05T15:42:38Z

src/commands/scaffold.ts

+      'List available templates',
+      (yargs) => yargs,
+      async (args) => {
+        // @ts-expect-error: TODO - Assert `args` is `CommonProps`


Let's make use of the yargs.Argv generic and remove this comment:

e.g.

export const builder = (argv: yargs.Argv<CommonProps>) => {

pffigueiredo · 2024-11-05T15:47:38Z

src/commands/scaffold.ts

+  'aws-eu-central-1',
+  'aws-us-east-2',
+  'aws-us-east-1',
+];


Should we also include Azure regions by now? cc @davidgomes

pffigueiredo · 2024-11-05T16:53:15Z

src/commands/scaffold.ts

+
+  if (!availableTemplates.includes(props.templateId)) {
+    log.error(
+      'Template not found. Please make sure the template exists and is public.',


nit: let's add the templateID to this error message, should be easier to follow up from a user perspective

pffigueiredo · 2024-11-05T16:54:39Z

src/commands/scaffold.ts

+      await getFileContent(
+        REPOSITORY_OWNER,
+        REPOSITORY,
+        (props.templateId as string) + '/config.neon.json',


What's this '/config.neon.json' for? If it's relevant, let's extract it into it's own const

The file config.neon.json is part of every "template" project and includes which files should be templated, copied and/or deleted in the scaffolding process. Example of such file: https://github.com/neon-scaffolder/templates/blob/main/astro/config.neon.json .

I'm not entirely sure what you mean by extracting it into it's own const. Currently, it is extracted into its own variable config, but I cannot change it to const, as it is assigned in the try-catch block.

pffigueiredo · 2024-11-05T17:07:12Z

src/commands/scaffold.ts

+      props.templateId,
+      dir,
+    );
+  } else {


If this a possible code path? seems like we are returning early in line 250 if the templateID doesn't exist?

Yes, you are right, this is dead code branch. Removed it.

pffigueiredo · 2024-11-05T17:11:06Z

src/utils/github.ts

+import path from 'path';
+import fs from 'fs';
+
+const MULTITHREADING_LIMIT = 10;


nit: can we rename this to CONCURRENT_OPERATIONS_LIMIT? The current name is clear, but it's not threads that we are referring here

pffigueiredo · 2024-11-05T17:13:18Z

src/utils/github.ts

+    '/' +
+    repository +
+    '/main/' +
+    path;


Can we extract this into a util function that builds the URL? Since it's used in multiple place

Extracted into own util function 👍

tavsec · 2024-11-09T18:15:30Z

@pffigueiredo Thanks for the review! I pushed some changes/fixes, feel free to review again :)

mrl5 and others added 2 commits August 24, 2024 15:36

fix: return correct neonctl version (neondatabase#271)

44e161c

closes neondatabase#196

feat(scaffolder): add scaffolder command

26cccea

duskpoet force-pushed the main branch from 47bf28c to 3764c5d Compare September 24, 2024 12:18

clarkbw reviewed Oct 5, 2024

View reviewed changes

ci: gh-workflow-stats-action: bump version and move action repo (neon…

51fe65f

…database#297) Move action repo from personal account to org, and bump to last version.

Merge branch 'neondatabase:main' into main

5832af9

chore(lint): fix lint

9b3c043

pffigueiredo self-requested a review October 18, 2024 08:48

duskpoet force-pushed the main branch from 2b2092e to 70addc3 Compare October 24, 2024 08:45

pffigueiredo reviewed Nov 5, 2024

View reviewed changes

tavsec and others added 3 commits November 9, 2024 19:03

chore: fix PR comments

d67f019

chore: fix PR comments

a1ab1cb

Merge branch 'main' into main

1ce5f06

duskpoet force-pushed the main branch from 3f9d6d8 to d86e9f4 Compare December 19, 2024 09:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(scaffolder): add scaffolder command #277

feat(scaffolder): add scaffolder command #277

tavsec commented Sep 19, 2024

davidgomes commented Oct 1, 2024

clarkbw Oct 5, 2024

clarkbw commented Oct 7, 2024

tavsec commented Oct 14, 2024

clarkbw commented Oct 16, 2024

tavsec commented Oct 17, 2024

davidgomes commented Oct 17, 2024

pffigueiredo commented Oct 18, 2024

tavsec commented Oct 18, 2024

pffigueiredo Nov 5, 2024

pffigueiredo Nov 5, 2024

pffigueiredo Nov 5, 2024

pffigueiredo Nov 5, 2024

pffigueiredo Nov 5, 2024

pffigueiredo Nov 5, 2024

pffigueiredo Nov 5, 2024

tavsec Nov 9, 2024

pffigueiredo Nov 5, 2024

tavsec Nov 9, 2024

pffigueiredo Nov 5, 2024

pffigueiredo Nov 5, 2024

tavsec Nov 9, 2024

tavsec commented Nov 9, 2024


		const MULTITHREADING_LIMIT = 10;

		export async function getContent(owner: string, repository: string) {


		const GENERATED_FOLDER_PREFIX = 'neon-';

		// TODO: Maybe move to constants file?

feat(scaffolder): add scaffolder command #277

Are you sure you want to change the base?

feat(scaffolder): add scaffolder command #277

Conversation

tavsec commented Sep 19, 2024

neonctl scaffold

list

start <template-name>

Templating

TODO:

davidgomes commented Oct 1, 2024

Choose a reason for hiding this comment

clarkbw commented Oct 7, 2024

tavsec commented Oct 14, 2024

clarkbw commented Oct 16, 2024

tavsec commented Oct 17, 2024

davidgomes commented Oct 17, 2024

pffigueiredo commented Oct 18, 2024

tavsec commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tavsec commented Nov 9, 2024

`neonctl scaffold`

`list`

`start <template-name>`