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

Conversation

@emidoots
Copy link
Member

@emidoots emidoots commented Jan 22, 2021

This PR causes the database migrations in migrations/codeinsights to be executed in dev deployments (not yet in other deployments, as we do not have TimescaleDB deployed yet.)

Depends on #17585 and #17588

Helps #17219

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

@emidoots emidoots added team/internal-contributor code-insights Issues related to the Code Insights product labels Jan 22, 2021
@emidoots emidoots added this to the 3.25 milestone Jan 22, 2021
@emidoots emidoots requested a review from efritz January 22, 2021 23:14
@codecov
Copy link

codecov bot commented Jan 22, 2021

Codecov Report

Merging #17586 (3ef723c) into main (7252a70) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main   #17586   +/-   ##
=======================================
  Coverage   51.53%   51.53%           
=======================================
  Files        1717     1717           
  Lines       85390    85391    +1     
  Branches     7652     7652           
=======================================
+ Hits        44004    44009    +5     
+ Misses      37509    37503    -6     
- Partials     3877     3879    +2     
Flag Coverage Δ *Carryforward flag
go 50.41% <100.00%> (+<0.01%) ⬆️
integration 30.73% <ø> (ø) Carriedforward from 7252a70
storybook 30.62% <ø> (ø) Carriedforward from 7252a70
typescript 54.24% <ø> (ø) Carriedforward from 7252a70
unit 34.60% <ø> (ø) Carriedforward from 7252a70

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
cmd/frontend/internal/cli/config.go 10.05% <100.00%> (+0.50%) ⬆️
...nal/campaigns/resolvers/changeset_apply_preview.go 65.92% <0.00%> (+1.48%) ⬆️
internal/leader/leader.go 86.66% <0.00%> (+13.33%) ⬆️

@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jan 22, 2021

Notifying subscribers in CODENOTIFY files for diff 7252a70...3ef723c.

Notify File(s)
@felixfbecker enterprise/internal/insights/insights.go
@keegancsmith internal/conf/conftypes/conftypes.go

emidoots pushed a commit that referenced this pull request Jan 22, 2021
This is a prerequisite to running codeinsights DB migrations: #17586

Helps #17219

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
emidoots pushed a commit that referenced this pull request Jan 25, 2021
This is a prerequisite to running codeinsights DB migrations: #17586

Helps #17219

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
@emidoots emidoots force-pushed the sg/insights-execute-migrations branch 2 times, most recently from a93252e to 84c53b3 Compare January 27, 2021 08:17
This PR causes the database migrations in `migrations/codeinsights` to be executed
in dev/testing deployments (not yet in other deployments, as we do not have
TimescaleDB deployed yet.)

Closes #17219

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
@emidoots emidoots force-pushed the sg/insights-execute-migrations branch from 84c53b3 to 3ef723c Compare January 27, 2021 08:22
@emidoots emidoots merged commit b877403 into main Jan 27, 2021
@emidoots emidoots deleted the sg/insights-execute-migrations branch January 27, 2021 08:29
emidoots pushed a commit that referenced this pull request Jan 27, 2021
emidoots pushed a commit that referenced this pull request Jan 27, 2021
emidoots referenced this pull request Jan 27, 2021
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 referenced this pull request Jan 27, 2021
#17731)

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 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 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>
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.

4 participants