Skip to content
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

fix(logs): srv_logs table now has timestamp instead of string #2197

Merged
merged 6 commits into from Aug 2, 2019

Conversation

@franklevasseur
Copy link
Collaborator

commented Jul 31, 2019

I tested the migration both on postgres and sqlite and everything seems to work fine.

@franklevasseur franklevasseur requested a review from botpress/core Jul 31, 2019

src/bp/core/logger/db-persister.ts Outdated Show resolved Hide resolved
src/bp/sdk/botpress.d.ts Outdated Show resolved Hide resolved
): Promise<sdk.MigrationResult> => {
const { client } = database.knex.client.config
if (client === 'sqlite3') {
return { success: true, message: 'No migration to run for sqlite' }

This comment has been minimized.

Copy link
@allardy

allardy Jul 31, 2019

Contributor

No migration for sqlite ? So the field stays a string like before? We should be consistent with both databases, if we change to the date type and one isn't, are date parsing gonna work through knex ?

This comment has been minimized.

Copy link
@franklevasseur

franklevasseur Jul 31, 2019

Author Collaborator

The data doesn't need to be change, just the column type. However sqlite does'nt support alter table.

I've implemented a new solution all in knex:

  1. rename timestamp to timestamp_temp
  2. create new column timestamp of type timestamp
  3. copy all the content of the first VARCHAR column to this one (type TIMESTAMP)

Does this seams good?

This comment has been minimized.

Copy link
@allardy

allardy Aug 1, 2019

Contributor

Yep, that makes sense !

src/bp/core/logger/logger.ts Outdated Show resolved Hide resolved
@allardy
allardy approved these changes Aug 2, 2019

@franklevasseur franklevasseur merged commit 63c3d7c into minor/12-1-0 Aug 2, 2019

2 checks passed

AWS CodeBuild us-east-1 (botpress-ce-tests) Build succeeded for project botpress-ce-tests
Details
license/cla Contributor License Agreement is signed.
Details

@slvnperron slvnperron deleted the fl_log_table_timestamp branch Aug 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.