Fix: per_page param

[angela-tran]

Closes #61

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
littlepay/api
client.py
products.py
Project Total

This report was generated by python-coverage-comment-action

[angela-tran]

@thekaveman In _get_list, do you think we should remove perPage from the params dict if per_page was passed in as a kwarg?

littlepay/littlepay/api/client.py

Lines 132 to 134 in 9191b43

	def _get_list(self, endpoint: str, **kwargs) -> Generator[dict, None, None]:
	params = dict(page=1, perPage=100)
	params.update(kwargs)

[thekaveman]

@thekaveman In _get_list, do you think we should remove perPage from the params dict if per_page was passed in as a kwarg?

Yeah that's a good question. I'm not super happy with all these duplicated per_page all over the place either. Maybe a different approach is to invert the logic, use per_page by default in _get_list and pass in the override perPage on the (I think only one??) endpoint it is needed.

It sounds like they are trying to normalize these and so hopefully soon we won't have to worry about the different versions.

As to your actual question, I don't think it's a problem to send both, and we can save ourselves a few cycles by not worrying about removing it / writing tests etc.

[angela-tran]

@thekaveman In _get_list, do you think we should remove perPage from the params dict if per_page was passed in as a kwarg?

Yeah that's a good question. I'm not super happy with all these duplicated per_page all over the place either. Maybe a different approach is to invert the logic, use per_page by default in _get_list and pass in the override perPage on the (I think only one??) endpoint it is needed.

It sounds like they are trying to normalize these and so hopefully soon we won't have to worry about the different versions.

As to your actual question, I don't think it's a problem to send both, and we can save ourselves a few cycles by not worrying about removing it / writing tests etc.

That all makes sense. I can make those changes, thanks for the suggestions 👍

[thekaveman]

@angela-tran I merged #62 so if you want to rebase on main to get that one as well 👍 🙏

[thekaveman]

Nice!