Search attribute Datetime need to support full ISO 8601 (and maybe more) #144

Activity

1. cretz

   added

   bugSomething isn't workingSomething isn't working

   on Sep 27, 2022
2. cretz

   changed the title [-]Search attribute Datetime types can be RFC3339Nano, not always ISO 8601[/-] [+]Search attribute Datetime need to support full ISO 8601 (and maybe more)[/+] on Sep 27, 2022
3. cretz commented on Sep 27, 2022

   

   cretz

   on Sep 27, 2022

   ContributorAuthor

   More actions

   From https://docs.python.org/3/library/datetime.html#datetime.datetime.fromisoformat:

   Caution
   This does not support parsing arbitrary ISO 8601 strings - it is only intended as the inverse operation of datetime.isoformat(). A more full-featured ISO 8601 parser, dateutil.parser.isoparse is available in the third-party package dateutil.
4. Oracen commented on Oct 30, 2022

   

   Oracen

   on Oct 30, 2022

   Contributor

   More actions

   I've been experimenting with the new Scheduler system released in Temporal 1.18.0. Workers using the Python API break on the same problem described here. This can be replicated with the following code snippet, then setting up a schedule either in the web UI or tctl.

   import asyncio
   from datetime import datetime
   
   from dateutil import parser
   from temporalio import workflow
   from temporalio.client import Client
   from temporalio.worker import Worker
   
   
   @workflow.defn
   class Replication:
       @workflow.run
       async def run(self):
           pass
   
   
   async def main():
       client = await Client.connect("localhost:7233")
       worker = Worker(client, workflows=[Replication], task_queue="replication")
   
       print("Starting worker")
       await worker.run()
   
   
   if __name__ == "__main__":
       print("Before:", datetime.fromisoformat("2022-10-30T05:13:00"))
       try:
           print("Won't run:", datetime.fromisoformat("2022-10-30T05:13:00Z"))
       except ValueError:
           print("Exception thrown")
       print("After:", parser.isoparse("2022-10-30T05:13:00Z"))
   
       asyncio.run(main())
   

   This produces the following stack trace:

   Failed handling activation on workflow with run ID 435abaf1-36bc-49ff-b003-4d6a8d833315
   Traceback (most recent call last):
     File "./.venv/lib/site-packages/temporalio/worker/workflow.py", line 174, in _handle_activation
       workflow = await self._create_workflow_instance(act)
     File "./.venv/lib/site-packages/temporalio/worker/workflow.py", line 321, in _create_workflow_instance
       search_attributes=temporalio.converter.decode_search_attributes(
     File "./.venv/lib/site-packages/temporalio/converter.py", line 704, in decode_search_attributes
       val = [datetime.fromisoformat(v) for v in val]
     File "./.venv/lib/site-packages/temporalio/converter.py", line 704, in <listcomp>
       val = [datetime.fromisoformat(v) for v in val]
   ValueError: Invalid isoformat string: '2022-10-30T06:28:30Z'
   

   I did some work on a fork that resolved down to a one-line fix + some unit tests for regression testing around the converter.decode_search_attributes function, linked here: https://github.com/Oracen/sdk-python

   I'll open a PR with the fix now, hopefully the problem is as simple as it appears
5. cretz commented on Oct 31, 2022

   

   cretz

   on Oct 31, 2022

   Last edited by cretz

   ContributorAuthor

   More actions

   👍 Saw the PR but wondering if we can avoid the additional dependency. Are there any snippets/approaches where we could do this using just the standard library?

   (EDIT: Starting to look unreasonable to vendor this or avoid the dependency. I'll update the PR)
6. cretz

   closed this as completedin #179on Nov 7, 2022