phalt / swapi

*NOT MAINTAINED - NO GUARENTEE TO BE UP*
https://phalt.github.io/pokeapi-and-swapi-going-forward/
BSD 3-Clause "New" or "Revised" License
890 stars 505 forks source link

Feature request: Return all IDs as v4 UUID #74

Open Joshfindit opened 7 years ago

Joshfindit commented 7 years ago

UUIDs have the benefits of being globally unique, which means that if someone brings in data from an API that returns a UUID, they can use that UUID as the internal ID as well.

If IDs are returned sequentially, the API caller have to create a different ID internally and then map it to the external ID for later updates.

(Also, for those that like an orderly database, internal UUIDs avoid the problem of leaving gaps in a sequence if removing/reordering things)

phalt commented 7 years ago

Hey @Joshfindit thanks for the feedback. I am 100% behind using UUID's instead of single integers. I tend to do this in any new system I create and I recommend it to everyone who asks for advice. The ID's in SWAPI are integers that just happen to be the primary key too (simplicity back when this was a side project). Interestingly I do get people asking me why certain ones don't exist (for example: https://github.com/phalt/swapi/issues/51) and this is because humans see incrementing numbers and assume some logic/relationship there. I do regret doing that, now!

I'm 100% behind someone updating the API so long as backwards compatibility is not affected (we have 100+ integrations and we can't guarantee they are fully HATEOAS and haven't hard-coded anything). Feel free to open a PR and start working on it and I'll be happy to give feedback.

Joshfindit commented 7 years ago

Excellent. If you don't mind, I'll add some notes on this issue; it's interesting enough to keep me coming back and hacking away at it, but I'm a B2C sysadmin turned hobby-dev with some experience in Ruby/Vue.js/Javascript/HTML/CSS, so I might not be the right person to implement it, but who knows I may end up adding Python to the list. :)

Joshfindit commented 7 years ago

Possible avenues to explore (feel free to comment/cherry-pick):

phalt commented 7 years ago

Django supports UUID fields by default: https://docs.djangoproject.com/en/1.10/ref/models/fields/#uuidfield

So that's point one resolved :)

Python also has built in uuid checking, so you don't need to do just the length check.

Joshfindit commented 7 years ago

Excellent. Putting this model code here for reference: uuid = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)

Looks like it will automatically generate without any massaging needed.

Joshfindit commented 7 years ago

As for

Python also has built in uuid checking, so you don't need to do just the length check.

Looks like there's a good definition of a function for is_valid_uuid on Stackoverflow

Basically:

try:
    uuid_obj = UUID(uuid_to_test, version=version)
except:
    return False

So then:

if is_valid_uuid(id_from_the_api_call):
   print "Query the database by UUID"
else:
   print "Query the database by integer"