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

Support x-databricks-path-style overrides at the operation level #562

Merged
merged 2 commits into from
Jul 27, 2023

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Jul 26, 2023

Changes

In order to support multiple styles of request under a single service, we introduce support for overriding x-databricks-path-style at the Operation level. This will allow RPC- and REST-style operations to exist under a single service. To determine the path style for a method, we check for a method-level override, falling back to the service-level setting. If neither is set, the path style is assumed to be "rest".

Tests

Tested by regenerating the Python SDK using a spec I edited by hand to include this extension at the operation level. The generated code was as I expected (using the operation ID to derive name in the Clusters service, which is normally an RPC style service).

  • make test passing
  • make fmt applied
  • relevant integration tests applied

@nfx nfx self-requested a review July 26, 2023 19:26
Copy link
Contributor

@nfx nfx left a comment

Choose a reason for hiding this comment

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

Where is it needed? Is it for Terminate/PermanentDelete? Can’t see this reflecting in Go SDK

@mgyucht
Copy link
Contributor Author

mgyucht commented Jul 26, 2023

It's needed for the permissions refactor being done by @Rchakmak. His permissions APIs are all RESTful, but they will be part of services that are marked as RPC style.

@mgyucht mgyucht requested a review from tanmay-db July 27, 2023 10:48
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

Patch coverage: 20.00% and project coverage change: -0.01% ⚠️

Comparison is base (cf4ac71) 18.73% compared to head (1a377a6) 18.73%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #562      +/-   ##
==========================================
- Coverage   18.73%   18.73%   -0.01%     
==========================================
  Files          85       85              
  Lines        9388     9405      +17     
==========================================
+ Hits         1759     1762       +3     
- Misses       7477     7489      +12     
- Partials      152      154       +2     
Files Changed Coverage Δ
openapi/code/method.go 17.74% <0.00%> (ø)
openapi/model.go 5.19% <0.00%> (-0.78%) ⬇️
openapi/code/service.go 46.36% <37.50%> (-0.15%) ⬇️
openapi/code/package.go 58.00% <100.00%> (ø)

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

@mgyucht mgyucht merged commit da7424a into main Jul 27, 2023
3 checks passed
@mgyucht mgyucht deleted the support-path-style-operation-overrides branch July 27, 2023 14:58
@mgyucht mgyucht mentioned this pull request Jul 27, 2023
mgyucht added a commit that referenced this pull request Jul 27, 2023
* Handle nested query parameters in Client.attempt ([#559](#559)).
* Support x-databricks-path-style overrides at the operation level ([#562](#562)).
mgyucht added a commit that referenced this pull request Jul 27, 2023
* Handled nested query parameters in Client.attempt
([#559](#559)).
* Supported x-databricks-path-style overrides at the operation level
([#562](#562)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants