Bulk table import #272

dtbuchholz · 2024-05-15T04:57:19Z

Summary

Adds new command for import table and import bulk
Exports variable from @tableland/validators to help with checking if table definition names in CSV are valid.
Adjust GH workflows to only occur on main and no longer use staging.

Details

You can now run studio import table <args> for a single table—which is the same as the old import-table command.
A new subcommand for import bulk <file> [--sanitize] lets you pass a CSV file table name, description, and new def name. The sanitize command will clean the new def names up for you, which is useful in case you have a lot of tables and are unsure why your def name isn't letting you import tables.

vercel · 2024-05-15T04:57:34Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 29, 2024 6:31pm

dtbuchholz · 2024-05-15T05:15:01Z

@joewagner @asutula i had to import 143 DIMO tables into the Studio, so i ended up creating a command to help me do it. you can find all of the tables here: https://studio.tableland.xyz/partners/dimo

idk if this command something we actually want to add, so I've just kept it in draft as a reference. for example, the tRPC / API could probably handle this more efficiently and batch calls.

a few notes:

here's original CSV file and the data mapped to the column ordering that the import-tables command expects:
dimo-tables.csv
data-formatted.csv
since the table definition names had spaces in them, i created a --sanitize flag to help fix those.
- idk if this is an actual feature that we want, though. the one argument for keeping it is that it's unclear what i can / can't use as a table def name—i kept hitting tRPC issues that provided no context as to why the table def was invalid until i added this feature. (in my scenario, it was just spaces, but it took me a minute to figure that out.)

and a couple of issues with the API:

the total time spent to import 143 tables: ~36 mins (~15s per table)
- this seems high, doesn't it? i.e., an arbitrum nova tx + validator materialization should only take a few seconds total. but as noted, I'm sure the API itself could altered to handle the bulk scenario vs. single calls per table import and significantly help with this
i hit a 500 twice while running this for no apparent reason. it happened at row 15 (“Bristol”) and 70 ("Liger").
also note that "Liger" has duplicative entries in the UI: one is deployed, but the one that hit a 500 is still in a non-deployed status: https://studio.tableland.xyz/partners/dimo/ligier (but you'll have to scroll the UI to find the failed one)

here's the 500 error:

TRPCClientError: Error saving defintion and deployment records.
    at TRPCClientError.from (file:///Users/dtb/tbl/studio/node_modules/@trpc/client/dist/TRPCClientError.mjs:26:20)
    at file:///Users/dtb/tbl/studio/node_modules/@trpc/client/dist/links/internals/createHTTPBatchLink.mjs:59:60
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  meta: {
    response: Response {
      [Symbol(realm)]: null,
      [Symbol(state)]: [Object],
      [Symbol(headers)]: [HeadersList]
    },
    responseJSON: [ [Object] ]
  },
  shape: {
    message: 'Error saving defintion and deployment records.',
    code: -32603,
    data: {
      code: 'INTERNAL_SERVER_ERROR',
      httpStatus: 500,
      path: 'tables.importTable',
      zodError: null
    }
  },
  data: {
    code: 'INTERNAL_SERVER_ERROR',
    httpStatus: 500,
    path: 'tables.importTable',
    zodError: null
  },
  [cause]: undefined
}

package-lock.json

dtbuchholz · 2024-05-15T05:19:33Z

packages/cli/src/commands/import-tables.ts

