feat: Handle resolving of empty maps and slices#430
Merged
kodiakhq[bot] merged 3 commits intocloudquery:mainfrom Nov 25, 2022
shimonp21:feat_json_handling_map_stringstring
Merged
feat: Handle resolving of empty maps and slices#430kodiakhq[bot] merged 3 commits intocloudquery:mainfrom shimonp21:feat_json_handling_map_stringstring
kodiakhq[bot] merged 3 commits intocloudquery:mainfrom
shimonp21:feat_json_handling_map_stringstring
Conversation
disq
reviewed
Nov 24, 2022
disq
approved these changes
Nov 24, 2022
yevgenypats
suggested changes
Nov 24, 2022
Contributor
yevgenypats
left a comment
There was a problem hiding this comment.
Looks good but it needs to be implemented in different place.
yevgenypats
suggested changes
Nov 25, 2022
Contributor
yevgenypats
left a comment
There was a problem hiding this comment.
looks good. One more improvement.
hermanschaaf
added a commit
that referenced
this pull request
Nov 25, 2022
We need slightly elevated permissions to make a comment when the PR comes from a fork, and I believe this should do the trick. Github Actions security when it comes to external PRs is a bit of a minefield, but because we're avoiding the `pull_request_target` trigger here, and not running any repository code in the delta-action itself, I believe this is safe. This should fix the issue we're seeing here #430
hermanschaaf
approved these changes
Nov 25, 2022
Contributor
hermanschaaf
left a comment
There was a problem hiding this comment.
With this change, is it possible to ever get a null entry in a column that has type map or slice? I guess not, so any queries checking for null to see if a particular config exists will need to be updated. That said, I think this change will solve more issues than it will create, and we have to choose one way or another, so I say we go for it.
yevgenypats
approved these changes
Nov 25, 2022
yevgenypats
suggested changes
Nov 25, 2022
Contributor
yevgenypats
left a comment
There was a problem hiding this comment.
wanted to request small change
yevgenypats
approved these changes
Nov 25, 2022
In fields like 'Tags', 'Annotations' - it is more consistent if empty/null
dictionaries show up as an empty JSON object `{}`, rather than being a postgres "null"
or a JSON "null". Same for slices.
Note that this PR will not handle the case of nested fields - but it should already be an improvement on current behavior.
candiduslynx
approved these changes
Nov 25, 2022
kodiakhq bot
pushed a commit
that referenced
this pull request
Nov 25, 2022
🤖 I have created a release *beep* *boop* --- ## [1.9.0](v1.8.2...v1.9.0) (2022-11-25) ### Features * Handle resolving of empty maps and slices ([#430](#430)) ([a5672b5](a5672b5)) ### Bug Fixes * Fix docs for deeply nested tables ([#434](#434)) ([48e0466](48e0466)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
shimonp21
added a commit
to cloudquery/cloudquery
that referenced
this pull request
Nov 28, 2022
<!-- 🎉 Thank you for making CloudQuery awesome by submitting a PR 🎉 --> #### Summary 1) Depends on cloudquery/plugin-sdk#430. 2) Caveat: This API doesn't support all authentication options (if trying to use application-default credentials, returns: ``` 2:50PM ERR table resolver finished with error error="rpc error: code = PermissionDenied desc = Your application has authenticated using end user credentials from the Google Cloud SDK or Google Cloud Shell which are not supported by the apikeys.googleapis.com. We recommend configuring the billing/quota_project setting in gcloud or using a service account through the auth/impersonate_service_account setting. For more information about service accounts and how to use them in your application, see https://cloud.google.com/docs/authentication/. If you are getting this error with curl or similar tools, you may need to specify 'X-Goog-User-Project' HTTP header for quota and billing purposes. For more information regarding 'X-Goog-User-Project' header, please check https://cloud.google.com/apis/docs/system-parameters.\nerror details: name = ErrorInfo reason = SERVICE_DISABLED domain = googleapis.com metadata = map[consumer:projects/<...> service:apikeys.googleapis.com]" client=cq-playground module=gcp-src table=gcp_apikeys_keys ``` The API was tested with service-account credentials (via GOOGLE_APPLICATION_CREDENTIALS) and worked OK. <!-- Explain what problem this PR addresses --> <!-- Use the following steps to ensure your PR is ready to be reviewed - [ ] Read the [contribution guidelines](../blob/main/CONTRIBUTING.md) 🧑🎓 - [ ] Test locally on your own infrastructure - [ ] Run `go fmt` to format your code 🖊 - [ ] Lint your changes via `golangci-lint run` 🚨 (install golangci-lint [here](https://golangci-lint.run/usage/install/#local-installation)) - [ ] Update or add tests 🧪 - [ ] Ensure the status checks below are successful ✅ --->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In fields like 'Tags', 'Annotations' - it is more consistent if empty/null dictionaries show up as an empty JSON object
{}, rather than being a postgres "null" or a JSON "null".Summary
Use the following steps to ensure your PR is ready to be reviewed
go fmtto format your code 🖊golangci-lint run🚨 (install golangci-lint here)