Is KV really the right name for this? A more common name for the stores this abstracts over is a blobstore. Or even just s3 like. uploadstore is a bad name to be fair. Also search's convenience package around this doesn't need to match whatever name we pick here, it should probably match what search cares about in this case, which is a result store or blob store.

I could very well live with blobstore, uploadstore does indeed seem to be too focused on uploads (which it was, LSIFUploads were the reason this was made) 😬

@eseliger @keegancsmith thanks for the feedback, I did not consider the conflict with the redis-associated packages. I don't want to re-use blobstore here because we already have the blobstore service elsewhere. Since we're storing "objects" in both S3 and GCS terminology (and we already have API like ExpireObjects), how about this alternate proposal:

1. Use object as the package name instead of uploadstore. For functions/types in the package which are too ambiguous, we can add Store appropriately.
2. Inline the contents of the (in this PR) searchkv package into internal/search? We do the same for embeddings today, where there is an uploadstore.go in internal/embeddings instead of in a separate uploadstore package.

object feels overly broad to me, I could be convinced otherwise but my initial inclination is to rather call it objectstore if we have to go with object - but I also wouldn't mind the overload of blobstore here, since cmd/blobstore hosts the API, and internal/blobstore provides the client to it. That feels in line with what cmd/gitserver and internal/gitserver are meant for, for example.

No strong opinion on (2)

but my initial inclination is to rather call it objectstore if we have to go with object

The most common usage in code referring to this package is referring to the Store type, so I don't want to have objectstore.Store in the name. This kind of guideline is also mentioned in Effective Go:

The importer of a package will use the name to refer to its contents, so exported names in the package can use that fact to avoid repetition. [..] For instance, the buffered reader type in the bufio package is called Reader, not BufReader, because users see it as bufio.Reader, which is a clear, concise name. [..] Similarly, the function to make new instances of ring.Ring—which is the definition of a constructor in Go—would normally be called NewRing, but since Ring is the only type exported by the package, and since the package is called ring, it's called just New, which clients of the package see as ring.New. Use the package structure to help you choose good names.

we could rename to {blob|object|store.Client perhaps?

AFAICT, we use the "Store" convention when it involves CRUD operations and "Client" when talking to 1st party and 3rd party services, so layering-wise, I think it would make sense for a Store to internally contain a Client based on whether it's talking to S3/GCS/blobstore.

I want to avoid the name punning/duplication because that has been cited as a frequent source of confusion whenever we've had someone new join the team and on-board onto the backend codebase, as it's hard to keep track of the same name meaning different things in different parts of the code as you're jumping around.

There's also this bit in Effective Go

It's helpful if everyone using the package can use the same name to refer to its contents, which implies that the package name should be good: short, concise, evocative. By convention, packages are given lower case, single-word names; there should be no need for underscores or mixedCaps. Err on the side of brevity, since everyone using your package will be typing that name

Thoughts:

• uploadstore.Store is a silly name, and uploadsstore definitely tripped me up
• Store is taken to mean db table wrapper in a lot of places, so I'd rather not reuse it here
• blob|object|upload.Storage might be a way to keep the connection with the non-db "blobstore" concept, and still give a hint that something different is going on?

I don't think there is a single solution which will satisfy everyone, so I'm going to pick something in-between and merge it on Monday unless someone complains loudly.

1. Using object for the package name, with object.Storage for the main interface (instead of object.Store or objectstore.Store or blobstore.Store)
2. I've inlined the search related wrapper into internal/search in a new file object_storage.go
3. I've changed the names in the embeddings code to be similar to the search related code (I think we can factor these out potentially, the configuration objects seem to be identical in 3 places).

This fixes the poor uploadstore naming which I think we all agree on. I've also avoided the key-value terminology as requested.