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

fix: fail on non-success gohbem init #254

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

Fabio1988
Copy link
Collaborator

fixes #214

@jfberry
Copy link
Collaborator

jfberry commented Dec 27, 2024

Fixes it in the sense that it exits if the pokemon data cannot be download? (And I assume therefore potentially goes into a restart loop?)

Should it instead try again in a short while while keeping golbat up?

@Fabio1988
Copy link
Collaborator Author

Fabio1988 commented Dec 27, 2024

Thought about that as well.. This would cause no-pvp data at all for at least 20-30 minutes until retry?! :)

this "fail on loading pokemon data" could be caused if e.g. https://raw.githubusercontent.com/WatWowMap/Masterfile-Generator/master/master-latest-basics.json is not reachable on startup

not sure which fix would be better :(

so you suggest to retry after 1-2 minutes? :)
how many retries?

@jfberry
Copy link
Collaborator

jfberry commented Dec 27, 2024

Thought about that as well.. This would cause no-pvp data at all for at least 20-30 minutes until retry?! :)

this "fail on loading pokemon data" could be caused if e.g. https://raw.githubusercontent.com/WatWowMap/Masterfile-Generator/master/master-latest-basics.json is not reachable on startup

not sure which fix would be better :(

I'm not sure either. I've summarised the options as I see them:

  1. exit - likely system behaviour restart (and retry) - but in some environments would be totally down
  2. hold startup, do not process raws and keep retrying in an orderly fashion until available (eg every minute)
  3. cache latest successful download, like we do for koji geofences, and fall back to that
  4. start with no pvp data, and keep retrying to download on a short or long cycle

Probably (3) is the most attractive

@jfberry
Copy link
Collaborator

jfberry commented Dec 27, 2024

  1. cache latest successful download, like we do for koji geofences, and fall back to that
    Probably (3) is the most attractive

I looked at the gohbem code and it has a function to load from disk already, so the whole download/save to golbat cache/load from disk/refresh on interval could move into golbat code.
Only issue I saw was that it doesn't clear its cache on a load from file, but the cache clear is public (or that could be added to the load from file in gohbem which is probably the right fix)

@Fabio1988
Copy link
Collaborator Author

I saw there is also a safetyCheck method in gohbem... This means instead of returning on o.FetchPokemonData we could also just assign the ohbem object to our internal variable holding the gohbem state?!
this means we would retry after 60 minutes. and we would log for each failing pvp request?! :)
But also the idea with the cache might not be that bad :)

@jfberry
Copy link
Collaborator

jfberry commented Dec 27, 2024

I saw there is also a safetyCheck method in gohbem... This means instead of returning on o.FetchPokemonData we could also just assign the ohbem object to our internal variable holding the gohbem state?! this means we would retry after 60 minutes. and we would log for each failing pvp request?! :) But also the idea with the cache might not be that bad :)

We're probably best just doing it in golbat.

We can try to download, if we succeed write to the file.
On initial startup if the initial download fails then just load from the file.
Then we can load from the file using the public method (and either clear the cache or fix gohbem)

@Fabio1988
Copy link
Collaborator Author

Probably this needs to update gohbem as well.... The masterfile watcher is fetching new data each hour by default.... This update would need to trigger a write to cache ....

@jfberry
Copy link
Collaborator

jfberry commented Jan 3, 2025

As per my description above I think golbat should have the timer, so the download, write the file and gohbem should be told to load from the cache file using the existing load from file function it has.

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.

Retry Gohbem Initialization on startup
2 participants