Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestGetConstructors.test_get_constructors fails #140

Open
youpong opened this issue Oct 10, 2024 · 3 comments
Open

TestGetConstructors.test_get_constructors fails #140

youpong opened this issue Oct 10, 2024 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers hacktoberfest Issue for hacktoberfest participants

Comments

@youpong
Copy link
Contributor

youpong commented Oct 10, 2024

Describe the bug

The following tests fail
TestGetConstructors.test_get_constructors

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'F1PyStats'
  2. Run the command below
$ poetry run nox --python 3.12 -s tests
  1. See error

Additional context

Currently, exactly 100 json records are returned when accessing the following url.
Previously, more than 100 records (for all constructors) were returned, and this error did not occur at that time.

https://ergast.com/api/f1/constructors.json?limit=230

@alec-kr
Copy link
Owner

alec-kr commented Oct 11, 2024

Thanks for following up with more details on this issue @youpong !

It’s possible that this behavior change could be linked to Ergast API shutting down or that they've changed their pagination logic. It seems like the default limit of 100 records is now enforced, and the previous method of retrieving all records might no longer work. Additionally, the way the pagination URL parameters work may have been modified.

I’ve tried a few solutions on my end, including tweaking the limit parameter and testing different approaches to pagination, but so far, I haven’t had much success in retrieving more than the default 100 records.

I'll continue to investigate this further, but if you or anyone else has insights or potential fixes, feel free to jump in!

@youpong
Copy link
Contributor Author

youpong commented Oct 12, 2024

The solution to #136 and this could be similar.

@alec-kr
Copy link
Owner

alec-kr commented Oct 14, 2024

I’ve been thinking of a couple of possible approaches to address the 100-record limitation in the API response:

  1. Modify the Tests: We could modify the tests (and potentially the code) to only work with the 100 records returned by default. This would be a simpler solution if 100 constructors are sufficient for our use case, but it might not be ideal if we need to ensure we always get all constructor data.

  2. Refactor the Code for Aggregation: Alternatively, we could refactor the code to handle pagination by fetching all constructors across multiple API calls. We could create a few utility functions to aggregate all constructors into a complete dataset before returning it. This would ensure we get all the data and avoid any issues with future API limits.

I’m happy to explore either of these approaches or discuss other possible solutions! Let me know your thoughts.

@alec-kr alec-kr added bug Something isn't working good first issue Good for newcomers hacktoberfest Issue for hacktoberfest participants labels Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers hacktoberfest Issue for hacktoberfest participants
Projects
None yet
Development

No branches or pull requests

2 participants