You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/05/04 10:25:32 UTC
[GitHub] [airflow] turbaszek opened a new issue #8703: Support for set in XCom serialization
turbaszek opened a new issue #8703:
URL: https://github.com/apache/airflow/issues/8703
<!--
Welcome to Apache Airflow! For a smooth issue process, try to answer the following questions.
Don't worry if they're not all applicable; just try to include what you can :-)
If you need to include code snippets or logs, please put them in fenced code
blocks. If they're super-long, please use the details tag like
<details><summary>super-long log</summary> lots of stuff </details>
Please delete these comment blocks before submitting the issue.
-->
**Description**
Add support for `set` in Xcom serialization method. This requires support for `set` in nested structures.
**Use case / motivation**
Once we switched to JSON as main serialization format the following error appeared:
```
TypeError: Object of type 'set' is not JSON serializable
```
**Related Issues**
Discussion started in https://github.com/apache/airflow/pull/8607#discussion_r419327697
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-625537935
XCom used to be pickle by default (and still is on 1.10?), that'll be why it has gone un-noticed for so long.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-721892281
The PR (https://github.com/apache/airflow/pull/9847) linked to this issue has milestone so we should be fine
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-633709610
Yes I think we should make it Serializable
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-623384203
As I said in https://github.com/apache/airflow/pull/8607#discussion_r419327697
> I was thinking about that and I am against it. JSON has no set structure and users should be aware of this. Additionally, if we want to support set in general then we have to support all possible nested structures (dicts + sets, list of sets, etc)
But I am happy to discuss. @kaxil @ashb @potiuk @mik-laj
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] removed a comment on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] removed a comment on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-623383306
Thanks for opening your first issue here! Be sure to follow the issue template!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-631986100
@kaxil @ashb @mik-laj @potiuk @zhongjiajie what's your opinion? Should we silently make `set` JSON-serializable or not?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-919473918
@kaxil https://github.com/apache/airflow/pull/16395 was on hold for a fix here.
Should we also close the linked issue https://github.com/apache/airflow/issues/16386 ?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-623386519
I am surprised though that we didn't get any issues regarding it yet (or maybe we just haven't looked) :D
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-623383306
Thanks for opening your first issue here! Be sure to follow the issue template!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-625700758
> XCom used to be pickle by default (and still is on 1.10?), that'll be why it has gone un-noticed for so long.
Imho the main reason were side effects in tests. Otherwise, we will spot it at once.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zhongjiajie commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-633334758
> @kaxil @ashb @mik-laj @potiuk @zhongjiajie what's your opinion? Should we silently make `set` JSON-serializable or not?
I think we should use JSON ser, and we going to use json as default serializable (as less in code base `TODO` mark said that)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] vikramkoka commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
vikramkoka commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-721297996
@turbaszek @kaxil Unclear on if this should have a milestone?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-623385925
We do it anyway for Serialized DAGs so we can just reuse the following code that takes care of set and other nested data structures:
https://github.com/apache/airflow/blob/caa60b1141ac02cdde1c33464be9adca114c2ed5/airflow/serialization/serialized_objects.py#L164-L209
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-623388643
> We do it anyway for Serialized DAGs
And if I am not mistaken we make an assumption that we will never reach max concurrency error?
> I am surprised though that we didn't get any issues regarding it yet (or maybe we just haven't looked) :D
Me too but I'm getting used to the fact that a lot of operators are never used by majority...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-873304633
https://github.com/apache/airflow/pull/16786 should fix this
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8703:
URL: https://github.com/apache/airflow/issues/8703#issuecomment-919074674
Let's use Custom XCom backend if needed -- this isn't critical
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil closed issue #8703: Support for set in XCom serialization
Posted by GitBox <gi...@apache.org>.
kaxil closed issue #8703:
URL: https://github.com/apache/airflow/issues/8703
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org