Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Cody: add repo-level context filters support#61641

Merged
taras-yemets merged 77 commits into
mainfrom
ty/apply-cody-context-filters-to-remote-context
Apr 15, 2024
Merged

Cody: add repo-level context filters support#61641
taras-yemets merged 77 commits into
mainfrom
ty/apply-cody-context-filters-to-remote-context

Conversation

@taras-yemets

@taras-yemets taras-yemets commented Apr 5, 2024

Copy link
Copy Markdown
Contributor

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

  1. Use different context filters for PLG and enterprise users.
  2. Implement repo-level Cody context filter based on the site config rules.

Follow-up work:

Notes to reviewers:

  1. Context filter logic depending on .cody/.ignore file remains unchanged, it was moved as-is to a separate file (dotcom.go).

Test plan

  • CI
  • Added unit tests
  • Tested manually: both enterprise and PLG scenarios work

@cla-bot cla-bot Bot added the cla-signed label Apr 5, 2024
@taras-yemets taras-yemets changed the base branch from main to ty/add-cody-ignore-rules-to-site-config April 5, 2024 12:48
Comment thread cmd/frontend/internal/context/init.go Outdated
@keegancsmith keegancsmith self-requested a review April 9, 2024 14:47
Comment thread internal/codycontext/dotcom.go Outdated

@keegancsmith keegancsmith left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the dotcom cache was likely not useful. I suspect the most expensive part before was reading the ignore file from git which I imagine was a pretty minor part of the work done in a codycontext request.

Comment thread internal/codycontext/context.go Outdated
Comment thread internal/codycontext/context.go Outdated
Comment thread internal/codycontext/dotcom.go Outdated
Comment thread internal/codycontext/dotcom.go Outdated
Comment thread internal/codycontext/dotcom.go Outdated
}

type filtersConfig struct {
cache *lru.Cache[api.RepoID, bool]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a decent chance this cache is also not that useful given you say the size of a list of repos from the client is relatively small. However, lets leave it in and consult the prometheus metrics in the future to determine if its atleast doing something. I think the main thing this protects against is an admin generating a very large config. But I'd be tempted to just not have the cache and rather monitor response times.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base automatically changed from ty/add-cody-ignore-rules-to-site-config to main April 15, 2024 20:38
@taras-yemets taras-yemets merged commit 1a13911 into main Apr 15, 2024
@taras-yemets taras-yemets deleted the ty/apply-cody-context-filters-to-remote-context branch April 15, 2024 21:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backend: Apply repo-level Cody context filter to remote context

4 participants