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.

Sign up for GitHub

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

Jump to bottom

fix(data_source_github_rest_api): just read body and convert bytes into string #2152

Merged
merged 2 commits into from Mar 12, 2024
Merged

fix(data_source_github_rest_api): just read body and convert bytes into string #2152

merged 2 commits into from Mar 12, 2024

Conversation

riezebosch
Copy link
Contributor

@riezebosch riezebosch commented Feb 16, 2024 •
edited

Resolves #1776

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@riezebosch
Copy link
Contributor Author

riezebosch commented Feb 16, 2024 •
edited

I was unable to get the the tests running locally, validated the work by testing it as a local provider. It is related to this PR: #2110 and also this would also solve #1776 and #1953

✅ I expect the test could also be expanded to validate for some or actual value instead of only presence in state file.

By not swallowing the err from the Do method you see the source of the original bug:

Error: json: cannot unmarshal object into Go value of type string

By passing nil into v the client leaves the body untouched.

**Update: **
I was able to get the tests running, updated the test to check for value and added another test to check for collection results.

Removed the err check so this change is not breaking, but would encourage to change that behaviour since it is now swallowing all exceptions.

This was referenced Feb 16, 2024
Merged
Closed
@riezebosch riezebosch closed this Feb 17, 2024
@riezebosch riezebosch reopened this Feb 17, 2024
@riezebosch
Copy link
Contributor Author

@kfcampbell ☝🏻 🥺

@kfcampbell
Copy link
Member

@riezebosch thanks for the attention here! The added test looks good to me. I'd be comfortable with adding an error check here; I think the user experience would be better if there was some kind of useful output rather than a swallowed error. Would you be comfortable adding that to this PR or would you prefer it to go in separately?

@riezebosch
Copy link
Contributor Author

@kfcampbell I already had added it in a separate PR: #2154 (but can combine the 2 if you prefer)

@riezebosch riezebosch changed the title fix(data_source_github_rest_api): just read body and convert bytes in to string fix(data_source_github_rest_api): just read body and convert bytes into string Mar 4, 2024
@riezebosch
Copy link
Contributor Author

riezebosch commented Mar 9, 2024 •
edited

@kfcampbell thanks for merging #2154, I've updated this one accordingly.

kfcampbell
kfcampbell approved these changes Mar 12, 2024
View reviewed changes
Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirming tests are passing for me. Thanks for the updates!

@kfcampbell kfcampbell merged commit af693fd into integrations:main Mar 12, 2024
3 checks passed
antnsn added a commit to CMCS-Norway/github-governance that referenced this pull request Mar 20, 2024
@antnsn
Update Terraform github to v6.2.0 (#9)
eb0808c 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github](https://registry.terraform.io/providers/integrations/github)
([source](https://togithub.com/integrations/terraform-provider-github))
| required_provider | minor | `6.1.0` -> `6.2.0` |

---

### Release Notes

<details>
<summary>integrations/terraform-provider-github (github)</summary>

###
[`v6.2.0`](https://togithub.com/integrations/terraform-provider-github/releases/tag/v6.2.0)

[Compare
Source](https://togithub.com/integrations/terraform-provider-github/compare/v6.1.0...v6.2.0)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### Features

- feat: add `github_enterprise_actions_runner_group` by
[@&#8203;bradam12](https://togithub.com/bradam12) in
[integrations/terraform-provider-github#2158
- Feat: Fixes abandoned PR
[#&#8203;2017](https://togithub.com/integrations/terraform-provider-github/issues/2017):
Add support for the require_last_push_approval flag in
github_branch_protection_v3 by
[@&#8203;georgekaz](https://togithub.com/georgekaz) in
[integrations/terraform-provider-github#2199

##### Bugfixes

- fix(data_source_github_rest_api): just read body and convert bytes
into string by [@&#8203;riezebosch](https://togithub.com/riezebosch) in
[integrations/terraform-provider-github#2152

##### 🛠️ Maintenance

- build(deps): bump golang.org/x/oauth2 from 0.17.0 to 0.18.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[integrations/terraform-provider-github#2189
- build(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[integrations/terraform-provider-github#2194

**Full Changelog**:
integrations/terraform-provider-github@v6.1.0...v6.2.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/CMCS-Norway/github-governance).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNDUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjI0NS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
kireque pushed a commit to kireque/home-ops that referenced this pull request Mar 22, 2024
@kireque-bot
feat(terraform-provider): update terraform github to 6.2.0 (#390)
91e5ee3 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github](https://registry.terraform.io/providers/integrations/github)
([source](https://togithub.com/integrations/terraform-provider-github))
| required_provider | minor | `6.0.1` -> `6.2.0` |

---

### Release Notes

<details>
<summary>integrations/terraform-provider-github (github)</summary>

###
[`v6.2.0`](https://togithub.com/integrations/terraform-provider-github/releases/tag/v6.2.0)

[Compare
Source](https://togithub.com/integrations/terraform-provider-github/compare/v6.1.0...v6.2.0)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### Features

- feat: add `github_enterprise_actions_runner_group` by
[@&#8203;bradam12](https://togithub.com/bradam12) in
[integrations/terraform-provider-github#2158
- Feat: Fixes abandoned PR
[#&#8203;2017](https://togithub.com/integrations/terraform-provider-github/issues/2017):
Add support for the require_last_push_approval flag in
github_branch_protection_v3 by
[@&#8203;georgekaz](https://togithub.com/georgekaz) in
[integrations/terraform-provider-github#2199

##### Bugfixes

- fix(data_source_github_rest_api): just read body and convert bytes
into string by [@&#8203;riezebosch](https://togithub.com/riezebosch) in
[integrations/terraform-provider-github#2152

##### 🛠️ Maintenance

- build(deps): bump golang.org/x/oauth2 from 0.17.0 to 0.18.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[integrations/terraform-provider-github#2189
- build(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[integrations/terraform-provider-github#2194

**Full Changelog**:
integrations/terraform-provider-github@v6.1.0...v6.2.0

###
[`v6.1.0`](https://togithub.com/integrations/terraform-provider-github/releases/tag/v6.1.0)

[Compare
Source](https://togithub.com/integrations/terraform-provider-github/compare/v6.0.1...v6.1.0)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

- fix: validation rule for `results_per_page` of `github_repositories`
data source by [@&#8203;dschniepp](https://togithub.com/dschniepp) in
[integrations/terraform-provider-github#2185
- fix: Prevent loading of allowed actions if not configured by
[@&#8203;Danielku15](https://togithub.com/Danielku15) in
[integrations/terraform-provider-github#2186
- fix(data_source_github_rest_api): only allow for 404 on err by
[@&#8203;riezebosch](https://togithub.com/riezebosch) in
[integrations/terraform-provider-github#2154
- fix: error if autolink reference not found by
[@&#8203;bradam12](https://togithub.com/bradam12) in
[integrations/terraform-provider-github#2164
- feat: Add `github_actions_enterprise_permissions` by
[@&#8203;ErikElkins](https://togithub.com/ErikElkins) in
[integrations/terraform-provider-github#2155
- docs: configure release notes categories based on labels by
[@&#8203;laughedelic](https://togithub.com/laughedelic) in
[integrations/terraform-provider-github#2184

#### New Contributors

- [@&#8203;dschniepp](https://togithub.com/dschniepp) made their first
contribution in
[integrations/terraform-provider-github#2185
- [@&#8203;riezebosch](https://togithub.com/riezebosch) made their first
contribution in
[integrations/terraform-provider-github#2154
- [@&#8203;bradam12](https://togithub.com/bradam12) made their first
contribution in
[integrations/terraform-provider-github#2164
- [@&#8203;ErikElkins](https://togithub.com/ErikElkins) made their first
contribution in
[integrations/terraform-provider-github#2155
- [@&#8203;laughedelic](https://togithub.com/laughedelic) made their
first contribution in
[integrations/terraform-provider-github#2184

**Full Changelog**:
integrations/terraform-provider-github@v6.0.1...v6.1.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNDAuMSIsInVwZGF0ZWRJblZlciI6IjM3LjI1Ny4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: kireque-bot[bot] <143391978+kireque-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kfcampbell kfcampbell kfcampbell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: github_rest_api supports list response
2 participants
@riezebosch @kfcampbell