Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(integrations): Use wraps on fastapi request call wrapper #2476

Merged
merged 3 commits into from Nov 8, 2023

Conversation

nkaras
Copy link
Contributor

@nkaras nkaras commented Nov 1, 2023

Use _functools.wraps on the fastapi:patch_get_request_handler local function _sentry_call.
Add unit test to compare endpoint and call __qualname__.

Fixes #2475

@jseadragon
Copy link

@nkaras Looks like the build is failing. I don't see any specific reason for it to be failing.. perhaps we are missing some sort of mock configuration in the tests?

Linter:

tests/integrations/fastapi/test_fastapi.py:397:5: E303 too many blank lines (2)

___________________ ERROR at setup of test_basic[python3.7] ____________________
tests/integrations/aws_lambda/test_aws.py:141: in lambda_client
    return get_boto_client()
tests/integrations/aws_lambda/client.py:197: in get_boto_client
    _get_or_create_lambda_execution_role()
tests/integrations/aws_lambda/client.py:179: in _get_or_create_lambda_execution_role
    response = iam_client.get_role(RoleName=AWS_LAMBDA_EXECUTION_ROLE_NAME)
.tox/py3.9-aws_lambda/lib/python3.9/site-packages/botocore/client.py:535: in _api_call
    return self._make_api_call(operation_name, kwargs)
.tox/py3.9-aws_lambda/lib/python3.9/site-packages/botocore/client.py:980: in _make_api_call
    raise error_class(parsed_response, operation_name)
E   botocore.exceptions.ClientError: An error occurred (InvalidClientTokenId) when calling the GetRole operation: The security token included in the request is invalid.

I'm not sure what this aws_lambda test is for. Seems unrelated, but maybe we are missing some mock configuration for the tests?

Hopefully a Sentry dev can make this more clear, I probably don't know enough about the CI configuration to be of much help

@sentrivana
Copy link
Contributor

Hey @jseadragon, the AWS Lambda test failure is not related to the changes in this PR. It seems we have a general problem with PRs from contributors hitting some AWS auth issue. We'll fix that.

@nkaras Thanks again for the PR! Could you please run the PR through black? The lint step is complaining about there being too many empty lines. (I'd really like to automate this at some point, but right now it needs to be done manually.)

@nkaras
Copy link
Contributor Author

nkaras commented Nov 3, 2023

@sentrivana I pushed the formatting change 🤞

@sentrivana
Copy link
Contributor

Created #2487 to track progress on the aws lambda test suite issue.

@sentrivana
Copy link
Contributor

@nkaras Looking good! Could you please merge master into this branch/rebase on master or set the PR to accept changes from maintainers (then we could do it without bothering you)? Once we're up to date with the base branch we should be good to merge.

@nkaras
Copy link
Contributor Author

nkaras commented Nov 8, 2023

@sentrivana Looks like there is an open issue allowing edits from maintainers across organizations https://github.com/orgs/community/discussions/5634. It's no bother though. I just merged master. I'm more than happy to bump versions or anything else needed along the way.

@sentrivana sentrivana enabled auto-merge (squash) November 8, 2023 15:06
@sentrivana
Copy link
Contributor

@nkaras I see, wasn't aware of that 🙈 Thanks for updating, that should be it.

@sentrivana sentrivana merged commit 2cb232e into getsentry:master Nov 8, 2023
312 of 314 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FastApiIntegration hides request handler function name
3 participants