-
Notifications
You must be signed in to change notification settings - Fork 2.8k
chore: Add goose providers list command #116
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
fa0b0a1
637da8e
35de085
3073f34
c412c16
376d35c
c9a5f73
cdbc0ac
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 |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import os | ||
| from abc import ABC, abstractmethod | ||
| from attrs import define, field | ||
| from typing import List, Optional, Tuple, Type | ||
|
|
@@ -14,10 +15,19 @@ class Usage: | |
|
|
||
|
|
||
| class Provider(ABC): | ||
| PROVIDER_NAME: str | ||
| REQUIRED_ENV_VARS: list[str] = [] | ||
|
|
||
| @classmethod | ||
| def from_env(cls: Type["Provider"]) -> "Provider": | ||
| return cls() | ||
|
|
||
| @classmethod | ||
| def check_env_vars(cls: Type["Provider"], instructions_url: Optional[str] = None) -> None: | ||
|
Collaborator
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. Nice! |
||
| for env_var in cls.REQUIRED_ENV_VARS: | ||
| if env_var not in os.environ: | ||
| raise MissingProviderEnvVariableError(env_var, cls.PROVIDER_NAME, instructions_url) | ||
elenazherdeva marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| @abstractmethod | ||
| def complete( | ||
| self, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,28 @@ def list_toolkits() -> None: | |
| print(f" - [bold]{toolkit_name}[/bold]: {first_line_of_doc}") | ||
|
|
||
|
|
||
| @goose_cli.group() | ||
| def providers() -> None: | ||
| """Manage providers""" | ||
| pass | ||
|
|
||
|
|
||
| @providers.command(name="list") | ||
| def list_providers() -> None: | ||
|
Collaborator
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. maybe write a test for this function?
Contributor
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. kinda hacky but if the |
||
| providers = load_plugins(group="exchange.provider") | ||
|
|
||
| for provider_name, provider in providers.items(): | ||
| lines_doc = provider.__doc__.split("\n") | ||
| first_line_of_doc = lines_doc[0] | ||
| print(f" - [bold]{provider_name}[/bold]: {first_line_of_doc}") | ||
| envs = provider.REQUIRED_ENV_VARS | ||
| if envs: | ||
| env_required_str = ", ".join(envs) | ||
| print(f" [dim]env vars required: {env_required_str}") | ||
|
Contributor
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. :nit: since these are required i think they should be emphasized instead of |
||
|
|
||
| print("\n") | ||
|
|
||
|
|
||
| def autocomplete_session_files(ctx: click.Context, args: str, incomplete: str) -> None: | ||
| return [ | ||
| f"{session_name}" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| from goose.toolkit.lint import lint_toolkits | ||
|
|
||
| from goose.toolkit.lint import lint_providers | ||
|
|
||
|
|
||
| def test_lint_toolkits(): | ||
| lint_toolkits() | ||
|
|
||
|
|
||
| def test_lint_providers(): | ||
| lint_providers() | ||
|
Collaborator
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 guess you followed the existing implementation pattern. I am not sure whether
Contributor
Author
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. @lukealvoeiro can describe it better, but the reason behind this is that
Contributor
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. Yup, we want this function to be exported so we can lint this in any open source or internal plugin repos as well. |
||
Uh oh!
There was an error while loading. Please reload this page.