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

Remove Google Analytics driver and related special case stuff #42792

Closed
camsaul opened this issue May 16, 2024 · 2 comments · Fixed by #43352
Closed

Remove Google Analytics driver and related special case stuff #42792

camsaul opened this issue May 16, 2024 · 2 comments · Fixed by #43352

Comments

@camsaul
Copy link
Member

camsaul commented May 16, 2024

We made the decision to deprecate it about a year ago because of #12905 (comment), now because it's blocking Metrics v2 we are finally going to drop it entirely. (We announced this in 47.) This needs to be done before we ship 50. See https://metaboat.slack.com/archives/C04DN5VRQM6/p1715888743074829 for more info.

We need to delete all the code related to GA in the repo including the CI job and any mentions of :googleanalytics in the codebase. We have a lot of special edge case stuff in the codebase for GA's hardcoded :metrics and :segments (which use string IDs instead of integers), we should remove that stuff from the MBQL schemas and all of the special case code. Examples of stuff to remove:

  • modules/drivers/googleanalytics
  • ;; Metrics are just 'macros' (placeholders for other aggregations with optional filter and breakout clauses) that get
    ;; expanded to other aggregations/etc. in the expand-macros middleware
    ;;
    ;; METRICS WITH STRING IDS, e.g. `[:metric "ga:sessions"]`, are Google Analytics metrics, not Metabase metrics! They
    ;; pass straight thru to the GA query processor.
    (def ^:private MetricID
    [:ref ::lib.schema.id/legacy-metric])
    (defclause metric
    metric-id [:or MetricID ::lib.schema.common/non-blank-string])
  • "modules/drivers/googleanalytics/test"
  • (mbql.u/ga-metric-or-segment? metric) (-> args first str (subs 3) str/capitalize)
  • GitHub Actions related to Google Analytics

Please be thorough and remove all the dead GA-related code!!! Please 🙇‍♂️

@camsaul
Copy link
Member Author

camsaul commented May 16, 2024

We should also double-check that things like the Database REST API endpoints don't blow up if we have a Database with a driver that no longer exists. The after-select method for :model/Database only calls metabase.driver.util/features if the driver is registered, so that should be good at least, but we should check other things as well just to be safe.

@camsaul camsaul changed the title Remove Google Analytics driver and related special case stuff Remove Google Analytics driver and related special case stuff. Ensure orphaned DBs do not blow up Metabase May 16, 2024
@crisptrutski
Copy link
Contributor

@camsaul do you have an ETA for landing the remaining work here?

@camsaul camsaul changed the title Remove Google Analytics driver and related special case stuff. Ensure orphaned DBs do not blow up Metabase Remove Google Analytics driver and related special case stuff May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants