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

Upgrade gorm dependency in Turing API #261

Merged
merged 5 commits into from
Oct 19, 2022

Conversation

krithika369
Copy link
Collaborator

@krithika369 krithika369 commented Oct 12, 2022

Upgrading to Gorm v2: https://gorm.io/

Changes similar to: caraml-dev/xp#43

Summary of Changes

  • api/turing/service/router_service.go - Use upsert in the place of Create / Update
  • api/turing/service/router_version_service.go - Use router version ID to test Create / Update. Where foreign key record creation is involved (Enricher / Ensembler), upsert cannot be used. Previously, the NewRecord function was testing the same thing. See: Is there an equivalent to *gorm.DB.NewRecord() that was in GORM 1? go-gorm/gorm#3400 and db.NewRecord() return false when I insert something first time go-gorm/gorm#1044
  • Count() query works with int64 now, as opposed to int.
  • api/turing/service/ensembler_service.go, api/turing/service/ensembling_job_service.go - The concurrent count query has been modified to serial execution as it sometimes causes data race in tests. As such, the gain may not be significant.
  • api/turing/models/pyfunc_ensembler.go - Update BeforeCreate to match the new signature - gorm.Scope has been removed.

@krithika369 krithika369 self-assigned this Oct 12, 2022
@krithika369 krithika369 marked this pull request as draft October 12, 2022 14:02
@krithika369 krithika369 marked this pull request as ready for review October 12, 2022 14:17
@krithika369 krithika369 requested a review from a team October 12, 2022 14:17
Copy link
Contributor

@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just left a tiny comment but otherwise everything looks good! Thanks for updating this! 🚀

api/turing/service/router_version_service.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants