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

Specify that Datetime values should be ISO strings #155

Closed
wants to merge 3 commits into from

Conversation

lorensr
Copy link
Contributor

@lorensr lorensr commented Mar 3, 2022

Fixes #154

(I think Datetime representation is the only ambiguity—the rest isn't needed, since precise field type info is only needed in AddSearchAttributesRequest)

@@ -56,6 +56,7 @@ message Payload {
}

// A user-defined set of *indexed* fields that are used/exposed when listing/searching workflows.
// Datetime fields are represented as ISO strings.
Copy link
Member

Choose a reason for hiding this comment

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

I think there is even more clarity than this needed. ISO8601 has multiple string formats (not to be confused w/ RFC3389 and other additions many things make to those). Can you specify exact format here? If unsure, we can start with where they are converted to date (Go? ES?) and then get the exact format from there.

Copy link
Contributor Author

@lorensr lorensr Mar 4, 2022

Choose a reason for hiding this comment

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

I think Go does

var val time.Time
json.Unmarshal(payload.GetData(), &val)

image

and json.Unmarshal calls this?

https://pkg.go.dev/time#Time.UnmarshalJSON

which uses an RFC 3339 string. I'll edit to say that.

Other research, for posterity:

image

On ES side, Server has a v7 client and v6 client. v7 is date_nanos, and v6 is date:
image

v6:

		case enumspb.INDEXED_VALUE_TYPE_DATETIME:
			typeMap = map[string]interface{}{"type": "date"}
		}

Neither appears to customize format, so

  • default date_nanos format: "strict_date_optional_time_nanos||epoch_millis"

image

image

  • default date format: "strict_date_optional_time||epoch_millis"

image

What's working for me in tctl and web UI:

tctl workflow list --query 'StartTime < "2022-03-04T16:11:33.388Z"'
tctl workflow list --query 'StartTime < "2022-03-04T16:11:33.388000Z"'
tctl workflow list --query 'StartTime < "1646410293388"'

Doesn't work:

tctl workflow list --query 'StartTime < 1646410293388'

Works for me in TS SDK (starting workflow with custom searchAttributes):

"2022-03-04T16:11:33.388Z"
"2022-03-04T16:11:33.388000Z"

Doesn't work:

1646410293388 // gRPC error '1.646410293388e+12 is not a valid value for search attribute CustomDatetimeField of type Datetime',
"1646410293388" // gRPC Error '1646410293388 is not a valid value for search attribute CustomDatetimeField of type Datetime',

Copy link
Member

Choose a reason for hiding this comment

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

Does 2001-02-03T04:05:06+0000 which is a perfectly valid ISO-8601 string work?

It does not in Go using RFC3339 format. If this does not work, we need to document that this is not exactly ISO-8601. Notice https://docs.temporal.io/visibility states:

a string in RFC3339Nano format (such as "2006-01-02T15:04:05.999999999Z07:00").

Need to clarify the same in docs here if it applies.

@lorensr lorensr requested review from a team as code owners June 4, 2023 04:02
@yiminc yiminc closed this Nov 21, 2024
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.

Document SearchAttributes parsing
3 participants