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

Add "json as a DB" methods #30

Open
rndquu opened this issue Nov 20, 2024 · 20 comments
Open

Add "json as a DB" methods #30

rndquu opened this issue Nov 20, 2024 · 20 comments

Comments

@rndquu
Copy link
Member

rndquu commented Nov 20, 2024

It makes sense to add SDK methods for storing arbitrary json in the .ubiquity-os partner repository.

@Keyrxng implemented smth similar but I'm not sure if it's production ready.

What should be done:

  1. Add SDK methods for storing and reading arbitrary json values from a single file located in the .ubiquity-os partner repository. You may use a 3rd party npm package like https://github.com/typicode/lowdb or https://github.com/Belphemur/node-json-db of https://github.com/gitrows/gitrows. In the end each plugin should correspond to its own plugin-name.json file.
  2. Add a quickstart section in the readme to show how to install the SDK package
  3. Add json related API methods in the readme (ideally they should be generated automatically using smth like https://jsdoc.app/)

Right now our biggest DB table is the permits table with 314 rows (except for the logs table which has 1M rows). This is not that much. In theory we could ditch the supabase DB at all and use only json files as a DB until we hit optimization issues. Json as a DB has lots of cons but simplicity (in some cases) outweighs them.

I feel like it's better to use a "single json file per plugin" approach compared to making supabase DB consistent and maintaining its single instance for all of the core plugins.

@rndquu
Copy link
Member Author

rndquu commented Nov 20, 2024

@gentlementlegen @whilefoo @EresDev Guys pls check the description if it's worth implementing this feature

@0x4007
Copy link
Member

0x4007 commented Nov 21, 2024

This method should also automatically set and get the file name as the organization/repository (or fallback to the package.json) name automatically.

That way we will always have separate JSON files per plugin.

In this repository it would be ubiquity-os/plugin-sdk.json

@gentlementlegen
Copy link
Member

@rndquu As long as you can solve the fact that wallets are linked to permits and users, that with this change would theoretically be in 3 separate json files, that's ok I guess.

@rndquu
Copy link
Member Author

rndquu commented Nov 21, 2024

@rndquu As long as you can solve the fact that wallets are linked to permits and users, that with this change would theoretically be in 3 separate json files, that's ok I guess.

We will have to introduce:

  1. Json schema typing (so that plugins that use some other plugin's json storage knew the exact data schema).
  2. Plugin's peer dependencies (similar to npm's one). So that if the text-conversation-rewards plugin is used then the /wallet plugin must also be set in the bot's config.

@gentlementlegen
Copy link
Member

We will probably also have to handle:

  • multiple simultaneous read / writes
  • merge strategies
  • migration and schema validation

@Keyrxng
Copy link

Keyrxng commented Nov 21, 2024

@Keyrxng implemented smth similar but I'm not sure if it's production ready.

It's pretty much ready yeah. I couldn't use it for kernel-telegram because I needed to store a sensitive session_string which meant it would have to be encrypted if pushed to .ubiquity-os, so the methods should have a param like {private: true} and the SDK handles encrypt/decrypt.

multiple simultaneous read / writes

I figured as long as I pull the most recent sha as close to the write as possible and retry by fetching a fresh sha each time we'd probs be okay but reads could become stale pretty quickly.

use only json files as a DB until we hit optimization issues...I feel like it's better to use a "single json file per plugin"

My thoughts when building what I did:

  1. Eventually we'll hit the file limit and have to split into multiple files and then have to deal with that, so build with "one json per table" from the beginning to save headaches down the line.
  2. read/write to a single file will become a v expensive operation esp in CF worker plugins
  3. One wrong move and you could negatively affect other "tables" you had no reason to touch
  4. Orgs could have standardized global objects such as UserBase A) UserBase.json is clearer than command-wallet.json for new devs B) kernel-telegram also has /register but is independent of wallet but collects the same info and more, would be nice if they could register a user globally. These stdized global objects we create for the org during setup and through the SDK are easier to work with from a plugin dev standpoint vs having to know 1. I need to fetch from "wallet" 2. I need to fetch from "users".... and cleaner than us doing that under the hood

@EresDev
Copy link

EresDev commented Nov 22, 2024

Interesting idea, but I am hesitant. If we go with JSON files, plugins are going to end up dealing with database concerns like integrity constraints. This adds to the responsibility of plugins and diverts their attention from their actual concerns. The DBMS is offering us out of the box what we need now and what we will need in future.

As given in the issue specs, the goal is to simplify. It is true if the JSON files are going to be modified rarely, like config files. But I see these files will be updated regularly. It seems to me it complicates things instead.

@gentlementlegen
Copy link
Member

To me it seems more like we are trying to get free hosting rather than simplifying things. Because we technically could have one Supabase DB per project which would be very simple to setup and to use, but expensive to host on Supabase itself. And we are still tied to Supabase when it comes to login and user sessions, which won't be solved by using JSON.

@Keyrxng
Copy link

Keyrxng commented Nov 28, 2024

I'm happy to continue on with my implementation and take on the burdens presented here. The vibe in this task feels like it headings toward a big fat no against implementing it but I also keep seeing @0x4007 repeating elsewhere that we need to get rid of the DB dependency and implement GH storage.

I'm free to work on this if it's a priority and no-one else is free to

@0x4007
Copy link
Member

0x4007 commented Nov 28, 2024

To me it seems more like we are trying to get free hosting rather than simplifying things. Because we technically could have one Supabase DB per project which would be very simple to setup and to use, but expensive to host on Supabase itself. And we are still tied to Supabase when it comes to login and user sessions, which won't be solved by using JSON.

There's no maintenance on the authentication app. Because of this, no contributors need to work on the database.

In order to implement authentication in any of our apps, two strings are required: the Supabase database url and the anon key. As I understand, these are public values and can be hard coded in the apps.

All the logic for apps to save and read custom app-specific data requires ongoing maintenance/changes to the database. This doesn't make sense in a DAO to manually add contributors to have access to the Supabase who may be transient (i.e. only work on one task.) QA also becomes more burdensome than it needs to be with manually adding contributors.

Something like Git based JSON storage makes the most sense for app maintenance by DAO contributors.

@rndquu
Copy link
Member Author

rndquu commented Nov 29, 2024

Ok, there's no consensus regarding whether we need to migrate all of our core plugins storages to json or not.

But at least there're no objections that we need SDK methods for managing json storages.

I suppose we could implement the methods and convert 1 plugin into json storage to see how it goes.

@rndquu
Copy link
Member Author

rndquu commented Dec 4, 2024

@gentlementlegen I've probably already asked but can't find the answer, do we need to install this app and why do we even need it?

Screenshot 2024-12-04 at 16 34 12

@gentlementlegen
Copy link
Member

As far as I recall it was created by @Keyrxng to solve the storage using json format, I don't know much more. @Keyrxng please explain 😄

@0x4007
Copy link
Member

0x4007 commented Dec 5, 2024

UbiquityOS is the only app that should be used.

@Keyrxng
Copy link

Keyrxng commented Dec 20, 2024

The storage app was created during the first PR of telegram bot where reviewers decided using the kernel's private key was too insecure, so I made another app so we could write to a private repo.

  1. Repo is no longer private.
  2. Kernel PK is now exposed via org secrets.

Not required anymore.

I had considered rate limits against our official app token as well but that was secondary to the above.

@sura1-0-1
Copy link

/help

Copy link

Available Commands

Command Description Example
/help List all available commands. /help
/allow Allows the user to modify the given label type. /allow @user1 label
/ask Ask any question about the repository, issue or pull request /ask
/query Returns the user's wallet, access, and multiplier information. /query @UbiquityOS
/start Assign yourself and/or others to the issue/task. /start
/stop Unassign yourself from the issue/task. /stop
/wallet Register your wallet address for payments. Use '/wallet unset' to unlink your wallet. /wallet ubq.eth

@sura1-0-1
Copy link

/start

Copy link

! You do not have the adequate role to start this task (your role is: member). Allowed roles are: collaborator, admin.

@sura1-0-1
Copy link

/help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants