Juniper / apstra-go-sdk

Go SDK for Apstra
Apache License 2.0
5 stars 1 forks source link

Move rendered config stuff from bp client to platform client #442

Closed chrismarget-j closed 1 month ago

chrismarget-j commented 1 month ago

These functions were initially written as methods on our TwoStageL3ClosClient object:

But they're not specific to the datacenter reference design. They are (appear to be?) equally applicable to freeform reference design.

This PR moves those methods from TwoStageL3ClosClient to Client and requires the blueprint ID to be passed as an argument since it's not embedded in the Client object.

No logic changes.

rajagopalans commented 1 month ago

What is the reasoning here? Don't we normally put blueprint-type functions in the blueprint client

chrismarget-j commented 1 month ago

A little bit of "trying to follow the lead established by the API", which makes distinctions between platform/datacenter/freeform, even when intermingling them at the same URL path, and a little bit of trying to avoid duplicate work in the SDK (re-writing the same method for each refdesign-specific client).

In this case, I didn't notice the issue until starting work on the apstra_blueprint_device_config data source. "blueprint" here, because it's equally applicable to both reference designs.

Should the data source implement datasourceWithSetClient, datasourceWithSetDcBpClientFunc or datasourceWithSetFfBpClientFunc?

The way I started writing it (with a DC client), it would never be usable in the freeform context.

There are likely other cases where I've made this mistake and not caught it, so I'm not claiming to be 100% consistent here. It's aspirational, I guess?

We could restore these methods on the TwoStageL3Clos and Freeform structs, but they should probably be no more than thin wrappers around the method on Client.

rajagopalans commented 1 month ago

Sounds good.