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

Allow to access external server IP via https. #7712

Merged

Conversation

LarsHill
Copy link
Contributor

@LarsHill LarsHill commented Nov 13, 2023

Describe your changes

Fallback to HTTPS endpoint call to check external IP if HTTP call failed.

This PR closes #7703

GitHub Issue Link (if applicable)

#7703

Testing Plan

We just add a second https url to retrieve external IPs
If the http url returns None (e.g. only https requests are allowed) the https url is used as fallback to retrieve the server IP.

Python unit tests added.

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

Copy link
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

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

Some tiny nits, but overall LGTM

Thanks @LarsHill for the contribution and @kajarenc for some additional tests!

Comment on lines 47 to 50
# try to get external IP from Amazon http url
response = _make_blocking_http_get(_AWS_CHECK_IP, timeout=5)

# try to get external IP from Amazon https url
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I feel like the code here is straightforward enough that the comments aren't needed

Comment on lines 41 to 50
# Test that by default we do call http endpoint
with requests_mock.mock() as m:
m.get(net_util._AWS_CHECK_IP, text="1.2.3.4")
m.get(net_util._AWS_CHECK_IP_HTTPS, text="5.6.7.8")
self.assertEqual("1.2.3.4", net_util.get_external_ip())
# Test that we do only one http call
self.assertEqual(m.call_count, 1)

def test_get_external_ip_https(self):
# Test that if http fails, we try https
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: same thing here -- we can probably drop these comments since the test names are descriptive enough + maybe edit the second test name a bit

with requests_mock.mock() as m:
m.get(net_util._AWS_CHECK_IP, exc=requests.exceptions.ConnectTimeout)
m.get(net_util._AWS_CHECK_IP_HTTPS, text="5.6.7.8")
self.assertEqual("5.6.7.8", net_util.get_external_ip())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be good to also verify that we make two endpoint calls

