feat: introduce Customizable Base URLs for API Providers #524
Conversation
WalkthroughThe pull request introduces modifications across several modules. In the account configuration, a new optional attribute ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as EditAccountDialog
participant A as Account Storage
U->>D: Open dialog
D->>D: init_ui() and init_data()
U->>D: Select provider and enter details (API key, organization, etc.)
D->>D: update_ui() (calls _set_api_key_data, _init_organization_data)
U->>D: Click OK
D->>D: _validate_form() & return validation result
alt Form Valid
D->>D: _save_account_data()
D->>A: Update/create account data (_update_existing_account / _create_new_account)
D->>U: Close dialog (success)
else Form Invalid
D->>U: Display error message
end
Possibly related PRs
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (3)
basilisk/gui/account_dialog.py (1)
357-442: UI initialization is well-structured and consistent.
Good job separating the creation of each control (provider label, combo, API key fields, etc.). The use of sizers appears appropriate for laying out the UI. Consider binding the combo event directly to a named handler (e.g. self.on_provider_changed) instead of using a lambda, for clarity.- self.provider_combo.Bind(wx.EVT_COMBOBOX, lambda e: self.update_ui()) + self.provider_combo.Bind(wx.EVT_COMBOBOX, self.on_provider_changed) def on_provider_changed(self, event): self.update_ui()basilisk/config/account_config.py (2)
124-143: Update docstring to reflect new validation behavior.The method now handles providers that don't require API keys, but this behavior isn't documented.
@field_validator("api_key", mode="before") @classmethod def validate_api_key( cls, value: Optional[Any], info: ValidationInfo ) -> Optional[SecretStr]: + """Validate API key based on storage method and provider requirements. + + Returns: + Optional[SecretStr]: The validated API key, or None if the provider doesn't require an API key. + + Raises: + ValueError: If the API key is invalid or missing when required. + """
163-174: Add custom_base_url validation in model validator.The
require_keysvalidator should ensure thatcustom_base_urlis only set when the provider allows it.@model_validator(mode="after") def require_keys(self) -> Account: if self.provider.require_api_key and not self.api_key: raise ValueError(f"API key for {self.provider.name} is required") + if self.custom_base_url and not self.provider.allow_custom_base_url: + raise ValueError(f"Custom base URL is not supported for {self.provider.name}") if ( not self.provider.organization_mode_available and self.active_organization_id ): raise ValueError( f"Organization mode is not available for {self.provider.name}" ) return self
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
basilisk/config/account_config.py(2 hunks)basilisk/gui/account_dialog.py(5 hunks)basilisk/provider.py(4 hunks)basilisk/provider_engine/openai_engine.py(1 hunks)
🔇 Additional comments (18)
basilisk/gui/account_dialog.py (15)
16-16: Good import of Provider and related functions.
No issues found with adding this import; it correctly references the new provider logic.
339-346: Constructor refactor with type hints looks clean.
The updated signature for EditAccountDialog and the use of type hints (e.g., tuple[int, int] and Optional[Account]) improves readability and maintainability.
443-465: init_data method provides a clear separation of concerns.
Initializing the UI fields based on the existing account data is done cleanly here. No obvious issues.
466-477: _set_api_key_data method is a neat approach.
Separating API key storage/combo box logic into a dedicated method is helpful for maintainability.
478-504: _init_organization_data: Great modular design.
Activating or skipping organization selection based on provider capabilities is both clear and flexible.
505-512: get_selected_provider logic is straightforward.
This helper clarifies transformation from combo selection to a Provider object.
513-523: update_ui effectively updates dependent fields.
Disabling and enabling fields based on provider attributes is well-structured.
524-538: _disable_all_fields is a concise utility.
Disabling each relevant widget when an invalid provider is selected helps avoid UI confusion.
539-549: _update_api_key_fields approach is solid.
Passing a boolean to enable or disable fields is a straightforward pattern.
550-554: _update_organization_fields addresses provider differences.
This is simple and keeps the code DRY.
570-584: Form submission logic is organized into smaller methods.
on_ok concisely delegates validation and saving. This is a best practice.
608-649: _save_account_data logically centralizes account updates.
Using separate logic for updating an existing account vs. creating a new one keeps things flexible.
651-666: _update_existing_account stores all relevant fields properly.
Everything updates in one place, reducing confusion.
667-685: _create_new_account is consistent with the existing approach.
This nicely mirrors _update_existing_account, keeping the flow uniform for new accounts.
708-713: Accounts label change.
A minor UI labeling improvement that aligns with the new structure.basilisk/provider.py (2)
37-37: Renaming to allow_custom_base_url sets a clear, descriptive field.
Switching the default to False removes assumptions about custom endpoints. Good approach to limit scope by default.
101-102: Selective enabling of custom base URLs for MistralAI, OpenAI, and OpenRouter.
This clearly scopes “custom base URL” capability to providers that are known to support it. Ensure future providers add or remove this flag appropriately.Also applies to: 113-114, 124-125
basilisk/provider_engine/openai_engine.py (1)
59-60: Conditional base_url usage is appropriate.
Using "self.account.custom_base_url or str(self.account.provider.base_url)" ensures custom URLs are honored if set, otherwise defaults to the provider’s base URL. This is exactly what we’d expect for user-defined overrides.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
basilisk/gui/account_dialog.py(6 hunks)
🔇 Additional comments (27)
basilisk/gui/account_dialog.py (27)
2-2: Import looks appropriate.
The newreimport is correctly used for pattern matching on custom base URLs.
21-23: Consider a more robust URL validator.
While this regex is a decent start, you might rely onurllib.parseor a dedicated library for more comprehensive validation of user-provided URLs.
[duplicate_comment, nitpick]
344-350: Constructor signature improvements.
Using type hints clarifies the parameters forEditAccountDialog. However, consider adding docstrings for better clarity and automated documentation.
351-351: Proper use of super().init.
Good to see standard usage ofsuper()to ensure correct initialization.
357-357: Immediate UI refresh.
Callingupdate_ui()right after initialization ensures the dialog reflects correct state upfront.
362-362: Type annotation for init_ui.
Defining-> Nonemakes the method signature consistent with the rest of the code.
377-389: Provider selection UI.
Creating a label and combo box for selecting providers is straightforward, and binding the combo event toupdate_ui()keeps the display consistent with provider requirements.
391-413: API key field group.
Introducing labeled fields and distinct combos for API key storage improves clarity and user experience.
425-434: Custom base URL UI.
Nice addition to handle user-defined endpoints. Consider adding a tooltip indicating safe usage guidelines or reminding users to include the full protocol (e.g.https://).
[duplicate_comment, nitpick]
440-440: OK button placement.
Properly adding the OK button to the sizer is consistent with wxPython layout practices.
444-444: Cancel button placement.
No issues here; this follows the same layout approach.
446-446: Buttons sizer addition.
Combining the OK and Cancel buttons into a single sizer block keeps the UI neatly aligned.
448-467: Initialize form data from account.
Good approach: default settings if no account exists, and structured initialization otherwise (_set_api_key_data,_init_organization_data, etc.).
471-482: Dedicated_set_api_key_datamethod.
Separating API key logic into its own method helps readability and potential reuse.
484-509: Organization data initialization.
Neatly enables/disables the organization combo box and handles default selections.
510-518:providerproperty for typed retrieval.
Providing a property for retrieving the selected provider name is elegant and helps avoid direct combo lookups elsewhere.
519-529: Holisticupdate_uiflow.
Clearly delegates updates to separate private methods (_update_api_key_fields,_update_organization_fields,_update_base_url_fields), enhancing maintainability.
530-544: Disable fields method.
Centralizing the disabling logic ensures consistent behavior when no provider is selected.
545-555: Toggle for API key fields.
A straightforward approach to conditionally enabling or disabling the API key elements.
556-560: Toggle for organization fields.
Similarly for organization elements, the code is clear and consistent.
561-575: Conditional custom base URL fields.
Overriding the label when a default base URL exists is a good usability improvement.
576-592:on_okwith validation flow.
Checking errors upfront and returning early prevents malformed data from being saved. Nicely done.
593-633: Form validation method.
Comprehensive checks for required fields, provider, API key, and custom URL correctness.
634-676:_save_account_dataimplementation.
Properly nullifiescustom_base_urlwhen empty or disallowed, and splits create/update logic.
677-692:_update_existing_account.
Well-structured assignment of updated values keeps the code DRY.
693-711:_create_new_account.
Constructing a newAccountobject ensures consistency with updated fields.
734-739: Static text for “Accounts”.
Maintaining translation context for this label is attentive to i18n.
| @@ -13,10 +14,14 @@ | |||
| accounts, | |||
| get_account_source_labels, | |||
| ) | |||
| from basilisk.provider import get_provider, providers | |||
| from basilisk.provider import Provider, get_provider, providers | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Check proper usage of imported classes and functions.
Ensure that Provider, get_provider, and providers are all utilized correctly, particularly if you rely on them for type hints or dynamic provider registration.
| self.organization_label = wx.StaticText( | ||
| panel, | ||
| # Translators: A label in account dialog | ||
| label=_("&Organization to use:"), | ||
| style=wx.ALIGN_LEFT, | ||
| ) | ||
| sizer.Add(label, 0, wx.ALL, 5) | ||
| self.organization = wx.ComboBox(panel, style=wx.CB_READONLY) | ||
| self.organization.Disable() | ||
| sizer.Add(self.organization, 0, wx.EXPAND) | ||
| sizer.Add(self.organization_label, 0, wx.ALL, 5) | ||
| self.organization_text_ctrl = wx.ComboBox(panel, style=wx.CB_READONLY) | ||
| sizer.Add(self.organization_text_ctrl, 0, wx.EXPAND) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Organization selection.
Providing a combo box for organization selection is sensible. Consider refreshing this list if the user switches providers that have different organization modes.
basilisk/config/account_config.py
Outdated
| custom_base_url: Optional[str] = Field( | ||
| default=None, pattern="^https?://[\\w.-]+(?::\\d+)?(?:/[\\w.-]*)*/?$" | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance URL validation and add documentation.
While the URL pattern validation is good, consider these improvements:
- Add docstring to document the purpose and format of
custom_base_url - Use a more restrictive pattern to prevent security issues
custom_base_url: Optional[str] = Field(
- default=None, pattern="^https?://[\\w.-]+(?::\\d+)?(?:/[\\w.-]*)*/?$"
+ default=None,
+ pattern="^https?://[a-zA-Z0-9][a-zA-Z0-9.-]+[a-zA-Z0-9](?::\\d{1,5})?(?:/[\\w.-]*)*/?$",
+ description="Custom base URL for the API provider. Must be a valid HTTP/HTTPS URL."
)The enhanced pattern:
- Prevents URLs with single-character domains
- Restricts port numbers to valid range (1-65535)
- Prevents domains starting or ending with dots/hyphens
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| custom_base_url: Optional[str] = Field( | |
| default=None, pattern="^https?://[\\w.-]+(?::\\d+)?(?:/[\\w.-]*)*/?$" | |
| ) | |
| custom_base_url: Optional[str] = Field( | |
| default=None, | |
| pattern="^https?://[a-zA-Z0-9][a-zA-Z0-9.-]+[a-zA-Z0-9](?::\\d{1,5})?(?:/[\\w.-]*)*/?$", | |
| description="Custom base URL for the API provider. Must be a valid HTTP/HTTPS URL." | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
basilisk/config/__init__.py(2 hunks)basilisk/config/account_config.py(4 hunks)basilisk/gui/account_dialog.py(5 hunks)
🔇 Additional comments (22)
basilisk/gui/account_dialog.py (17)
2-2: Import used for URL validation.The newly added
import reis logical, since regex-based validation is introduced for custom base URLs.
10-10: Importing the custom base URL pattern.Bringing
CUSTOM_BASE_URL_PATTERNinto scope is correct to enable client-side validation of the custom base URL.
18-18: Provider imports are appropriate.The addition of
Provider, get_provider, providersimports ensures valid references for managing provider-specific logic and UI.
341-348: Constructor improvements with type hints and UI initialization.Defining constructor parameters with explicit type hints (
wx.Window,str, etc.) and usingsuper().__init__is a good practice for clarity and maintainability. Callingself.update_ui()afterinit_data()helps ensure the UI reflects the loaded account state.Also applies to: 354-354
359-359: Enhanced method signature.Adding the return type annotation
-> Noneimproves clarity in theinit_uimethod.
374-386: Provider selection UI is well-structured.The static text label, combo box, and event binding cleanly separate the provider selection logic. This approach is consistent with the rest of the dialog.
388-410: API key storage method and key input fields.These new fields (labels, combo, text controls) are systematically added, following the same structure as other fields. This helps keep the layout and forms uniform.
412-420: Organization handling improvements.Introducing a separate label and combo box for selecting the organization is consistent with the existing UI design for accounts. This lays a good foundation for advanced organization management features.
432-442: Standard OK/Cancel button sizer.This layout is consistent with the rest of the dialog design. No concerns here.
445-466: Initialization of data for the new fields.Correctly populating the UI controls (provider combo, custom base URL field, etc.) keeps everything in sync with existing account data.
468-479: API key data population.The
_set_api_key_datamethod neatly consolidates the logic for reading and displaying the stored API key. The combo box selection approach is consistent with the broader design.
480-506: Organization data initialization.The
_init_organization_datamethod properly manages organization selectors. Early return for providers that do not require organizations is also a clean approach.
507-515: Provider property accessor.The property-based approach to retrieving the selected
Provideris a convenient pattern, allowing the rest of the code to remain simpler.
516-526: Context-aware UI updates.The
_disable_all_fields,_update_api_key_fields,_update_organization_fields, and_update_base_url_fieldsmethods keep the controls aligned with the chosen provider. This modular approach is easy to extend if new fields are added.Also applies to: 527-541, 542-552, 553-557, 558-572
573-585: Form validation with custom base URL checks.The
_validate_formmethod includes a robust check ofcustom_base_urlagainst theCUSTOM_BASE_URL_PATTERN. This aligns well with the new feature’s requirements, ensuring invalid URLs are caught early.Also applies to: 590-629
631-673: Account data persistence.Methods
_save_account_data,_update_existing_account, and_create_new_accountneatly split up the logic of reflecting new fields (custom_base_url) into the model. This structure is solid for maintainability.Also applies to: 674-689, 690-708
731-736: UI label for accounts.Declaring the _("Accounts") label is consistent with the existing localization strategy.
basilisk/config/__init__.py (2)
1-6: Newly enhanced import block.Pulling in
CUSTOM_BASE_URL_PATTERNalongside other essential entities is organized and aligns with the PR’s objective that promotes reusability of constants.
34-34: AddedCUSTOM_BASE_URL_PATTERNto all.Exporting
CUSTOM_BASE_URL_PATTERNin__all__ensures it’s readily available outside the module for validations in the UI or elsewhere.basilisk/config/account_config.py (3)
4-4: Importingrefor regex functionality.This aligns with the subsequent definition and usage of
CUSTOM_BASE_URL_PATTERN.
112-116: Introducing thecustom_base_urlfield.Adding the optional
custom_base_urlwith an associated regex pattern is central to the new feature. The inline description is helpful for end-users.
149-150: Logical handling for optional API key.When the provider does not require an API key, returning
Noneis appropriate, avoiding validation errors.
| self.custom_base_url_label = wx.StaticText( | ||
| panel, | ||
| # Translators: A label in account dialog | ||
| label=_("Custom &base URL:"), | ||
| style=wx.ALIGN_LEFT, | ||
| ) | ||
| sizer.Add(self.custom_base_url_label, 0, wx.ALL, 5) | ||
| self.custom_base_url_text_ctrl = wx.TextCtrl(panel) | ||
| sizer.Add(self.custom_base_url_text_ctrl, 0, wx.EXPAND) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Custom base URL fields introduced.
Adding a label and text control for the custom_base_url addresses the PR objective of customizable base URLs. Consider optionally adding a placeholder text or tooltip for user guidance.
| CUSTOM_BASE_URL_PATTERN = re.compile( | ||
| r"^https?://[\w.-]+(?::\d{1,5})?(?:/[\w-]+)*/?$" | ||
| ) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Definition of CUSTOM_BASE_URL_PATTERN.
The regex appears sufficiently flexible for basic HTTP/HTTPS URL validation, including optional ports and simple path segments.
Consider clarifying whether you need to support IPv6 addresses, user info, or other URL components. If not, this pattern is acceptable.
- Introduced a `custom_base_url` field to the `Account` model in `account_config.py` for providers that allow custom base URLs. - Updated `EditAccountDialog` in `account_dialog.py` to include a new input field for the custom base URL and adjusted the UI to reflect this change. - Modified `Provider` class in `provider.py` to include an `allow_custom_base_url` attribute, enabling custom base URL configuration for specific providers. - Updated provider configurations for `mistralai`, `openai`, and `openrouter` to support custom base URLs. - Adjusted OpenAIEngine logic to prioritize the custom base URL over the default provider base URL if provided.
- Removed `TYPE_CHECKING` imports and directly imported `Provider`. - Refactored the `EditAccountDialog` class to improve clarity and maintainability: - Introduced type hints for method parameters and return types. - Improved method names and added docstrings for better readability. - Split large methods into smaller, more focused methods. - Added error messages for form validation as comments for translators. - Reduced code duplication by consolidating similar code blocks. - Revised organization and API key fields handling for clarity.
- Introduced a regex pattern `CUSTOM_BASE_URL_PATTERN` for validating custom base URLs. - Changed `get_selected_provider` method into a `provider` property for enhanced code readability. - Updated `_validate_form` to return a tuple of error message and associated field. - Added focus control to invalid form fields to enhance user experience by directing attention to the error. - Refactor form validation to include custom base URL validation.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
for more information, see https://pre-commit.ci
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…L validation - Move the `CUSTOM_BASE_URL_PATTERN` regex from `account_dialog.py` to `account_config.py`. - Update the `Account` class definition to use this centralized pattern for `custom_base_url`. - Adjust imports in `__init__.py` and `account_dialog.py` to utilize the centralized `CUSTOM_BASE_URL_PATTERN`.
AAClause
force-pushed
the
customBaseURL
branch
from
bc5ddf8 to
d669042
Compare
Replaced 'Optional[T]' with 'T | None' for type hinting in the account_dialog.py file. This change modernizes the code to make use of the Python 3.10+ feature which improves readability.
Enhanced the account dialog to display the default base URL when available, aiding in customization clarity.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
basilisk/gui/account_dialog.py(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build_app / build_windows (x86)
- GitHub Check: build_app / build_windows (x64)
🔇 Additional comments (20)
basilisk/gui/account_dialog.py (20)
4-4: Importing theremodule for regex validation.
No issues identified; this aligns with the needs for custom URL validation.
11-11: Usage ofCUSTOM_BASE_URL_PATTERN.
Effectively centralizes regex logic for validating custom base URLs.
18-18: Ensure correct usage of imported provider-related classes and functions.
This matches a previous recommendation regarding verifyingProvider,get_provider, andprovidersusage.
32-32: Improved clarity with type annotation fororganization.
Explicitly specifyingAccountOrganization | Nonemakes the interface more self-documenting.
432-438: Switching tosuper().__init__and callingupdate_uipromptly.
This ensures proper initialization and a consistent UI state upon dialog creation.
463-476: Refactored provider combo box with a dedicated label and event binding.
Consider handling empty or missing providers gracefully if the list is ever dynamic.
477-489: Added UI elements for API key storage method selection.
This separation clarifies different storage options based on the chosen provider.
491-499: Dedicated API key label and text control.
Providing distinct credentials fields aligns with best practices for user input.
511-520: New custom base URL fields.
Fulfills the PR objective. Consider adding a tooltip to guide users on valid endpoint usage.
521-532: Horizontal sizer for dialog action buttons.
Straightforward layout approach with no identified concerns.
552-553: Delegating API key logic to_set_api_key_data().
Keeps initialization cleanup and avoids clutteringinit_data.
561-572:_set_api_key_datacentralizes the combo selection and API key population.
This promotes maintainability by consolidating credential assignment in one place.
585-586: Setting items in the organization combo box with a "Personal" placeholder.
Enhances user understanding of non-organizational context.
598-599: Auto-selecting the active organization by index.
Improves user experience by preserving the user's previously selected organization.
601-612:providerproperty for streamlined retrieval of the chosen provider.
Well-typed approach that clarifies the expected return type.
613-666: Granular UI update methods (_disable_all_fields,_update_api_key_fields,_update_organization_fields,_update_base_url_fields).
These helper methods keep logic segmented, boosting readability and maintainability.
667-689:on_okmethod delegating to_validate_formand_save_account_data.
Good layering of responsibilities: validation before final save ensures data consistency.
733-775: Consolidating account data persistence into_save_account_data.
Promotes single-responsibility design, simplifying testing and future modifications.
776-791:_update_existing_accountcleanly updates all relevant fields.
No redundant steps or missed fields observed.
854-859: New "Accounts" label for the account list UI.
Consistent naming adheres to the established design language.
| self.organization_label = wx.StaticText( | ||
| panel, | ||
| # Translators: A label in account dialog | ||
| label=_("&Organization to use:"), | ||
| style=wx.ALIGN_LEFT, | ||
| ) | ||
| sizer.Add(label, 0, wx.ALL, 5) | ||
| self.organization = wx.ComboBox(panel, style=wx.CB_READONLY) | ||
| self.organization.Disable() | ||
| sizer.Add(self.organization, 0, wx.EXPAND) | ||
| sizer.Add(self.organization_label, 0, wx.ALL, 5) | ||
| self.organization_text_ctrl = wx.ComboBox(panel, style=wx.CB_READONLY) | ||
| sizer.Add(self.organization_text_ctrl, 0, wx.EXPAND) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Introducing organization selection combo box.
Ensure correct refresh or re-population if organizations are updated elsewhere in real time.
| @@ -526,34 +538,52 @@ | |||
| and organization settings are set in the dialog. | |||
| """ | |||
| if not self.account: | |||
| self.api_key_storage_method.SetSelection(0) | |||
| self.api_key_storage_method_combo.SetSelection(0) | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Default combo selection set to index 0 for API key storage.
Consider validating an actual default method or user preference to reduce surprises.
| def _init_organization_data(self) -> None: | ||
| """Initialize organization related fields.""" | ||
| self.organization_text_ctrl.Enable( |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Initialization method _init_organization_data.
Clear separation of organization-related logic, though additional edge case handling for zero organizations might help.
| def _validate_form(self) -> tuple[str, wx.Window] | None: | ||
| """Validate form data and return a tuple of error message and field to focus on if any. | ||
|
|
||
| Returns None if form data is valid. | ||
| """ | ||
| if not self.name.GetValue(): | ||
| # Translators: An error message in account dialog | ||
| return _("Please enter a name"), self.name | ||
|
|
||
| provider = self.provider | ||
| if not provider: | ||
| # Translators: An error message in account dialog | ||
| return _("Please select a provider"), self.provider_combo | ||
|
|
||
| if provider.require_api_key: | ||
| if self.api_key_storage_method.GetSelection() == -1: | ||
| msg = _("Please select an API key storage method") | ||
| wx.MessageBox(msg, _("Error"), wx.OK | wx.ICON_ERROR) | ||
| self.api_key_storage_method.SetFocus() | ||
| return | ||
| if not self.api_key.GetValue(): | ||
| msg = _( | ||
| if self.api_key_storage_method_combo.GetSelection() == wx.NOT_FOUND: | ||
| # Translators: An error message in account dialog | ||
| return _( | ||
| "Please select an API key storage method" | ||
| ), self.api_key_storage_method_combo | ||
|
|
||
| if not self.api_key_text_ctrl.GetValue(): | ||
| # Translators: An error message in account dialog | ||
| return _( | ||
| "Please enter an API key. It is required for this provider" | ||
| ) | ||
| wx.MessageBox(msg, _("Error"), wx.OK | wx.ICON_ERROR) | ||
| self.api_key.SetFocus() | ||
| return | ||
| organization_index = self.organization.GetSelection() | ||
| ), self.api_key_text_ctrl | ||
|
|
||
| if ( | ||
| self.provider.allow_custom_base_url | ||
| and self.custom_base_url_text_ctrl.GetValue() | ||
| ): | ||
| if not re.match( | ||
| CUSTOM_BASE_URL_PATTERN, | ||
| self.custom_base_url_text_ctrl.GetValue(), | ||
| ): | ||
| # Translators: An error message in account dialog | ||
| return _( | ||
| "Please enter a valid custom base URL" | ||
| ), self.custom_base_url_text_ctrl | ||
|
|
||
| return None | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Robust _validate_form checks, including custom base URL regex validation.
Consider more detailed feedback for invalid URLs beyond simple pattern matching.
| self, | ||
| provider: Provider, | ||
| active_organization: str | None, | ||
| api_key_storage_method: KeyStorageMethodEnum | None, | ||
| api_key: SecretStr | None, | ||
| custom_base_url: str | None, | ||
| ) -> None: | ||
| """Create new account from form data.""" | ||
| self.account = Account( | ||
| name=self.name.GetValue(), | ||
| provider=provider, | ||
| api_key_storage_method=api_key_storage_method, | ||
| api_key=api_key, | ||
| active_organization_id=active_organization, | ||
| source=AccountSource.CONFIG, | ||
| custom_base_url=custom_base_url, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
_create_new_account incorporates new custom base URL and other fields.
Consider whether a default organization is required if organization_mode_available is set.
Docstrings generation was requested by @clementb49. * #524 (comment) The following files were modified: * `basilisk/config/account_config.py` * `basilisk/gui/account_dialog.py` * `basilisk/provider_engine/openai_engine.py`
|
Note Generated docstrings for this pull request at #528 |
Fixes #338
Summary by CodeRabbit
New Features
Refactor