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

Add default enabled sentry integrations dynamically to hidden imports #71

Merged
merged 7 commits into from
Nov 19, 2020

Conversation

SimonIT
Copy link
Contributor

@SimonIT SimonIT commented Nov 18, 2020

Sentry added in PR getsentry/sentry-python#625 by default enabled integrations. We need to add them to the hidden imports to be able to launch a application with sentry.

I added the hasattr check for backwards compatibility

@SimonIT SimonIT requested review from a team and Legorooj and removed request for a team November 18, 2020 19:06
@SimonIT SimonIT mentioned this pull request Nov 18, 2020
Copy link
Member

@bwoodsend bwoodsend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you've dealt with my review comments could you launch our CI tester? From the root of your copy of this repo, check your in your sentry_django_integration branch, then run:

python scripts\cloud-test.py --os=windows --os=ubuntu --py=3.6,3.8 --browser sentry-sdk==0.15.1, sentry-sdk==0.19.3

You'll need to create a github authentication token if you don't already have one because github have stopped accepting username and password.

@SimonIT
Copy link
Contributor Author

SimonIT commented Nov 19, 2020

@bwoodsend How should I handle the trailing whitespace problem as reported by the linter? Or does python inside exec_statement need no indentation?

@rokm
Copy link
Member

rokm commented Nov 19, 2020

@bwoodsend How should I handle the trailing whitespace problem as reported by the linter? Or does python inside exec_statement need no indentation?

Linter is complaining about the extra whitespace at the end of the line 30 (after "integrations").

@bwoodsend
Copy link
Member

Looks good.

Once you've dealt with my review comments could you launch our CI tester? From the root of your copy of this repo, check into your sentry_django_integration branch, then run:

python scripts\cloud-test.py --os=windows --os=ubuntu --py=3.6,3.8 --browser sentry-sdk==0.15.1, sentry-sdk==0.19.3

You'll need to create a github authentication token if you don't already have one because github have stopped accepting username and password.

Can you go ahead and do this now? If it passes then we should be ready to merge this.

@SimonIT
Copy link
Contributor Author

SimonIT commented Nov 19, 2020

@bwoodsend
Copy link
Member

Perfect - thanks for your contribution!

@bwoodsend bwoodsend merged commit c20340f into pyinstaller:master Nov 19, 2020
@SimonIT SimonIT deleted the sentry_django_integration branch November 19, 2020 19:40
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.

3 participants