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

Conversation

@emidoots
Copy link
Member

It appears that we introduced this in this PR
and, although there is no description, the intent was to prevent this code from running.

However, it appears that code was never merged - at least blame says it has
not existed since Sourcegraph became open source.

Also, the code is already protected by the user agent check - so I don't
think we need this.

Most importantly: having DEPLOY_TYPE=dev is incredibly bad because
it means we are not e2e testing the actual version of Sourcegraph we are
releasing! conf.IsDeployTypeSingleDockerContainer() will return false
when this is set, and various dev-only features will be turned on.

I only caught this because my PR to run some code in dev-only mode failed.

Helps #17218

Signed-off-by: Stephen Gutekanst stephen@sourcegraph.com

It appears that we introduced this in [this PR](https://github.com/sourcegraph/sourcegraph/pull/2975)
and, although there is no description, the intent was to prevent [this code from running](https://github.com/sourcegraph/sourcegraph/pull/2971/files#diff-8715b8ced982264b18aaeee9100672eec47a519289087c1d57c9c116b230836eR138).

However, it appears that code was never merged - at least blame says it has
not existed [since Sourcegraph became open source](https://github.com/sourcegraph/sourcegraph/blame/main/cmd/frontend/internal/auth/userpasswd/handlers.go#L188).

Also, the code is already protected by the user agent check - so I don't
think we need this.

**Most importantly:** having `DEPLOY_TYPE=dev` is *incredibly* bad because
it means we are not e2e testing the actual version of Sourcegraph we are
releasing! `conf.IsDeployTypeSingleDockerContainer()` will return *false*
when this is set, and various dev-only features will be turned on.

I only caught this because my PR to run some code in dev-only mode [failed](https://github.com/sourcegraph/sourcegraph/pull/17586).

Helps #17218

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
@emidoots emidoots requested a review from efritz January 27, 2021 23:49
@emidoots emidoots merged commit bbdbdd9 into main Jan 27, 2021
@emidoots emidoots deleted the sg/no-deploy-type-override branch January 27, 2021 23:52
@emidoots emidoots added this to the 3.25 milestone Jan 28, 2021
emidoots pushed a commit that referenced this pull request Jan 28, 2021
…)"

This reverts commit 5fca3e6.

This brings back the execution of code insights database migrations, which
is safe to do now that #17731 has landed.

Helps #17218

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
@emidoots emidoots added code-insights Issues related to the Code Insights product team/internal-contributor labels Jan 28, 2021
emidoots pushed a commit that referenced this pull request Jan 28, 2021
…)" (#17732)

This reverts commit 5fca3e6.

This brings back the execution of code insights database migrations, which
is safe to do now that #17731 has landed.

Helps #17218

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
@emidoots emidoots mentioned this pull request Jan 28, 2021
27 tasks
emidoots pushed a commit that referenced this pull request Jan 28, 2021
These are effectively end-to-end tests, so setting this is very bad
for the same reason described in #17731

Should also fix the current CI failure on `main`.

Helps #17218

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
emidoots pushed a commit that referenced this pull request Jan 28, 2021
* dev/ci: do not set DEPLOY_TYPE=dev for backend integration tests

These are effectively end-to-end tests, so setting this is very bad
for the same reason described in #17731

Should also fix the current CI failure on `main`.

Helps #17218

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>

* code insights: do not run on sourcegraph/server

Code Insights will not, at least initially, be supported on demo sourcegraph/server
deployments. This codifies that.

See #17222

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

code-insights Issues related to the Code Insights product slimsag-basic-scaffolding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants