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 url property to manifest.json #33

Closed
rndquu opened this issue Nov 26, 2024 · 44 comments · Fixed by #36
Closed

Add url property to manifest.json #33

rndquu opened this issue Nov 26, 2024 · 44 comments · Fixed by #36

Comments

@rndquu
Copy link
Member

rndquu commented Nov 26, 2024

We need to know an exact plugin URL (in case it works as a cloudflare worker) in order to set it in the partner onboarding UI.

What should be done:

  1. Add a new optional url property to manifest.json
  2. Update the worker-deploy.yml workflow to set the url property in manifest.json on each deployment of the development branch. I think it's enough for now to use the development plugin instances + https://github.com/ubiquity-os/ubiquity-os-plugin-installer will simply have to fetch a default plugin's github branch in order to get a plugin URL.
  3. Make sure all plugins from https://github.com/ubiquity-os-marketplace have the url property set in the manifest.json file and updated worker-deploy.yml workflow

Original comment

@rndquu
Copy link
Member Author

rndquu commented Nov 26, 2024

@gentlementlegen Pls check the description

@gentlementlegen
Copy link
Member

How do we deal with plugins that offers both Actions and Workers like the telegram chatroom?

@rndquu
Copy link
Member Author

rndquu commented Nov 26, 2024

How do we deal with plugins that offers both Actions and Workers like the telegram chatroom?

I don't have much context on the telegram kernel but it seems the plugin architecture must be optimized if it requires both github action and a worker.

@Keyrxng
Copy link
Contributor

Keyrxng commented Nov 26, 2024

How do we deal with plugins that offers both Actions and Workers like the telegram chatroom?

We could do: manifest.urls.action: ...

I don't have much context on the telegram kernel but it seems the plugin architecture must be optimized if it requires both github action and a worker.

It's split into worker/action because we needed a node env to run MTProto API to implement the chatroom feature.

It should be converted to a single worker plugin like it was originally now that 0x4007 decided to export the KERNEL_PRIVATE_KEY as an org secret,, so it can be simplified in the sense of it requiring only one config entry.

@rndquu
Copy link
Member Author

rndquu commented Nov 27, 2024

@gentlementlegen I need you blessing on this task.

I think plugins that work both as a worker and a github action are outliers and it's safe to introduce the url property in the manifest.json file.

@gentlementlegen
Copy link
Member

Sure, we only have one edge case so far. Maybe we can think of an option later like "install as Worker" and "install as Action". We can safely introduce the url property in the meantime.

@whilefoo
Copy link

whilefoo commented Dec 2, 2024

We need to know an exact plugin URL (in case it works as a cloudflare worker) in order to set it in the partner onboarding UI.

How will the onboarding UI know if the plugin is a worker or action?
We could have https:// if it's a worker and github://ubiquity-os/marketplace@main if it's an action

We could also have urls property that would have 2 urls for hybrid plugins but that would also require changes in the kernel.

It's split into worker/action because we needed a node env to run MTProto API to implement the chatroom feature.

It should be converted to a single worker plugin like it was originally now that 0x4007 decided to export the KERNEL_PRIVATE_KEY as an org secret,, so it can be simplified in the sense of it requiring only one config entry.

Don't we still need node runtime for MTPRroto, so how can we convert to single worker plugin?

@rndquu
Copy link
Member Author

rndquu commented Dec 2, 2024

How will the onboarding UI know if the plugin is a worker or action?

If the url property is present in the manifest.json file then it's a worker

Don't we still need node runtime for MTPRroto, so how can we convert to single worker plugin?

@Keyrxng knows better

@0x4007
Copy link
Member

0x4007 commented Dec 13, 2024

We need to know an exact plugin URL (in case it works as a cloudflare worker) in order to set it in the partner onboarding UI.

This is no longer the case so perhaps this can be closed as unplanned.

@rndquu
Copy link
Member Author

rndquu commented Dec 13, 2024

We need to know an exact plugin URL (in case it works as a cloudflare worker) in order to set it in the partner onboarding UI.

This is no longer the case so perhaps this can be closed as unplanned.

How to get a plugin URL for a 3rd party plugin created by some other developer?

