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

Remove unnecessary timestamp columns from models #219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danielkza
Copy link
Contributor

Remove the timestamp columns from the KalibroModule, ModuleResult,
MetricResult and ProcessingTime models. They were not used by anything,
but needed to be populated and inserted in millions of records.

This will require a KalibroClient change in the future.

@rafamanzo
Copy link
Member

Have you tested how does this affect kalibro_client?

@danielkza
Copy link
Contributor Author

It will be necessary to change the processor entities to not expect those columns. We just need to make sure we release a new kalibro_client stable version together with the next stable processor version.

Remove the timestamp columns from the KalibroModule, ModuleResult,
MetricResult and ProcessingTime models. They were not used by anything,
but needed to be populated and inserted in millions of records.

This will require a KalibroClient change in the future.
@danielkza
Copy link
Contributor Author

Rebased to master. Ping @mezuro/core.

@rafamanzo
Copy link
Member

I'm waiting for #225 to get merged before this, so the release of 1.3.3 is easier.

@diegoamc
Copy link
Contributor

Ok for me 👍

@danielkza
Copy link
Contributor Author

This probably should go into a new major version.

@rafamanzo
Copy link
Member

A question came to me right now: do we have any reference if this has any impact on insertion time? Or select times? Or DB size in disk?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants