Lite: Fix the analytics module to use asyncio to work in the Wasm env #5045

whitphx · 2023-07-31T03:03:02Z

Description

Following the discussion at #4967 (comment)

Technically, the current analytics module doesn't work in the Wasm mode because it relies on the threading module that doesn't work on the Pyodide runtime.
So in this PR they are replaced with asyncio and pyodide.http.pyfetch() for the Wasm env.

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Tests

PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh
You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

vercel · 2023-07-31T03:03:06Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
gradio	✅ Ready (Inspect)	Visit Preview	Aug 4, 2023 1:00pm

gradio-pr-bot · 2023-07-31T03:03:40Z

🦄 change detected

This Pull Request includes changes to the following packages.

Package	Version
`gradio`	`minor`

Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Lite: Fix the analytics module to use asyncio to work in the Wasm env

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

Maintainers can change the version label to modify the version bump.
If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

gradio-pr-bot · 2023-07-31T03:06:02Z

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-5045-all-demos

You can install the changes in this PR by running:

pip install https://gradio-builds.s3.amazonaws.com/4df38b4e52be5e958834fd8dabbe1569c2c745c6/gradio-3.39.0-py3-none-any.whl

abidlabs · 2023-07-31T10:36:49Z

.changeset/rare-feet-travel.md

@@ -0,0 +1,5 @@
+---


We can delete this changeset file

OK, will delete it.

btw, it was added by the CI bot. Did it detect something wrong?

I'm not sure why it added two different changeset files! Will keep an eye out to see if it happens again

Sorry, I added this one, and the bot added the another one: .changeset/hungry-bobcats-own.md

Should only this file about @gradio/lite be deleted, or another one about the gradio Python package too?

Oh you don't need to add a changeset manually, the bot will take care of it. You can delete the changeset you created.

I don't think we have the automatic changeset support for @gradio/lite at the moment. Would be good to add it closer to the release

I see.
While it may also work, I have added the changeset for @gradio/lite in each PR, like #4779 (comment), btw.

gradio/analytics.py

gradio/blocks.py

abidlabs · 2023-07-31T10:52:50Z

Nice @whitphx! Thanks for making this addition. Generally looks good.

I couldn't figure out why the test is failing? It looks like its trying to do analytics using the WASM approach, but this should not be the case since the tests don't run on pyiodide by default.

Btw it would be good to add a similar test to

gradio/test/test_analytics.py

Line 40 in 683bdf7

def test_error_analytics_successful(self, mock_post, monkeypatch):

to ensure that the request is being made in WASM mode (ok to add pyiodide as a test requirement)

This reverts commit 052f2bd.

…Wasm mode

… module

whitphx · 2023-08-02T13:01:03Z

gradio/analytics.py

    data["ip_address"] = get_local_ip_address()
    try:
        requests.post(url, data=data, timeout=5)
    except (requests.ConnectionError, requests.exceptions.ReadTimeout):
        pass  # do not push analytics if no network


+async def _do_wasm_analytics_request(url: str, data: dict[str, Any]) -> None:
+    data["ip_address"] = "No internet connection"


Since the current get_local_ip_address() doesn't work in the Wasm env, and even if we make its Wasm-compatible version, gathering users' public IP-address looks too much, I hardcoded the string literal representing the failure of IP address detection.

gathering users' public IP-address looks too much

What do you mean by looks too much. Do you mean that its too intrusive? Or that its not techincally feasible?

The reason that I ask is that main benefit of collecting analytics is that it allows us to measure daily, weekly, and monthly unique users which we might not be able to do if we don't collect IP addresses.

I felt it's too intrusive.
In the case of Gradio-lite, this telemetry will be sent from clients' device immediately after they open the app page, where they don't have any chance to set analytics_enabled=false to opt-out it. I think some amount of people consider it as a bad behavior to automatically access the third-party API (https://checkip.amazonaws.com/) to get the IP address and send it as analytics data, under the recent situation where people are sensitive to data privacy.
However, getting IP addresses itself might be allowed as long as they are not logged or properly stored securely (I'm not sure), so I would like some advice from experts.

I pushed a commit including the IP address fetcher (b77247c), though we should revert it if it looks not good anyway.

Nice @whitphx. Let's keep it for now -- as part of #4226, we might replace the IP address with a hash of the IP address so that we can still keep a track of unique users without sending IP addresses.

gradio/blocks.py

abidlabs · 2023-08-04T13:18:06Z

LGTM @whitphx! This looks good to go. Apologies this took a while to review

whitphx · 2023-08-04T14:02:05Z

@abidlabs Thank you!

Fix the analytics module to use asyncio to work in the Wasm env

d4be722

add changeset

42f04ad

vercel bot deployed to Preview July 31, 2023 03:04 View deployment

whitphx changed the title ~~Fix the analytics module to use asyncio to work in the Wasm env~~ Lite: Fix the analytics module to use asyncio to work in the Wasm env Jul 31, 2023

add changeset

90127ae

vercel bot deployed to Preview July 31, 2023 03:14 View deployment

whitphx marked this pull request as ready for review July 31, 2023 03:15

whitphx mentioned this pull request Jul 31, 2023

Lite: Disable the analytics function forcefully in the Wasm mode #4967

Closed

Add changeset

052f2bd

vercel bot deployed to Preview July 31, 2023 03:24 View deployment

abidlabs reviewed Jul 31, 2023

View reviewed changes

gradio/analytics.py Show resolved Hide resolved

abidlabs reviewed Jul 31, 2023

View reviewed changes

gradio/blocks.py Outdated Show resolved Hide resolved

whitphx added 4 commits August 1, 2023 21:06

Revert "Add changeset"

07a341c

This reverts commit 052f2bd.

Add the is_wasm field to the analytics telemetry

96c4f56

Restore the initialization code in Blocks.launch() in the Wasm env

70bbfc2

Remove a call of analytics.version_check() in Blocks.launch() in the …

152db85

…Wasm mode

vercel bot deployed to Preview August 2, 2023 07:37 View deployment

Add a type: ignore directive to the line of importing the pyodide…

2b87bc8

… module

vercel bot deployed to Preview August 2, 2023 07:50 View deployment

Fix a test case

f134298

vercel bot deployed to Preview August 2, 2023 08:49 View deployment

Refactor _do_wasm_analytics_request() and add a unit test for it

2c87d7c

vercel bot deployed to Preview August 2, 2023 09:49 View deployment

whitphx commented Aug 2, 2023

View reviewed changes

gradio/blocks.py Show resolved Hide resolved

whitphx requested a review from abidlabs August 2, 2023 13:12

Merge branch 'main' into wasm-compatible-analytics

6f58292

vercel bot deployed to Preview August 2, 2023 13:14 View deployment

vercel bot deployed to Preview August 3, 2023 03:30 View deployment

Get the IP address for analytics in the Wasm mode

b77247c

whitphx force-pushed the wasm-compatible-analytics branch from 43af9a3 to b77247c Compare August 3, 2023 04:04

vercel bot deployed to Preview August 3, 2023 04:05 View deployment

Merge branch 'main' into wasm-compatible-analytics

593803b

vercel bot deployed to Preview August 4, 2023 13:00 View deployment

abidlabs merged commit 3b9494f into gradio-app:main Aug 4, 2023
12 checks passed

whitphx deleted the wasm-compatible-analytics branch August 4, 2023 14:01

pngwn mentioned this pull request Aug 9, 2023

chore: update versions #5038

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lite: Fix the analytics module to use asyncio to work in the Wasm env #5045

Lite: Fix the analytics module to use asyncio to work in the Wasm env #5045

whitphx commented Jul 31, 2023 •

edited

vercel bot commented Jul 31, 2023 •

edited

gradio-pr-bot commented Jul 31, 2023 •

edited

Something isn't right?

gradio-pr-bot commented Jul 31, 2023 •

edited

abidlabs Jul 31, 2023

whitphx Jul 31, 2023 •

edited

abidlabs Jul 31, 2023

whitphx Aug 1, 2023

abidlabs Aug 1, 2023

whitphx Aug 2, 2023

abidlabs commented Jul 31, 2023

whitphx Aug 2, 2023

abidlabs Aug 2, 2023

whitphx Aug 3, 2023 •

edited

whitphx Aug 3, 2023 •

edited

abidlabs Aug 4, 2023

abidlabs commented Aug 4, 2023

whitphx commented Aug 4, 2023

Lite: Fix the analytics module to use asyncio to work in the Wasm env #5045

Lite: Fix the analytics module to use asyncio to work in the Wasm env #5045

Conversation

whitphx commented Jul 31, 2023 • edited

Description

🎯 PRs Should Target Issues

Tests

vercel bot commented Jul 31, 2023 • edited

gradio-pr-bot commented Jul 31, 2023 • edited

🦄 change detected

This Pull Request includes changes to the following packages.

With the following changelog entry.

Something isn't right?

gradio-pr-bot commented Jul 31, 2023 • edited

Choose a reason for hiding this comment

whitphx Jul 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs commented Jul 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whitphx Aug 3, 2023 • edited

Choose a reason for hiding this comment

whitphx Aug 3, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs commented Aug 4, 2023

whitphx commented Aug 4, 2023

whitphx commented Jul 31, 2023 •

edited

vercel bot commented Jul 31, 2023 •

edited

gradio-pr-bot commented Jul 31, 2023 •

edited

gradio-pr-bot commented Jul 31, 2023 •

edited

whitphx Jul 31, 2023 •

edited

whitphx Aug 3, 2023 •

edited

whitphx Aug 3, 2023 •

edited