Skip to content
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

[BYOM] Support for .local (and other?) testing domains #42367

Open
jonathansampson opened this issue Nov 19, 2024 · 0 comments
Open

[BYOM] Support for .local (and other?) testing domains #42367

jonathansampson opened this issue Nov 19, 2024 · 0 comments
Assignees
Labels
browser-ai feature-request OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. privacy

Comments

@jonathansampson
Copy link
Contributor

jonathansampson commented Nov 19, 2024

Platforms

all

Description

brave/brave-core#26475, in part, originally sought to add support for .local domains, but has deferred this effort due to various challenges. This issue exists to encourage proper consideration for this feature, and to lay out an ideal path forward.

Relevant Links

@jonathansampson jonathansampson self-assigned this Nov 19, 2024
jonathansampson added a commit to brave/brave-core that referenced this issue Nov 19, 2024
Giving proper support to .local domains requires adequate address resolution, which can be somewhat tricky across platforms. For now, we will defer adding support to a later date. Track brave/brave-browser#42367 for additional details and development.
@thypon thypon self-assigned this Nov 19, 2024
@mattmcalister mattmcalister added priority/P3 The next thing for us to work on. It'll ride the trains. OS/Desktop labels Nov 21, 2024
@mattmcalister mattmcalister moved this to In Progress in Browser AI Nov 21, 2024
jonathansampson added a commit to brave/brave-core that referenced this issue Nov 21, 2024
Giving proper support to .local domains requires adequate address resolution, which can be somewhat tricky across platforms. For now, we will defer adding support to a later date. Track brave/brave-browser#42367 for additional details and development.
mihaiplesa pushed a commit to brave/brave-core that referenced this issue Nov 21, 2024
* adds feature flag for private ip addresses

* Adds implementation, and updates tests

Moved some of the earlier endpoint tests into the newer param-based suite for better organization.

* Shows an error when attempting to use a model with an invalid endpoint

Current approach is to check the validity of the endpoint when the first human interaction takes place. If the model endpoint is invalid, an error is shown to the user. The displayed error invites the user to check their model's configuration.

* checks endpoint-validity on model activation

Rather than waiting for the user to interact with the model, this change proactively notifies the user of an invalid model endpoint when the model is selected for use.

* ui change; remove unnecessary gap in list

With the gap, there is an extra bit of padding above each model listing after the first. This gives the impression that the first model in the list is shorter (i.e., its bounding box size) than all that follow. Further, the gap causes asymmetry between the top and bottom padding on every model listing after the first.

* adds informative error message

We aim to provide an instructive error message to the user when they have provided an endpoint value that would only be valid with the enabling of optional private IPs.

* Enhanced model endpoint validation

This change introduces an alternative approach to endpoint validation. Some endpoints are only valid with the enabling of the optional brave-ai-chat-allow-private-ips flag. The new approach informs the user when their provided endpoint URL [would be] valid as a private IP address.

* presubmit fixes

* distinguish between endpoint error types

Upon saving a custom model, the URL may be deemed invalid. This change gives a more detailed message to the frontend regarding the endpoint validity, enabling us to present a more helpful error message to the user.

* clear orphaned error messages

Switching to a model with an invalid endpoint results in an error message being displayed. This change causes the error message to be cleared when switching to another model.

* fixes private endpoint validation logic

* Adds modal and warning label for private endpoints

If the user attempts to save a configuration with a private endpoint, and the optional flag has not been enabled, the user will be presented with a modal dialog informing them as much. If the optional flag has been enabled, and the user attempts to use a private endpoint, we will display a label warning them of the risk they're accepting.

* minor refactor of error-msg logic

Each condition checked `apiHasError`, so we can simplify by moving that to the top-most conditional, and turning the rest of the logic into a switch-case, dropping unnecessary parents around JSX items.

* presubmit fixes and cleanup

* removes unnecessary `async`

This method no longer queries the backend itself, and therefore no longer needs to be async.

* Improve string identifiers and content

To ease efforts for translators, more descriptive identifiers are provided.

* fixes typo

* tighten up expectations

Though not likely to happen, it's possible our method could be called with an invalid number and/or type of arguments. We'll make sure our expectations are clear, and that we reject early otherwise.

* supplemental comment(s) in mojom files

* presubmit fixes following rebase

* removes attempted-support of .local domains

Giving proper support to .local domains requires adequate address resolution, which can be somewhat tricky across platforms. For now, we will defer adding support to a later date. Track brave/brave-browser#42367 for additional details and development.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser-ai feature-request OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. privacy
Projects
Status: In Progress
Development

No branches or pull requests

3 participants