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

[pull] main from sourcegraph:main #1

Open
wants to merge 10,000 commits into
base: main
Choose a base branch
from
Open

Conversation

pull[bot]
Copy link

@pull pull bot commented Jan 16, 2023

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

chrsmith and others added 26 commits July 10, 2024 14:27
A couple of minor changes to minimize the diff for "large completions
API refactoring".

Most changes are just a refactoring of the `openai` completions
provider, which I apparently missed in
#63731. (There are still
some smaller tweaks that can be made to the `fireworks` or `google`
completion providers, but they aren't as meaningful.

This PR also removes a couple of unused fields and methods. e.g.
`types.CompletionRequestParameters::Prompt`. There was a comment to the
effect of it being long since deprecated, and it is no longer read
anywhere on the server side. So I'm assuming that a green CI/CD build
means it is safe to remove.

## Test plan

CI/CD

## Changelog

NA
This will correct6 upgrade path for mvu plan creation

## Test plan

CI test

<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->

---------

Co-authored-by: Release Bot <[email protected]>
Co-authored-by: Jean-Hadrien Chabran <[email protected]>
Co-authored-by: Anish Lakhwara <[email protected]>
Co-authored-by: Jean-Hadrien Chabran <[email protected]>
Co-authored-by: Anish Lakhwara <[email protected]>
Removes the `sg telemetry` command that pertains to the legacy V1
exporter that is specific to Cloud instances.

I got asked about this recently, and especially with the new `sg
analytics` for usage of the `sg` CLI, this has the potential to be
pretty confusing.

Part of https://linear.app/sourcegraph/issue/CORE-104

## Test plan

n/a

## Changelog

