Bulk table import - Githubissues

dtbuchholz commented 4 months ago

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[bot] commented 4 months ago

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 commented 4 months ago

@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
}

joewagner commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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

joewagner commented 4 months ago

(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.

tablelandnetwork / studio

Bulk table import #272

Summary

Details