tvalentyn commented on code in PR #28564:
URL: https://github.com/apache/beam/pull/28564#discussion_r1344626645


##########
sdks/python/apache_beam/transforms/environments.py:
##########
@@ -124,9 +124,9 @@ def __init__(self,
         dict(resource_hints) if resource_hints else {})
 
   def __eq__(self, other):
+    # don't compare artifacts since they have different hashes in their names.
     return (
-        self.__class__ == other.__class__ and
-        self._artifacts == other._artifacts

Review Comment:
   that is a bizzare behavior. we should work on fixing comparison. I am not 
sure what is 'type_payload', but as far as file payloads are concerned, i think 
it's reasonably safe to compare file hashes; alternatively we can look and see 
why   the file path is different and if we should fix that instead. without any 
additional context and digging further, first options sounds more promising.



##########
sdks/python/apache_beam/transforms/environments.py:
##########
@@ -124,9 +124,9 @@ def __init__(self,
         dict(resource_hints) if resource_hints else {})
 
   def __eq__(self, other):
+    # don't compare artifacts since they have different hashes in their names.
     return (
-        self.__class__ == other.__class__ and
-        self._artifacts == other._artifacts

Review Comment:
   that is a bizzare behavior. we should work on fixing comparison. I am not 
sure what is 'type_payload', but as far as file payloads are concerned, i think 
it's reasonably safe to compare file hashes; alternatively we can look and see 
why   the file path is different and if we should fix that instead. without any 
additional context and digging further, first option sounds more promising.



-- 
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