datasource int overflow

[danielnachtrub]

Fixes an issue on datasource types that export counters that can reach values larger than an int32. Switched to TypeFloat because it's underlying type is float64.

[dokmic]

Hey @danielnachtrub,

Thanks for the contribution - that's a great job. But could you please share some examples of the numbers that cause fields to overflow on your end?

I quickly checked both our implementation and terraform-plugin-sdk. And they both use strconv underneath, so that the numbers should be converted to int64 and not int32.

To be honest, I don't think it's correct to use TypeFloat here, as those numbers cannot be fractional. It would probably be more correct to use TypeString or just get rid of the counters at all, as in #299.

[danielnachtrub]

Technically float is for sure not the right type because numbers are integers - just not int32.

Here's what we've seen in the logs:

│ Error: strconv.Atoi: parsing "2618960812": value out of range for 'rx_byte' field
│
│   with module.routeros_interface_bond[0].data.routeros_interfaces.existing_interfaces,
│   on .terraform\modules\routeros_interface_bond\main.tf line 45, in data "routeros_interfaces" "existing_interfaces":
│   45: data "routeros_interfaces" "existing_interfaces" {}
│
╵
╷
│ Error: strconv.Atoi: parsing "2772712184": value out of range for 'rx_byte' field
│
│   with module.routeros_interface_bond[0].data.routeros_interfaces.existing_interfaces,
│   on .terraform\modules\routeros_interface_bond\main.tf line 45, in data "routeros_interfaces" "existing_interfaces":
│   45: data "routeros_interfaces" "existing_interfaces" {}
│
╵
╷
│ Error: strconv.Atoi: parsing "2621148278": value out of range for 'rx_byte' field
│
│   with module.routeros_interface_bond[0].data.routeros_interfaces.existing_interfaces,
│   on .terraform\modules\routeros_interface_bond\main.tf line 45, in data "routeros_interfaces" "existing_interfaces":
│   45: data "routeros_interfaces" "existing_interfaces" {}

[vaerh]

@danielnachtrub @dokmic
Greetings, friends!
It is impossible to convert counters from int32 to int64 directly. This is an old problem related to building modules for x32 architecture. I think Michael pointed out the right option earlier - to make these fields string or remove them from the schemа if they are counters.

[danielnachtrub]

@vaerh @dokmic Okay, i can update it in this regard. As these fields are generating constantly changes and probably no deployment should rely on counters like this - do we need to expose them at all?

[vaerh]

I can't think of a case where full use of counters is possible within a deployment scenario.
I would ignore them.

[danielnachtrub]

I agree on this one. So, should I prepare a PR that removes them instead of adjusting the value types?

[vaerh]

Sure, I don't mind.

[vaerh]

@danielnachtrub

Hi!

I'll leave a link to a similar change I made yesterday.

terraform-provider-routeros/routeros/resource_tool_netwatch.go

Lines 28 to 40 in d0d88f2

	MetaSkipFields: PropSkipFields(
	// Generic
	"since", "status", "done_tests", "failed_tests",
	// ICMP
	"sent_count", "response_count", "loss_count", "loss_percent", "rtt_avg", "rtt_min", "rtt_max", "rtt_jitter",
	"rtt_stdev",
	// TCP
	"tcp_connect_time",
	// HTTP, HTTPS
	"http_status_code",
	// DNS
	"ip", "ip6", "mail_servers", "name_servers",
	),

#535

We can't just remove unnecessary attributes, as the user will get warnings about fields missing in the schema.

[danielnachtrub]

@vaerh better now?

[vaerh]

@danielnachtrub

better now?

Yes!

I have one questions: ingress_priority & random these two metrics, based on MT's description, are meaningful and should not change over time. Does it make sense to reduce them?

[vaerh]

🎉 This PR is included in version 1.59.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