Support for parsing custom scopes in config file#581
Conversation
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
force-pushed
the
custom-scopes
branch
from
54ffcc4 to
782d3c9
Compare
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
force-pushed
the
custom-scopes
branch
from
782d3c9 to
f1b8d08
Compare
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
force-pushed
the
custom-scopes
branch
from
f1b8d08 to
42aef75
Compare
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
changed the title
| @ConfigAttribute(env = "DATABRICKS_CLIENT_SECRET", auth = "oauth", sensitive = true) | ||
| private String clientSecret; | ||
|
|
||
| @ConfigAttribute(env = "DATABRICKS_SCOPES", auth = "oauth") |
There was a problem hiding this comment.
It actually wasn't possible to pass scopes through environment variable because Lists weren't parsed correctly, so removing this is not a breaking change.
Removing the environment variable attribute for consistency with the other SDKs where we are not supporting setting scopes via environment variables.
|
|
||
| @ConfigAttribute(env = "DATABRICKS_SCOPES", auth = "oauth") | ||
| @ConfigAttribute(auth = "oauth") | ||
| private List<String> scopes; |
There was a problem hiding this comment.
This is a backward-incompatible change. What's the rationale for removing this environment variable?
There was a problem hiding this comment.
I've explained the rationale in this comment: #581 (comment)
databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigAttributeAccessor.java
Outdated
Show resolved
Hide resolved
databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigAttributeAccessor.java
Outdated
Show resolved
Hide resolved
databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigAttributeAccessor.java
Outdated
Show resolved
Hide resolved
databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigAttributeAccessor.java
Outdated
Show resolved
Hide resolved
databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java
Outdated
Show resolved
Hide resolved
databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java
Show resolved
Hide resolved
databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java
Outdated
Show resolved
Hide resolved
| field.set(cfg, seconds > 0 ? Duration.ofSeconds(seconds) : null); | ||
| } else if (field.getType() == ProxyConfig.ProxyAuthType.class) { | ||
| field.set(cfg, ProxyConfig.ProxyAuthType.valueOf(value)); | ||
| } else if (List.class.isAssignableFrom(field.getType())) { | ||
| // Handle List<String> fields (e.g., scopes) |
There was a problem hiding this comment.
I have a question about this big if-else block. This block doesn't have a default else, which throws an error when an unexpected type appears in the config. So is the current code simply ignoring those types?
There was a problem hiding this comment.
Yes, it silently ignores those types. I can't think of a good reason for it to be this way.
There was a problem hiding this comment.
We considered adding a default block that would fail for unexpected types, but rejected it for this PR, as it could lead to breaking changes.
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
tejaskochar-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
2 participants
Summary
The Java SDK already had support for custom scopes in all three OAuth flows. It only lacked support for configuring scopes in a config file. This PR covers that gap.
Testing
Tested by loading configurations from a test config file and resolving them.
NO_CHANGELOG=true