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

Allow skipping nil bytes for WithCredentialsJSON #2647

Closed
lexcao opened this issue Jun 21, 2024 · 1 comment · Fixed by #2667
Closed

Allow skipping nil bytes for WithCredentialsJSON #2647

lexcao opened this issue Jun 21, 2024 · 1 comment · Fixed by #2667
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@lexcao
Copy link

lexcao commented Jun 21, 2024

Background

I added a small feature to a repository authzed/spicedb#1942 that supports passing WithCredentialsJSON to Spanner options. Since this is just library code, it simply forwards the option directly.

I discovered that WithCredentialsJSON doesn't work correctly with nil slice, as converting them to an empty slice results in invalid JSON. To address this issue, I submitted another PR: authzed/spicedb#1948.

Problem

I believe the issue lies in how the WithCredentialsJSON function handles a nil slice within the option pattern, treating it as an empty slice.

For user code, I agree with that use case; we call the option as needed.

However, for library code, it's more ergonomic to forward options directly without needing to consider how to pass them in such cases.

Solution

We can skip nil slice for the value.
This way the library code can simply forward the option directly.

Additional context

It seems there aren't enough use cases for handling slices in the option pattern.
I am looking for other libraries that handle this case, but I have only found one.

@lexcao lexcao added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jun 21, 2024
@codyoss
Copy link
Member

codyoss commented Jul 2, 2024

Although I think this would fix the issue I think the bigger problem is under the hood there are a couple of spots that are checking != nil instead of len() > 0. I will send out a patch for this.

codyoss added a commit to codyoss/google-api-go-client that referenced this issue Jul 2, 2024
noahdietz pushed a commit that referenced this issue Jul 2, 2024
gcf-merge-on-green bot pushed a commit that referenced this issue Jul 9, 2024
🤖 I have created a release *beep* *boop*
---


## [0.188.0](https://togithub.com/googleapis/google-api-go-client/compare/v0.187.0...v0.188.0) (2024-07-09)


### Features

* **all:** Auto-regenerate discovery clients ([#2665](https://togithub.com/googleapis/google-api-go-client/issues/2665)) ([e84fa65](https://togithub.com/googleapis/google-api-go-client/commit/e84fa6508ebc498c3435668c48001185fbc9ce83))
* **all:** Auto-regenerate discovery clients ([#2669](https://togithub.com/googleapis/google-api-go-client/issues/2669)) ([6df3749](https://togithub.com/googleapis/google-api-go-client/commit/6df37492965b6323c6bcefa2a1ccd192b92981b4))
* **all:** Auto-regenerate discovery clients ([#2671](https://togithub.com/googleapis/google-api-go-client/issues/2671)) ([0d54a85](https://togithub.com/googleapis/google-api-go-client/commit/0d54a8540060cc79f830892fdd1fba46d73034c1))
* **all:** Auto-regenerate discovery clients ([#2673](https://togithub.com/googleapis/google-api-go-client/issues/2673)) ([88240e3](https://togithub.com/googleapis/google-api-go-client/commit/88240e3d98f3e944398c50379372eb071ebac0a2))
* **all:** Auto-regenerate discovery clients ([#2674](https://togithub.com/googleapis/google-api-go-client/issues/2674)) ([d465cec](https://togithub.com/googleapis/google-api-go-client/commit/d465cec68dbc2616c665e6ea240cd1e32c01418d))
* **all:** Auto-regenerate discovery clients ([#2675](https://togithub.com/googleapis/google-api-go-client/issues/2675)) ([a9177bd](https://togithub.com/googleapis/google-api-go-client/commit/a9177bdbdbba60c86b22bda4a7061c31d3485e4a))
* **all:** Auto-regenerate discovery clients ([#2677](https://togithub.com/googleapis/google-api-go-client/issues/2677)) ([5dd2fb2](https://togithub.com/googleapis/google-api-go-client/commit/5dd2fb237802349250c97c0ebdbb54cbd088884d))
* **all:** Auto-regenerate discovery clients ([#2678](https://togithub.com/googleapis/google-api-go-client/issues/2678)) ([d17f6be](https://togithub.com/googleapis/google-api-go-client/commit/d17f6beb5a531910b563a4383acaa383dbd3ee43))


### Bug Fixes

* Allow ForceSendFields to work for map types ([#2670](https://togithub.com/googleapis/google-api-go-client/issues/2670)) ([40b5113](https://togithub.com/googleapis/google-api-go-client/commit/40b5113127c4d66d533df16fe201898855c7c0cc))
* Check []bytes > 0 instead of nil ([#2667](https://togithub.com/googleapis/google-api-go-client/issues/2667)) ([711eb91](https://togithub.com/googleapis/google-api-go-client/commit/711eb913fe455ffe5c9d717b44762801820c0e8c)), refs [#2647](https://togithub.com/googleapis/google-api-go-client/issues/2647)

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
abs3ntdev pushed a commit to abs3ntdev/gspot that referenced this issue Jul 10, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [google.golang.org/api](https://github.com/googleapis/google-api-go-client) | require | minor | `v0.187.0` -> `v0.188.0` |

---

### Release Notes

<details>
<summary>googleapis/google-api-go-client (google.golang.org/api)</summary>

### [`v0.188.0`](https://github.com/googleapis/google-api-go-client/releases/tag/v0.188.0)

[Compare Source](googleapis/google-api-go-client@v0.187.0...v0.188.0)

##### Features

-   **all:** Auto-regenerate discovery clients ([#&#8203;2665](googleapis/google-api-go-client#2665)) ([e84fa65](googleapis/google-api-go-client@e84fa65))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2669](googleapis/google-api-go-client#2669)) ([6df3749](googleapis/google-api-go-client@6df3749))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2671](googleapis/google-api-go-client#2671)) ([0d54a85](googleapis/google-api-go-client@0d54a85))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2673](googleapis/google-api-go-client#2673)) ([88240e3](googleapis/google-api-go-client@88240e3))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2674](googleapis/google-api-go-client#2674)) ([d465cec](googleapis/google-api-go-client@d465cec))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2675](googleapis/google-api-go-client#2675)) ([a9177bd](googleapis/google-api-go-client@a9177bd))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2677](googleapis/google-api-go-client#2677)) ([5dd2fb2](googleapis/google-api-go-client@5dd2fb2))
-   **all:** Auto-regenerate discovery clients ([#&#8203;2678](googleapis/google-api-go-client#2678)) ([d17f6be](googleapis/google-api-go-client@d17f6be))

##### Bug Fixes

-   Allow ForceSendFields to work for map types ([#&#8203;2670](googleapis/google-api-go-client#2670)) ([40b5113](googleapis/google-api-go-client@40b5113))
-   Check \[]bytes > 0 instead of nil ([#&#8203;2667](googleapis/google-api-go-client#2667)) ([711eb91](googleapis/google-api-go-client@711eb91)), refs [#&#8203;2647](googleapis/google-api-go-client#2647)

</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://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MjYuMiIsInVwZGF0ZWRJblZlciI6IjM3LjQyNi4yIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Co-authored-by: Renovate Bot <renovate-bot@gitea.com>
Reviewed-on: https://git.asdf.cafe/abs3nt/gspot/pulls/17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
2 participants