jolpica / jolpica-f1

Apache License 2.0
41 stars 1 forks source link

Standings endpoints do not work without specifying a year #29

Open theOehrly opened 4 months ago

theOehrly commented 4 months ago

When the driver or constructor standings endpoint is called without specifying a year, the end of season standings for multiple years should be returned, starting with the oldest season. Currently, a server error is encountered and status code 500 is returned.

Example: http://api.jolpi.ca/ergast/f1/driverstandings.json?limit=100 http://ergast.com/api/f1/driverstandings.json?limit=100

This is likely similar to #25.

jolpica commented 4 months ago

I think this might be worth marking as an "intentional difference" as it would require a lot of work to get working in the same way as ergast (multiple tables in a response just wont work with how its currently implemented).

It is definitely possible, but I would like to know how widely used this endpoint is, and whether its worth the effort.

The endpoint has been updated to return this response for now:

{
    "detail": "Bad Request: Missing one of the required parameters ['season_year']."
}
theOehrly commented 4 months ago

I think this is probably fine. The only use case that I see for the current behaviour is getting all/many standings in as few requests as possible. This is probably not that common?

Also, I imagine that many people might still be iterating year by year for that anyway. You don't really know what data you get in advance for any given limit and offset query parameter values. And the first and last entries may be incomplete and split over multiple queries. So this is more involved to do client side, even if it is more efficient due to needing fewer requests.

@chrisfree do we want to have an "intentional difference" label and a "requires documentation" label for things like this so that we can properly keep track of that and don't forget to document it?

chrisfree commented 4 months ago

@jolpica and @theOehrly I think your logic here makes good sense.

I am wondering, how difficult it would be to retain the current alias that Ergast has, wherein URLs like the following:

...simply return the standings for the current season.

I will add some labels now, as suggested. Solid way for us to keep track of these decisions over time.

jolpica commented 4 months ago

Are you suggesting that http://api.jolpi.ca/ergast/f1/driverStandings/ should redirect to http://api.jolpi.ca/ergast/f1/current/driverStandings/

Or that we should just keep these current aliases as they are?

theOehrly commented 4 months ago

I think Chris meant the opposite, but this is already the current behaviour in Ergast. So his suggestion is how it actually works right now?

@chrisfree The "current" keyword is equivalent to using "2024" and returns the standings for only the current season. Using no specifier for the season returns the standings for multiple seasons until the query limit is reached.

So I don't think there is a need to change the behaviour of the alias (except for the change suggested in #42).

chrisfree commented 4 months ago

I think I got us all confused here. When I was testing the current variant over the weekend I was getting 500 errors, but those disappeared once I replaced current with 2024. So I think my point is now moot. Sorry for the confusion. I was just trying to ensure that those that currently rely on the current parameter didn't see a major regression.