-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Typeorm updation #1606
Typeorm updation #1606
Changes from 30 commits
7892463
8c62634
e478009
d95ffc7
442680d
5a2c73f
c441ab4
35e8709
2ecf865
3fcd3d0
cb163d0
c6b15e7
f254123
9ea7d2d
ea21f5c
53f1771
dcc9fee
e1f733f
6da36d4
37e5414
e5566ec
f678bf0
c969037
851a2c8
18d0244
b2d4d95
3595a59
c275654
d5e650c
521ffac
8d1d8c5
8b32f28
ce16d2f
56e8bb4
92d7893
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1014,7 +1014,7 @@ export class ExperimentClientController { | |
@Delete('clearDB') | ||
public async clearDB(@Req() request: AppRequest): Promise<string> { | ||
// if DEMO mode is enabled, then clear the database: | ||
if (env.app.demo) { | ||
if (!env.app.demo) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please remove. i have to assume someone was doing this in debugging and accidentally committed it, if this change snuck into prod, anyone with a token would be able to delete our database by doing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. code updated |
||
await this.experimentUserService.clearDB(request.logger); | ||
return 'DB truncate successful'; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the separate new env var needed for the typeorm update? does it need to be an array if it's just one thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it was part of improvement we found while updating replica connection syntax during this PR. Its kept as an array for consistency with the other replica hostnames var. We can make it a string if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, this complicates promoting this to production as it's an unexpected configuration change that I don't remember being talked about. @bcb37 @VivekFitkariwala what do you think