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 2022/12/05 16:18:16 UTC

[GitHub] [airflow] potiuk opened a new pull request, #28124: Better description of the import limiting for deserialization

potiuk opened a new pull request, #28124:
URL: https://github.com/apache/airflow/pull/28124

   Introduce better description and significant change in release notes for import limiting for deserialization.
   
   Follow up after #27887
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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] potiuk closed pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #28124: Better description of the import limiting for deserialization
URL: https://github.com/apache/airflow/pull/28124


-- 
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] potiuk commented on pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28124:
URL: https://github.com/apache/airflow/pull/28124#issuecomment-1337663000

   But yeah. anyhow let's wait for the user to respond how he got there, I am also curious why it was triggered at all.


-- 
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] bolkedebruin commented on pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on PR #28124:
URL: https://github.com/apache/airflow/pull/28124#issuecomment-1337670617

   This is the follow up https://github.com/apache/airflow/pull/28067


-- 
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] potiuk commented on pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28124:
URL: https://github.com/apache/airflow/pull/28124#issuecomment-1337661842

   > But we can change the message then ? can we? For now it's only XCom, as I understand.
   
   Just some reasin why - It took me quite a while to look it up (going through Git History and finding PR) so I can only imagine how hard it might be for the user, and I think we should give as good a hint where it is from as 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-unsubscribe@airflow.apache.org

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


[GitHub] [airflow] potiuk commented on pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28124:
URL: https://github.com/apache/airflow/pull/28124#issuecomment-1337656810

   > The serializer will be generic and not just tied to XCom so referring to XCom doesn't make sense
   
   But we can change the message then ? can we? For now it's only XCom, as I understand. 


-- 
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] bolkedebruin commented on pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on PR #28124:
URL: https://github.com/apache/airflow/pull/28124#issuecomment-1337655192

   The serializer will be generic and not just tied to XCom so referring to XCom doesn't make sense


-- 
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] potiuk commented on pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28124:
URL: https://github.com/apache/airflow/pull/28124#issuecomment-1338011757

   > As I intend to further refactor it in the earlier mentioned PR and in the intention is to have that out as part of 2.5.1 and this is just clarification I think I'd better solve it there.
   
   Cool. Closing it then :)


-- 
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] bolkedebruin commented on pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on PR #28124:
URL: https://github.com/apache/airflow/pull/28124#issuecomment-1337768726

   As I intend to further refactor it in the earlier mentioned PR and in the intention is to have that out as part of 2.5.1 and this is just clarification I think I'd better solve it there. 


-- 
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 a diff in pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
kaxil commented on code in PR #28124:
URL: https://github.com/apache/airflow/pull/28124#discussion_r1039847814


##########
airflow/utils/json.py:
##########
@@ -212,7 +212,11 @@ def object_hook(self, dct: dict) -> object:
                     break
 
             if not cls:
-                raise ImportError(f"{classname} was not found in allow list for import")
+                raise ImportError(
+                    f"{classname} was not found in allow list for import in XCom. "
+                    "If you want to continue to use your class in XCom, add it to "
+                    "allowed_deserialization_classes config in core section of config."

Review Comment:
   ```suggestion
                       "allowed_deserialization_classes config in core section of airflow.cfg."
   ```



##########
airflow/utils/json.py:
##########
@@ -212,7 +212,11 @@ def object_hook(self, dct: dict) -> object:
                     break
 
             if not cls:
-                raise ImportError(f"{classname} was not found in allow list for import")
+                raise ImportError(
+                    f"{classname} was not found in allow list for import in XCom. "

Review Comment:
   ```suggestion
                       f"{classname} was not found in allow list for imports. "
   ```



-- 
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 a diff in pull request #28124: Better description of the import limiting for deserialization

Posted by GitBox <gi...@apache.org>.
kaxil commented on code in PR #28124:
URL: https://github.com/apache/airflow/pull/28124#discussion_r1039847814


##########
airflow/utils/json.py:
##########
@@ -212,7 +212,11 @@ def object_hook(self, dct: dict) -> object:
                     break
 
             if not cls:
-                raise ImportError(f"{classname} was not found in allow list for import")
+                raise ImportError(
+                    f"{classname} was not found in allow list for import in XCom. "
+                    "If you want to continue to use your class in XCom, add it to "
+                    "allowed_deserialization_classes config in core section of config."

Review Comment:
   ```suggestion
                       "allowed_deserialization_classes config in core section of airflow.config."
   ```



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