✨ Kf id prefix #135
Merged
✨ Kf id prefix #135
Conversation
This file contains 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
dankolbman
added
feature
dankolbman
force-pushed
the
kf_id_prefix
branch
2 times, most recently
from
aa36bc8 to
444e6ef
Compare
dankolbman
force-pushed
the
kf_id_prefix
branch
3 times, most recently
from
14f2a1c to
6bea2d9
Compare
parimalak
approved these changes
Feb 19, 2018
dankolbman
force-pushed
the
kf_id_prefix
branch
4 times, most recently
from
5cb61dc to
8dce359
Compare
znatty22
reviewed
Feb 22, 2018
dataservice/api/README.md
Outdated
| and seperated by an underscore. | ||
| Some examples: | ||
|
|
||
| - `PH_CZHXGVPB` |
There was a problem hiding this comment.
Minor nitpick..should we make the examples a little more clear here. So something like:
PH_CZHXGVPB(Phenotype entity)
dataservice/api/common/id_service.py
Outdated
| @@ -10,15 +10,26 @@ def uuid_generator(): | |||
| return str(uuid.uuid4()) | |||
|
|
|||
|
|
|||
| def kf_id_generator(): | |||
| def kf_id_generator(prefix=None): | |||
There was a problem hiding this comment.
Why not make prefix a required argument since it cannot be None as indicated by the assert below?
dataservice/api/common/model.py
Outdated
|
|
||
| @declared_attr | ||
| def kf_id(cls): | ||
| kf_id = db.Column(KfId(), unique=True, primary_key=True, |
There was a problem hiding this comment.
Remove unique = True since this is a primary key and we're going to wipe out migrations
dataservice/api/common/model.py
Outdated
| default=kf_id_generator(cls.__prefix__)) | ||
| return kf_id | ||
|
|
||
| uuid = db.Column(db.String(36), unique=True, default=uuid_generator) |
There was a problem hiding this comment.
Why aren't we using Postgres UUID?
dankolbman
force-pushed
the
kf_id_prefix
branch
2 times, most recently
from
fe3c4ad to
2ded275
Compare
dankolbman
force-pushed
the
kf_id_prefix
branch
from
c04702e to
4ed5261
Compare
Merged
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.
Adds a two-character prefix per table on the kf_id delimited by an underscore.
Resolves #127