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

Properly parse ISO date strings #473

Merged
merged 5 commits into from
Dec 7, 2023
Merged

Properly parse ISO date strings #473

merged 5 commits into from
Dec 7, 2023

Conversation

fjakobs
Copy link
Contributor

@fjakobs fjakobs commented Dec 5, 2023

Pre Python 3.11 the datetime.fromisoformat isn't a generic date time parser and can only be used to parse ISO dates generated by the python datatime module. This PR normalized the date string the same way the google oauth2 library does by discarding the timezone and millisecond part of the date string.

Changes

Make date parsing for tokens from the CLI more robust

Tests

  • Added unit test
  • Manually tested with CLI auth provider

@fjakobs fjakobs requested a review from mgyucht December 5, 2023 13:03
@fjakobs fjakobs force-pushed the ES-959603 branch 3 times, most recently from 8acd84d to 8a6a1e9 Compare December 6, 2023 08:28
@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5955333) 57.53% compared to head (ca4964a) 57.53%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #473   +/-   ##
=======================================
  Coverage   57.53%   57.53%           
=======================================
  Files          38       38           
  Lines       25554    25555    +1     
=======================================
+ Hits        14703    14704    +1     
  Misses      10851    10851           

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

@fjakobs fjakobs force-pushed the ES-959603 branch 2 times, most recently from ea75f44 to b62ea96 Compare December 6, 2023 08:42
@fjakobs fjakobs changed the title Use "fromisoformat" to parse tokens from the CLI Properly parse ISO date strings Dec 6, 2023
Copy link
Contributor

@mgyucht mgyucht left a comment

Choose a reason for hiding this comment

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

Small nit on the tests but otherwise LGTM.

Comment on lines 56 to 58
CliTokenSource._parse_expiry("2023-12-01T15:19:48.007742617Z")
CliTokenSource._parse_expiry("2023-12-05T15:59:01.40081+11:00")
CliTokenSource._parse_expiry("2023-12-06 10:06:05")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just add an assertion on the actual returned values?
BTW: good practice here would be to use pytest.mark.parameterize annotation, e.g.

@pytest.mark.parametrize(
    "date_string,expected",
    [("2023-12-01T15:19:48.007742617Z", <the expected date>),
      ("2023-12-05T15:59:01.40081+11:00", <the expected date>),
      ("2023-12-06 10:06:05", <the expected date>)])
def test_databricks_cli_token_parse_expiry(date_string, expected):

@fjakobs fjakobs added this pull request to the merge queue Dec 6, 2023
@mgyucht mgyucht removed this pull request from the merge queue due to a manual request Dec 6, 2023
@fjakobs fjakobs added this pull request to the merge queue Dec 7, 2023
Merged via the queue into main with commit 741238b Dec 7, 2023
9 checks passed
@fjakobs fjakobs deleted the ES-959603 branch December 7, 2023 12:18
hectorcast-db added a commit that referenced this pull request Dec 12, 2023
Bugfixes:

* Fixed accidental rename ([#471](#471)).
* Fixed parsing of ISO date strings ([#473](#473)).

Other changes:

* Updated GCP OAuth Readme ([#464](#464)).
* Reference Documentation Refactoring ([#467](#467)).
* Installed local library when generating docs ([#469](#469)).
* Fixed readme links in pypi ([#472](#472)).
* Updated a note for installing Python SDK on Databricks Runtime 13.1+ ([#474](#474)).
* Updated GCP auth readme ([#470](#470)).

API Changes:

 * Changed `update()` method for [w.connections](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/connections.html) workspace-level service with new required argument order.
 * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.CreateStorageCredential`.
 * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.StorageCredentialInfo`.
 * Changed `name` field for `databricks.sdk.service.catalog.UpdateCatalog` to be required.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateCatalog`.
 * Changed `name` field for `databricks.sdk.service.catalog.UpdateConnection` to no longer be required.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateConnection`.
 * Changed `name` field for `databricks.sdk.service.catalog.UpdateExternalLocation` to be required.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateExternalLocation`.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateMetastore`.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateRegisteredModelRequest`.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateSchema`.
 * Changed `name` field for `databricks.sdk.service.catalog.UpdateStorageCredential` to be required.
 * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.UpdateStorageCredential`.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateStorageCredential`.
 * Added `new_name` field for `databricks.sdk.service.catalog.UpdateVolumeRequestContent`.
 * Added `cloudflare_api_token` field for `databricks.sdk.service.catalog.ValidateStorageCredential`.
 * Added `databricks.sdk.service.catalog.CloudflareApiToken` dataclass.
 * Removed `continuous` field for `databricks.sdk.service.jobs.BaseRun`.
 * Removed `continuous` field for `databricks.sdk.service.jobs.Run`.
 * Changed `job_parameters` field for `databricks.sdk.service.jobs.RunJobTask` to `databricks.sdk.service.jobs.ParamPairs` dataclass.
 * Added `run_if` field for `databricks.sdk.service.jobs.SubmitTask`.
 * Added `run_job_task` field for `databricks.sdk.service.jobs.SubmitTask`.
 * Changed `update_config()` method for [w.serving_endpoints](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/serving_endpoints.html) workspace-level service with new required argument order.
 * Added `put()` method for [w.serving_endpoints](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/serving_endpoints.html) workspace-level service.
 * Added `rate_limits` field for `databricks.sdk.service.serving.CreateServingEndpoint`.
 * Changed `served_models` field for `databricks.sdk.service.serving.EndpointCoreConfigInput` to no longer be required.
 * Added `auto_capture_config` field for `databricks.sdk.service.serving.EndpointCoreConfigInput`.
 * Added `served_entities` field for `databricks.sdk.service.serving.EndpointCoreConfigInput`.
 * Added `auto_capture_config` field for `databricks.sdk.service.serving.EndpointCoreConfigOutput`.
 * Added `served_entities` field for `databricks.sdk.service.serving.EndpointCoreConfigOutput`.
 * Added `served_entities` field for `databricks.sdk.service.serving.EndpointCoreConfigSummary`.
 * Added `served_entities` field for `databricks.sdk.service.serving.EndpointPendingConfig`.
 * Added `extra_params` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `input` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `max_tokens` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `messages` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `n` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `prompt` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `stop` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `stream` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Added `temperature` field for `databricks.sdk.service.serving.QueryEndpointInput`.
 * Changed `predictions` field for `databricks.sdk.service.serving.QueryEndpointResponse` to no longer be required.
 * Added `choices` field for `databricks.sdk.service.serving.QueryEndpointResponse`.
 * Added `created` field for `databricks.sdk.service.serving.QueryEndpointResponse`.
 * Added `data` field for `databricks.sdk.service.serving.QueryEndpointResponse`.
 * Added `id` field for `databricks.sdk.service.serving.QueryEndpointResponse`.
 * Added `model` field for `databricks.sdk.service.serving.QueryEndpointResponse`.
 * Added `object` field for `databricks.sdk.service.serving.QueryEndpointResponse`.
 * Added `usage` field for `databricks.sdk.service.serving.QueryEndpointResponse`.
 * Changed `workload_size` field for `databricks.sdk.service.serving.ServedModelInput` to `databricks.sdk.service.serving.ServedModelInputWorkloadSize` dataclass.
 * Changed `workload_type` field for `databricks.sdk.service.serving.ServedModelInput` to `databricks.sdk.service.serving.ServedModelInputWorkloadType` dataclass.
 * Added `task` field for `databricks.sdk.service.serving.ServingEndpoint`.
 * Added `task` field for `databricks.sdk.service.serving.ServingEndpointDetailed`.
 * Added `databricks.sdk.service.serving.Ai21LabsConfig` dataclass.
 * Added `databricks.sdk.service.serving.AnthropicConfig` dataclass.
 * Added `databricks.sdk.service.serving.AutoCaptureConfigInput` dataclass.
 * Added `databricks.sdk.service.serving.AutoCaptureConfigOutput` dataclass.
 * Added `databricks.sdk.service.serving.AutoCaptureState` dataclass.
 * Added `databricks.sdk.service.serving.AwsBedrockConfig` dataclass.
 * Added `databricks.sdk.service.serving.AwsBedrockConfigBedrockProvider` dataclass.
 * Added `databricks.sdk.service.serving.ChatMessage` dataclass.
 * Added `databricks.sdk.service.serving.ChatMessageRole` dataclass.
 * Added `databricks.sdk.service.serving.CohereConfig` dataclass.
 * Added `databricks.sdk.service.serving.DatabricksModelServingConfig` dataclass.
 * Added `databricks.sdk.service.serving.EmbeddingsV1ResponseEmbeddingElement` dataclass.
 * Added `databricks.sdk.service.serving.EmbeddingsV1ResponseEmbeddingElementObject` dataclass.
 * Added `databricks.sdk.service.serving.ExternalModel` dataclass.
 * Added `databricks.sdk.service.serving.ExternalModelConfig` dataclass.
 * Added `databricks.sdk.service.serving.ExternalModelProvider` dataclass.
 * Added `databricks.sdk.service.serving.ExternalModelUsageElement` dataclass.
 * Added `databricks.sdk.service.serving.FoundationModel` dataclass.
 * Added `databricks.sdk.service.serving.OpenAiConfig` dataclass.
 * Added `databricks.sdk.service.serving.PaLmConfig` dataclass.
 * Added `databricks.sdk.service.serving.PayloadTable` dataclass.
 * Added `databricks.sdk.service.serving.PutRequest` dataclass.
 * Added `databricks.sdk.service.serving.PutResponse` dataclass.
 * Added `databricks.sdk.service.serving.QueryEndpointResponseObject` dataclass.
 * Added `databricks.sdk.service.serving.RateLimit` dataclass.
 * Added `databricks.sdk.service.serving.RateLimitKey` dataclass.
 * Added `databricks.sdk.service.serving.RateLimitRenewalPeriod` dataclass.
 * Added `databricks.sdk.service.serving.ServedEntityInput` dataclass.
 * Added `databricks.sdk.service.serving.ServedEntityOutput` dataclass.
 * Added `databricks.sdk.service.serving.ServedEntitySpec` dataclass.
 * Added `databricks.sdk.service.serving.ServedModelInputWorkloadSize` dataclass.
 * Added `databricks.sdk.service.serving.ServedModelInputWorkloadType` dataclass.
 * Added `databricks.sdk.service.serving.V1ResponseChoiceElement` dataclass.
 * Removed [a.account_network_policy](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_network_policy.html) account-level service.
 * Removed `databricks.sdk.service.settings.AccountNetworkPolicyMessage` dataclass.
 * Removed `databricks.sdk.service.settings.DeleteAccountNetworkPolicyRequest` dataclass.
 * Removed `databricks.sdk.service.settings.DeleteAccountNetworkPolicyResponse` dataclass.
 * Removed `databricks.sdk.service.settings.ReadAccountNetworkPolicyRequest` dataclass.
 * Removed `databricks.sdk.service.settings.UpdateAccountNetworkPolicyRequest` dataclass.
 * Removed `name` field for `databricks.sdk.service.sharing.UpdateCleanRoom`.
 * Changed `name` field for `databricks.sdk.service.sharing.UpdateProvider` to be required.
 * Added `new_name` field for `databricks.sdk.service.sharing.UpdateProvider`.
 * Changed `name` field for `databricks.sdk.service.sharing.UpdateRecipient` to be required.
 * Added `new_name` field for `databricks.sdk.service.sharing.UpdateRecipient`.
 * Changed `name` field for `databricks.sdk.service.sharing.UpdateShare` to be required.
 * Added `new_name` field for `databricks.sdk.service.sharing.UpdateShare`.
 * Added `statement_ids` field for `databricks.sdk.service.sql.QueryFilter`.
 * Added `databricks.sdk.service.sql.StatementId` dataclass.

OpenAPI SHA: 63caa3cb0c05045e81d3dcf2451fa990d8670f36, Date: 2023-12-12
@hectorcast-db hectorcast-db mentioned this pull request Dec 12, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2023
Bugfixes:

* Fixed accidental rename
([#471](#471)).
* Fixed parsing of ISO date strings
([#473](#473)).


Other changes:

* Updated GCP OAuth Readme
([#464](#464)).
* Reference Documentation Refactoring
([#467](#467)).
* Installed local library when generating docs
([#469](#469)).
* Fixed readme links in pypi
([#472](#472)).
* Updated a note for installing Python SDK on Databricks Runtime 13.1+
([#474](#474)).
* Updated GCP auth readme
([#470](#470)).


API Changes:

* Changed `update()` method for
[w.connections](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/connections.html)
workspace-level service with new required argument order.
* Added `cloudflare_api_token` field for
`databricks.sdk.service.catalog.CreateStorageCredential`.
* Added `cloudflare_api_token` field for
`databricks.sdk.service.catalog.StorageCredentialInfo`.
* Changed `name` field for
`databricks.sdk.service.catalog.UpdateCatalog` to be required.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateCatalog`.
* Changed `name` field for
`databricks.sdk.service.catalog.UpdateConnection` to no longer be
required.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateConnection`.
* Changed `name` field for
`databricks.sdk.service.catalog.UpdateExternalLocation` to be required.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateExternalLocation`.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateMetastore`.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateRegisteredModelRequest`.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateSchema`.
* Changed `name` field for
`databricks.sdk.service.catalog.UpdateStorageCredential` to be required.
* Added `cloudflare_api_token` field for
`databricks.sdk.service.catalog.UpdateStorageCredential`.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateStorageCredential`.
* Added `new_name` field for
`databricks.sdk.service.catalog.UpdateVolumeRequestContent`.
* Added `cloudflare_api_token` field for
`databricks.sdk.service.catalog.ValidateStorageCredential`.
 * Added `databricks.sdk.service.catalog.CloudflareApiToken` dataclass.
 * Removed `continuous` field for `databricks.sdk.service.jobs.BaseRun`.
 * Removed `continuous` field for `databricks.sdk.service.jobs.Run`.
* Changed `job_parameters` field for
`databricks.sdk.service.jobs.RunJobTask` to
`databricks.sdk.service.jobs.ParamPairs` dataclass.
 * Added `run_if` field for `databricks.sdk.service.jobs.SubmitTask`.
* Added `run_job_task` field for
`databricks.sdk.service.jobs.SubmitTask`.
* Changed `update_config()` method for
[w.serving_endpoints](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/serving_endpoints.html)
workspace-level service with new required argument order.
* Added `put()` method for
[w.serving_endpoints](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/serving_endpoints.html)
workspace-level service.
* Added `rate_limits` field for
`databricks.sdk.service.serving.CreateServingEndpoint`.
* Changed `served_models` field for
`databricks.sdk.service.serving.EndpointCoreConfigInput` to no longer be
required.
* Added `auto_capture_config` field for
`databricks.sdk.service.serving.EndpointCoreConfigInput`.
* Added `served_entities` field for
`databricks.sdk.service.serving.EndpointCoreConfigInput`.
* Added `auto_capture_config` field for
`databricks.sdk.service.serving.EndpointCoreConfigOutput`.
* Added `served_entities` field for
`databricks.sdk.service.serving.EndpointCoreConfigOutput`.
* Added `served_entities` field for
`databricks.sdk.service.serving.EndpointCoreConfigSummary`.
* Added `served_entities` field for
`databricks.sdk.service.serving.EndpointPendingConfig`.
* Added `extra_params` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `input` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `max_tokens` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `messages` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `n` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `prompt` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `stop` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `stream` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Added `temperature` field for
`databricks.sdk.service.serving.QueryEndpointInput`.
* Changed `predictions` field for
`databricks.sdk.service.serving.QueryEndpointResponse` to no longer be
required.
* Added `choices` field for
`databricks.sdk.service.serving.QueryEndpointResponse`.
* Added `created` field for
`databricks.sdk.service.serving.QueryEndpointResponse`.
* Added `data` field for
`databricks.sdk.service.serving.QueryEndpointResponse`.
* Added `id` field for
`databricks.sdk.service.serving.QueryEndpointResponse`.
* Added `model` field for
`databricks.sdk.service.serving.QueryEndpointResponse`.
* Added `object` field for
`databricks.sdk.service.serving.QueryEndpointResponse`.
* Added `usage` field for
`databricks.sdk.service.serving.QueryEndpointResponse`.
* Changed `workload_size` field for
`databricks.sdk.service.serving.ServedModelInput` to
`databricks.sdk.service.serving.ServedModelInputWorkloadSize` dataclass.
* Changed `workload_type` field for
`databricks.sdk.service.serving.ServedModelInput` to
`databricks.sdk.service.serving.ServedModelInputWorkloadType` dataclass.
* Added `task` field for
`databricks.sdk.service.serving.ServingEndpoint`.
* Added `task` field for
`databricks.sdk.service.serving.ServingEndpointDetailed`.
 * Added `databricks.sdk.service.serving.Ai21LabsConfig` dataclass.
 * Added `databricks.sdk.service.serving.AnthropicConfig` dataclass.
* Added `databricks.sdk.service.serving.AutoCaptureConfigInput`
dataclass.
* Added `databricks.sdk.service.serving.AutoCaptureConfigOutput`
dataclass.
 * Added `databricks.sdk.service.serving.AutoCaptureState` dataclass.
 * Added `databricks.sdk.service.serving.AwsBedrockConfig` dataclass.
* Added `databricks.sdk.service.serving.AwsBedrockConfigBedrockProvider`
dataclass.
 * Added `databricks.sdk.service.serving.ChatMessage` dataclass.
 * Added `databricks.sdk.service.serving.ChatMessageRole` dataclass.
 * Added `databricks.sdk.service.serving.CohereConfig` dataclass.
* Added `databricks.sdk.service.serving.DatabricksModelServingConfig`
dataclass.
* Added
`databricks.sdk.service.serving.EmbeddingsV1ResponseEmbeddingElement`
dataclass.
* Added
`databricks.sdk.service.serving.EmbeddingsV1ResponseEmbeddingElementObject`
dataclass.
 * Added `databricks.sdk.service.serving.ExternalModel` dataclass.
 * Added `databricks.sdk.service.serving.ExternalModelConfig` dataclass.
* Added `databricks.sdk.service.serving.ExternalModelProvider`
dataclass.
* Added `databricks.sdk.service.serving.ExternalModelUsageElement`
dataclass.
 * Added `databricks.sdk.service.serving.FoundationModel` dataclass.
 * Added `databricks.sdk.service.serving.OpenAiConfig` dataclass.
 * Added `databricks.sdk.service.serving.PaLmConfig` dataclass.
 * Added `databricks.sdk.service.serving.PayloadTable` dataclass.
 * Added `databricks.sdk.service.serving.PutRequest` dataclass.
 * Added `databricks.sdk.service.serving.PutResponse` dataclass.
* Added `databricks.sdk.service.serving.QueryEndpointResponseObject`
dataclass.
 * Added `databricks.sdk.service.serving.RateLimit` dataclass.
 * Added `databricks.sdk.service.serving.RateLimitKey` dataclass.
* Added `databricks.sdk.service.serving.RateLimitRenewalPeriod`
dataclass.
 * Added `databricks.sdk.service.serving.ServedEntityInput` dataclass.
 * Added `databricks.sdk.service.serving.ServedEntityOutput` dataclass.
 * Added `databricks.sdk.service.serving.ServedEntitySpec` dataclass.
* Added `databricks.sdk.service.serving.ServedModelInputWorkloadSize`
dataclass.
* Added `databricks.sdk.service.serving.ServedModelInputWorkloadType`
dataclass.
* Added `databricks.sdk.service.serving.V1ResponseChoiceElement`
dataclass.
* Removed
[a.account_network_policy](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_network_policy.html)
account-level service.
* Removed `databricks.sdk.service.settings.AccountNetworkPolicyMessage`
dataclass.
* Removed
`databricks.sdk.service.settings.DeleteAccountNetworkPolicyRequest`
dataclass.
* Removed
`databricks.sdk.service.settings.DeleteAccountNetworkPolicyResponse`
dataclass.
* Removed
`databricks.sdk.service.settings.ReadAccountNetworkPolicyRequest`
dataclass.
* Removed
`databricks.sdk.service.settings.UpdateAccountNetworkPolicyRequest`
dataclass.
* Removed `name` field for
`databricks.sdk.service.sharing.UpdateCleanRoom`.
* Changed `name` field for
`databricks.sdk.service.sharing.UpdateProvider` to be required.
* Added `new_name` field for
`databricks.sdk.service.sharing.UpdateProvider`.
* Changed `name` field for
`databricks.sdk.service.sharing.UpdateRecipient` to be required.
* Added `new_name` field for
`databricks.sdk.service.sharing.UpdateRecipient`.
* Changed `name` field for `databricks.sdk.service.sharing.UpdateShare`
to be required.
* Added `new_name` field for
`databricks.sdk.service.sharing.UpdateShare`.
* Added `statement_ids` field for
`databricks.sdk.service.sql.QueryFilter`.
 * Added `databricks.sdk.service.sql.StatementId` dataclass.

OpenAPI SHA: 63caa3cb0c05045e81d3dcf2451fa990d8670f36, Date: 2023-12-12
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.

3 participants