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(dev): Patch firefox_profile.py from Selenium package #29887

Merged
merged 5 commits into from
Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion requirements-base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,5 @@ phabricator==0.7.0

# test dependencies, but unable to move to requirements-test until
# sentry.utils.pytest and sentry.testutils are moved to tests/
selenium==3.141.0
selenium==3.141.0 # XXX: Remove hack from lib.sh when upgrading
Copy link
Member

Choose a reason for hiding this comment

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

@armenzg can you comment the PR link/number here for future reference?

sqlparse==0.2.4
8 changes: 8 additions & 0 deletions scripts/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ install-py-dev() {
# https://github.com/unbit/uwsgi/issues/2361
pip install https://storage.googleapis.com/python-arm64-wheels/uWSGI-2.0.19.1-cp38-cp38-macosx_11_0_universal2.whl
fi
# This hack is until we can upgrade to a newer version of Selenium
fx_profile=.venv/lib/python3.8/site-packages/selenium/webdriver/firefox/firefox_profile.py
# Remove this block when upgrading the selenium package
if grep -q "or setting is" "${fx_profile}"; then
echo "We are patching ${fx_profile}. You will see this message only once."
sed -i .bak 's/or setting is/or setting ==/' ${fx_profile}
Copy link
Member

Choose a reason for hiding this comment

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

Could you instead generate a patch and put it in scripts/patches, then idempotently patch? Patching shouldn't be done like this.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, lemme take this over and show you what I mean.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. WFM

diff -U1 ${fx_profile} ${fx_profile}.bak
fi
# SENTRY_LIGHT_BUILD=1 disables webpacking during setup.py.
# Webpacked assets are only necessary for devserver (which does it lazily anyways)
# and acceptance tests, which webpack automatically if run.
Expand Down