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

Support new schema endpoint and metadata operations #13

Merged
merged 8 commits into from
Nov 16, 2023
Merged

Support new schema endpoint and metadata operations #13

merged 8 commits into from
Nov 16, 2023

Conversation

dvdsgl
Copy link
Member

@dvdsgl dvdsgl commented Nov 16, 2023

  • Adds glide.getApps() and glide.getAppNamed(...).
  • Adds app.getTables() and app.getTableNamed(...).
  • Adds table.getSchema().

They allow more flexible handling of apps and tables–for example:

// Get all users of all apps on team:
const allUsers = [];
const apps = (await glide.getApps()) ?? [];
for (const app of apps) {
      const usersTable = await app.getTableNamed("Users");
      const users = await usersTable!.getRows();
      allUsers.push(...users);
}

An Enterprise customer asked for these.

@dvdsgl dvdsgl marked this pull request as ready for review November 16, 2023 05:14
@dvdsgl dvdsgl self-assigned this Nov 16, 2023
@dvdsgl dvdsgl requested a review from chkn November 16, 2023 05:14
@dvdsgl dvdsgl enabled auto-merge (squash) November 16, 2023 06:04
@dvdsgl dvdsgl changed the title Support new schema endpoint Support new schema endpoint and metadata operations Nov 16, 2023
Copy link

@chkn chkn left a comment

Choose a reason for hiding this comment

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

Lovely improvements 🧑‍🍳

Just left a couple minor thoughts/suggestions, nothing blocking.

export function makeClient({ token = process.env.GLIDE_TOKEN! }: { token?: string } = {}) {
return {
get(route: string, r: RequestInit = {}) {
return fetch(`https://functions.prod.internal.glideapps.com/api${route}`, {
Copy link

Choose a reason for hiding this comment

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

I feel like we're sort of committing to this endpoint, at least for some period of time, by shipping it here.. Just making sure we're cool with that /cc @timwellswa

Copy link
Member Author

Choose a reason for hiding this comment

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

I am prepared to change it. We will need to coordinate when we do, yes.

@@ -42,6 +67,9 @@ await inventory.setRow(rowID, {

// Delete a row
await inventory.deleteRow(rowID);

// Get table schema info (columns and their types)
const schema = await inventory.getSchema();
Copy link

Choose a reason for hiding this comment

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

This doesn't make it clear to me that it's actually fetching the schema from Glide. Earlier in this sample, inventory is created by specifying the columns and their types, so I'd assume this would just return that again:

  columns: {
    Item: "string",
    Description: "string",
    Price: "number",

    // Handle internal column names != display names
    Assignee: { type: "string", name: "7E42F8B3-9988-436E-84D2-5B3B0B22B21F" },
  },`

I'm assuming that is still needed to get strongly typed rows? Might be good just to clarify the relationship between these

};
this.client = makeClient({
token: process.env.GLIDE_TOKEN!,
Copy link

Choose a reason for hiding this comment

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

Shouldn't this just be this.props.token now?

Suggested change
token: process.env.GLIDE_TOKEN!,
token: this.props.token!,

constructor(props: AppProps) {
this.props = { ...props, token: props.token ?? process.env.GLIDE_TOKEN! };
this.client = makeClient({
token: process.env.GLIDE_TOKEN!,
Copy link

Choose a reason for hiding this comment

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

Same again?

Suggested change
token: process.env.GLIDE_TOKEN!,
token: this.props.token!,

@dvdsgl dvdsgl merged commit fc9ace7 into main Nov 16, 2023
1 check passed
@dvdsgl dvdsgl deleted the schema branch November 16, 2023 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants