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

(sdk) add helper to create studio project aliases #48

Closed
wants to merge 2 commits into from
Closed

Conversation

joewagner
Copy link
Contributor

@joewagner joewagner commented Oct 2, 2023

Overview

This PR exposes a helper function that takes a Studio project ID and returns an aliases Object that can be used to instantiate a DB which uses the given project's table names.

Details

The end goal here is to make the SDK feel like a normal database, specifically table names don't need their universally unique suffixes. A dev can create a project in the Studio, then simply instantiate Databases with the projectId.
e.g. new Database({ projectId: "123-xyz" });

This is a first draft and there's a few things to consider:

I ended up calling the config param projectId. The names environmentId, and databaseId are being discussed as well. For this draft I landed on projectId for the following reasons:

  • The term Environment doesn't exist in the Studio and using it seemed like I was adding unecessary complexity.
  • The term Database seems overloaded, and also doesn't exist in the Studio.
  • The project page is where people will end up looking for the projectId.
  • The existing api endpoint that returns the name mapping we want is deployments.projectDeployments, which takes a projectId argument.

I did write some tests to make development of the feature easier, but they only work when the tests can connect to the studio with pre-populated data. I used a personal test wallet and created projects on the current production studio, but I'm skipping the tests here because the studio instance url and data might change as we develop and having access to the wallet in github ci is not trivial. For now the tests only serve as an example usage.

The @tableland/studio-client package is going to need to be published. As an alternative to this PR, we could export this feature as a function in the @tableland/studio-client package. thoughts?

@joewagner joewagner requested a review from asutula October 2, 2023 22:19
// NOTE: In the future we may need to use `environmentId` instead of `projectId`, but
// there is currently no concept of an environment for a user, and the api doesn't
// support querying for deployments based on environment.
export function studioAliases(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the meat of the feature. The studio project name map is loaded and used for read and insert only.

return _map;
},
write: async function () {
throw new Error("cannot create project tables via studio sdk aliases");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The write function may never be used since creating tables must be done via Studio

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect.

@@ -1,6 +1,6 @@
{
"name": "@tableland/sdk",
"version": "4.5.3-dev.0",
"version": "4.6.0-pre.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will need a minor version bump

const aliases = studioAliases(
"6f254b66-d9cf-482b-a4b9-76cfe5eb2f19",
"https://studio-neon.vercel.app/"
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

usage with the SDK is fairly straight forward. Just provide a projectId and the URL of the studio. We can remove the requirement for studio url once we have a permanent url.

@@ -1,3 +1,4 @@
import { api as studioApi } from "@tableland/studio-client";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The @tableland/studio-client package is not published yet. Anyone wanting to try out this feature will have to npm link. Linking between to two monorepos is tricky. I can help with that via voice if needed.

Copy link
Contributor

@asutula asutula left a comment

Choose a reason for hiding this comment

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

This is looking like a good start. I left some critical comments.

My opinion is that we should not actually do this work in the SDK for now. It's adding too much complexity with having to npm link monorepos for development, publish the @tableland/studio-client package, etc. Plus, the current API for this helper is very alpha right now since we haven't properly introduced environments (or databases if we call them that). For those reasons, I vote to just create a tiny package in the Studio monorepo that publishes this helper.

url: apiUrl,
});
const loadMap = async function (): Promise<void> {
const res = await api.deployments.projectDeployments.query({ projectId });
Copy link
Contributor

Choose a reason for hiding this comment

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

This only works now because there is a single environment called default. Once there are multiple environments, you'll end up with duplicate aliases in the map. Actually they will just overwrite each other and the map will end up with a mix of tables from different environments.

The root of this problem is using projectId as the parameter. We should have the user pass in the environment id. We can call it something opaque sounding like key or whatever since "environment id" isn't a user facing concept yet.

If we do that, we can create a new api function to return deployments for an environment id, then the map will be accurate.

// map the response to a `NameMapping` Object
// { tokenId: string; tableId: string; tableName: string; environmentId: string; chainId: number; blockNumber: number | null; txnHash: string | null; createdAt: string; }
res.forEach(function (row) {
_map[row.tableName.split("_").slice(0, -2).join("_")] = row.tableName;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not quite right. You're keying the map based on the Tableland table name. We want to key it on the Studio Table name in the Blueprint. It is true that these should be the same, but if we introduce the ability to rename a Studio Table, they will be different. They can also be different for imported tables because the user chooses the Studio table name for the imported Tableland table.

This can be easily addressed when we implement the deploymentsForEnvId function mentioned in the above comment... we can join against the tables table to get the Studio table name.

return _map;
},
write: async function () {
throw new Error("cannot create project tables via studio sdk aliases");
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect.

@joewagner
Copy link
Contributor Author

My opinion is that we should not actually do this work in the SDK for now. It's adding too much complexity with having to npm link monorepos for development, publish the @tableland/studio-client package, etc. Plus, the current API for this helper is very alpha right now since we haven't properly introduced environments (or databases if we call them that). For those reasons, I vote to just create a tiny package in the Studio monorepo that publishes this helper.

I like the idea of moving the helper to the studio repo. What about putting it in the client package?

@asutula
Copy link
Contributor

asutula commented Oct 3, 2023

Sounds good to me.

@joewagner
Copy link
Contributor Author

replaced by: tablelandnetwork/studio#85

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