Skip to content

Conversation

justinclift
Copy link
Member

@justinclift justinclift commented Apr 5, 2024

What type of PR is this?

  • Other

Description

This PR removes:

  • all of the code (and user interface pieces) that shares usage data with redash.io domains
  • the two sign ups (newsletter, security alerts) on the first run configuration page
  • the checking for a new redash version, which is currently breaking when trying to parse the 24.04.0-dev string

With this PR, the failing version check showing up in the logs is no longer present:

worker_1            | [2024-04-04 23:45:51,490][PID:92][ERROR][root] Failed checking for new version (probably bad/non-JSON response).
worker_1            | Traceback (most recent call last):
worker_1            |   File "/app/redash/version_check.py", line 78, in run_version_check
worker_1            |     _compare_and_update(latest_version)
worker_1            |   File "/app/redash/version_check.py", line 97, in _compare_and_update
worker_1            |     is_newer = semver.compare(current_version, latest_version) == -1
worker_1            |   File "/usr/local/lib/python3.8/site-packages/semver.py", line 282, in compare
worker_1            |     v1, v2 = parse(ver1), parse(ver2)
worker_1            |   File "/usr/local/lib/python3.8/site-packages/semver.py", line 65, in parse
worker_1            |     raise ValueError('%s is not valid SemVer string' % version)
worker_1            | ValueError: 24.04.0-dev is not valid SemVer string

I'm not super sure we want this though. Thoughts? 😄

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)

It's passing both the unit and Cypress tests, and watching the backend docker logs as the Cypress tests run isn't showing anything bad. 😄

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

After this change, the first run configuration page no longer has two checkboxes asking for subscribing to things:

image

Home page no longer asks if the user wants to share anonymous usage data:

image

Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 63.82%. Comparing base (a0f5c70) to head (6c3bc74).

❗ Current head 6c3bc74 differs from pull request most recent head c21210b. Consider uploading reports for the commit c21210b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6852      +/-   ##
==========================================
+ Coverage   63.69%   63.82%   +0.13%     
==========================================
  Files         162      161       -1     
  Lines       13126    13060      -66     
  Branches     1814     1803      -11     
==========================================
- Hits         8360     8335      -25     
+ Misses       4464     4425      -39     
+ Partials      302      300       -2     
Files Coverage Δ
redash/app.py 100.00% <ø> (ø)
redash/handlers/setup.py 45.71% <100.00%> (+0.47%) ⬆️
redash/settings/__init__.py 98.64% <ø> (-0.01%) ⬇️
redash/settings/organization.py 80.00% <ø> (ø)
redash/tasks/__init__.py 100.00% <ø> (ø)
redash/tasks/general.py 36.17% <ø> (-2.02%) ⬇️
redash/tasks/schedule.py 84.44% <100.00%> (+3.59%) ⬆️
redash/handlers/authentication.py 73.17% <85.71%> (+0.71%) ⬆️

@justinclift justinclift force-pushed the version_and_data_sharing_v1 branch from 6c3bc74 to c21210b Compare April 5, 2024 08:18
@justinclift justinclift changed the title WIP. Remove version check and all of the data sharing Remove version check and all of the data sharing Apr 5, 2024
@justinclift justinclift marked this pull request as ready for review April 5, 2024 08:20
Copy link
Collaborator

@eradman eradman left a comment

Choose a reason for hiding this comment

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

Logs are clean; everything seems to work properly!

@justinclift
Copy link
Member Author

Awesome! Thanks heaps for reviewing this @eradman. 😁

@justinclift justinclift merged commit 4eb5f4e into master Apr 5, 2024
@justinclift justinclift deleted the version_and_data_sharing_v1 branch April 5, 2024 14:02
mirkan1 added a commit to dataminelab/datareporter that referenced this pull request Jul 29, 2024
arikfr added a commit that referenced this pull request Oct 29, 2024
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Jan 8, 2025
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.

2 participants