OscarValerock / Clockify-PowerBI

This repository contains the project for a Power BI Custom connector that connects to Clockify
MIT License
14 stars 5 forks source link

PBI Template `Bad Request` - Clockify `Time Entries` Max `page-size` Changed #2

Closed JonParton closed 3 years ago

JonParton commented 3 years ago

The Clockify API seems to have had a change introduced that now makes the Time Entries endpoint give an error of Bad Request if you request a page-size of 100000 like the template currently does in the Time Entries Fx function.

Changing this to 5000 makes it work again but may be too low for a lot of use cases.

(Workspace,User)=>

let
    Source = Json.Document(
        Web.Contents(
            "https://api.clockify.me",
            [
                RelativePath = "/api/v1/workspaces/"&Workspace&"/user/"&User&"/time-entries?page-size=5000", 
                Headers=[
                    #"X-Api-Key"=(#"X-Api-Key")
                ]
            ]
        )
    ),
// .... Extra code removed for brevity

I've messaged Clockify to see if this change was intentional (as it's not listed in their API changelog and seems to be the way they themselves have suggested doing it) and if it is, what the maximum value is.

If it is intentional though the template may now need to loop getting more pages until there is no more to get! ... only issue will be the 10 reqs / second API limit so there may have to be some waiting introduced if that is possible in Power Query!...

I'll wait to see what they come back with and update here!

OscarValerock commented 3 years ago

Hey Jon, thanks for raising this issue; now, I will only change the page size to 5000 to make the connector "usable." I will need to implement pagination into the code 😑; if you get to know the max page size in your communication with Clockify, please let me know.

JonParton commented 3 years ago

Hi @OscarValerock ... Clockify have come back with the below:

Hi Jonathan,

Thank you for reaching out.

The maximum value of the "page-size" field was recently been changed and now is 1000. To make sure your requests work, please make sure not to set the value of the page-size parameter above 1000.

I hope this helps! If there's anything else I can do for you, don't hesitate to let me know and I'll be happy to help.

Kind regards, Milan Support Team | Clockify

I've gone back with them as below just to get confirmation as 5000 is deffinitely working at the moment not just 1000...

Hi Milan,

Thanks for confirming this.

If this is a new policy change from Clockify should this be listed in the API Changelog on the docs and maybe documented on the "page-size" field?

image (1)

image (2)

Be aware that this will be a breaking change for quite a few people using the API as it seems to have become an accepted pattern due to some support answers etc! I expect a lot of people will also now hit your 10 queries / second rate limiting as 1000 is quite low and there will be a lot of people implementing loop logic!

This is the Github Issue I have already raised on a project that uses this which will now have to switch to looping!

Also ... Just so you know the API currently accepts up to 5000 not just 1000... Which may be an error but one that will reduce repeated calls to your API 5 fold at least ...

Can you please confirm the actual intended maximum just so people don't have to do code changes twice?

Cheers, Jon

I'll update again soon ...

OscarValerock commented 3 years ago

Thanks Jon! And confirmed, there is a limit of queries per second 😁. image

OscarValerock commented 3 years ago

Here is the pagination logic that needs to be implemented, I paste it in here as it may take some time for being included in the custom connector.

Source = List.Generate( ()=> [ currentPage = 1, URL = Json.Document( Web.Contents( baseurl, [ RelativePath = "/workspaces/5da58c2a21b3d66467e2ef9d/user/5da58c2a21b3d66467e2ef9a/time-entries?page-size=1000&page="&Number.ToText(currentPage), Headers = headers ] ) ) ], each List.IsEmpty([URL]) = false, each[ currentPage = [currentPage]+1, URL = Json.Document( Web.Contents( baseurl, [ RelativePath = "/workspaces/5da58c2a21b3d66467e2ef9d/user/5da58c2a21b3d66467e2ef9a/time-entries?page-size=1000&page="&Number.ToText(currentPage), Headers = headers ] ) ) ], each [URL] )

image

JonParton commented 3 years ago

Cracking, thanks @OscarValerock !!

