Update OpenAPI spec to include Permissions OpenAPI Refactor #567

mgyucht · 2023-08-01T17:41:55Z

Changes

Update the Go SDK to include the updated structures for the permissions APIs.

Tests

make test passing
make fmt applied
relevant integration tests applied

examples/test.go

nfx · 2023-08-02T15:49:00Z

service/iam/api.go

-// access for various users on different objects and endpoints.
+// access for various users on different objects and endpoints:
+//
+// * **[Cluster permissions](clusters)** — Manage which users can manage,


run make doc. this may break the https://pkg.go.dev/github.com/databricks/databricks-sdk-go docs, as it points to non-existing links.

No links to break if there are no links :)
Any prior art w.r.t. links within the REST API docs?

From what I can tell, it is not possible to link to a service in our current OpenAPI model. I filed a ticket with @arusanov to support this. For now, I'm going to revert the links in this file

yep, also in the openapi spec. this just breaks the logical flow of code comments.

nfx · 2023-08-02T15:51:27Z

service/oauth2/model.go

@@ -11,6 +11,9 @@ type CreateCustomAppIntegration struct {
 	Name string `json:"name"`
 	// List of oauth redirect urls
 	RedirectUrls []string `json:"redirect_urls"`
+	// OAuth scopes granted to the application. Supported scopes: all-apis, sql,
+	// offline_access, openid, profile, email.
+	Scopes []string `json:"scopes,omitempty"`


seems unrelated, if you're only doing permissions change :)

This may have other OpenAPI changes in it as well, I just wanted to update ASAP to detect any potential issues.

nfx · 2023-08-02T15:52:02Z

service/pkg.go

@@ -82,7 +82,7 @@
 //
 // - [oauth2.OAuthEnrollmentAPI]: These APIs enable administrators to enroll OAuth for their accounts, which is required for adding/using any OAuth published/custom application integration.
 //
-// - [iam.PermissionsAPI]: Permissions API are used to create read, write, edit, update and manage access for various users on different objects and endpoints.
+// - [iam.PermissionsAPI]: Permissions API are used to create read, write, edit, update and manage access for various users on different objects and endpoints: * **[Cluster permissions](clusters)** — Manage which users can manage, restart, or attach to clusters.


may break go pkg docs

nfx · 2023-08-02T15:55:56Z

workspace_client.go

-	// access for various users on different objects and endpoints.
+	// access for various users on different objects and endpoints:
+	//
+	// * **[Cluster permissions](clusters)** — Manage which users can manage,


we have to use different syntax for cross-service referencing or links. See Deep linking: methods, classes, and external links from http://go/openapi/spec.

can you also add a linter to prevent these from being added in universe?

Yes, will add a linter there.

That style of link also does not seem to be handled appropriately?

Filed a ticket with @arusanov to add linting

codecov-commenter · 2023-08-03T12:33:47Z

Codecov Report

Patch coverage has no change and project coverage change: -0.77% ⚠️

Comparison is base (da7424a) 18.73% compared to head (a5645f2) 17.97%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #567      +/-   ##
==========================================
- Coverage   18.73%   17.97%   -0.77%     
==========================================
  Files          85       85              
  Lines        9405     9805     +400     
==========================================
  Hits         1762     1762              
- Misses       7489     7889     +400     
  Partials      154      154

Files Changed	Coverage Δ
service/compute/api.go	`12.65% <0.00%> (-0.64%)`	⬇️
service/compute/impl.go	`10.74% <0.00%> (-3.11%)`	⬇️
service/compute/model.go	`0.00% <0.00%> (ø)`
service/iam/api.go	`0.00% <0.00%> (ø)`
service/iam/impl.go	`0.00% <0.00%> (ø)`
service/iam/model.go	`0.00% <0.00%> (ø)`
service/jobs/api.go	`0.00% <0.00%> (ø)`
service/jobs/impl.go	`0.00% <0.00%> (ø)`
service/jobs/model.go	`0.00% <0.00%> (ø)`
service/ml/api.go	`0.00% <0.00%> (ø)`
... and 15 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nfx

does godoc support bold highlighting? I don't think so... but if we want to roll it out ASAP, then just fix the sha file

nfx · 2023-08-03T13:32:16Z

.codegen/_openapi_sha

looks very unintentional ;) i'd prefer if we remove the ability to put rfs into this file.

PS: I just associate refish with "a fish named RE". Can we change that? =P

I suppose we could call it ref?

ref is better :)

mgyucht · 2023-08-03T13:52:41Z

Bold does not seem to be supported. I filed a ticket to track better doc generation for our SDKs on the backlog.

## Changes This includes a change to how we expose the permissions-related APIs, for example (for the `JobsService`): * `GetJobPermissionLevels` -> `GetPermissionLevels` * `GetJobPermissions` -> `GetPermissions` * `SetJobPermissions` -> `SetPermissions` * `UpdateJobPermissions` -> `UpdatePermissions` These were added in #567 and first released as part of v0.15.0. ## Tests - [x] `make test` passing - [x] `make fmt` applied - [x] relevant integration tests applied

nfx self-requested a review August 2, 2023 15:41

nfx suggested changes Aug 2, 2023

View reviewed changes

nfx suggested changes Aug 3, 2023

View reviewed changes

Rebuild from master

f72b016

mgyucht force-pushed the update-openapi-permissions branch from 43476ea to f72b016 Compare August 4, 2023 09:03

mgyucht requested a review from nfx August 4, 2023 09:04

mgyucht enabled auto-merge (squash) August 4, 2023 09:04

Merge branch 'main' into update-openapi-permissions

a5645f2

nfx approved these changes Aug 4, 2023

View reviewed changes

mgyucht merged commit ffc82a1 into main Aug 4, 2023
3 checks passed

nfx deleted the update-openapi-permissions branch August 4, 2023 13:52

pietern mentioned this pull request Sep 4, 2023

Update OpenAPI spec #600

Merged

3 tasks

pietern mentioned this pull request Sep 14, 2023

Error json.UnmarshalTypeError in workspace Dashboards.Get() call #565

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update OpenAPI spec to include Permissions OpenAPI Refactor #567

Update OpenAPI spec to include Permissions OpenAPI Refactor #567

mgyucht commented Aug 1, 2023

nfx Aug 2, 2023

mgyucht Aug 3, 2023

mgyucht Aug 3, 2023

nfx Aug 3, 2023

nfx Aug 2, 2023

mgyucht Aug 3, 2023

nfx Aug 2, 2023

mgyucht Aug 3, 2023

nfx Aug 2, 2023

mgyucht Aug 3, 2023

mgyucht Aug 3, 2023

mgyucht Aug 3, 2023

codecov-commenter commented Aug 3, 2023 •

edited

Loading

nfx left a comment

nfx Aug 3, 2023

mgyucht Aug 3, 2023

nfx Aug 3, 2023

mgyucht commented Aug 3, 2023

Update OpenAPI spec to include Permissions OpenAPI Refactor #567

Update OpenAPI spec to include Permissions OpenAPI Refactor #567

Conversation

mgyucht commented Aug 1, 2023

Changes

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 3, 2023 • edited Loading

Codecov Report

nfx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgyucht commented Aug 3, 2023

codecov-commenter commented Aug 3, 2023 •

edited

Loading