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

result.id of GET accountsettings function of cloudshell is returning nil #159

Closed
kavya498 opened this issue Oct 29, 2021 · 13 comments
Closed

Comments

@kavya498
Copy link
Member

Describe the bug
result.id of GET accountsettings function of cloudshell is returning nil

Expected behavior
It should have returned unique ID

@padamstx
Copy link
Member

@kavya498 Could you please make sure that you are using the latest version of the platform-services-go-sdk, and also ensure that debug logging has been turned on in the go core (I think there is a standard way to do this within the tf environment). In that scenario, you should see request and response messages logged as the various operations are being invoked. That should tell us if the correct info is in fact coming back from the server. Once we verify that it is, we can then make sure that we're unmarshalling it correctly within the SDK code.

In the generated service code in the Go SDK, I see this within the UnmarshalAccountSettings function (used to unmarshal the response body for the GetAccountSettings() operation:

func UnmarshalAccountSettings(m map[string]json.RawMessage, result interface{}) (err error) {
	obj := new(AccountSettings)
	err = core.UnmarshalPrimitive(m, "_id", &obj.ID)
	if err != nil {
		return
	}

So, the SDK is certainly TRYING to unmarshal the "_id" field from the response body json string :)
The trace of the request/response messages should show us if it's actually being sent back from the server.

@padamstx
Copy link
Member

@kavya498 I ran the Go SDK integration test for the service and here are the debug request/response messages for the GetAccountSettings operation:

2021/10/29 13:09:22 [Debug] Request:
GET /api/v1/user/accounts/2de836702ef00a3435bab2a105c5452b/settings HTTP/1.1
Host: api.shell.cloud.ibm.com
User-Agent: platform-services-go-sdk/0.22.2 (lang=go; arch=amd64; os=linux; go.version=go1.16.8)
Accept: application/json
Authorization: [redacted]
Accept-Encoding: gzip


2021/10/29 13:09:22 [DEBUG] GET https://api.shell.cloud.ibm.com/api/v1/user/accounts/2de836702ef00a3435bab2a105c5452b/settings
2021/10/29 13:09:22 [Debug] Response:
HTTP/2.0 200 OK
Cf-Cache-Status: DYNAMIC
Cf-Ray: 6a5e5ac4694e67a8-DFW
Content-Type: application/json; charset=utf-8
Date: Fri, 29 Oct 2021 18:09:22 GMT
Expect-Ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
Server: cloudflare
X-Request-Id: 497ab6e8-bd89-485c-bd46-f843238e7cdd

{"type":"account_settings","account_id":"2de836702ef00a3435bab2a105c5452b","enabled":true,"regions":[{"key":"eu-de","enabled":true},{"key":"jp-tok","enabled":true},{"key":"us-south","enabled":true}],"default_enable_new_regions":true,"features":[{"key":"server.file_manager","enabled":true},{"key":"server.web_preview","enabled":true}],"default_enable_new_features":true}

And you can see that the _id field is not present in the response body, so this is why you're not seeing that being unmarshalled. Looks like this is a problem with the server if the _id field should in fact be returned in all scenarios.
I think you'll need to pursue this with the service team so the problem can be addressed in the service implementation.

@padamstx
Copy link
Member

@hartyt ^^^

@padamstx
Copy link
Member

BTW, it looks like the following fields from the "AccountSettings" schema are missing from the operation's response:
_id, _rev, created_at, created_by, updated_at, updated_by.

@seancallanan
Copy link

@padamstx I'm not familiar with the sdk but I tried to do an example:

`package main

import (
"github.com/IBM/platform-services-go-sdk/ibmcloudshellv1"
"fmt"
"encoding/json"
)

var accountID="681694096ef8338ad439a95a1b897e00"

func main() {
serviceClientOptions := &ibmcloudshellv1.IBMCloudShellV1Options{}
ibmCloudShellService, err := ibmcloudshellv1.NewIBMCloudShellV1UsingExternalConfig(serviceClientOptions)
getAccountSettingsOptions := &ibmcloudshellv1.GetAccountSettingsOptions{
AccountID: &accountID,
}

accountSettings, _, err := ibmCloudShellService.GetAccountSettings(getAccountSettingsOptions)
if err != nil {
    panic(err)
}
b, _ := json.MarshalIndent(accountSettings, "", "  ")
fmt.Println(string(b))
fmt.Println(*accountSettings.ID)

}`

I get all the values printed out and I get the ID printed. Am I missing something or is this a bug only affecting particular accounts?

@padamstx
Copy link
Member

padamstx commented Nov 1, 2021

Not sure why the service would behave differently for different accounts, if that is in fact happening. I've contacted the team that supports the service, so hopefully they'll have some answers soon.

@seancallanan
Copy link

Sorry @padamstx I'm from the team.

The issue we are seeing happens only the first time a user access the AccountSettings because there is no database entry created. You should be able to update the settings and in the process all those fields will be created.

@padamstx
Copy link
Member

padamstx commented Nov 2, 2021

@seancallanan

Sorry @padamstx I'm from the team.

Oh, sorry about that... I had pinged Bo on slack and didn't know that you were working on the cloudshell service :)

From a user's standpoint, it seems weird that they would first need to update the account settings before they can retrieve the account settings (and see all the fields filled in) for an existing account. This seems sorta like a bug in the service to me. The GET operation has a certain set of properties defined in the response, and only some of them are getting set.
Would it be possible to modify the behavior of the GET operation so that if there is no db entry, the service first creates one so that the server-provided property values are populated, then it returns the full response (including all the currently missing properties)? What do you think?

@padamstx
Copy link
Member

padamstx commented Nov 2, 2021

@kavya498 Given the above discussion about the current behavior of the GET operation, it seems like that would cause some potential issues in the terraform provider, wouldn't it? I mean... it seems like the terraform provider would be much more straightforward if the GET operation just always returned the values associated with the server-populated properties (e.g. _id, _rev, etc.).

@seancallanan
Copy link

@seancallanan

Sorry @padamstx I'm from the team.

Oh, sorry about that... I had pinged Bo on slack and didn't know that you were working on the cloudshell service :)

From a user's standpoint, it seems weird that they would first need to update the account settings before they can retrieve the account settings (and see all the fields filled in) for an existing account. This seems sorta like a bug in the service to me. The GET operation has a certain set of properties defined in the response, and only some of them are getting set. Would it be possible to modify the behavior of the GET operation so that if there is no db entry, the service first creates one so that the server-provided property values are populated, then it returns the full response (including all the currently missing properties)? What do you think?

That makes sense, I'll put it in the backlog and will try to get to it in the next iteration

@padamstx
Copy link
Member

padamstx commented Nov 2, 2021

Thanks @seancallanan Could you add a link to the new issue here as well?

@kavya498 Once Sean provides the link to the new issue, can we close this out? You could then monitor the other issue for when the service will be updated. Once that change is made, the Go SDK behavior should just change accordingly.

@seancallanan
Copy link

@kavya498 Given the above discussion about the current behavior of the GET operation, it seems like that would cause some potential issues in the terraform provider, wouldn't it? I mean... it seems like the terraform provider would be much more straightforward if the GET operation just always returned the values associated with the server-populated properties (e.g. _id, _rev, etc.).

https://github.ibm.com/cloudshell/cloudshell-api/issues/262

@padamstx
Copy link
Member

Since this issue will need to be addressed in the cloudshell service implementation, I've cc'd Kavya in the cloudshell issue linked above, and I will close out this one...
@kavya498 Please follow the above issue for info about when the problem has been resolved.

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

No branches or pull requests

3 participants