Does this handle the too many requests situation too? (Can't see any logic in the code you posted!). My guess is that the above if it gets an error will just receive an empty list and so think that is the last page?

Also - I'm currently using the PBI Template so I'll have to see if I can patch the same code in there!

Currently waiting for a final confirmation from Clockify with the last email received last night:

Milan (Clockify)

Sep 8, 2021, 20:54 GMT+2

Thank you for bringing this to my attention. I'll double-check with the team what is the exact limit on the endpoint you are using.

The Reporting endpoints have a maximum value of 1000 as stated, however, I'll have to make sure that this is also the case with the GET /workspaces/{workspaceId}/user/{userId}/time-entries endpoint also.

I'll send you an email as soon as I receive a response from the rest of the team regarding this case.

Kind regards, Milan Support Team | Clockify

If I don't get an answer tomorrow, I will prod again!

JonParton commented 3 years ago

So with our current code I think the adapted version to replace the TimeEntries Fx for the PowerBI Template version is

(Workspace,User)=>

let
    Source
    = List.Generate(
    () => [
    currentPage = 1, 
    URL = Json.Document(
        Web.Contents(
        "https://api.clockify.me", 
        [
            RelativePath = "/api/v1/workspaces/"&Workspace&"/user/"&User&"/time-entries?page-size=5000&page="
            & Number.ToText(currentPage), 
            Headers=[
                    #"X-Api-Key"=(#"X-Api-Key")
                ]
        ]
        )
    )
    ], 
    each List.IsEmpty([URL]) = false, 
    each [
    currentPage = [currentPage] + 1, 
    URL = Json.Document(
        Web.Contents(
        "https://api.clockify.me", 
        [
            RelativePath = "/api/v1/workspaces/"&Workspace&"/user/"&User&"/time-entries?page-size=5000&page="
            & Number.ToText(currentPage), 
            Headers=[
                    #"X-Api-Key"=(#"X-Api-Key")
                ]
        ]
        )
    )
    ], 
    each [URL]
    ),
    #"Unioned List" = List.Union(Source),
    #"Converted to Table" = Table.FromList(#"Unioned List", Splitter.SplitByNothing(), null, null, ExtraValues.Error),
    #"Expanded Column1" = Table.ExpandRecordColumn(#"Converted to Table", "Column1", {"id", "description", "tagIds", "userId", "billable", "taskId", "projectId", "timeInterval", "workspaceId", "isLocked", "customFieldValues", "hourlyRateDto"}, {"id", "description", "tagIds", "userId", "billable", "taskId", "projectId", "timeInterval", "workspaceId", "isLocked", "customFieldValues", "hourlyRateDto"}),
    #"Expanded Column1.timeInterval" = Table.ExpandRecordColumn(#"Expanded Column1", "timeInterval", {"start", "end", "duration"}, {"start", "end", "duration"}),
    #"Changed Type1" = Table.TransformColumnTypes(#"Expanded Column1.timeInterval",{ {"start", type datetime}, {"end", type datetime}}),
    #"Added Custom" = Table.AddColumn(#"Changed Type1", "Duration.1", each [end]-[start]),
    #"Changed Type" = Table.TransformColumnTypes(#"Added Custom",{{"Duration.1", type duration}}),
    #"Extracted Date" = Table.TransformColumns(#"Changed Type",{{"start", DateTime.Date, type date}, {"end", DateTime.Date, type date}})
in
    #"Extracted Date"

I had to List.Union the source otherwise I got errors later on! I've also used a page-size of 5000 as I know that works at the moment at least ... I will edit if Clockify come back to use with a lower amount!

Also - Did a test on the Too Many Requests problem by setting the page-size really small so I knew I would hit the limit ... Seems your query or the power of powerbi just ploughs on and retries errors until it gets all the records ... Winner! 👍🏼 ... Can't see where this happens though 🤷🏼‍♂️🎩↗🐇

JonParton commented 3 years ago

Oh Also - Prodded clockify again this morning for a response.

OscarValerock commented 3 years ago

Jon, thanks for looking into the code. Before having the chance to read your message I pushed a new version of the code with a very similar function, please give me your comments; if everything works fine I will close the issue. And yes, PQ seems that after too many requests it retries it, I do not think the limit per second will be an issue at this time.

OscarValerock commented 3 years ago

Also, at the moment the connector only pages TimeEntries; additional changes will be needed if a user has +1000 (5000) Users, Groups, Projects, etc...

JonParton commented 3 years ago

Cracking Oscar - see you have updated the cconnector 👍but I'm guessing not the template file?

That's what my bit of code refers to above!

Are you going to update that too or want me to try and do a pull request?

JonParton commented 3 years ago

Also, at the moment the connector only pages TimeEntries; additional changes will be needed if a user has +1000 (5000) Users, Groups, Projects, etc...

Yeah did think that too... Probably the lesser issue but if someone uses it really at scale it may hit!... One to do at some point but not as important!

OscarValerock commented 3 years ago

Cracking Oscar - see you have updated the cconnector 👍but I'm guessing not the template file?

That's what my bit of code refers to above!

Are you going to update that too or want me to try and do a pull request?

Would be awesome if you could do the Pull request.

JonParton commented 3 years ago

Hi @OscarValerock !

Sorry for the delay was away from the computer for a few days! I've now created pull request #3 that updates the template file.

I also got confirmation from Clockify that the max page-size for all non reporting endpoints is 5,000 so I've included an update to make it 5,000 instead of 1,000 in the connector so we limit the number of calls needed (5x reduction!). Only thing I couldn't do was build the .mez file if you would be so kind as to do the honours...

See confirmation email below:

Hi Jonathan,

I have finally received an update from the dev team about what is the exact limit on the page-size parameter.

On the endpoint you are currently using, the limit of the page-size parameter is 5000. On all other endpoints where the page-size is passed as a parameter, the limit is 5000. Only on the reports API, on the Detailed report to be precise, the page-size can be set to up to 1000.

I hope this helps! If you have any additional questions or concerns, don't hesitate to let me know and I'll be happy to help.

Kind regards, Milan Support Team | Clockify

OscarValerock commented 3 years ago

Hi @OscarValerock !

Sorry for the delay was away from the computer for a few days! I've now created pull request #3 that updates the template file.

I also got confirmation from Clockify that the max page-size for all non reporting endpoints is 5,000 so I've included an update to make it 5,000 instead of 1,000 in the connector so we limit the number of calls needed (5x reduction!). Only thing I couldn't do was build the .mez file if you would be so kind as to do the honours...

See confirmation email below:

Hi Jonathan, I have finally received an update from the dev team about what is the exact limit on the page-size parameter. On the endpoint you are currently using, the limit of the page-size parameter is 5000. On all other endpoints where the page-size is passed as a parameter, the limit is 5000. Only on the reports API, on the Detailed report to be precise, the page-size can be set to up to 1000. I hope this helps! If you have any additional questions or concerns, don't hesitate to let me know and I'll be happy to help. Kind regards, Milan Support Team | Clockify

Hey @JonParton , thanks for the collaboration! I just updated the MEZ file. great work!

JonParton commented 3 years ago

@OscarValerock Cracking stuff! Glad to have helped 🎉.

My partner and I both use the project for reporting on Timesheets etc so it's nice to be able to pay it back a bit!

Cheers again for creating it in the first place!! 😊