sdk
sdk copied to clipboard
Setup consistent way for optional parameters (like `page` and `perpage`) through all the SDK functions
It is about other ways to solve the problem fixed in #46.
There are a lot of functions in Grafana API that could accept page
and perpage
as optional params. In a number of functions, there is also query
parameter. I think it would be good for clarity to setup them all in a consistent way through all the SDK. It could be reached in a number of ways:
- Add
page
andpergage
explicitly to the functions where it applicable. Additionally, define the functions withAll
suffix likeGetAllUsers
that would get whole object lists without paging parameters. - Setup
page
andperpage
as optional slice like that:GetUsers(paging ...int)
. It may be not obvious though and requires a careful explanation in the function comments. - Pass optional functional parameters to all the functions of the SDK where optional params allowed. So it would allow pass them in any order:
sdk.GetUsers()
sdk.GetUsers(sdk.Page(200))
sdk.GetUsers(sdk.Query("match"), sdk.Page(1), sdk.Perpage(10))
It could be refactored for the next major release with breaking changes. So the current state could be fixed with v1.0 (it has 0.x numeration yet). And the change proposed above marked with v2.0
I don't decide that variant would be better for the API clarity yet. The latest way with funcparams looks more attractive for me.
It is about other ways to solve the problem fixed in #46.
There are a lot of functions in Grafana API that could accept
page
andperpage
as optional params. In a number of functions, there is alsoquery
parameter. I think it would be good for clarity to setup them all in a consistent way through all the SDK. It could be reached in a number of ways:
- Add
page
andpergage
explicitly to the functions where it applicable. Additionally, define the functions withAll
suffix likeGetAllUsers
that would get whole object lists without paging parameters.- Setup
page
andperpage
as optional slice like that:GetUsers(paging ...int)
. It may be not obvious though and requires a careful explanation in the function comments.- Pass optional functional parameters to all the functions of the SDK where optional params allowed. So it would allow pass them in any order:
sdk.GetUsers() sdk.GetUsers(sdk.Page(200)) sdk.GetUsers(sdk.Query("match"), sdk.Page(1), sdk.Perpage(10))
It could be refactored for the next major release with breaking changes. So the current state could be fixed with v1.0 (it has 0.x numeration yet). And the change proposed above marked with v2.0
I don't decide that variant would be better for the API clarity yet. The latest way with funcparams looks more attractive for me.
I think the third option looks the best out of these as well. However, not sure that we should release v1.0
with the current code because it might not work in cases where someone might have lots of users, for example (https://github.com/grafana-tools/sdk/pull/46). IMO we should opt for correctness here before v1.0
.
Well, there is no deadline for the release here (for me anyway :) so I agree. Let make API consistent before v1.0 appeared. 0.x numeration assumes that API not stable yet.