Skip to content

fIx failing masking tests for python < 3.10#27337

Merged
potiuk merged 1 commit intoapache:mainfrom
potiuk:fix-main-test-for-pre-3-10
Oct 28, 2022
Merged

fIx failing masking tests for python < 3.10#27337
potiuk merged 1 commit intoapache:mainfrom
potiuk:fix-main-test-for-pre-3-10

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Oct 28, 2022

Seems that the number of times user is printed in stack trace depend on Python version. The fix in #27335 seems to only have worked for Python 3.10 with the 1.0.0 of exceptiongroup the stack trace has less stack levels.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

Seems that the number of times user is printed in stack trace
depend on Python version. The fix in apache#27335 seems to only have
worked for Python 3.10 with the 1.0.0 of exceptiongroup the
stack trace has less stack levels.
@potiuk potiuk added the full tests needed We need to run full set of tests for this PR to merge label Oct 28, 2022
@potiuk
Copy link
Member Author

potiuk commented Oct 28, 2022

Running for "full tests" and with change in setup.py to get latest version of exceptiongroup - apparently the test results sligthly differ for different Python version

@potiuk
Copy link
Member Author

potiuk commented Oct 28, 2022

just the trino case (which means BTW it is not question of resources - just race)

@potiuk potiuk merged commit fcec804 into apache:main Oct 28, 2022
@potiuk potiuk deleted the fix-main-test-for-pre-3-10 branch October 28, 2022 07:36
@Taragolis
Copy link
Contributor

Finally I could reproduce this error locally.

I do not know about actual nature of this error but what I found

  • Error not happen in case of exceptiongroup==1.0.0rc9
  • exceptiongroup monkey patch traceback.TracebackException on import as result tests not failed if no import exceptiongroup. However this test run in the end of all Test: Core
  • In PyCharm Debugger with Python 3.9.9 caplog.text returns expected value. I thought Debugger also might monkey patch traceback.TracebackException
ERROR Err
Traceback (most recent call last):
  File ".../test_secrets_masker.py", line 165, in test_masking_in_implicit_context_exceptions
    raise RuntimeError(f"Cannot connect to user:{p}")
RuntimeError: Cannot connect to user:***

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".../test_secrets_masker.py", line 167, in test_masking_in_implicit_context_exceptions
    raise RuntimeError(f"Exception: {ex1}")
RuntimeError: Exception: Cannot connect to user:***

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".../test_secrets_masker.py", line 169, in test_masking_in_implicit_context_exceptions
    raise RuntimeError(f"Exception: {ex2}")
RuntimeError: Exception: Exception: Cannot connect to user:***
  • In the other hand pytest get this value.
ERROR Err
Traceback (most recent call last):
  File ".../test_secrets_masker.py", line 167, in test_masking_in_implicit_context_exceptions
    raise RuntimeError(f"Exception: {ex1}")
RuntimeError: Exception: Cannot connect to user:***

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".../test_secrets_masker.py", line 169, in test_masking_in_implicit_context_exceptions
    raise RuntimeError(f"Exception: {ex2}")
RuntimeError: Exception: Exception: Cannot connect to user:***
  • Set env var EXCEPTIONGROUP_NO_PATCH to 1 not help

  • Add parameter --tb=native to pytest run not help

  • exceptiongroup is dependency of cattrs

root@4b131dd73f72:/opt/airflow# pipdeptree --reverse --packages exceptiongroup --python `which python`
exceptiongroup==1.0.0rc9
  - cattrs==22.2.0 [requires: exceptiongroup]
    - apache-airflow==2.5.0.dev0 [requires: cattrs>=22.1.0]
    - looker-sdk==22.18.0 [requires: cattrs>=1.3]

@potiuk
Copy link
Member Author

potiuk commented Oct 28, 2022

Yeah. it's weird but eventually i think the approach i used works fine. We really do not care how many details and levels are in the stack trace we just care that password is not leaked. And checking that there is no plain password and that there are some 'masked' passwords (and more thab one in this case) is 'good enough' I think

@potiuk
Copy link
Member Author

potiuk commented Oct 28, 2022

I think what really happens here is that sometimes we have one less exception in the chain because some of the monkey patching is done earlier or later (and it does not really matter) - but this is not something we should care about in this test - we just care about masking implicit passwords in stack trace

@potiuk
Copy link
Member Author

potiuk commented Oct 28, 2022

BTW. We got the main "green" again :) 🎉

@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.3 milestone Nov 9, 2022
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 9, 2022
ephraimbuddy pushed a commit that referenced this pull request Nov 9, 2022
Seems that the number of times user is printed in stack trace
depend on Python version. The fix in #27335 seems to only have
worked for Python 3.10 with the 1.0.0 of exceptiongroup the
stack trace has less stack levels.

(cherry picked from commit fcec804)
ephraimbuddy pushed a commit that referenced this pull request Nov 9, 2022
Seems that the number of times user is printed in stack trace
depend on Python version. The fix in #27335 seems to only have
worked for Python 3.10 with the 1.0.0 of exceptiongroup the
stack trace has less stack levels.

(cherry picked from commit fcec804)
ephraimbuddy pushed a commit that referenced this pull request Nov 9, 2022
Seems that the number of times user is printed in stack trace
depend on Python version. The fix in #27335 seems to only have
worked for Python 3.10 with the 1.0.0 of exceptiongroup the
stack trace has less stack levels.

(cherry picked from commit fcec804)
ephraimbuddy pushed a commit that referenced this pull request Nov 9, 2022
Seems that the number of times user is printed in stack trace
depend on Python version. The fix in #27335 seems to only have
worked for Python 3.10 with the 1.0.0 of exceptiongroup the
stack trace has less stack levels.

(cherry picked from commit fcec804)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:logging changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants