luoyuliuyin commented on pull request #16734:
URL: https://github.com/apache/airflow/pull/16734#issuecomment-872206401


   > And like Ash explains in [#15395 
(comment)](https://github.com/apache/airflow/pull/15395#discussion_r614418669) 
, list and tuple's should be sorted.
   > 
   > If there is an issue with how we are storing `deps` we should convert it 
to a set
   
   I agree with ashb, `[2, 0, 3]` and` [0, 2, 3]` are not the same.
   
   We now need to serialize dag to json, and then calculate the hash of 
json_str to determine whether the dag has changed. But json doesn't directly 
work with sets because the data stored in the set is not stored as properties, 
we usually convert set to list to solve this problem. So in the current 
serialization scenario, it is reasonable to convert set to list.
   
   For the case of `sorted(list)` throwing an exception, we need to ensure that 
the elements in the list are of the Iterable type and that the elements are 
comparable. In the current modification, both deps and task_group.xxx_ids are 
str, so they can be compared. At the same time, we are not sure whether the 
return of `cls._serialize(v)` is comparable, nor whether the semantics will be 
changed after sorting, so we should not sort in the external function, and we 
should give the choice to the specific execution function.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to