Open felixlut opened 1 month ago
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs
. You & others like you are the reason all of this works! So thank you & happy coding! 🚀
I appreciate this super thoughtful bug report, and thank you for working to bring the new SDK into the Terraform provider. That's great!
I've reproduced what you've done here on a dummy repo (kfcampbell-terraform-provider/tf-acc-test-destroy-trfiz), and I agree there's something wonky going on in the generated code. I'm able to add custom properties successfully via the UI, and via a CURL request like the following:
curl -L -X PATCH -H "Accept: application/vnd.github+json" -H "Authorization: Bearer $(gh auth token)" -H "X-GitHub-Api-Version: 2022-11-28" https://api.github.com/repos/kfcampbell-terraform-provider/tf-acc-test-destroy-trfiz/properties/values -d '{"properties":[{"property_name":"environment","value":"test-from-curl"}]}'
When using the SDK, I'm using the following code (after creating the client as we do in the example token CLI):
customPropertyValue := models.NewCustomPropertyValue()
propertyName := "environment"
propertyValue := "test-from-go-sdk"
customPropertyValue.SetPropertyName(&propertyName)
value := models.NewCustomPropertyValue_CustomPropertyValue_value()
value.SetString(&propertyValue)
customPropertyValue.SetValue(value)
patchRequestBody := repos.NewItemItemPropertiesValuesPatchRequestBody()
patchRequestBody.SetProperties([]models.CustomPropertyValueable{customPropertyValue})
err = client.Repos().ByOwnerId("kfcampbell-terraform-provider").ByRepoId("tf-acc-test-destroy-trfiz").
Properties().Values().Patch(context.Background(), patchRequestBody, nil)
if err != nil {
log.Fatalf("error setting custom properties: %v", err)
}
This fails with a 400 Bad Request error, and I see the payload going out as:
"{\"properties\":[{\"property_name\":\"environment\",\"value\":\"test-from-go-sdk\"}]}"
When I attempt a curl request with that as the payload (as I'm somewhat suspicious of the backslashes), I get a 422, not a 400.
I think this is an error with Kiota, our generation engine, so I've filed https://github.com/microsoft/kiota/issues/4995 and will do some investigation with them.
@felixlut this should be fixed as of v0.0.23!
Thanks! I'll take a stab after work today or tomorrow
@kfcampbell setting non-multi_select
property values works now!
multi_select
values are still broken though from what I can tell, for the reason I stated in the original issue:
- The
value
attribute of anCustomPropertyValue
should implement theCustomPropertyValue_CustomPropertyValue_valueable
interface, which only have the two methodsGetString()
andSetString
, which implies that the value of a custom property can only be a string. But it can be either a string or a list of string (multi_select
custom property type). Due to this this SDK can only work with the other property types currently (except for the stuff mentioned above 😄). Thecurl
call below is valid, but there is no way of replicating it with this SDK from what I can tellcurl -L \ -X PATCH \ -H "Accept: application/vnd.github+json" \ -H "Authorization: Bearer $(gh auth token)" \ -H "X-GitHub-Api-Version: 2022-11-28" \ https://api.github.com/repos/YesWeKanske/test-custom-properties/properties/values -d '{"properties": [{"property_name": "multi-select","value": ["option1","option2"]}]}'
This might warrant its own issue though 😄
I'll also note that the Organization Ruleset API seems to handle the fact that a property value can be either a string or a list of strings in the way I proposed here, i.e., by just treating it as a list of strings at all times. It's pretty deeply nested, but Org Rulesets can target repos by custom properties, and this is where this occurs. .conditions.repository_property_and_ref_name.repository_property.include.property_values
always takes a list of strings
What happened?
I'm playing around with this sdk as a means of contributing to https://github.com/integrations/terraform-provider-github/issues/1956, and as such my focus is on the endpoints related to repository custom properties.
The issue I'm facing occurs when calling octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName).Properties().Values().Patch(), which is mapped to the repos/OWNER/REPO/properties/values
PATCH
endpoint. I'll list the code I'm using further down in the message.When running the code below I receive the following error:
error status code received from the API
I've ran the debugger and dug deeper into the call chain, and found that the error code is
400
, but haven't been able to find an error message. Digging down all the way to thenet/http
layer and inspecting the request body being sent, it looks like this:"{\"properties\":[{\"property_name\":\"text\",\"value\":\"test\"}]}"
Which isn't too dissimilar to what the API expects (example from the docs):
'{"properties":[{"property_name":"environment","value":"production"},{"property_name":"service","value":"web"},{"property_name":"team","value":"octocat"}]}'
After playing around with
curl
a bit, I've managed to receive a400
error by using malformed JSON in the body (see example below). My guess is that this SDK somewhere along the line parses the custom property incorrectly, or that I'm passing theproperty_name
andvalue
incorrectly to it.I've also explored the following:
The
value
attribute of anCustomPropertyValue
should implement theCustomPropertyValue_CustomPropertyValue_valueable
interface, which only have the two methodsGetString()
andSetString
, which implies that the value of a custom property can only be a string. But it can be either a string or a list of string (multi_select
custom property type). Due to this this SDK can only work with the other property types currently (except for the stuff mentioned above 😄). Thecurl
call below is valid, but there is no way of replicating it with this SDK from what I can tellBoth
models.CustomPropertyValue
andrepos.ItemItemPropertiesValuesPatchRequestBody
supports a function calledSetAdditionalData()
. I've tried playing around with those two, but they seem to mess up the json body that is being sent to the GitHub API by adding key value pairs to it (the two"test": "text"
below):Versions
I've used the latest version of the provider at the time of writing this bug report (
go get github.com/octokit/go-sdk@a74a3a2a13654bd84794fd91a6a0fbc96f883722
, wherea74a3a2a13654bd84794fd91a6a0fbc96f883722
is a commit to this repo). Mygo.mod
file looks like so:Code of Conduct