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

fix: Unmarshal error on meroxa api --json #171

Merged
merged 2 commits into from
Aug 2, 2021
Merged

Conversation

raulb
Copy link
Member

@raulb raulb commented Aug 2, 2021

Reverts #169


--json already worked and only showing the body content as expected however #169 did caught an issue with arrays, which is what this pull-request is addressing.

before

❯ .m api get /v1/connectors --json
could not unmarshal: %!w(*json.UnmarshalTypeError=&{array 0x1500580 1  })
Error: json: cannot unmarshal array into Go value of type map[string]interface {}

now

❯ .m api get /v1/connectors --json
[
	{
		"id": 583,
		"name": "my-resource-149-954565",
		"type": "jdbc-source",
		"config": {},
		"state": "failed",
		"trace": "trace",
		"resource_id": 149,
		"pipeline_id": 11,
		"pipeline_name": "default",
		"streams": {
			"output": [
				"resource-149-942792-public.User"
			],
			"dynamic": false
		},
		"metadata": {
			"mx:connectorType": "source"
		},
		"created_at": "2021-04-29T13:34:32.66516Z",
		"updated_at": "2021-08-02T14:35:58.671148Z"
	}
]

@raulb raulb requested a review from lovromazgon August 2, 2021 13:40
@raulb
Copy link
Member Author

raulb commented Aug 2, 2021

However, #169 did capture an issue with unmarshaling arrays.

Here's a test on how it works for single objects:

❯ .m api get /v1/resources/149
> GET /v1/resources/149
< 200 OK HTTP/2.0
< Content-Length 414
< Meroxa-Request-Id 7088ef1d-cfc2-4d44-b367-401b3cb177b4
< Vary Origin
< Date Mon, 02 Aug 2021 13:43:41 GMT
< Content-Type application/json; charset=utf-8
{
	"id": 149,
	"uuid": "4430bf85-50d4-4b34-b979-b9668ecf8a35",
	"name": "your-resources",
	"type": "postgres",
	"url": "postgres://meroxa-demo.cwyhi7jl0lln.us-east-2.rds.amazonaws.com:5432/dboP24ThrN",
	"metadata": null,
	"connector_count": 1,
	"ssh_tunnel": null,
	"status": {
		"state": "ready",
		"details": "",
		"last_updated_at": "2021-06-14T15:04:13.384547Z"
	},
	"created_at": "2021-03-31T11:38:20.09416Z",
	"updated_at": "2021-06-03T08:05:57.684458Z"
}

cli on  revert-169-meroxa_api_fix [$!] via 🐹 v1.16.3 on ☁️  (us-east-1)
❯ .m api get /v1/resources/149 --json
{
	"connector_count": 1,
	"created_at": "2021-03-31T11:38:20.09416Z",
	"id": 149,
	"metadata": null,
	"name": "your-resources",
	"ssh_tunnel": null,
	"status": {
		"details": "",
		"last_updated_at": "2021-06-14T15:04:13.384547Z",
		"state": "ready"
	},
	"type": "postgres",
	"updated_at": "2021-06-03T08:05:57.684458Z",
	"url": "postgres://meroxa-demo.cwyhi7jl0lln.us-east-2.rds.amazonaws.com:5432/dboP24ThrN",
	"uuid": "4430bf85-50d4-4b34-b979-b9668ecf8a35"
}

This PR addresses the issue with arrays.

@raulb raulb added the bug Something isn't working label Aug 2, 2021
@raulb raulb self-assigned this Aug 2, 2021
@raulb raulb changed the title Revert "Allow output JSON only for meroxa api" fix: Unmarshal error on meroxa api --json Aug 2, 2021
Copy link
Member

@lovromazgon lovromazgon left a comment

Choose a reason for hiding this comment

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

LGTM (even though acceptance test is failing, but I checked and it's not even using this command, so I don't think it's related)

@raulb raulb merged commit bf7673b into master Aug 2, 2021
@raulb raulb deleted the revert-169-meroxa_api_fix branch August 2, 2021 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants