[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049301967


##
airflow/models/xcom.py:
##
@@ -723,6 +723,21 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to go through the trouble of serializing the entire

Review Comment:
   yeah... sounds about right



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049297956


##
airflow/models/xcom.py:
##
@@ -723,6 +723,21 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to go through the trouble of serializing the entire

Review Comment:
   maybe it is through! what do i know! maybe _to_ is british



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049296838


##
airflow/models/xcom.py:
##
@@ -723,6 +723,21 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to go through the trouble of serializing the entire

Review Comment:
   ```suggestion
   # We don't want to go to the trouble of serializing the entire
   ```
   sorry, i had it wrong earlier... source : 
https://dictionary.cambridge.org/us/dictionary/english/go-to-the-trouble-to



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049263076


##
airflow/models/xcom.py:
##
@@ -723,6 +723,20 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to get into the trouble to pickle the entire SQLAlchemy

Review Comment:
   wait it's "go _to_ the trouble" :) had to google, myself



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049262354


##
airflow/models/xcom.py:
##
@@ -723,6 +723,20 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to get into the trouble to pickle the entire SQLAlchemy

Review Comment:
   got you ... ok then i would just suggest changing to "go _through the 
trouble of pickling the"



##
airflow/models/xcom.py:
##
@@ -723,6 +723,20 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to get into the trouble to pickle the entire SQLAlchemy

Review Comment:
   got you ... ok then i would just suggest changing to "go _through the 
trouble of pickling_ the"



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049218067


##
airflow/models/xcom.py:
##
@@ -723,6 +723,20 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to get into the trouble to pickle the entire SQLAlchemy
+# query object, but there's no good way to extract query information.
+# The SQL string for DBAPI is the best we can get. THeoratically we can

Review Comment:
   ```suggestion
   # The SQL string for DBAPI is the best we can get. Theoratically we 
can
   ```



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049217806


##
airflow/models/xcom.py:
##
@@ -723,6 +723,20 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to get into the trouble to pickle the entire SQLAlchemy

Review Comment:
   it's a little unclear whether you mean it's a big effort to do, or it's a 
problematic thing to do.



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [airflow] dstandish commented on a diff in pull request #28191: Add custom pickling hooks to LazyXComAccess

2022-12-14 Thread GitBox


dstandish commented on code in PR #28191:
URL: https://github.com/apache/airflow/pull/28191#discussion_r1049217806


##
airflow/models/xcom.py:
##
@@ -723,6 +723,20 @@ def __eq__(self, other: Any) -> bool:
 return all(x == y for x, y in z)
 return NotImplemented
 
+def __getstate__(self) -> Any:
+# We don't want to get into the trouble to pickle the entire SQLAlchemy

Review Comment:
   ```suggestion
   # We don't want to get into the trouble to pickle the entire 
SQLAlchemy
   ```
   it's a little unclear whether you mean it's a big effort to do, or it's a 
problematic thing to do.



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org