+}
+
+export const command = "import-tables <file> [sanitize]";
+export const desc = wrapText(


note: i created this helper bc i couldnt get the yargs help command's formatting to print properly

dtbuchholz · 2024-05-15T05:21:05Z

packages/cli/src/commands/import-tables.ts

+          definitionName || helpers.getPrefixFromTableName(tableName);
+        // Ensure custom definition name is valid
+        try {
+          await defNameSchema.parseAsync(defName);


i needed this method from the studio-validators package to check if the user-defined table def name was usable.

and below this is where i do the "sanitize" step that may or may not be useful in the generalized use case...but it was for me w/ DIMO!

dtbuchholz · 2024-05-15T05:22:36Z

packages/validators/src/index.ts

-export * from "./validators/projects";
-export * from "./validators/auth";
-export * from "./validators/tables";
+export { defNameSchema } from "./common.js";


lastly, i had to add the .js extension to make this work with the CLI

joewagner · 2024-05-15T14:44:20Z

We already have a command for importing tables. I've been trying to avoid having two commands that do the same thing and only differ by being plural or not.
The orginal naming with singular vs. plural wasn't meant to signify anything. For example if you create a single project the command is project, and if you list many projects the command is still project.

Curios if this makes sense to others, but maybe we refactor so that the command can take a file as input or use command parameters? Alternatively a more different name might make sense?

dtbuchholz · 2024-05-15T19:58:59Z

packages/cli/src/commands/import-table.ts

-      argv.description,
-      "definition description is required",
-    );
-    const projectId = helpers.getStringValue(


note: i dropped this from being a positional since it's already a global flag. thus, this command can use the context, if set, vs explicitly being passed

dtbuchholz · 2024-05-15T20:03:07Z

@joewagner so i removed the duplicate-ish commands and consolidated them into a single import command that replaces the current import-table command. this has table and bulk subcommands. it felt a bit more natural to me vs. trying to figure out flow for import-table --bulk or having separate commands that do that ~same thing:

> studio import table -h
cli.js import table <tableName> <description> [definitionName]

Import an existing tableland table into a project with description and,
optionally, with a new definition name

Positionals:
  tableName       The full Tableland table name to import    [string] [required]
  description     A description of the table being imported  [string] [required]
  definitionName  Optional new definition name for the table            [string]

and

> studio import bulk -h                           
cli.js import bulk <file> [sanitize]

Import existing Tableland tables into a project with CSV file that includes:
- Full Tableland table name, a description, & (optional) new definition name
- CSV header order: 'tableName', 'description', 'definitionName'

Optionally, sanitize the custom definition names to ensure they are valid by
replacing any invalid characters with underscores.

Positionals:
  file  The CSV file to import                               [string] [required]

Options:
      --sanitize          Sanitize the custom definition names for validity
                                                      [boolean] [default: false]

fwiw, i thought about also including the import-data command, but it's less about "importing" and more so just writing data. but, it could easily be another subcommand if we wanted it to...

dtbuchholz · 2024-05-15T20:23:38Z

btw, getting an error for the web build, but everything else works:

TypeError: Cannot read properties of null (reading 'useContext')
    at exports.useContext (/Users/dtb/tbl/studio/node_modules/react/cjs/react.production.min.js:24:118)
    at StyleRegistry (/Users/dtb/tbl/studio/node_modules/styled-jsx/dist/index/index.js:450:30)
    at Wc (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:68:44)
    at Zc (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:70:253)
    at Z (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:76:89)
    at Zc (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:74:209)
    at Z (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:76:89)
    at Zc (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:74:209)
    at Z (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:76:89)
    at Zc (/Users/dtb/tbl/studio/packages/web/node_modules/react-dom/cjs/react-dom-server.browser.production.min.js:74:209)

someone noted issues with the NODE_ENV var, so if i put NEXT_PUBLIC_NODE_ENV="" in the .env file, the error is at least somewhat fixed:

> nrb

> @tableland/studio@0.0.0 build
> next build

 ✓ Creating an optimized production build    
 ✓ Compiled successfully
   Skipping linting
 ✓ Checking validity of types    
   Collecting page data  ...PageNotFoundError: Cannot find module for page: /_not-found
    at getPagePath (/Users/dtb/tbl/studio/packages/web/node_modules/next/dist/server/require.js:94:15)
    at requirePage (/Users/dtb/tbl/studio/packages/web/node_modules/next/dist/server/require.js:99:22)
    at /Users/dtb/tbl/studio/packages/web/node_modules/next/dist/server/load-components.js:59:84
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async loadComponentsImpl (/Users/dtb/tbl/studio/packages/web/node_modules/next/dist/server/load-components.js:59:26)
    at async /Users/dtb/tbl/studio/packages/web/node_modules/next/dist/build/utils.js:1045:32
    at async Span.traceAsyncFn (/Users/dtb/tbl/studio/packages/web/node_modules/next/dist/trace/trace.js:105:20) {
  code: 'ENOENT'
}

> Build error occurred
Error: Failed to collect page data for /_not-found
    at /Users/dtb/tbl/studio/packages/web/node_modules/next/dist/build/utils.js:1171:15
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  type: 'Error'
}

joewagner · 2024-05-15T20:32:51Z

btw, getting an error for the web build, but everything else works:

@dtbuchholz I am getting the same TypeError: Cannot read properties of null (reading 'useContext') error, I'll try modifying NODE_ENV

fwiw, the original issue with node modules seems to be originating with the siwe package's peer dependencies have an entry of "ethers": "^5.6.8 || ^6.0.8". The || (i.e. an or clause) results in ethers 5 being set as a peer dep in package-lock.
However when I manually update the lock file, I ran into the useContext error.

joewagner · 2024-05-15T20:39:16Z

Next's prerender error explanation page barely unhelpful.
https://nextjs.org/docs/messages/prerender-error

Basically they suggest that you review the entire code base for one of 8 errors that might be the problem.

dtbuchholz · 2024-05-15T21:07:30Z

I remember running into this exact issue when I started the ethers refactor. I was trying out different next and react and react-dom versions...but I can't quite remember what I ended up doing to fix it.

It might've been a .next caching issue, and iirc, Vercel has a way to reset that (maybe the lockfile nuke caused issues there, idk). But that could be way off base.

joewagner · 2024-05-17T20:51:10Z

@dtbuchholz I got the package-lock issues in #266 worked out, but I think you'll want to undo the changes you made to package-lock then rebase this on top of my most recent commit.

dtbuchholz · 2024-05-23T00:24:09Z

.github/workflows/test.yml

  pull_request:
-    branches: ["main", "staging"]


@joewagner wait...was i supposed to remove this line? i can't remember / find the original comment about this GH workflow change.

should it be: branches: ["main"]

packages/cli/package.json

dtbuchholz · 2024-05-23T00:26:52Z

packages/cli/test/import-table.test.ts

added new testfile for table imports

joewagner

This looks good, and is a really nice feature. The only issue I was having is with the definitionName not being optional.
I'm ok if we want to require the header, but if that's the case we should update the help text

joewagner · 2024-05-28T20:56:38Z

packages/cli/src/commands/import-table.ts

+          const rows = dataObject.slice(1);
+          // Must be CSV file and have headers in the order: `tableName`,
+          // `description`, and `definitionName` (optional)
+          if (headers.length < 3) {


I tried modifying an import file to do the import without definitionName, but I can't get it to work.
If I remove the header, it doesn't pass this check.
If I remove the values, I get a zod error that says "String must contain at least 1 character(s)"

@joewagner good catch, fixed and added a couple more tests for this.

a random callout—i'm not enforcing the actual csv headers to be of a specific naming convention but only to have correct ordering. i think that's fine, right? i.e., your headers could be blah,blah,blah, but we just care that those are tableName,description,definitionName in terms of the actual values (where definitionName is actually optional, now).

@joewagner good catch, fixed and added a couple more tests for this.

Excellent!

a random callout—i'm not enforcing the actual csv headers to be of a specific naming convention but only to have correct ordering. i think that's fine, right? i.e., your headers could be blah,blah,blah, but we just care that those are tableName,description,definitionName in terms of the actual values (where definitionName is actually optional, now).

That seems fine, if anyone has an issue we can re-evaluate.

This isnt necessarily the best design but a starting point. For example, it might make sense to combine this with the singular `import-table` and have a `--bulk` flag or something. Or, maybe keeping them separate is ideal.

Rather than having multiple table import commands, this consolidates them into one. You can either `import table` or `import bulk` to import a single table or a series of them under the same parent.

dtbuchholz · 2024-05-29T18:38:39Z

(note: tests pass, aside from the known nonce tests issue)

joewagner · 2024-05-29T20:41:13Z

(note: tests pass, aside from the known nonce tests issue)

Perfect, I'm re-running now.
I've got a potential fix for the nonce tests in the works.

dtbuchholz commented May 15, 2024

View reviewed changes

package-lock.json Outdated Show resolved Hide resolved

dtbuchholz commented May 15, 2024

View reviewed changes

joewagner marked this pull request as ready for review May 15, 2024 15:27

joewagner marked this pull request as draft May 15, 2024 15:28

vercel bot had a problem deploying to Preview May 15, 2024 19:47 Failure

dtbuchholz commented May 15, 2024

View reviewed changes

dtbuchholz force-pushed the dtb/bulk-import branch from b31024d to e76df18 Compare May 15, 2024 20:02

dtbuchholz had a problem deploying to staging May 15, 2024 20:02 — with GitHub Actions Failure

vercel bot had a problem deploying to Preview May 15, 2024 20:05 Failure

Base automatically changed from joe/cli-ethers-v6 to main May 21, 2024 04:57

dtbuchholz force-pushed the dtb/bulk-import branch from e76df18 to 3f5c6b1 Compare May 22, 2024 19:29

vercel bot had a problem deploying to Preview May 22, 2024 19:32 Failure

dtbuchholz force-pushed the dtb/bulk-import branch from 3f5c6b1 to 9c0200e Compare May 22, 2024 19:37

dtbuchholz temporarily deployed to staging May 22, 2024 19:37 — with GitHub Actions Inactive

vercel bot deployed to Preview May 22, 2024 19:40 View deployment

dtbuchholz temporarily deployed to staging May 23, 2024 00:18 — with GitHub Actions Inactive

dtbuchholz marked this pull request as ready for review May 23, 2024 00:18

vercel bot deployed to Preview May 23, 2024 00:20 View deployment

dtbuchholz requested a review from joewagner May 23, 2024 00:22

dtbuchholz commented May 23, 2024

View reviewed changes

packages/cli/package.json Outdated Show resolved Hide resolved

dtbuchholz commented May 23, 2024

View reviewed changes

packages/cli/test/import-table.test.ts Outdated

Copy link

Contributor Author

dtbuchholz May 23, 2024

Choose a reason for hiding this comment

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

added new testfile for table imports

dtbuchholz temporarily deployed to staging May 23, 2024 04:30 — with GitHub Actions Inactive

vercel bot deployed to Preview May 23, 2024 04:31 View deployment

joewagner reviewed May 28, 2024

View reviewed changes

dtbuchholz added 8 commits May 29, 2024 10:40

feat(validators): export 'defNameSchema' | cjs compatability

bdca1e0

feat(cli): bulk table import

fd8b0fe

This isnt necessarily the best design but a starting point. For example, it might make sense to combine this with the singular `import-table` and have a `--bulk` flag or something. Or, maybe keeping them separate is ideal.

chor(repo): adjust review/test workflow triggers

8ad2408

refactor(cli): import with subcommands for tables

c81eb7d

Rather than having multiple table import commands, this consolidates them into one. You can either `import table` or `import bulk` to import a single table or a series of them under the same parent.

chore(cli): remove comment

7246f75

test(cli): add import table & bulk tests

9e2785f

chore(cli): move dev dep to root package file

cb83aae

fix: allow optional def name in bulk csv

8767691

dtbuchholz force-pushed the dtb/bulk-import branch from 0b03782 to 8767691 Compare May 29, 2024 18:29

dtbuchholz temporarily deployed to staging May 29, 2024 18:31 — with GitHub Actions Inactive

vercel bot deployed to Preview May 29, 2024 18:31 View deployment

joewagner self-requested a review May 29, 2024 21:07

joewagner approved these changes May 29, 2024

View reviewed changes

dtbuchholz merged commit 766fde5 into main May 29, 2024
4 checks passed

dtbuchholz deleted the dtb/bulk-import branch May 29, 2024 23:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bulk table import #272

Bulk table import #272

dtbuchholz commented May 15, 2024 •

edited

Loading

vercel bot commented May 15, 2024 •

edited

Loading

dtbuchholz commented May 15, 2024

dtbuchholz May 15, 2024

dtbuchholz May 15, 2024

dtbuchholz May 15, 2024

joewagner commented May 15, 2024

dtbuchholz May 15, 2024

dtbuchholz commented May 15, 2024

dtbuchholz commented May 15, 2024 •

edited

Loading

joewagner commented May 15, 2024

joewagner commented May 15, 2024

dtbuchholz commented May 15, 2024 •

edited

Loading

joewagner commented May 17, 2024

dtbuchholz May 23, 2024 •

edited

Loading

dtbuchholz May 23, 2024

joewagner left a comment

joewagner May 28, 2024

dtbuchholz May 29, 2024

joewagner May 29, 2024

dtbuchholz commented May 29, 2024

joewagner commented May 29, 2024

Bulk table import #272

Bulk table import #272

Conversation

dtbuchholz commented May 15, 2024 • edited Loading

Summary

Details

vercel bot commented May 15, 2024 • edited Loading

dtbuchholz commented May 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joewagner commented May 15, 2024

Choose a reason for hiding this comment

dtbuchholz commented May 15, 2024

dtbuchholz commented May 15, 2024 • edited Loading

joewagner commented May 15, 2024

joewagner commented May 15, 2024

dtbuchholz commented May 15, 2024 • edited Loading

joewagner commented May 17, 2024

dtbuchholz May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joewagner left a 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

dtbuchholz commented May 29, 2024

joewagner commented May 29, 2024

dtbuchholz commented May 15, 2024 •

edited

Loading

vercel bot commented May 15, 2024 •

edited

Loading

dtbuchholz commented May 15, 2024 •

edited

Loading

dtbuchholz commented May 15, 2024 •

edited

Loading

dtbuchholz May 23, 2024 •

edited

Loading