- `sg`: the deprecated `sg telemetry` command for allowlisting export of
V1 telemetry from Cloud instances has been removed. Use telemetry V2
instead.
This PR fixes the following:
- Handles source range translation in the occurrences API
  (Fixes https://linear.app/sourcegraph/issue/GRAPH-705)
- Handles range translation when comparing with document occurrences in
   search-based and syntactic usagesForSymbol implementations

Throwing this PR up in its current state as I think adding the bulk
conversion
API will be a somewhat complex task, so we should split them into
separate
PRs anyways, and I don't have time to continue working on this right
now.

Some design notes:
- We want to avoid passing around full CompletedUpload and RequestState
objects,
which is why I chose to create a smaller UploadSummary type and decided
to pass
around GitTreeTranslator as that is the minimal thing we need to handle
range re-mapping.
- Yes, this PR increases the surface of the UploadLike type, but I think
it's still quite manageable.

## Test plan

manual testing, existing tests on gittreetranslator
---------

Co-authored-by: Christoph Hegemann <[email protected]>
This commit removes files/dependencies that we are not using (anymore).
In the case of `@sourcegraph/wildcard` we never want to import
dependencies from it, but have done so accidentally in the past. I hope
that we can prevent this by removing it from dependencies (and we don't
need anyway).

## Test plan

`pnpm build` and CI
Instead of fetching the file for every node, this passes in a
request-scoped cache to minimize the number of gitserver roundtrips, but
does no fetching if `surroundingContent` is not requested by the caller.
Fixes srch-717

This commit fixes the line numbers for unified diff views, which are
used on the commit page and for inline diffs.

## Test plan

Manual testing.

| Before | After |
|--------|--------|
|
![2024-07-11_10-40](https://github.com/sourcegraph/sourcegraph/assets/179026/170ac815-d038-4239-80fe-7d35cecfa832)
|
![2024-07-11_10-38](https://github.com/sourcegraph/sourcegraph/assets/179026/3606cb34-ad87-43bf-9664-414bf9250fa4)
|
When `internal/env/env.Get` detects a difference in between already
registered descriptions, it panics (good). But the error message doesn't
tell you what's the difference and you're left out to put a few prints
for yourself in the code to try to understand what's wrong.

See also: #63786

Before:

<img width="1109" alt="image"
src="https://github.com/sourcegraph/sourcegraph/assets/10151/56b2d65c-ef87-4134-bfc0-67248aa48350">

After: 

![CleanShot 2024-07-11 at 15 26
13@2x](https://github.com/sourcegraph/sourcegraph/assets/10151/406bca90-2b87-481d-aad3-6550afaca29f)


## Test plan

CI + local run 

## Changelog

- When conflicting env var are detected, print the two to ease
debugging.
This PR fixes an important bug in #62976, where we didn't properly map the
symbol line match to the return type. Instead, we accidentally treated symbol
matches like file matches and returned the start of the file.

## Test plan

Add new unit test for symbol match conversion. Extensive manual testing.
…3779)

If we failed getting a secret via a tool - we return CommandErr which
contains SecretErr
If we failed getting a secret via Google - we return GoogleSecretErr
which contains SecretErr

Depending on the error we get while trying to persist Analytics we
suggest different fixes the user can try.

Below is how it looks when we get a GoogleSecretErr

![Screenshot 2024-07-11 at 11 11
40](https://github.com/sourcegraph/sourcegraph/assets/1001709/12479561-c1f5-4de7-b00e-01a1fbb49ece)

## Test plan
Tested locally
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
**chore(appliance): extract constant for configmap name**

To the reconciler, this is just a value, but to higher-level packages
like appliance, there is a single configmap that is an entity. Let's
make sure all high-level orchestration packages can reference our name
for it. This could itself be extracted to injected config if there was a
motivation for it.



**chore(appliance): extract NewRandomNamespace() in k8senvtest**

From reconciler tests, so that we can reuse it in self-update tests.



**feat(appliance): self-update**

Add a worker thread to the appliance that periodically polls release
registry for newer versions, and updates its own Kubernetes deployment.

If the APPLIANCE_DEPLOYMENT_NAME environment variable is not set, this
feature is disabled. This PR will be accompanied by one to the
appliance's helm chart to add this variable by default.



**fix(appliance): only self-update 2 minor versions above deployed SG**




**chore(appliance): self-update integration test extra case**

Check that self-update doesn't run when SG is not yet deployed.

https://linear.app/sourcegraph/issue/REL-212/appliance-can-self-upgrade
`REDIS_ENDPOINT` is now registered by gateway, but it is also registered
in `internal/redispool` as the fallback for when the other values are
not set.

The real fix would be to not have env vars in that package, and instead
each service creates one instance of each of those two in their `cmd/`,
but that's a lot of work so short-term fixing it by reading the fallback
using os.Getenv.

Test plan:

`sg run cody-gateway` doesn't panic.

---------

Co-authored-by: Jean-Hadrien Chabran <[email protected]>
…#63790)

The OTEL upgrade #63171
bumps the `prometheus/common` package too far via transitive deps,
causing us to generate configuration for alertmanager that altertmanager
doesn't accept, at least until the alertmanager project cuts a new
release with a newer version of `promethues/common`.

For now we forcibly downgrade with a replace. Everything still builds,
so we should be good to go.

## Test plan
`sg start` and `sg run prometheus`. On `main`, editing
`observability.alerts` will cause Alertmanager to refuse to accept the
generated configuration. With this patch, all is well it seems - config
changes go through as expected. This is a similar test plan for
#63329

## Changelog

- Fix Prometheus Alertmanager configuration failing to apply
`observability.alerts` from site config
A couple of tweaks to the commit / diff view:

- Linking both file paths in the header for renamed files
- Collapse renamed file diffs without changes by default
- Move "no changes" out of `FileDiffHunks` to not render a border around
the test.
- Add description for binary files
- Adjust line height and font size to match what we use in the file view
- Added the `visibly-hidden` utility class to render content for a11y
purposes (I didn't test the changes I made with a screenreader though)

Contributes to SRCH-523


## Test plan

Manual testing
…itly (#63782)

I went through all call sites of the 3 search APIs (Stream API, GQL API,
SearchClient (internal)) and made sure that the query syntax version is
set to "V3".

Why?

Without this change, a new default search syntax version might have
caused a change in behavior for some of the call sites.

## Test plan
- No functional change, so relying mostly on CI
- The codeintel GQL queries set the patternType explicitly, so this
change is a NOP.

I tested manually
- search based code intel sends GQL requests with version "V3"
- repo badge still works
- compute GQL returns results
The background publisher was started regardless if analytics was
disabled or not. This PR makes it so that we only publish analytics if
it is enabled.

To make it work and not duplicate the disabled analytics check, I moved
the usershell + background context creation to happen earlier.

## Test plan
CI and tested locally

## Changelog
* sg - only start the analytics background publisher when analytics are
enabled

---------

Co-authored-by: Jean-Hadrien Chabran <[email protected]>
This PR overhauls the UI a bunch to make it look more in line with other
pages, and fixes various smaller papercuts and bugs.

Closes SRC-377

Test plan:

Added storybooks and made sure existing ones still look good, created,
updated, deleted various webhooks locally.
…-box after successful completion (#59645)

## Linked Issues
- Closes #38348


## Motivation and Context:
<!--- Why is this change required? What problem does it solve? --> 
- Improves the UX of the password reset flow

## Changes Made:
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. List any
dependencies that are required for this change. -->
- Made changes to the following 2 flows:
	- On the new password entry screen:
- Added a text which displays the email of the account for which the
password change request has been raised
- Added a back button to allow the users to go back to the previous
email entry screen if the account they want to reset the password for is
different
- On the sign-in screen which comes after successful password reset
request completion:
- Made changes to auto-populate the email text-box with the email linked
to the account on which the password reset request was completed
recently


## Type of change:

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactoring (altering code without changing its external
behaviour)
- [ ] Documentation change
- [ ] Other


## Checklist:

- [x] Development completed
- [ ] Comments added to code (wherever necessary)
- [ ] Documentation updated (if applicable)
- [x] Tested changes locally


## Follow-up tasks (if any):

- None

## Test Plan
<!--- Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce. -->

- Setup the codebase locally along with configuring a custom SMTP to
enable the email delivery functionality with the help of this
documentation:
https://docs.sourcegraph.com/admin/config/email#configuring-sourcegraph-to-send-email-using-another-provider

 - Tested the entire flow locally end-to-end.
- Screen recording of the password reset screen where the current email
ID is added along with a back button:
	 	

https://github.com/sourcegraph/sourcegraph/assets/51479159/a79fc338-ace0-4281-86d2-de7cc68eae20
	 
- Screen recording of the sign-in screen after password reset is
successfully done where the email ID is auto-populated in the text-box:
	 	

https://github.com/sourcegraph/sourcegraph/assets/51479159/be7db65d-9421-4621-a1e9-a04a546b9757



    
## Additional Comments
- Please let me know if I need to make any further design changes from
the frontend side or any API contract related changes from the backend
side

---------

Co-authored-by: Vincent <[email protected]>
Co-authored-by: Shivasurya <[email protected]>
We register ctrl+backspace to go to the repository root, but that should
not trigger when an input field, such as the fuzzy finder, is focused.

Fixes srch-681

## Test plan

Manual testing.
The "Exclude" options in the filter panels are very useful, but many are specific to Go. This change generalizes them so they apply in many more cases:
* All files with suffix `_test` plus extension (covers Go, Python, some Ruby, C++, C, more)
* All files with suffix `.test` plus extension (covers Javascript, some Ruby)
* Ruby specs
* Third party folders (common general naming pattern)

Relates to SPLF-70
This adds two new components for the common situation of needing to
display a styled path.

- `DisplayPath` handles splitting, coloring, spacing, slotting in a file
icon, adding a copy button, and ensuring that no spaces get introduced
when copying the path manually
- `ShrinkablePath` is built on top of `DisplayPath` and adds the ability
to collapse path elements into a dropdown menu

These are used in three places:
- The file header. There should be no change in behavior except maybe a
simplified DOM. This makes use of the "Shrinkable" version of the
component.
- The file search result header. This required carefully ensuring that
the text content of the node is exactly equal to the path so that the
character offsets are correct.
- The file popover, where it is used for both the repo name (unlinkified
version) and the file name (linkified version).

Fixes SRCH-718
Fixes SRCH-690
Closes
https://linear.app/sourcegraph/issue/CODY-2847/change-experimental-labels-to-beta

## Test plan
- Check that cody web page and cody web side panel have beta badges
…ore models) (#63797)

This PR if what the past dozen or so
[cleanup](#63359),
[refactoring](#63731),
and [test](#63761) PRs
were all about: using the new `modelconfig` system for the completion
APIs.

This will enable users to:

- Use the new site config schema for specifying LLM configuration, added
in #63654. Sourcegraph
admins who use these new site config options will be able to support
many more LLM models and providers than is possible using the older
"completions" site config.
- For Cody Enterprise users, we no longer ignore the
`CodyCompletionRequest.Model` field. And now support users specifying
any LLM model (provided it is "supported" by the Sourcegraph instance).

Beyond those two things, everything should continue to work like before.
With any existing "completions" configuration data being converted into
the `modelconfig` system (see
#63533).

## Overview

In order to understand how this all fits together, I'd suggest reviewing
this PR commit-by-commit.

### [Update internal/completions to use
modelconfig](e6b7eb1)

The first change was to update the code we use to serve LLM completions.
(Various implementations of the `types.CompletionsProvider` interface.)

The key changes here were as follows:

1. Update the `CompletionRequest` type to include the `ModelConfigInfo`
field (to make the new Provider and Model-specific configuration data
available.)
2. Rename the `CompletionRequest.Model` field to
`CompletionRequest.RequestedModel`. (But with a JSON annotation to
maintain compatibility with existing callers.) This is to catch any bugs
related to using the field directly, since that is now almost guaranteed
to be a mistake. (See below.)

With these changes, all of the `CompletionProvider`s were updated to
reflect these changes.

- Any situation where we used the
`CompletionRequest.Parameters.RequestedModel` should now refer to
`CompletionRequest.ModelConfigInfo.Model.ModelName`. The "model name"
being the thing that should be passed to the API provider, e.g.
`gpt-3.5-turbo`.
- In some situations (`azureopenai`) we needed to rely on the Model ID
as a more human-friendly identifier. This isn't 100% accurate, but will
match the behavior we have today. A long doc comment calls out the
details of what is wrong with that.
- In other situations (`awsbedrock`, `azureopenai`) we read the new
`modelconfig` data to configure the API provider (e.g.
`Azure.UseDeprecatedAPI`), or surface model-specific metadata (e.g. AWS
Provisioned Throughput ARNs). While the code is a little clunky to avoid
larger refactoring, this is the heart and soul of how we will be writing
new completion providers in the future. That is, taking specific
configuration bags with whatever data that is required.

### [Fix bugs in
modelconfig](75a51d8)

While we had lots of tests for converting the existing "completions"
site config data into the `modelconfig.ModelConfiguration` structure,
there were a couple of subtle bugs that I found while testing the larger
change.

The updated unit tests and comments should make that clear.

### [Update frontend/internal/httpapi/completions to use
modelconfig](084793e)

The final step was to update the HTTP endpoints that serve the
completion requests. There weren't any logic changes here, just
refactoring how we lookup the required data. (e.g. converting the user's
requested model into an actual model found in the site configuration.)

We support Cody clients sending either "legacy mrefs" of the form
`provider/model` like before, or the newer mref
`provider::apiversion::model`. Although it will likely be a while before
Cody clients are updated to only use the newer-style model references.

The existing unit tests for the competitions APIs just worked, which was
the plan. But for the few changes that were required I've added comments
to explain the situation.

### [Fix: Support requesting models just by their
ID](99715fe)

> ... We support Cody clients sending either "legacy mrefs" of the form
`provider/model` like before ...

Yeah, so apparently I lied 😅 . After doing more testing, the extension
_also_ sends requests where the requested model is just `"model"`.
(Without the provider prefix.)

So that now works too. And we just blindly match "gtp-3.5-turbo" to the
first mref with the matching model ID, such as
"anthropic::unknown::gtp-3.5-turbo".

## Test plan

Existing unit tests pass, added a few tests. And manually tested my Sg
instance configured to act as both "dotcom" mode and a prototypical Cody
Enterprise instance.

## Changelog

Update the Cody APIs for chat or code completions to use the "new style"
model configuration. This allows for great flexibility in configuring
LLM providers and exposing new models, but also allows Cody Enterprise
users to select different models for chats.

This will warrant a longer, more detailed changelog entry for the patch
release next week. As this unlocks many other exciting features.
kritzcreek and others added 30 commits July 25, 2024 08:04
Closes
https://linear.app/sourcegraph/issue/GRAPH-766/batch-api-for-fetching-scip-documents

## Test plan
Tested by being used for the single-path retrieval path, will be tested
more once its used in follow-up PRs with more batching
This moves the `refreshAnalyticsCache` job from `frontend` to `worker`

Notes:
the `adminanalytics` package uses a global cache which made testing
awkward. That's why I introduced the `store()` abstraction to swap the
store at test time.

## Test plan
- new unit test
Fixes GRAPH-649

<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->

This PR adds non-local references support to Java (and more generally to
our locals handling code).

The main idea is to cherrypick nodes that _can_ contain global or
resolved (see below) references, handle them first, and then mark
everything else as locals.

- Pure Global references are formed from types of definitions that
_can't_ be locals in our code. Currently it's only methods in Java that
we treat as always global references

- Pure local references - these references should only be resolved
against the locals scope tree and definitions within it - never emitting
a non-local references

- Resolved references - these are first resolved as locals, and if that
doesn't succeed, a global reference is emitted. These will be used most
frequently, as TS grammars don't carry enough information for us to
identify more pure global references. For example, in Java's type bounds
`class Hello<N extends Number>`, Number can refer to both the local type
parameter of an enclosing class (in which case we should emit a local
reference), or it can refer to `java.lang.Number` (in which case we
should emit a global reference)

## Test plan

- New snapshot results
- New evaluation results

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->

---------

Co-authored-by: Christoph Hegemann <[email protected]>
…ocking quota (#63911)

Closes
[DINF-51](https://linear.app/sourcegraph/issue/DINF-51/failed-back-compat-doesnt-count-towards-branch-locking-quota)

## Context

If a back-compat step on main fails, the build is marked as having
failed. However, we don't treat that as a failure in build-tracker,
resulting in no #buildkite-main post and not counting towards failed
build quota for locking main.

The reason why this was happening is that the Backcompat build wasn't
linked to the main Sourcegraph build in anyway. However, when a
backcompat fails the main build reflects the status of this failure, but
we do not use this field when determining the status of a build, so it
doesn't work for our use case.

![CleanShot 2024-07-18 at 15 04
15@2x](https://github.com/user-attachments/assets/9553330a-ad98-45cc-b4ce-03a22ca1b99d)

We [instead do a walkthrough of all the jobs associated with a build to
figure
out](https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/dev/build-tracker/main.go?L349-372)
if the build has failed, fixed or is passing.

With this logic, it means we have to link the steps from child builds
that a particular build triggers to it's parent.

## Test plan

* Create a build that'll have backcompat failing
* The build tracker event associated with the main build will be
reported with a state of failed to buildkite.

![CleanShot 2024-07-18 at 15 10
45@2x](https://github.com/user-attachments/assets/1bf503ab-0020-47bf-9512-b3a9ee5d4e36)


## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
…4018)

## Test plan

Test each link manually.

---------

Co-authored-by: Bolaji Olajide <[email protected]>
For precise usagesForSymbols, we want to propagate usages everywhere
(with associated symbol names, not just 'Location' values). This PR
introduces the new Usage type, and unifies the old GetBulkSymbolUsages and
GetMinimalBulkSymbolUsages APIs into a single GetSymbolUsages API.

We convert the Usage values to Location to avoid changing a lot of code
at once.

We also change the DB query to do grouping and aggregation for us
instead of doing it in Go code.

---------

Co-authored-by: Christoph Hegemann <[email protected]>
…l server lock (round 2) (#64079)

Second attempt at #64044,
now that rate limit is set right. So lets boost the concurrency to all
cores!

## Test plan

main dry-run
https://buildkite.com/sourcegraph/sourcegraph/builds/284268#0190e9cf-b902-4f7c-a1ad-fca8700b8fa0

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Sorry missed this during the review. No need to allow `rEfErEnCe` as the
capture name. Noticed this while checking the samply profile and seeing
string allocations (gets us back 20ms on spring-framework 🎉)

## Test plan

Covered by existing tests
**Appliance points ingress-facing service to itself by default**

Not frontend.

**feat(appliance): healthchecker manages ingress-facing service**

Add a new background goroutine to the appliance. It does nothing until a
"begin" channel closes. The idea is that another part of the appliance
will close this channel if the configmap state is set to a post-install
value (or on startup if this is already the case when an appliance
boots).

After this barrier is lifted, the healtchecker periodically checks the
readiness (using k8s conditions) of each pod returned by the frontend
deployment's label selector. If even a single pod is ready, it ensures
that the service points to frontend. Otherwise, it waits for a grace
period, checks again, and if downtime persists, it points the service to
the appliance.

This should cover the following cases:
- The service is pointed to frontend after the admin clicks "go" after
  an initial successful install.
- The service is pointed to appliance after frontend downtime that
  exceeds the grace period.
- The service is promptly pointed to frontend after downtime ends.
Creates the Wolfi image for the Appliance Maintenance UI

## Test plan

```
bazel test \
      //internal/appliance/frontend/maintenance:image_test \
      //docker-images/appliance-frontend:image_test
```
…ets (#64085)

Changes in client-side code should not result in expensive rebuilds of
`frontend` locally when using bazel with `sg start`. For some reason,
iBazel still thinks it needs to be rebuilt (possibly it uses `query`
instead of `cquery` and therefore doesnt resolve `selects`), but bazel
underneath determines nothing needs to be rebuilt so theres very little
cost

## Test plan

Command set with just `frontend` in a `bazelCommands` section and then
`sg start <command set>`

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
…mpat) (#64075)

Fixes GRAPH-751
Fixes GRAPH-636

Reverts #64046 which itself was a revert of
#63876 due to backwards
compatibility concerns.

This new PR adds two extra commits which restore the `indexingEnabled`
field, and make `syntacticIndexingEnabled` field optional.

---

Fixes GRAPH-636
Fixes GRAPH-751

This PR aims to allow admins to enable syntactic indexing on per-policy
basis using the existing UI and APIs with the following amendments:

- New UI toggle for syntactic indexing – **only visible if the
experimental feature is enabled** (see screenshots below)
- New field in GraphQL API for syntactic indexing
- New Go resolvers to handle the new field

To support this on the backend, we also update the policies handling
code and SQL queries to ensure we can retrieve and update the
`syntactic_indexing_enabled` field.

<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->

## Test plan

- Backend part covered by updated policies tests
- Manual testing of UI

With experimental feature **enabled**:

![CleanShot 2024-07-22 at 12 56
56](https://github.com/user-attachments/assets/e95aa224-be9a-40a6-9e42-6f409478e2fc)

With experimental feature **disabled**:
![CleanShot 2024-07-22 at 12 57
21](https://github.com/user-attachments/assets/0d46a65d-95bc-4695-a3df-ad9aa86dbd36)

<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

## Changelog

- In site-admin APIs for updating code intelligence policies:
  - field `indexingEnabled` is renamed to `preciseIndexingEnabled`
- a required `syntacticIndexingEnabled` is added (only takes effect if
experimental feature is enabled)
  - field `forIndexing` is renamed to `forPreciseIndexing`

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
)

Adds a read-only "known baddies" list of feature/action combination
where we know the event will produce marshalling or validation errors,
and skips the error-logging step.

Note that the code fails the entire batch the way it works today, but in
practice, none of our clients batch events yet, so this is something to
fix when we start doing that.

## Test plan
```
sg start enterprise
```

```gql
mutation {
  telemetry {
    recordEvents(
      events: [{feature: "cody.completion", action: "persistence:present", source: {client: "VSCode.Cody", clientVersion: "0.14.1"}, parameters: {version: 0, privateMetadata: 12}}]
    ) {
      alwaysNil
    }
  }
}
```

get an error response, but does not record a log in `sg start` output
…nd appliance cfg defaults (#64021)

<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
This PR implements [this
comment](https://linear.app/sourcegraph/issue/REL-218/ingressservice-consistency-for-helm-deployment#comment-6e1b88b5)
from
[REL-218](https://linear.app/sourcegraph/issue/REL-218/ingressservice-consistency-for-helm-deployment).
~~Currently, not included in `reconcileFrontendService` or
`reconcileFronteendIngress`. It's still a **work in progress**,
requesting review to make sure I'm on the right path, the test doesn't
currently pass.~~
Got the tests to pass locally, and added them to
`reconcileFronteendService` and `reconcileFronteendIngress`, however,
I'd like to deploy this locally with helm to ease my concern in it
breaking.

~~This PR alone isn't enough to close REL-218 either, we'll need
something like @craigfurman's [PR in
deploy-sourcegraph-helm](sourcegraph/deploy-sourcegraph-helm#509)
as well~~
This PR no longer requires that helm values be serialized, since we
adopt the object that exists in k8s rather than reading serialized helm
values.

## Test plan

<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
Unit tests included

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
- feat/appliance: Include existing objects when constructing Frontend's
`Service` & `Ingress`

---------

Co-authored-by: Craig Furman <[email protected]>
This PR makes a maintenance splash page intended to display service
health after installation completes.
Intended as WIP no nav yet

<img width="973" alt="Screenshot 2024-07-24 at 12 00 44 AM"
src="https://github.com/user-attachments/assets/3e2ecd67-3ef7-4bdc-9f45-d8740eb426f8">

## Test plan
Run local Golden Tests

<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Relates to #64041

This refactors the `adminanalytics` package to set the cache explicitly
instead of implicitly relying on the global `redispool.Store`. The
global store obfuscated the dependency and also made testing a bit
awkward.

Test plan:
- new unit test
- I ran a local instance and checked for panics in the logs from the
worker job that updates the cache on startup.
- Checked that the following GQL query returned results 

```GQL
query {
  site {
    analytics {
      search(dateRange: LAST_MONTH, grouping: WEEKLY) {
        searches {
          nodes {
            date
            count
          }
          summary {
            totalCount
            totalUniqueUsers
            totalRegisteredUsers
          }
        }
      }
    }
  }
}
```
- I deleted the cache and ran the GQL query again and verified that
cache had the following new entries
```
1) "adminanalytics:Search:Searches:LAST_MONTH:WEEKLY:nodes"
2) "adminanalytics:Search:Searches:LAST_MONTH:WEEKLY:summary"
```
…ed matching (#64082)

At the heart of the loop for extracting usages across a Sourcegraph
instance is the `extractLocationsFromPosition` function, which
extracts related symbols and source ranges from a single SCIP
Document. (Source ranges for returning to the user directly,
and related symbols to do further lookups, e.g. in the case
of inheritance.)

Since we want to perform matching based on symbol names in the upcoming
precise usagesForSymbol API, and also return symbol names for each
associated source range, this function needs to be updated to:
1. Be able to take a symbol name for doing lookups. This is done using
    the new `FindUsagesKey` type which allows two cases - position-based and
    symbol-based.
2. Be able to return symbol names associated with every source range.
    This is done by creating a new `UsageBuilder` type which somewhat subsumes
    the `Location` type. We avoid copying the same 'UploadID' and 'Path'
    fields eagerly for clarity; that will be handled by callers in the future when
    they mix `UsageBuilder` values across different Documents (by first calling `build`).

For the above, I've introduced a new func `extractRelatedUsagesAndSymbolNames`,
and `extractLocationsFromPosition` delegates to that. In the future,
`extractLocationsFromPosition` will be removed.
…low to webhooks (#64036)

Closes SRCH-741
Closes SRCH-716

This PR removes the GitHub installation code from the redirect flow to a
webhook-based appraoch. We expect that the GitHub server calls the
webhook when the installation is ready, and therefore shouldn't see the
errors explained in the issues above.

To handle the potential delay until the webhook is called and the
credential is set up, I added a scrappy info notice that the user should
refresh their page:

<img width="928" alt="Screenshot 2024-07-24 at 13 48 24"
src="https://github.com/user-attachments/assets/4d298f2a-d7b8-423b-9e2f-2ae53fbce1ac">

Below is what you see after you refreshed (or if the webhook was called
faster than the user being redirected back to the settings):

<img width="929" alt="Screenshot 2024-07-24 at 13 50 14"
src="https://github.com/user-attachments/assets/b6826158-8561-476d-b20e-e36f8cfb86fd">

I'm able to create PRs for sourcegraph-testing with an app that was
created this way.

<img width="1171" alt="Screenshot 2024-07-24 at 16 16 06"
src="https://github.com/user-attachments/assets/86e20acb-136f-4a46-a33b-bdfdd0d51d71">

I'm seeing an error when getting an access token with a personal github
app to run a batch change, but that will be handled with another PR.

<img width="1053" alt="Screenshot 2024-07-24 at 16 38 38"
src="https://github.com/user-attachments/assets/5655ba91-1ae4-453a-8d5c-1bcdbe34bc17">

## Test plan

Manual testing locally, and more testing to be done on S2 where we have
a more production like environment

## Changelog

- When installing a GitHub app for batch changes, the instance now waits
for a callback from GitHub to complete the installation to avoid issues
from eventual consistency.

---------

Co-authored-by: Peter Guy <[email protected]>
Most other code host connections we support have this property to
control the generated name, but for some reason it was forgotten for
Gerrit.

This PR adds it, plus a few tests.

![Screenshot 2024-07-26 at
[email protected]](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/Xvbrpl1hwVbe4tb9QeLp/6b7f2e35-1147-4e6d-912c-628ea94f2f33.png)

Test plan: Added tests, cloned a gerrit repo locally.

## Changelog 

Added support for the `repositoryPathPattern` setting for Gerrit code
host connections.
If a build is triggered from the web the variable BUILDKITE_BUILD_AUTHOR
is not set which the msp_deploy.sh script requires. This PR uses
BUILDKITE_BUILD_CREATOR as a fallback if _AUTHOR is missing

## Test plan
Tested locally
Leftover that somehow slipped through 🤦🏻 

## Test plan
CI
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
This only contains one commit which has a performance improvement
experiment hidden behind an environment variable.

- sourcegraph/zoekt@12ce07a298 index:
experiment to limit ngram lookups for large snippets

Test Plan: CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet