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

feat: sdk #16

Merged

Conversation

gentlementlegen
Copy link
Member

Resolves #15

gentlementlegen and others added 12 commits October 14, 2024 14:26
Replace custom logger with @ubiquity-os/ubiquity-os-logger and update types.
Removed server start code and simplified plugin initialization.
Removed @hono/node-server from dependencies in package.json and updated GitHub Action workflow.
Updated wrangler version, renamed node_compat to nodejs_compat, and enabled observability.
Updated wrangler.toml configuration, package scripts, and dependency versions.
Replaces console log and explicit return with simplified promise chaining.
Changed type casting in tests from PluginInputs to CommandContext to match updated codebase.
Renamed src/worker.ts to src/index.ts and updated relevant references.
Copy link
Contributor

github-actions bot commented Oct 14, 2024

Unused dependencies (2)

Filename dependencies
package.json @octokit/rest
@octokit/webhooks

@gentlementlegen
Copy link
Member Author

/query @gentlementlegen

Copy link
Contributor

ubiquity-os bot commented Oct 14, 2024

Property Value
Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED

@gentlementlegen
Copy link
Member Author

/query @gentlementlegen

Copy link
Contributor

ubiquity-os bot commented Oct 14, 2024

Property Value
Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED

@gentlementlegen
Copy link
Member Author

@whilefoo I had a question. Now that we have the SDK, I included it in this plugin. Everything seems fine but now every time it runs the verification fails. Shall this plugin have something set in the environment (although I see there is already a public key included) or is it something missing on the kernel side?

PS: I guess it will void this pull request: #4

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Oct 15, 2024

@whilefoo thank you I missed that step, I got it to work with my own workers.

QA: Meniole#4 (comment)

That raises a few questions on my side:

First, the SDK will probably have to be changed because for the environment it reads process.env, which exists within Cloudflare but is not populated with the Env set by wrangler, which instead passes it as an argument of fetch. That forced me to wrap the creation of the kernel like so, and the environment validation cannot be run by the SDK.

Second, what is exactly the purpose of the key validation? The kernel configuration can only be changed by admins so it will call only the plugins we allow. And on the plugin side, since we embed the public key, anyone who would fork the plugin could have potentially this fork running if somehow they get to set the URL in the configuration? Maybe I miss something but I can't see what this protection is for. I just understood that it is the other way around, to avoid having a kernel calling any plugin freely.

@gentlementlegen
Copy link
Member Author

/query @gentlementlegen

Copy link
Contributor

ubiquity-os bot commented Oct 15, 2024

Property Value
Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED

@whilefoo
Copy link
Contributor

First, the SDK will probably have to be changed because for the environment it reads process.env, which exists within Cloudflare but is not populated with the Env set by wrangler, which instead passes it as an argument of fetch.

The Hono server automatically reads the env from Cloudflare fetch or from process.env if it's run in Node.
I didn't even notice that I'm validating the env from the process.env instead of from Hono context.

gentlementlegen and others added 3 commits October 16, 2024 15:32
Added envSchema to ensure environment variables are validated.
Updated @ubiquity-os/ubiquity-os-kernel to version 2.3.0.
@gentlementlegen
Copy link
Member Author

@whilefoo Updated and upgraded the kernel, tried to simplify the fetch but I don't think I can just export default because I need the env.KERNEL_PUBLIC_KEY to be passed in the options of createPlugin.

wrangler.toml Show resolved Hide resolved
@gentlementlegen gentlementlegen merged commit b8091a0 into ubiquity-os-marketplace:development Oct 18, 2024
@gentlementlegen gentlementlegen deleted the feat/sdk branch October 18, 2024 23:58
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.

Update logger and plug SDK
3 participants