Skip to content

rm TAAR service integrations #23147

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

Merged
merged 2 commits into from
Mar 12, 2025
Merged

rm TAAR service integrations #23147

merged 2 commits into from
Mar 12, 2025

Conversation

eviljeff
Copy link
Member

@eviljeff eviljeff commented Mar 10, 2025

Fixes: mozilla/addons#15377

Description

Removes all TAAR integration, and attempts to rename what's left to not reference TAAR.

Context

Testing

Verify that the two endpoints in the issue return results

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@eviljeff eviljeff marked this pull request as ready for review March 10, 2025 11:16
@eviljeff eviljeff requested review from a team and KevinMind and removed request for a team March 10, 2025 11:17
@@ -957,7 +957,7 @@ Replacement Add-ons

.. _addon-replacement-addons:

This endpoint returns a list of suggested replacements for legacy add-ons that are unsupported in Firefox 57. Added to support the TAAR recommendation service.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add this to the addons DRF viewset in the relevant method? Just good hygiene to continually update the methods so we eventually don't have to maintain the separate api docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you link to docs explaining how to do this?

@KevinMind
Copy link
Contributor

Could you link the relevant endpoints in the PR.. I don't actually see anything in the issue or the linked doc at least not obviously.

@eviljeff
Copy link
Member Author

Could you link the relevant endpoints in the PR.. I don't actually see anything in the issue or the linked doc at least not obviously.

Mat linked to the docs that detail the endpoints in the comment directly preceding yours. They are:

@KevinMind
Copy link
Contributor

KevinMind commented Mar 10, 2025

Could you link the relevant endpoints in the PR.. I don't actually see anything in the issue or the linked doc at least not obviously.

Mat linked to the docs that detail the endpoints in the comment directly preceding yours. They are:

* https://mozilla.github.io/addons-server/topics/api/v4_frozen/discovery.html#discovery-content

* https://mozilla.github.io/addons-server/topics/api/addons.html#recommendations

Thanks for clarifying. FWIW the confusion for me was that there are 2 links, with a total of 6 endpoints.. and it wasn't even clear that was the comment that I should have been referring to. Might have been obvious to you, but definitely wasn't to me.

@eviljeff
Copy link
Member Author

Could you link the relevant endpoints in the PR.. I don't actually see anything in the issue or the linked doc at least not obviously.

Mat linked to the docs that detail the endpoints in the comment directly preceding yours. They are:

* https://mozilla.github.io/addons-server/topics/api/v4_frozen/discovery.html#discovery-content

* https://mozilla.github.io/addons-server/topics/api/addons.html#recommendations

Thanks for clarifying. FWIW the confusion for me was that there are 2 links, with a total of 6 endpoints.. and it wasn't even clear that was the comment that I should have been referring to. Might have been obvious to you, but definitely wasn't to me.

It's just two endpoints - the two directly linked to (are you counting the other endpoints in different sections on the same page? Or v3, v4, v5 * 2?). /api/v4/discovery/ and /api/v5/addons/recommendations/ are the ones that must keep working, as per the issue.

@eviljeff eviljeff requested a review from KevinMind March 11, 2025 16:12
Copy link
Contributor

@KevinMind KevinMind left a comment

Choose a reason for hiding this comment

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

http://olympia.test/api/v4/discovery/

{"results":[],"count":0}

http://olympia.test/api/v5/addons/recommendations/?guid=@spicy-pancake

{"outcome":"curated","fallback_reason":null,"page_size":1,"page_count":1,"count":0,"next":null,"previous":null,"results":[]}

I have no idea if this is "still working" I'm getting empty responses but they are not raising. It would be helpful to have specific test criteria for this since you have context on what exactly we should be expecting? That is not only helpful for me to verify, but also for QA to verify and finally for future us to debug/reverify if there is any issue that links back to this PR.

@eviljeff
Copy link
Member Author

http://olympia.test/api/v4/discovery/

{"results":[],"count":0}

http://olympia.test/api/v5/addons/recommendations/?guid=@spicy-pancake

{"outcome":"curated","fallback_reason":null,"page_size":1,"page_count":1,"count":0,"next":null,"previous":null,"results":[]}

I have no idea if this is "still working" I'm getting empty responses but they are not raising. It would be helpful to have specific test criteria for this since you have context on what exactly we should be expecting? That is not only helpful for me to verify, but also for QA to verify and finally for future us to debug/reverify if there is any issue that links back to this PR.

Ahhh, you need some extra set-up locally to replicate the state on addons-dev (and stage, prod).

  • For api/v5/addons/recommendations you need to have add-ons with the hard-coded guids in RECOMMENDED - easiest way is to just change the guid values of existing public addon instances in django admin|shell
  • For api/v4/discovery/ there should be some DiscoveryItem instances created in django admin

@eviljeff eviljeff merged commit 8b7c26d into mozilla:master Mar 12, 2025
41 checks passed
diox pushed a commit to diox/olympia that referenced this pull request Mar 13, 2025
* rm TAAR service integrations

* delete waffle switch too
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.

[Task]: Remove TAAR integration
2 participants