Commit e51c2c6
## What changes are proposed in this pull request?
This pull request refactors the Databricks Java SDK to replace all
usages of LocalDateTime with Instant for representing date-time values.
The primary motivation is to ensure that all date-time handling is
unambiguous, always in UTC, and consistent across the SDK.
## How does time currently operate in the SDK?
Currently, tokens in the SDK use LocalDateTime for expiry timestamps,
which by itself is timezone-agnostic and could be ambiguous. Thus, it is
not clear how time is handled from the type alone and we need to look at
the implementation details of the ClockSupplier used.
The SDK currently uses system time through the `SystemClockSupplier`
class as defined
[here](https://github.com/databricks/databricks-sdk-java/blob/main/databricks-sdk-java/src/main/java/com/databricks/sdk/core/utils/SystemClockSupplier.java).
This implementation of the ClockSupplier interface returns
`Clock.systemDefaultZone()`, meaning it uses the actual system time from
the operating system. The `SystemClockSupplier` is the used throughout
the SDK to conduct OAuth token expiration checks as seen
[here](https://github.com/databricks/databricks-sdk-java/blob/main/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/Token.java)
in the `isExpired()` method of the Token class. While `LocalDateTime`
doesn't explicitly store timezone information, the system's timezone is
being implicitly used for these comparisons through
`Clock.systemDefaultZone()`. In other words, token expiry times are
implicitly measured in the system's timezone.
## Converting to UTC
To convert these expiry times to UTC while maintaining the SDK's current
behavior, we need to handle two distinct scenarios:
### 1. Tokens with `expires_in`
- The response includes the time until expiry.
- **Conversion:**
Add the `expires_in` value to the current UTC time.
- **Result:**
This approach maintains the original expiration behavior while using UTC
internally.
---
### 2. Tokens with `expires_on` (e.g., Azure CLI, Databricks CLI)
#### With Time Zone Indicator
- Tokens (such as those from the Databricks CLI) provide expiry times in
RFC 3339 format, including a time zone indicator.
- **Conversion:**
These can be easily converted to UTC using standard library methods.
#### Without Time Zone Indicator
- Example: The `expiresOn` field from Azure CLI
[Reference](https://github.com/Azure/azure-cli/blob/dev/src/azure-cli-core/azure/cli/core/_profile.py#L384)
- Tokens provide expiry timestamps without explicit time zone
information.
- The timestamp is implicitly in the system’s local time zone.
- **Conversion steps:**
1. Interpret the timestamp as local time.
2. Add the system timezone.
3. Convert this to UTC.
We do this with **`LocalDateTime.atZone(systemDefault).toInstant()`**
## How is this tested?
- Existing unit tests to ensure no regressions.
- Added new tests in
#462 which
verifies that this change does not change overall behavior of
CilTokenSource.
## Future Work
- The SDK currently parses expiry strings from external sources like
Azure CLI
- Azure CLI provides two timestamp fields:
- `expiresOn`: Local datetime without timezone (maintained for backward
compatibility)
- `expires_on`: POSIX timestamp (seconds since epoch) - preferred format
### Current Implementation
- The Java SDK currently uses the deprecated `expiresOn` field
- `LocalDateTime` creates timezone interpretation challenges
### To Do
1. **Phase out `expiresOn`**
This legacy field should be deprecated in favor of `expires_on`
2. **Update parsing logic**
Future PRs should prioritize using `expires_on` (POSIX timestamp) which:
- Provides unambiguous UTC-based expiration time
- Avoids timezone conversion complexities
NO_CHANGELOG=true
---------
Co-authored-by: Parth Bansal <[email protected]>
1 parent 7567b41 commit e51c2c6
File tree
17 files changed
+142
-98
lines changed- databricks-sdk-java/src
- main/java/com/databricks/sdk/core
- oauth
- utils
- test/java/com/databricks/sdk/core
- oauth
17 files changed
+142
-98
lines changedLines changed: 33 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
| 13 | + | |
| 14 | + | |
12 | 15 | | |
13 | 16 | | |
14 | 17 | | |
| |||
36 | 39 | | |
37 | 40 | | |
38 | 41 | | |
39 | | - | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
40 | 66 | | |
41 | 67 | | |
42 | 68 | | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
| 69 | + | |
49 | 70 | | |
50 | 71 | | |
51 | 72 | | |
52 | 73 | | |
53 | | - | |
| 74 | + | |
54 | 75 | | |
55 | | - | |
| 76 | + | |
56 | 77 | | |
57 | 78 | | |
58 | | - | |
| 79 | + | |
| 80 | + | |
59 | 81 | | |
60 | 82 | | |
61 | 83 | | |
| |||
83 | 105 | | |
84 | 106 | | |
85 | 107 | | |
86 | | - | |
| 108 | + | |
87 | 109 | | |
88 | 110 | | |
89 | 111 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| |||
166 | 166 | | |
167 | 167 | | |
168 | 168 | | |
169 | | - | |
| 169 | + | |
170 | 170 | | |
171 | 171 | | |
172 | 172 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
| 5 | + | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| |||
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
90 | | - | |
| 90 | + | |
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | | - | |
| 80 | + | |
81 | 81 | | |
82 | 82 | | |
83 | 83 | | |
Lines changed: 2 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
9 | | - | |
| 8 | + | |
10 | 9 | | |
11 | 10 | | |
12 | 11 | | |
| |||
70 | 69 | | |
71 | 70 | | |
72 | 71 | | |
73 | | - | |
| 72 | + | |
74 | 73 | | |
75 | 74 | | |
76 | 75 | | |
| |||
Lines changed: 10 additions & 12 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | | - | |
| 7 | + | |
9 | 8 | | |
10 | 9 | | |
11 | 10 | | |
| |||
19 | 18 | | |
20 | 19 | | |
21 | 20 | | |
22 | | - | |
23 | | - | |
| 21 | + | |
| 22 | + | |
24 | 23 | | |
25 | | - | |
| 24 | + | |
26 | 25 | | |
27 | 26 | | |
28 | 27 | | |
29 | 28 | | |
30 | | - | |
| 29 | + | |
31 | 30 | | |
32 | 31 | | |
33 | 32 | | |
34 | 33 | | |
35 | | - | |
36 | | - | |
| 34 | + | |
37 | 35 | | |
38 | 36 | | |
39 | 37 | | |
| |||
43 | 41 | | |
44 | 42 | | |
45 | 43 | | |
46 | | - | |
| 44 | + | |
47 | 45 | | |
48 | 46 | | |
49 | 47 | | |
| |||
52 | 50 | | |
53 | 51 | | |
54 | 52 | | |
55 | | - | |
| 53 | + | |
56 | 54 | | |
57 | 55 | | |
58 | 56 | | |
| |||
71 | 69 | | |
72 | 70 | | |
73 | 71 | | |
74 | | - | |
75 | | - | |
| 72 | + | |
| 73 | + | |
76 | 74 | | |
77 | 75 | | |
78 | 76 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
| 8 | + | |
9 | 9 | | |
10 | 10 | | |
Lines changed: 1 addition & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | 10 | | |
12 | 11 | | |
13 | 12 | | |
| |||
25 | 24 | | |
26 | 25 | | |
27 | 26 | | |
28 | | - | |
| 27 | + | |
29 | 28 | | |
30 | 29 | | |
31 | 30 | | |
| |||
Lines changed: 53 additions & 23 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
| |||
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| 18 | + | |
17 | 19 | | |
| 20 | + | |
18 | 21 | | |
19 | 22 | | |
20 | 23 | | |
| |||
27 | 30 | | |
28 | 31 | | |
29 | 32 | | |
30 | | - | |
31 | 33 | | |
32 | 34 | | |
33 | 35 | | |
| |||
157 | 159 | | |
158 | 160 | | |
159 | 161 | | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
164 | 204 | | |
165 | 205 | | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
170 | | - | |
171 | | - | |
172 | | - | |
173 | | - | |
174 | | - | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | | - | |
181 | | - | |
182 | | - | |
183 | | - | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
184 | 214 | | |
185 | 215 | | |
186 | 216 | | |
Lines changed: 1 addition & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | | - | |
16 | 15 | | |
17 | 16 | | |
18 | 17 | | |
| |||
232 | 231 | | |
233 | 232 | | |
234 | 233 | | |
235 | | - | |
| 234 | + | |
236 | 235 | | |
237 | 236 | | |
238 | 237 | | |
| |||
0 commit comments