Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

RegionSearchLocalizationProvider is slow on builds without MLS key #5989

Closed
boek opened this issue Feb 19, 2020 · 5 comments
Closed

RegionSearchLocalizationProvider is slow on builds without MLS key #5989

boek opened this issue Feb 19, 2020 · 5 comments
Assignees
@pocmo
Labels
E5 Estimation points: 5 <location> Component: feature-location
Projects
A-C: Android Components Sprint Planning
Milestone
35.0.0 ✨

Comments

@boek
Copy link
Contributor

boek commented Feb 19, 2020 •
edited by data-sync-user
Loading

MozillaLocationService.fetchRegion caches a result, but if it doesn't get a result it will continue to to make that request every single time we use the RegionSearchLocalizationProvider

┆Issue is synchronized with this Jira Task
@pocmo
Copy link
Contributor

pocmo commented Feb 20, 2020

Afaik you should also get a result with a dummy key - but it is rate limited. Are you sure you do not get anything back? 🤔

@boek
Copy link
Contributor Author

boek commented Feb 20, 2020

Oops! We were using an empty string. I will try a dummy key and close this out!

@boek
Copy link
Contributor Author

boek commented Feb 20, 2020

@pocmo Is there a specific dummy key to use? I am getting this response {"error":{"errors":[{"domain":"usageLimits","reason":"keyInvalid","message":"Missing or invalid API key."}],"code":400,"message":"Missing or invalid API key."}} so it always returns null bypassing the cache

@boek boek mentioned this issue Feb 20, 2020
Closed
@pocmo pocmo self-assigned this Feb 21, 2020
@pocmo pocmo added the <location> Component: feature-location label Feb 21, 2020
@pocmo
Copy link
Contributor

pocmo commented Feb 21, 2020

I'll take a look.

@pocmo
Copy link
Contributor

pocmo commented Feb 21, 2020

  • I prepared a patch that moves the location service behind an interface. So in situations where we do not want to use it we can use a dummy location service (and AC will provide one).

  • SearchEngineManager is doing only one request to determine the location. And this is done from loadSearchEngines() which is supposed to be called only once (or if the configuration changes). Where are the additional calls into RegionSearchLocalizationProvider coming from?

pocmo added a commit to pocmo/android-components that referenced this issue Feb 21, 2020
@pocmo
Issue mozilla-mobile#5989: Create LocationService interface and make …
0d5c86f 
…MozillaLocationService implement it.
pocmo added a commit to pocmo/android-components that referenced this issue Feb 21, 2020
@pocmo
Issue mozilla-mobile#5989: Create LocationService interface and make …
3796cb5 
…MozillaLocationService implement it.
pocmo added a commit to pocmo/android-components that referenced this issue Feb 21, 2020
@pocmo
Issue mozilla-mobile#5989: Create LocationService interface and make …
d2d3dd1 
…MozillaLocationService implement it.
@pocmo pocmo added this to 🏃‍♀️ In Progress in A-C: Android Components Sprint Planning Feb 24, 2020
pocmo added a commit to pocmo/android-components that referenced this issue Feb 25, 2020
@pocmo
Issue mozilla-mobile#5989: Create LocationService interface and make …
728dd04 
…MozillaLocationService implement it.
pocmo added a commit to pocmo/android-components that referenced this issue Feb 27, 2020
@pocmo
Issue mozilla-mobile#5989: Create LocationService interface and make …
113f7e5 
…MozillaLocationService implement it.
bors bot pushed a commit that referenced this issue Feb 27, 2020
@MozLando @pocmo @denschub
Merge #6021 #6082
f999065 
6021: Issue #5989: Create LocationService interface and make MozillaLocatioService implement it. r=jonalmeida a=pocmo



6082: Bug 1618128 - Import WebCompat 8.0.0 sources into feature-webcompat. r=pocmo a=denschub

It's close to the end of a Gecko release cycle, which brings us to the end of the WebCompat release cycle. New version, wohoo!



Co-authored-by: Sebastian Kaspari <s.kaspari@gmail.com>
Co-authored-by: Dennis Schubert <dschubert@mozilla.com>
pocmo added a commit to pocmo/fenix that referenced this issue Feb 28, 2020
@pocmo
FenixSearchEngineProvider: Use dummy location service in debug builds.
8fdc86b 
Workaround for issue described in:
mozilla-mobile/android-components#5989

For debug builds it is unnecessary to use the actual location provider since those builds
do not have an API key configured. With that patch we replace the location provider with
a dummy implementation in debug builds.
@pocmo pocmo mentioned this issue Feb 28, 2020
Merged
boek pushed a commit to mozilla-mobile/fenix that referenced this issue Feb 28, 2020
@pocmo @boek
FenixSearchEngineProvider: Use dummy location service in debug builds.
d89fbd7 
Workaround for issue described in:
mozilla-mobile/android-components#5989

For debug builds it is unnecessary to use the actual location provider since those builds
do not have an API key configured. With that patch we replace the location provider with
a dummy implementation in debug builds.
@jonalmeida jonalmeida moved this from 🏃‍♀️ In Progress to 🏁 Done in A-C: Android Components Sprint Planning Mar 2, 2020
@pocmo pocmo closed this as completed Mar 4, 2020
@pocmo pocmo added the E5 Estimation points: 5 label Mar 4, 2020
@pocmo pocmo added this to the 35.0.0 ✨ milestone Mar 4, 2020
gmierz pushed a commit to gmierz/fenix that referenced this issue Mar 5, 2020
@pocmo @gmierz
FenixSearchEngineProvider: Use dummy location service in debug builds.
d431b76 
Workaround for issue described in:
mozilla-mobile/android-components#5989

For debug builds it is unnecessary to use the actual location provider since those builds
do not have an API key configured. With that patch we replace the location provider with
a dummy implementation in debug builds.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@pocmo pocmo

Labels
E5 Estimation points: 5 <location> Component: feature-location
Projects
No open projects
A-C: Android Components Sprint Planning
  
🏁 Done
Milestone
35.0.0 ✨
Development

No branches or pull requests
2 participants
@pocmo @boek