sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.12k stars 1.29k forks source link

remove xlang-java gradle, artifact enviroment variables from site configuration #1654

Open ggilmore opened 6 years ago

ggilmore commented 6 years ago

All of the above are configuration options that are only used by deploy-sourcegraph's templating logic in order to set environment variables for the xlang-java deployment. These have no effect on sourcegraph/sourcegraph's logic (i.e., these options do nothing for sourcegraph:server). Since we aren't using the templating logic for the pure yaml language server deployments (sourcegraph/deploy-sourcegraph#68), we won't be able to rely on this.

I think the longer term solution is to make xlang-java be able to read these options over workspace/configuration. For now though, I think it's fine to:

slimsag commented 6 years ago

Yeah this is not good, this is all debt from when we used sourcegraph-server-gen. We used to have all the data center options inside the site config, but that should no longer be the case / we should move away from that approach going forward.

I suspect that unfortunately Uber is probably relying on these options currently, cc @beyang to confirm. I'm not sure if other customers would be.

I think the longer term solution is to make xlang-java be able to read these options over workspace/configuration.

I agree 👍

Also note that we have something that configures the language server itself (rather than the workspace), which can act a bit quicker in the langserver's startup procedure. You can set arbitrary initialize request parameters via the site config here:

https://sourcegraph.sgdev.org/github.com/sourcegraph/sourcegraph/-/blob/schema/site.schema.json#L488-517

(I'm fine with either immediate solution, as long as we avoid breaking anyone's data center deployment that relies on these)

beyang commented 6 years ago

Yes, Uber currently uses them (and Lyft might also use them). Before they are removed, we will need (1) an alternate way to get these values into xlang-java (probably through workspace/configuration or initialize params as @ggilmore suggests); and (2) migration instructions for Uber.

nicksnyder commented 6 years ago

Action item is to add info to our migration doc in our yaml branch on deploy-sourcegraph

nicksnyder commented 6 years ago

Then in 2.12 (or whenever existing customers update), we can fully remove these from site config

ggilmore commented 6 years ago

https://github.com/sourcegraph/deploy-sourcegraph/commit/f1287cc082e866eb47acd2c674fbc26d1e3e5e32

stale[bot] commented 5 years ago

This issue has not had any recent activity. To keep our open issue list representative of our actual priorities, it will be closed if no further activity occurs. If this issue should stay open, simply remove the stale label. Thank you for your contributions.