-
Notifications
You must be signed in to change notification settings - Fork 30
Add support for server addons #454
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,6 +96,14 @@ def initialize | |
raise InitializationError, @stderr.read | ||
end | ||
|
||
sig { params(server_addon_path: String).void } | ||
def register_server_addon(server_addon_path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 perhaps we don't.... @vinistock ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I wanted to bring up for context that we need a way to limit who can use our addon. Right now we can do it in the addon while triggering There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They run on different Ruby processes, so I'm not sure how we could combine theses two. Also, why do we need to limit who uses the addon? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought it wasn't necessary but now I see that
We need to do some testing first on a partial rollout. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We can manage that on the Tapioca side though, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, there's no way to perform a partial rollout. But we could inform addons if experimental features are enabled so that they can decide whether to do something or not. That would give you some control. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said above, with this implementation, rollout can be controlled by the individual addons through the trigger of |
||
send_notification("server_addon/register", server_addon_path: server_addon_path) | ||
rescue IncompleteMessageError | ||
$stderr.puts("Ruby LSP Rails failed to register server addon #{server_addon_path}") | ||
nil | ||
end | ||
|
||
sig { params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) } | ||
def model(name) | ||
make_request("model", name: name) | ||
|
Uh oh!
There was an error while loading. Please reload this page.