feat(protocol): add update subscription message type #2192
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
Add "(aside)" to your comment to have me ignore it. |
There was a problem hiding this comment.
I'm assuming that we intend to add support for this event to Lazer? Also, I believe we do need to tag a release for the SDK in order to use this change. I wonder if there's a way to add a GitHub CI to automatically make a release if a change to this line is detected. Would this be something we would want to add?
| pub subscription_id: SubscriptionId, | ||
| pub add_price_feed_ids: Vec<PriceFeedId>, | ||
| pub remove_price_feed_ids: Vec<PriceFeedId>, | ||
| pub add_properties: Vec<PriceFeedProperty>, | ||
| pub remove_properties: Vec<PriceFeedProperty>, |
There was a problem hiding this comment.
I can understand updating price feed IDs. But, is subscription ID and properties something we want to allow updates for? I'm actually not even sure what these properties even are...
There was a problem hiding this comment.
I think we can allow updates for all parameters (except SubscriptionId obviously, which just specifies the subscription to update).
I'd like UpdateSubscriptionRequest to be idempotent. The user should specify full SubscriptionParams when updating subscription, so UpdateSubscriptionRequest should have the same structure as SubscribeRequest.
| pub subscription_id: SubscriptionId, | ||
| pub add_price_feed_ids: Vec<PriceFeedId>, | ||
| pub remove_price_feed_ids: Vec<PriceFeedId>, | ||
| pub add_properties: Vec<PriceFeedProperty>, | ||
| pub remove_properties: Vec<PriceFeedProperty>, |
There was a problem hiding this comment.
I think we can allow updates for all parameters (except SubscriptionId obviously, which just specifies the subscription to update).
I'd like UpdateSubscriptionRequest to be idempotent. The user should specify full SubscriptionParams when updating subscription, so UpdateSubscriptionRequest should have the same structure as SubscribeRequest.
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct SubscriptionUpdatedResponse { | ||
| pub subscription_id: SubscriptionId, | ||
| } |
There was a problem hiding this comment.
I think Response::Subscribed will be a suitable response. Let's try to keep the API minimal and not add this one.
devin-ai-integration
bot
force-pushed
the
devin/1734020211-add-update-subscription
branch
from
3aeddab to
61ba4e1
Compare
Co-Authored-By: Jayant Krishnamurthy <[email protected]>
Co-Authored-By: Jayant Krishnamurthy <[email protected]>
devin-ai-integration
bot
force-pushed
the
devin/1734020211-add-update-subscription
branch
from
e84d163 to
175631a
Compare
Description
Add UpdateSubscription message type to allow dynamic subscription updates.
Changes
Testing
Implementation Details
Link to Devin run: https://app.devin.ai/sessions/9c04ee7e379a442ab53ad30c4a689203