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

fix: trigger Action for base rate update #72

Draft
wants to merge 14 commits into
base: development
Choose a base branch
from

Conversation

gentlementlegen
Copy link
Member

Resolves #69

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Jan 14, 2025

@whilefoo Struggling a bit here. The idea is that when it is a push event, I want to forward the event to the corresponding action from the worker. This part seems to work fine, but every time I get "invalid signature" from the Action run.

I thought the order of the inputs was changing, but even when manually ordering them the problem remained the same. Two questions:

  • do you think this is the right approach to mix actions and workers without needing to specify both in the configuration
  • are signature from payloads incompatible between workers and actions (and eventually I would need to sign the payload again to forward it)?

Edit: to answer my own question, it seems that indeed both payloads are different which most likely implies a different signature:
https://github.com/ubiquity-os/ubiquity-os-kernel/blob/c5b77f32fc1518d5bf5ed525dcc185d429b87c9d/src/github/types/plugin.ts#L47-L80

Wouldn't it be beneficial to have the same payloads and let the SDK decode them properly so both payloads can be forwarded seamlessly between workers and actions? This would most likely be helpful for plugins needing both like telegram or much likely the user personal agent, that has a very complex bridge at the moment.

@whilefoo
Copy link
Member

  • are signature from payloads incompatible between workers and actions (and eventually I would need to sign the payload again to forward it)?

Yeah I was also thinking about making the payloads the same in actions and workers, Action plugins were added later and it only supports strings and not objects so that's why the worker still has objects

  • do you think this is the right approach to mix actions and workers without needing to specify both in the configuration

I like to avoid that approach but if there's no other way then it's fine. What is the problem that you're trying to solve by mixing actions and workers?

@gentlementlegen
Copy link
Member Author

Okay I'll test having the same payload and same signature for both and see how that goes.

What I am trying to solve is that because this plugin should react quickly on label change (so a Worker), but when it is running due to changes in the base multiplier, it has to check all the issues and update the price which is slow (so an Action). Recently everything was moved to an action due to the base rate change handling, but it would be much more user friendly to have instant price change on label change and have the long running update in the background (which is essentially what the spec describes).

@0x4007
Copy link
Member

0x4007 commented Jan 15, 2025

Well we can make this just a Worker and then make a different plugin that handles only base rate changes as an Action.

@gentlementlegen
Copy link
Member Author

That would be nice to be able to have hybrids easily because this would not be the only plugin who would benefit from this. And splitting the functionality to another plugin would mean copy/paste the configuration for each configuration using this plugin, which is why it is not preferable.

@0x4007
Copy link
Member

0x4007 commented Jan 15, 2025

Perhaps the telegram bridge can be used as reference but I'm skeptical it's the best implementation of a dual runtime plugin.

@whilefoo
Copy link
Member

What I am trying to solve is that because this plugin should react quickly on label change (so a Worker), but when it is running due to changes in the base multiplier, it has to check all the issues and update the price which is slow (so an Action). Recently everything was moved to an action due to the base rate change handling, but it would be much more user friendly to have instant price change on label change and have the long running update in the background (which is essentially what the spec describes).

Maybe what we need to do is move away from Cloudflare because it doesn't have Node and has low time limits. If we use another platform (Azure?) that has decent start-up time, has Node and higher time limits then we wouldn't even need to use Actions.

@Keyrxng
Copy link
Contributor

Keyrxng commented Jan 15, 2025

Maybe what we need to do is move away from Cloudflare because it doesn't have Node and has low time limits. If we use another platform (Azure?) that has decent start-up time, has Node and higher time limits then we wouldn't even need to use Actions.

  1. I like it because CF can be temperamental at times and is v restrictive because of env and limits.
  2. I thought we used CF and leverage actions to keep overheads to a bare minimum, ideally, cost free? Wouldn't Azure incur costs (even if subsidised when partnered with MS)? And if so then maybe just a paid CF plan might be the fix

@gentlementlegen
Copy link
Member Author

@whilefoo and maybe it would fix the random skipping of events we currently experience. however this long running task of updating all the labels is really long (around 10 to 15 minutes depending of the amount of issues) which wouldn't probably fit in Azure either.

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.

Remove Base Rate Logic
4 participants