frontend: Warn when multiple rate limiters configured

Created by: ryanslade

We'll display a warning if more than one rate limiter is configured for a single code host:

NOTE: The issues below have now been solved by using the ValidateConfig in frontend/db

There are a couple of issues which are not blocking, but would be nice to solve later:

One, when saving external service config the warning is not displayed immediately, only when you navigate away from the page or refresh.

Two, we need to load rate limit config in both repo-updater, to actually enforce the limits and now the frontend to display warnings. I moved common code to the extsvc package to help with this. However, we can't do the DB call in extsvc since we have two db packages, one from frontend and one from repo-updater. Importing either in extsvc leads to an import cycle. Also, both frontend and repo-updater have their own definitions for the ExternalService struct so we need to pull out the common fields and pass those to the extsvc package.

(As a side note, we actually have a third definition of ExternalService here: https://github.com/sourcegraph/sourcegraph/blob/8bea5eb42c72423a163932d6f31d9b5cf9ebb21d/internal/api/api.go#L118)

Ideally we should consolidate the different definitions of external service to a central package but I don't think we should do that as part of this PR.

Closes: https://github.com/sourcegraph/sourcegraph/issues/10026

Created by: eseliger

One, when saving external service config the warning is not displayed immediately, only when you navigate away from the page or refresh.

To address that, would it be simple to add it also to the warning property in the GQL resolver of updateExternalService?

Created by: ryanslade

One, when saving external service config the warning is not displayed immediately, only when you navigate away from the page or refresh.

To address that, would it be simple to add it also to the warning property in the GQL resolver of updateExternalService?

That was actually what I tried first, see: https://gitlab.sgdev.org/root/sourcegraph/-/issues/10026#issuecomment-627444625

Created by: eseliger

I see, one answer to (1) would be to use markdown in the warning, it's already rendered as markdown in the webapp. Ex:

# Error one
This is really serious!

# Error two
Another veery bad problem

type warning struct {
    warning string
}
func (w *warning) AddWarning(title, body string) {
    w.warning += fmt.Sprintf("# %s\n%s\n", title, body)
}

I agree with that it should also be a banner, as this is a bad config error. Would be nice to immediately include it in there as well, though. Especially because it's immediately visible to the user :)

But that won't block this PR, we can file a separate ticket for it to keep track of it.

Deferring the actual review to core people

Created by: mrnugget

What's the reasoning against using this?

https://github.com/sourcegraph/sourcegraph/blob/1800cf25e4902d817bb47ee1a3774d68ffa9b6fd/cmd/frontend/db/external_services.go#L73-L75

It's used when creating

https://github.com/sourcegraph/sourcegraph/blob/1800cf25e4902d817bb47ee1a3774d68ffa9b6fd/cmd/frontend/db/external_services.go#L215-L219

and when updating an external service in the database:

https://github.com/sourcegraph/sourcegraph/blob/1800cf25e4902d817bb47ee1a3774d68ffa9b6fd/cmd/frontend/db/external_services.go#L237-L251

The errors are displayed in the warning automatically. No string formatting etc. needed, since the multierror output is displayed correctly.

Created by: ryanslade

What's the reasoning against using this?

I didn't know about it

When I asked about how to tackle this I was pointed towards the approach I've taken in this PR: https://gitlab.sgdev.org/root/sourcegraph/-/issues/10026#issuecomment-627448627

I'll take a look into your suggested approach now

Created by: ryanslade

@mrnugget PTAL

Created by: ryanslade

@unknwon Up to 58% now