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

fix: use local file for sp_whoisactive install #169

Closed
wants to merge 4 commits into from

Conversation

jkrilov
Copy link
Contributor

@jkrilov jkrilov commented Dec 19, 2022

Description

Use a local file to install sp_whoisactive during integration tests to avoid 403 error

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue) - Fixes #
  • New feature (non-breaking change which adds functionality)

Checklist:

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2022

Codecov Report

Patch coverage has no change and project coverage change: -4.26% ⚠️

Comparison is base (9a5a958) 93.80% compared to head (dc9495a) 89.55%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
- Coverage   93.80%   89.55%   -4.26%     
==========================================
  Files          64       52      -12     
  Lines        2196     1445     -751     
==========================================
- Hits         2060     1294     -766     
- Misses        136      151      +15     
Flag Coverage Δ
sanity 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 15 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lowlydba
Copy link
Owner

I'm game to add this as an additional integration test, but want to keep the download option in the tests for sure to get proper end-to-end coverage since that is likely how most people will be using it.

I don't see any issues reported in dbatools about this, or incidents for Github.com so am going to guess it may be transient and will resolve itself soon. It hasn't been a problem before.

@jkrilov
Copy link
Contributor Author

jkrilov commented Dec 19, 2022

Fair enough. I'll revert the changes, and add the local file as an additional test.

@lowlydba
Copy link
Owner

Fair enough. I'll revert the changes, and add the local file as an additional test.

Thanks! I also opened a Github support issue to see if this can get resolved faster that way.

@lowlydba
Copy link
Owner

This hasn't been a problem for a while, so perhaps how the VMs are allocated, or API throttling has been changed on GitHub's side. I'm going to close for now but feel free to re-open and continue work if it becomes an issue again.

@lowlydba lowlydba closed this Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants