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

   This reverts commit 77d2fc7d7591679aa99c1924daba678463a7b7bb.
   
   <!--
    Licensed to the Apache Software Foundation (ASF) under one
    or more contributor license agreements.  See the NOTICE file
    distributed with this work for additional information
    regarding copyright ownership.  The ASF licenses this file
    to you under the Apache License, Version 2.0 (the
    "License"); you may not use this file except in compliance
    with the License.  You may obtain a copy of the License at
   
      http://www.apache.org/licenses/LICENSE-2.0
   
    Unless required by applicable law or agreed to in writing,
    software distributed under the License is distributed on an
    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    KIND, either express or implied.  See the License for the
    specific language governing permissions and limitations
    under the License.
    -->
   
   <!--
   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/
   -->
   
   Reason for reverts:
   1. Test placed into the `tests/test_sentry.py` which do not pick up by 
selective checks, it should be placed into the `tests/core/test_sentry.py`
   2. Test do not pass, first with mocking context manager, even with fix this 
one by replace `mock_configure_scope.return_value` to 
`mock_configure_scope.return_value.__enter__.return_value` it starts fail with 
mock assertion errors
   3. Test use inclusive words
   
   ```console
   root@6eb4af1c407d:/opt/airflow# pytest tests/test_sentry.py
   ====================================================================== test 
session starts 
=======================================================================
   platform linux -- Python 3.8.19, pytest-7.4.4, pluggy-1.4.0 -- 
/usr/local/bin/python
   cachedir: .pytest_cache
   rootdir: /opt/airflow
   configfile: pyproject.toml
   plugins: custom-exit-code-0.3.0, instafail-0.5.0, rerunfailures-14.0, 
asyncio-0.23.6, cov-5.0.0, xdist-3.5.0, timeouts-1.2.1, anyio-4.3.0, 
icdiff-0.9, mock-3.14.0, requests-mock-1.11.0, time-machine-2.14.1
   asyncio: mode=strict
   setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
   collected 1 item                                                             
                                                                                
    
   
   tests/test_sentry.py::test_configured_sentry_add_tagging FAILED              
                                                                              
[100%]
   
   ============================================================================ 
FAILURES 
============================================================================
   _______________________________________________________________ 
test_configured_sentry_add_tagging 
_______________________________________________________________
   
   mock_configure_scope = <MagicMock name='configure_scope' 
id='281473541367312'>
   
       @patch("sentry_sdk.configure_scope")
       def test_configured_sentry_add_tagging(mock_configure_scope):
           mock_scope = create_autospec(Scope)
           mock_configure_scope.return_value = mock_scope
       
           from airflow.sentry.configured import ConfiguredSentry
       
           sentry = ConfiguredSentry()
       
           dummy_tags = ConfiguredSentry.SCOPE_DAG_RUN_TAGS | 
ConfiguredSentry.SCOPE_TASK_INSTANCE_TAGS
       
           # It should not raise error with both "dag_run" and "task" 
attributes, available.
           task_instance_1 = create_autospec(TaskInstance)
           task_instance_1.dag_run = MagicMock()
           task_instance_1.task = "task_1"
           for tag in dummy_tags:
               setattr(task_instance_1.dag_run, tag, "dummy")
   >       sentry.add_tagging(task_instance_1)
   
   tests/test_sentry.py:44: 
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
_ _ _ 
   
   self = <airflow.sentry.configured.ConfiguredSentry object at 
0xffffaa74faf0>, task_instance = <MagicMock spec='TaskInstance' 
id='281473497709872'>
   
       def add_tagging(self, task_instance):
           """Add tagging for a task_instance."""
           dag_run = task_instance.dag_run
           # See TaskInstance definition, the "task" attribute may not be set
           task = getattr(task_instance, "task")
       
   >       with sentry_sdk.configure_scope() as scope:
   E       AttributeError: __enter__
   
   airflow/sentry/configured.py:108: AttributeError
   --------------------------------------------------------------------- 
Captured stdout setup 
----------------------------------------------------------------------
   ========================= AIRFLOW ==========================
   Home of the user: /root
   Airflow home /root/airflow
   Skipping initializing of the DB as it was initialized already.
   You can re-initialize the database by adding --with-db-init flag when 
running tests.
   ======================================================================== 
warnings summary 
========================================================================
   tests/test_sentry.py::test_configured_sentry_add_tagging
     
/usr/local/lib/python3.8/site-packages/flask_appbuilder/models/sqla/__init__.py:105:
 MovedIn20Warning: Deprecated API features detected! These feature(s) are not 
compatible with SQLAlchemy 2.0. To prevent incompatible upgrades prior to 
updating applications, ensure requirements files are pinned to 
"sqlalchemy<2.0". Set environment variable SQLALCHEMY_WARN_20=1 to show all 
deprecation warnings.  Set environment variable 
SQLALCHEMY_SILENCE_UBER_WARNING=1 to silence this message. (Background on 
SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
       @as_declarative(name="Model", metaclass=ModelDeclarativeMeta)
   
   tests/test_sentry.py::test_configured_sentry_add_tagging
     
/usr/local/lib/python3.8/site-packages/marshmallow_sqlalchemy/convert.py:17: 
DeprecationWarning: The '__version__' attribute is deprecated and will be 
removed in in a future version. Use feature detection or 
'importlib.metadata.version("marshmallow")' instead.
       _META_KWARGS_DEPRECATED = Version(ma.__version__) >= Version("3.10.0")
   
   -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
   ==================================================================== short 
test summary info 
=====================================================================
   FAILED tests/test_sentry.py::test_configured_sentry_add_tagging - 
AttributeError: __enter__
   ================================================================= 1 failed, 
2 warnings in 4.77s 
==================================================================
   ```
   
   So this make me think that this could be either wrong implemented test or 
implementation do not work
   
   cc: @LipuFei 
   
   <!-- Please keep an empty line above the dashes. -->
   ---
   **^ Add meaningful description above**
   Read the **[Pull Request 
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.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: [email protected]

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

Reply via email to