Rocket model properties are not aligned across endpoints

philipengberg commented 6 years ago

Thanks for an amazing API. I'm working on an iOS app that will consume more or less the whole thing 🚀

I've come across one peculiarity, however. On /rockets the properties look like this:

{
    "id": "falcon1",
    "type": "Merlin A",
    "name": "Falcon 1"
}

whereas on /launces those properties are prepended with rocket_ like this:

{
    "rocket": {
        "rocket_id": "falcon1",
        "rocket_type": "Merlin A",
        "rocket_name": "Falcon 1"
    }
}

This makes it a little more troublesome to make global model serialization.

Is there a reason for this misalignment? 🙂

Cheers 🙌

jakewmeyer commented 6 years ago

Understood. Changing it would be easy, but I don't know how much stuff is using those particular id's, so it might break a lot of stuff 🤷‍♂️

But I do agree, they should be the same for consistency's sake.

philipengberg commented 6 years ago

Do you want me to do a PR? Then you can decide whether to merge it or not 🙂

jakewmeyer commented 6 years ago

It's not something set by the application, but by the database, so a PR can't change it unfortunately.

Not sure I'm comfortable with the change, because it breaks backwards compatibility for quite a few applications. I agree that it should be named differently, but I also want to preserve backwards compatibility to an extent.

philipengberg commented 6 years ago

I see, alright 🙂 I understand. Are you aware of similar cases elsewhere in the data model?

jakewmeyer commented 6 years ago

That't the only one I've been made aware of. Most of the endpoints have very little overlap for the launches, except the rocket name, core serials, cap serials, and the launch site.

philipengberg commented 6 years ago

Cool thanks :)

By the way, if any, how many apps are you aware of that is using this API?

jakewmeyer commented 6 years ago

Some projects in the GitHub org use it, and I've also seen this app.

But in addition, just searching the api url on Github gives back 1,039 results in public repos alone. While the bigger projects here in the org could switch easily, I worry more about the +95% that are unknown to myself and the org.

If you have any ideas about how this could be solved while retaining backwards compatibility, I'm completely on board, because you shouldn't have to jump through crazy hoops just to use the data easily.

jhpratt commented 6 years ago

Would it be feasible to edit the resulting object for those endpoints only? It wouldn't be difficult to duplicate those properties onto a different key. I imagine a middleware could be created for anything matching a certain regex, which could edit the results accordingly.

On Thu, Aug 23, 2018, 14:07 Jake Meyer notifications@github.com wrote:

Some projects in the GitHub org https://github.com/r-spacex use it, and I've also seen this app https://play.google.com/store/apps/details?id=com.danielscholte.spacexlaunches .

But in addition, just searching the api url on Github gives back 1,039 results in public repos alone. While the bigger projects here in the org could switch easily, I worry more about the +95% that are unknown to myself and the org.

If you have any ideas about how this could be solved while retaining backwards compatibility, I'm completely on board, because you shouldn't have to jump through crazy hoops just to use the data easily.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/r-spacex/SpaceX-API/issues/133#issuecomment-415514958, or mute the thread https://github.com/notifications/unsubscribe-auth/ADA9M8hPNCZngEdQlatyVOBAj1kLHJheks5uTu9zgaJpZM4WGa1e .

philipengberg commented 6 years ago

I just came across a similar case:

/launces has

"launch_site": {
    "site_id": "kwajalein_atoll",
    "site_name": "Kwajalein Atoll",
    "site_name_long": "Kwajalein Atoll Omelek Island"
},

whereas /launchpads has

{
    "padid": 1,
    "id": "kwajalein_atoll",
    "full_name": "Kwajalein Atoll Omelek Island",
}

jakewmeyer commented 6 years ago

@jhpratt They could be easily edited before shipping the json out.

I see two potential solutions here:

Bite the bullet and bring rockets, launchpads, and other endpoints in line with the keys in launches
Iterate to a version 3 of the API, and correct the json response for each request

Options 1 is the lowest overhead option by far, and I can't imagine that option 2 won't have any performance hit for subbing a ton of the json objects for every response.

jakewmeyer commented 6 years ago

Honestly at this point, I think I'd just rather change the keys outright, because I'd also like to tweak the launch schema slightly, to remove some redundant fields.

jhpratt commented 6 years ago

@jakewmeyer Maybe create some benchmarks for basic key replacement? I don't think it will be as much of a difference as you're expecting; the value is stored by reference, so all that would be required server-wise is assigning a key to an integer (the pointer), while removing the old pointer.

jakewmeyer commented 6 years ago

I'll run some tests

jakewmeyer commented 6 years ago

Negligible performance difference, one example is live for rockets. https://api.spacexdata.com/v3/rockets

philipengberg commented 6 years ago

Awesome 👏

Just wanna make sure that you are aware of two very similar properties on that endpoint now: rocketid and rocket_id. There might be some serializers that aren't happy about that?

Maybe rocketid should just be id?

jakewmeyer commented 6 years ago

It should be just id. In #1 there was a feature request for it, and because we already had an id field, it din't make sense at the time. Now that we have rocket_id instead, an id field makes more sense.

jakewmeyer commented 6 years ago

Rockets and Launchpads are done

https://api.spacexdata.com/v3/rockets https://api.spacexdata.com/v3/launchpads

Let me know if you find any other inconsistencies

philipengberg commented 6 years ago

@jakewmeyer Just to be clear: Is it only the /rockets and /launchpads endpoints that have been bumped to /v3?

jakewmeyer commented 6 years ago

Correct. All the other endpoints will be added to v3 likely over the weekend, or as soon as I finish up the ships data, and add the ship id's to launches.

I won't officially release v3 for a few weeks, but as I add the new ones, they will become available.

jakewmeyer commented 6 years ago

Also planning on making the endpoints a bit more REST friendly for v3. Currently these are the v2 endpoints:

Some are too nested, and some don't accept any kind of query parameters. v3 will look like this:

HiKaylum commented 6 years ago

Nice to know, getting ready to publish my Python wrapper for the API and currently implementing V3 rocket and launch. Knowing this I can get ready for same day push update.

Will you be deprecating V2 on release or just removing it all together?

jakewmeyer commented 6 years ago

Not deprecating v2 in the foreseeable future. Creating v3 with backwards compatibility in mind.

r-spacex / SpaceX-API

Rocket model properties are not aligned across endpoints #133