remove unused comments
addd additional assertion
more descriptive test name
@kajarenc kajarenc merged commit 836ec49 into streamlit:develop Nov 17, 2023
43 checks passed
willhuang1997 added a commit that referenced this pull request Nov 28, 2023
… a key (#7760)

* Add support for timedelta type to `st.dataframe`, `st.data_editor` and `st.table`. (#7689)

* Add support for timedelta type

* Add tests

* Move timedelta to supported types

* Improve timedelta support

* Improve implementation

* Rename to convert to seconds

* Fix type util test

* Fix unit tests

* Add timedelta index

* Update notices file

* Update snapshots

* Update snapshots

* Apply review comments

* Add Python 3.12 support (#7663)

Updates Streamlit for Python 3.12 compatibility

We have a test that checks interaction with langchain (callback handler), which is not yet 3.12-compatible. These are skipped when running on 3.12.
ALL_PYTHON_VERSIONS are extended with 3.12 for our Github Actions runners
Refactor a few tests to use self.assertEqual instead of deprecated  self.assertEqualS
Bump revision of insert-licence pre-commit hook to work with Python 3.12

* Add border parameter to container and form (#7455)

* Add border parameter to container and form

* Add border parameter

* Add snapshots

* Add note to form docstring

* Add exhaustive proto stability tests (#7635)

* temp fix mypy upper bound, to avoid broken CI (#7713)

* Enforce pyarrow version for arrow-based custom components (#7695)

* Release Streamlit version 1.28.2 (#7716)

* Build releases directly from tags instead of release branches (#7580)

* Get rid of unused release candidate workflow

* Just build releases directly from tag instead of release branch

* Get rid of unused scripts

* Add back branch check to verify that corresponding PR exists

* Docstrings for App testing (#7587)

* Make the outline for expanders appear on focus-visible and not focus (#7592)

* Add new libraries to attributions (#7599)

* Add new libraries to attributions

* Update metrics_util.py

* Up version to 1.28.0

* Fix: MPA Nav expand arrow (#7634)

Update dependency array for isOverflowing check - does not always register changes in clientHeight after initial expand & collapse. Add expanded to dependency array to ensure check of scrollHeight vs. clientHeight

* Tweak "forgot config" SnowflakeConnection error message (#7652)

* Fix app testing repr bug for `st.container` (#7644)

* Ensure file_uploader doesn't trigger needless reruns (#7641)

* Fix trigger value regression with `st.rerun` (#7643)

* Greatly narrow errors that we retry in SnowflakeConnection (#7645)

* Fix issue for st.camera_input and chat_input, add regression test for file_uploader (#7646)

Add playwright regression test for issue With st.file_uploader and st.chat_input, first message disappears #7556
Replicate changes from Ensure file_uploader doesn't trigger needless reruns #7641 for st.camera_input

* Up version to 1.28.1

* Add fix to enforce pyarrow version

* temp fix mypy upper bound, to avoid broken CI (#7713)

* Update version to 1.28.2

---------

Co-authored-by: Vincent Donato <vincent@streamlit.io>
Co-authored-by: Debbie Matthews <debbie.matthews@snowflake.com>
Co-authored-by: Ken McGrady <ken.mcgrady@gmail.com>
Co-authored-by: Maya Barnes <63436329+mayagbarnes@users.noreply.github.com>
Co-authored-by: Amanda Walker <amanda@amandawalker.io>
Co-authored-by: Karen Javadyan <kajarenc@gmail.com>

* Bump actions/setup-node from 3 to 4 (#7628)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 3 to 4.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v3...v4)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump axios from 0.27.2 to 1.6.0 in /frontend (#7708)

* Bump axios from 0.27.2 to 1.6.0 in /frontend

Bumps [axios](https://github.com/axios/axios) from 0.27.2 to 1.6.0.
- [Release notes](https://github.com/axios/axios/releases)
- [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md)
- [Commits](axios/axios@v0.27.2...v1.6.0)

---
updated-dependencies:
- dependency-name: axios
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update NOTICES

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Vincent Donato <vincent@streamlit.io>

* Directly use package name in setup.py (#7717)

* Directly use package name in setup.py

* Fix update name script to replace correct name

* Remove unnecessary PIL dependency in st.image docstring (#7728)

* Allow to access external server IP via https. (#7712)

Fallback to HTTPS endpoint call to check external IP if HTTP call failed.

---------

Co-authored-by: Lars Hillebrand <lars.patrick.hillebrand@iais.fraunhofer.de>
Co-authored-by: Karen Javadyan <kajarenc@gmail.com>

* Allow a service managing a Streamlit runtime to set its own session IDs (#7544)

* Move `dg_stack` into a `ContextVar` to support `with` blocks in separate threads (#7715)

* Bump actions/github-script from 6 to 7 (#7738)

Bumps [actions/github-script](https://github.com/actions/github-script) from 6 to 7.
- [Release notes](https://github.com/actions/github-script/releases)
- [Commits](actions/github-script@v6...v7)

---
updated-dependencies:
- dependency-name: actions/github-script
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Improve st.connection caching behavior (#7730)

* hack to compute connection cache function keys based on ttl, max_entries and dsn

* Fix ttl and connection name behavior for all connections and factory

* Add explanatory comments

* More explanatory comment tweaks

* Avoid adding new `.` characters to function qualnames

---------

Co-authored-by: Vincent Donato <vincent@streamlit.io>

* Pin playwright version to < 1.40 (#7747)

* Pin playwright version to < 1.40

* Add comment

* Use "loading skeletons" throughout Streamlit  (#7598)

* Implement both app-wide skeleton and suspense skeleton. Still need tests.

* Slow down the animation a little bit.

* Awww yisss. MFing tests.

* Address comments

* Make skeleton look nicer in sidebar

* Make DeckGlJsonChart withMapboxToken use Skeleton instead of "Loading..."

* Lint

* Fix assignment to const

* Add test for mapbox skeleton

* Make skeleton color overridable with postMessage

* Make a couple more e2e tests use AppSkeleton rather than "Please wait..."

* Make e2e tests not depend on skeletons. Instead, use new data-teststate attribute.

* Add query params for: no loading screen; old "Please wait" loading screen

* Fix AppNode test elements.alert check

* Lint

* Fix tests by creating a test-only "initial" state

* Fix e2e bug. "should not exist" -> "should exist"

* Pin playwright version to < 1.40 (#7747)

* Pin playwright version to < 1.40

* Add comment

* Add skeleton_background_color to NewSession.proto

* Fix arguments to cypress, whenever we call it manually.

* Add ability to pass function to matchThemedSnapshots()

* Ooops, remove Cypress upgrade code that was accidentally committed.

---------

Co-authored-by: Lukas Masuch <Lukas.Masuch@gmail.com>

* Migrate `st.button` e2e test to playwright (#7755)

* Add wait time

* Migrate button test to playwright

* Add snapshots

* Replace deprecated `find_executable` from `distutils.spawn` with recommended `which` from `shutil` (#7756)

Replace deprecated find_executable from distutils.spawn with recommended which from shutil
PEP: https://peps.python.org/pep-0632/

* add app_heartbeat BackMsg type and send it when receiving SEND_APP_HEARTBEAT window message (#7682)

* Remove outdated snapshots (#7758)

* Update docstrings for release 1.29 (#7759)

* Style tweaks for connections docstrings

* Add border to container example

* Update data_editor docsring for timedelta addition

* Fix link format

* Clarify data_editor note re: timedelta

* Add ability to set query param key to tuple or set, but not dictionary

* Fix minor typo in server.scriptHealthCheckEnabled help (#7762)

* fix: component dev url typo (#7746)

* Don't use hash on floats in hashing.py (#7754)

* Don't use hash on floats

Python's hash is salted with a random number. This will not work across restarts. Instead, use struct.pack to get the bytes of a float.

* Run autoformatter

---------

Co-authored-by: Vincent Donato <vincent@streamlit.io>

* Fix another url typo (#7764)

* Fix typing issues with altair (#7769)

* Fix typing issues with altair

* Fix mypy issues

* Fix mypy issues

* Remove altair from test dependencies

* Address comments

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Lukas Masuch <Lukas.Masuch@gmail.com>
Co-authored-by: Karen Javadyan <kajarenc@gmail.com>
Co-authored-by: Vincent Donato <vincent@streamlit.io>
Co-authored-by: Debbie Matthews <debbie.matthews@snowflake.com>
Co-authored-by: Ken McGrady <ken.mcgrady@gmail.com>
Co-authored-by: Maya Barnes <63436329+mayagbarnes@users.noreply.github.com>
Co-authored-by: Amanda Walker <amanda@amandawalker.io>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Snehan Kekre <snehan.minerva@gmail.com>
Co-authored-by: Lars Hillebrand <37187985+LarsHill@users.noreply.github.com>
Co-authored-by: Lars Hillebrand <lars.patrick.hillebrand@iais.fraunhofer.de>
Co-authored-by: eric-skydio <eric@skydio.com>
Co-authored-by: Thiago Teixeira <103002884+sfc-gh-tteixeira@users.noreply.github.com>
Co-authored-by: Pat Finnigan <pat.finnigan@snowflake.com>
Co-authored-by: Samuel Dion-Girardeau <samueldg@users.noreply.github.com>
Co-authored-by: Observed Observer <270001151@qq.com>
Co-authored-by: Andreas Kirsch <blackhc@gmail.com>
eric-skydio pushed a commit to eric-skydio/streamlit that referenced this pull request Dec 20, 2023
Fallback to HTTPS endpoint call to check external IP if HTTP call failed.

---------

Co-authored-by: Lars Hillebrand <lars.patrick.hillebrand@iais.fraunhofer.de>
Co-authored-by: Karen Javadyan <kajarenc@gmail.com>
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Mar 22, 2024
Fallback to HTTPS endpoint call to check external IP if HTTP call failed.

---------

Co-authored-by: Lars Hillebrand <lars.patrick.hillebrand@iais.fraunhofer.de>
Co-authored-by: Karen Javadyan <kajarenc@gmail.com>
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Apr 16, 2024
Fallback to HTTPS endpoint call to check external IP if HTTP call failed.

---------

Co-authored-by: Lars Hillebrand <lars.patrick.hillebrand@iais.fraunhofer.de>
Co-authored-by: Karen Javadyan <kajarenc@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable https to access external IP
3 participants