v2 API draft #86
v2 API draft #86
Conversation
v2/devices.go
Outdated
| @@ -8,6 +8,14 @@ import ( | |||
| "time" | |||
| ) | |||
|
|
|||
| type Devices struct { | |||
There was a problem hiding this comment.
If we're keeping things in the same package, do we really want to use these simpler names for these services? I suspect we're more likely to run into conflicts with types we want to use for data types within the API. DevicesService or similar?
There was a problem hiding this comment.
I'm open to qualifying the names somehow, though I don't like Service since our API is explicitly not service oriented by entity-oriented/RESTful.
There was a problem hiding this comment.
How about DevicesResource, since we're modeling REST resources here?
v2/client_test.go
Outdated
| @@ -125,6 +125,7 @@ func TestACL_Unmarshal(t *testing.T) { | |||
| Name: "It should handle HuJSON ACLs", | |||
| ACLContent: huJSONACL, | |||
| UnmarshalFunc: func(b []byte, v interface{}) error { | |||
| b = append([]byte{}, b...) | |||
There was a problem hiding this comment.
Should we fix this in v1 too?
There was a problem hiding this comment.
| @@ -0,0 +1,213 @@ | |||
| package tailscale | |||
There was a problem hiding this comment.
Named this file policyfile to match the api docs section naming?
v2/devices.go
Outdated
| @@ -17,7 +25,7 @@ type ( | |||
|
|
|||
| // SetDeviceSubnetRoutes sets which subnet routes are enabled to be routed by a device by replacing the existing list | |||
| // of subnet routes with the supplied routes. Routes can be enabled without a device advertising them (e.g. for preauth). | |||
| func (c *Client) SetDeviceSubnetRoutes(ctx context.Context, deviceID string, routes []string) error { | |||
| func (c *Devices) SetDeviceSubnetRoutes(ctx context.Context, deviceID string, routes []string) error { | |||
There was a problem hiding this comment.
Thoughts on making this func (d *Devices)...
And similar for all the others
There was a problem hiding this comment.
It had occurred to me too. Probably makes sense to rename.
There was a problem hiding this comment.
Done. The names might have to change if we don't go with the Resource suffix.
v2/dns.go
Outdated
| // that changing the list of DNS nameservers may also affect the status of MagicDNS (if MagicDNS is on). | ||
| func (c *DNS) SetDNSNameservers(ctx context.Context, dns []string) error { | ||
| func (c *DNS) SetNameservers(ctx context.Context, dns []string) error { |
There was a problem hiding this comment.
Not sure if you already did a scan, but would be nice to have these names match our api docs
There was a problem hiding this comment.
BTW, I assume you're referring to the operationId? If so, we have two problems:
- The operationId is
setDnsNameservers, which stutters - The operationId doesn't appear to actually be shown to the user anywhere in the openapi docs (not even in URLs)
oxtoacart
force-pushed
the
percy/v2-2
branch
2 times, most recently
from
17343c4 to
0728d12
Compare
Updates tailscale/corp#21867 Signed-off-by: Percy Wegmann <percy@tailscale.com>
One of the unit test modified huJSONACL in place, even while parallel tests are using the same value. Updates tailscale/corp#21867 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21867 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21867 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21867 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Helps distinguish them from actual data types, for example a 'DevicesResource' may provide access . Updates tailscale/corp#21867 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21876 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21876 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21876 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21876 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Updates tailscale/corp#21876 Signed-off-by: Percy Wegmann <percy@tailscale.com>
|
Closing in favor of #89 |
It's useful to look at the commits individually.