Minor Server Tweaks #612
Minor Server Tweaks #612
Conversation
|
@slundberg if you choose to merge this, please use the description as the commit message. The individual commit messages will not be very helpful. |
guidance/models/_model.py
Outdated
| class RemoteModel(Model): | ||
| def __init__(self, endpoint, echo=True, **kwargs): | ||
| from ._remote import RemoteEngine | ||
|
|
||
| engine = RemoteEngine(endpoint, **kwargs) | ||
| super().__init__(engine, echo=echo) |
There was a problem hiding this comment.
@riedgar-ms we're thinking we can just have users pass in a URL to the Model subclass directly to indicate a RemoteModel, instead of needing another layer
There was a problem hiding this comment.
So users would just do
m_remote = models.Model("https://my.url.endpoint/")?
There was a problem hiding this comment.
Yes (with appropriate auth params if needed too)
There was a problem hiding this comment.
This strikes me as a bad idea, but I've adjusted the code accordingly.
There was a problem hiding this comment.
I don't think users will want to use models.Model directly, because the client needs to know what kind of syntax to use for the model etc. So while we allow the base class to contain this, note that it mostly used in the subclass constructors
There was a problem hiding this comment.
I would have put RemoteModel back if this hadn't already been merged.....
| @@ -603,7 +607,7 @@ def __init__(self): | |||
| class Join(GrammarFunction): | |||
| __slots__ = ("nullable", "values", "name", "hidden", "commit_point", "capture_name", "max_tokens") | |||
There was a problem hiding this comment.
Related to this... I'm seeing quite a few places where __slots__ contains "hidden", "commit_point", "capture_name" ... is there a base class lurking beneath these? Should it be GrammarFunction itself?
There was a problem hiding this comment.
There probably is :) ...however we can address that in another PR. I think we are likely to call these grammar objects something like "GuidanceScript" objects in the future (similar to torchscript), because they will encode more than similar grammar idioms.
|
This looked good to me so I merged! Any updates can fall in later PRs |
|
Been on travel :) |
Some minor tweaks to the server code:
.gitignoreto skip binary produced bysetup.pyon Windowsresponse.raise_for_status()in the server code