refactor: migrate fields to snake_case with alias#917
Draft
HeloiseJoffe wants to merge 3 commits into
Draft
Conversation
996b067 to
7fa7c14
Compare
7fa7c14 to
8f93f08
Compare
8f93f08 to
cc983b1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #618
The FastAPI mapping is not blocking, since this can be handled correctly with the
by_alias=Trueparameter.I migrated the classes from
diracx/core/models/job.pyto snake_case with alias but not the other files fromdiracx/core/models/because they are already in snake_case.I also started working on other
BaseModelusages still defined in PascalCase, including :diracx/cli/internal/legacyanddiracx/core/config/schema.pyHowever, after re-reading the issue, I am unsure whether these should also be migrated as part of this scope.
In the issue the discussion is about the python models and the DB fields.
Could you confirm @aldbr the intended scope of #618 ?
Should all remaining
BaseModelclasses be migrated to snake_case + aliases for consistency? Or is the goal limited to models directly tied to DB field mapping ?