@0x4007
Copy link
Member

0x4007 commented Dec 13, 2024

I don't have full context on this repository but specifically in the plugin installer UI I fixed this problem.

@gentlementlegen
Copy link
Member

The problem is that the plugin installer cannot guess the URL to put it in the configuration which is the problem. It only knows the GitHub repository. One workaround would be if we had deployment urls linked within the github repo we could use these.

@rndquu
Copy link
Member Author

rndquu commented Dec 17, 2024

The problem is that the plugin installer cannot guess the URL to put it in the configuration which is the problem. It only knows the GitHub repository. One workaround would be if we had deployment urls linked within the github repo we could use these.

Then we also have to force partners to setup github deployment URLs which might not be the too convenient for them.

A single url property is basically a "set and forget" approach when partners set plugin URL in the manifest.json only once to the production URL and never(?) get back to it.

@gentlementlegen
Copy link
Member

/start

@gentlementlegen
Copy link
Member

/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

@gentlementlegen
Copy link
Member

The schema for start/stop has changed and it is not up to date here which is why the command didn't work. fixing.

@gentlementlegen
Copy link
Member

/start

@gentlementlegen
Copy link
Member

My understanding is that it happens the other way around:
the plugin installer needs to know if the plugin you're installing should be treated as a worker / action, so that it generates the proper value in the ubiquity-os.config.yml file by settings either a url to the worker, or a org/repo instead, which is why we need the URL in the manifest to be present.

@0x4007
Copy link
Member

0x4007 commented Dec 17, 2024

Then why don't we just try calling it as an external url first and if it fails then run as action

@gentlementlegen
Copy link
Member

How do you guess the deployment url? This is why the proposal makes it populated during the deployment step

@whilefoo
Copy link

My understanding is that it happens the other way around:
the plugin installer needs to know if the plugin you're installing should be treated as a worker / action, so that it generates the proper value in the ubiquity-os.config.yml file by settings either a url to the worker, or a org/repo instead, which is why we need the URL in the manifest to be present.

This is a fine approach but another way would be that plugin installer sets org/repo in the ubiquity-os.config.yml and the kernel will check the URL in the manifest or the lack of URL would imply it's an Action. (anyway the kernel already fetches the manifest).

This way we also solve the problem with changing URLs, because if the URL is in the manifest then the kernel can check it and you don't need to update URLs.

I was only addressing @0x4007 concern that we couldn't use closed-source plugins anymore, but it's not true because you could still specify a https URL in the ubiquity-os.config.yml.

@gentlementlegen
Copy link
Member

@whilefoo I like this idea because that would streamline the configuration file which would only contain org/repo for any plugin, and you would never have to think about "is this a plugin or a worker".

However we have that one plugin for telegram that uses both, but that's the only one (we might want to look into it to change that).

@0x4007
Copy link
Member

0x4007 commented Dec 17, 2024

I don't think we should make accommodations based on the telegram plugin architecture.

To be honest I haven't been able to benefit from it, like subscribing to events never worked etc. there's a high chance it will be completely redone in the future.

@rndquu
Copy link
Member Author

rndquu commented Dec 18, 2024

My understanding is that it happens the other way around:
the plugin installer needs to know if the plugin you're installing should be treated as a worker / action, so that it generates the proper value in the ubiquity-os.config.yml file by settings either a url to the worker, or a org/repo instead, which is why we need the URL in the manifest to be present.

This is a fine approach but another way would be that plugin installer sets org/repo in the ubiquity-os.config.yml and the kernel will check the URL in the manifest or the lack of URL would imply it's an Action. (anyway the kernel already fetches the manifest).

This way we also solve the problem with changing URLs, because if the URL is in the manifest then the kernel can check it and you don't need to update URLs.

I was only addressing @0x4007 concern that we couldn't use closed-source plugins anymore, but it's not true because you could still specify a https URL in the ubiquity-os.config.yml.

Added 2 tasks, for the kernel and UI installer:

Copy link

+ Evaluating results. Please wait...

Copy link

 [ 153.855 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueTask1150
IssueComment43.855
Conversation Incentives
CommentFormattingRelevancePriorityReward
How do we deal with plugins that offers both Actions and Workers…
1.06
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.530.4125
Sure, we only have one edge case so far. Maybe we can think of a…
2.05
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 35
  wordValue: 0.1
  result: 2.05
131.53
The problem is that the plugin installer cannot guess the URL to…
2.54
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 45
  wordValue: 0.1
  result: 2.54
131.92
The schema for start/stop has changed and it is not up to date h…
1.49
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 24
  wordValue: 0.1
  result: 1.49
03-0.0075

 [ 150 WXDAI ] 

@rndquu
⚠️ Your rewards have been limited to the task price of 150 WXDAI.
Contributions Overview
ViewContributionCountReward
IssueSpecification195.16
IssueComment756.208
ReviewComment11.365
Conversation Incentives
CommentFormattingRelevancePriorityReward
We need to know an exact plugin URL (in case it works as a cloud…
31.72
content:
  content:
    p:
      score: 0
      elementCount: 6
    ol:
      score: 0
      elementCount: 1
    li:
      score: 0.5
      elementCount: 3
    a:
      score: 5
      elementCount: 5
  result: 26.5
regex:
  wordCount: 105
  wordValue: 0.1
  result: 5.22
1395.16
@gentlementlegen Pls check the description
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.230.234
I don't have much context on the telegram kernel but it seems th…
1.7
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 28
  wordValue: 0.1
  result: 1.7
0.532.55
@gentlementlegen I need you blessing on this task.I think plug…
2
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 34
  wordValue: 0.1
  result: 2
136
If the `url` property is present in the `manifest.js…
1.06
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.531.59
How to get a plugin URL for a 3rd party plugin created by some o…
1.06
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.832.544
Then we also have to force partners to setup github deployment U…
2.78
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 50
  wordValue: 0.1
  result: 2.78
138.34
Added 2 tasks, for the kernel and UI installer:- https://githu…
11.65
content:
  content:
    p:
      score: 0
      elementCount: 3
    ul:
      score: 0
      elementCount: 1
    li:
      score: 0.5
      elementCount: 2
    a:
      score: 5
      elementCount: 2
  result: 11
regex:
  wordCount: 9
  wordValue: 0.1
  result: 0.65
1334.95
@gentlementlegen So `homepage_url` is basically the work…
0.65
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 9
  wordValue: 0.1
  result: 0.65
0.731.365

 [ 25.02 WXDAI ] 

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueComment125.02
Conversation Incentives
CommentFormattingRelevancePriorityReward
We could do: `manifest.urls.action: ...`It's split int…
8.34
content:
  content:
    p:
      score: 0
      elementCount: 3
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 62
  wordValue: 0.1
  result: 3.34
1325.02

 [ 65.322 WXDAI ] 

@whilefoo
Contributions Overview
ViewContributionCountReward
IssueComment435.94
ReviewComment429.382
Conversation Incentives
CommentFormattingRelevancePriorityReward
How will the onboarding UI know if the plugin is a worker or act…
3.88
content:
  content:
    p:
      score: 0
      elementCount: 4
  result: 0
regex:
  wordCount: 74
  wordValue: 0.1
  result: 3.88
1311.64
Also we can still support raw https URLs, so if it's a https URL…
2.83
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 51
  wordValue: 0.1
  result: 2.83
138.49
I meant if `org/repo` is set in the repo config, for man…
1.44
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 23
  wordValue: 0.1
  result: 1.44
030
This is a fine approach but another way would be that plugin ins…
5.27
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 106
  wordValue: 0.1
  result: 5.27
1315.81
I think it's a good idea to make this a shared workflow so it wo…
1.65
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 27
  wordValue: 0.1
  result: 1.65
0.733.465
Isn't it better to use `github.sha` so that you're worki…
2.59
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 46
  wordValue: 0.1
  result: 2.59
0.936.993
do we need to force?
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.430.468
Just did some research and it's possible to make it appear like …
6.44
content:
  content:
    p:
      score: 0
      elementCount: 1
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 23
  wordValue: 0.1
  result: 1.44
0.8318.456

 [ 13.113 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment47.308
ReviewComment35.805
Conversation Incentives
CommentFormattingRelevancePriorityReward
This is no longer the case so perhaps this can be closed as unpl…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
0.531.41
I don't have full context on this repository but specifically in…
1.28
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 20
  wordValue: 0.1
  result: 1.28
0.732.688
Then why don't we just try calling it as an external url first a…
1.38
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 22
  wordValue: 0.1
  result: 1.38
0.431.656
I don't think we should make accommodations based on the telegra…
2.59
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 46
  wordValue: 0.1
  result: 2.59
0.231.554
Ensure it's part of the manifest specification. If not then pref…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
0.832.256
@UbiquityOS is url a standard property part of the manifest.json…
0.83
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 12
  wordValue: 0.1
  result: 0.83
0.531.245
Not sure if it's for the same purpose. If it isn't then prefix i…
1.28
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 20
  wordValue: 0.1
  result: 1.28
0.632.304

@gentlementlegen
Copy link
Member

gentlementlegen commented Dec 18, 2024

I will updat all the related plugins that should be treated as workers and link the pull-requests here.

@gentlementlegen
Copy link
Member

I think this is supposed to be a long-running Action? rfc @sshivaditya2019

@rndquu
Copy link
Member Author

rndquu commented Dec 18, 2024

I think this is supposed to be a long-running Action? rfc @sshivaditya2019

Right now it is set as a worker

@gentlementlegen
Copy link
Member

And it is set as an Action (actually it is set even twice). My guess is that it started as a Worker but was a long running process and eventually moved to an Action.

Seems to run fine as an Action: https://github.com/ubiquity-os-marketplace/text-vector-embeddings/actions/workflows/compute.yml

@rndquu
Copy link
Member Author

rndquu commented Dec 18, 2024

And it is set as an Action (actually it is set even twice). My guess is that it started as a Worker but was a long running process and eventually moved to an Action.

Seems to run fine as an Action: https://github.com/ubiquity-os-marketplace/text-vector-embeddings/actions/workflows/compute.yml

In other orgs it's set as a worker (one, two).

@sshivaditya2019 How should https://github.com/ubiquity-os-marketplace/text-vector-embeddings plugin be set, as a worker or a github action? Or it doesn't matter?

Overall, as far as I remember, the general concept was that for "easy" plugins that run fast we use workers while for long running plugins we use github actions. https://github.com/ubiquity-os-marketplace/text-vector-embeddings basically saves data to a DB on a request so it falls in the "easy" plugins section.

@gentlementlegen
Copy link
Member

Now that our plugins use the SDK and hono basically all can run on either workers or actions so indeed it just depends on their run duration. I am fine having it as a worker as long as it is fast! Will open a PR then.

@whilefoo
Copy link

It seems to run only 10 seconds but I would keep it as an Action because Cloudflare has tight time limits so it might timeout and we don't need it to be fast because it's not responding to users

@gentlementlegen
Copy link
Member

Worst case we can just disable the workflow that deploys to cloudflare and remove the url from the manifest to get it back as actions.

@rndquu rndquu removed this from Development Dec 19, 2024
@Keyrxng
Copy link
Contributor

Keyrxng commented Dec 20, 2024

However we have that one plugin for telegram that uses both, but that's the only one (we might want to look into it to change that).

I don't think we should make accommodations based on the telegram plugin architecture.

To be honest I haven't been able to benefit from it, like subscribing to events never worked etc. there's a high chance it will be completely redone in the future.

lol guys I have said it like 5 times now. @0x4007 exposed the private key via org secrets after it holding back the telegram bot for a week or so in debate, so it doesn't need to exist like it does now.

It ONLY exists like that because review pushed me that way but it should be a single worker entry.


Glad you came to a resolution with it @gentlementlegen

@0x4007
Copy link
Member

0x4007 commented Dec 20, 2024

so it doesn't need to exist like it does now.

You can file a proposal so we can fix it

@Keyrxng
Copy link
Contributor

Keyrxng commented Dec 20, 2024

so it doesn't need to exist like it does now.

You can file a proposal so we can fix it

ubiquity-os-marketplace/ubiquity-os-kernel-telegram#20

Been open for ages

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

Successfully merging a pull request may close this issue.

5 participants