search

lowlydba / lowlydba.sqlserver

:spoon: A cross-platform Ansible collection using PowerShell to configure and maintain SQL Server.
https://galaxy.ansible.com/ui/repo/published/lowlydba/sqlserver
GNU General Public License v3.0
20 stars 12 forks source link

Deal with rate limiting better on sp_whoisactive #176

Closed briantist closed 1 year ago

briantist commented 1 year ago

https://github.com/lowlydba/lowlydba.sqlserver/actions/runs/4224563658/jobs/7335679027#step:9:1676

For tests of modules that need to reach out to download things (I think there might be some others besides this one), we might want to add retries/until to those tasks to make the CI a little more resilient to them.

lowlydba commented 1 year ago

Unfortunately so far all my retry attempts have just escalated the rate limiting :( Still thinking about a novel approach that doesn't involve reducing testing.

briantist commented 1 year ago

Hm, looking through the code, all I can really see as an alternative is testing the local_file parameter instead. It looks like that isn't tested yet in the integration tests, so I think adding tests for that is great.

Keep the standard test, and used failed_when or ignore_errors with conditions that only allow the failure then the message includes the rate limiting error; this test is really only trying to test that the parameters were passed to the dbatools cmdlet correctly, so we don't really need the download to succeed, and the local_file test works for end-to-end.

The local version doesn't have to be the real version either, so we don't necessarily have to commit it (I'm not sure how it's licensed), in theory I guess you can create a no-op local file to use for this test.

lowlydba commented 1 year ago

Yeah thats true. I know there is enough difference in the underlying DbaTools cmdlet between local and download options that I wanted to be sure that nothing weird happened with the download, but ignore_errors does seem like perhaps the way to go.

lowlydba commented 1 year ago

This seems to have self-resolved at some point 🤷🏻 probably some changes on how runners are allocated/provisioned that resulted in us no longer getting rate limited often. Going to close this for now.