fix: fail on non-success gohbem init

[Fabio1988]

fixes #214

[jfberry]

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]

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]

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]

3. 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]

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]

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]

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]

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.

[lenisko]

Since everything is already in Gohbem I see no point to moving a logic to Golbat. For now, I have added a proposal of the ability to set up a path to auto update a MasterFile from within a watcher routine in Gohbem.

https://github.com/UnownHash/gohbem/pull/16/files

Gohbem have few methods we can use to achieve it:

• o.FetchPokemonData() - Tries to fetch a new MasterFile and stores it inside internal struct
• o.LoadPokemonData(filePath string) - Loads external MasterFile and override internal struct with it's content
• o.SavePokemonData(filePath string) - Saves internal struct to external MasterFile path
• o.WatchPokemonData() - Watches for MasterFile changes, and updates internal struct + when o.MasterFileCachePath != "" it will auto save changes to MasterFile once fetched & changes are detected.

If that works for you, all we need is to make a few changes in Golbat. Right now we call only a Fetch and Watch.

		if err := o.FetchPokemonData(); err != nil {
			log.Errorf("ohbem.FetchPokemonData: %s", err)
			return
		}

		_ = o.WatchPokemonData()

So when it can't fetch a MasterFile for whatever reason, it will fail. To avoid that, we could store MasterFile under Golbat cache directory in git and load it o.LoadPokemonData(filePath string) first. Also passing MasterFileCachePath to Gohbem struct, we will also make sure to keep it updated when it changes. Changes on this file should be ignored in .gitignore after adding it to repository.

This would make sure it won't fail initial run anymore. In worst case we will have outdated data till Gohbem will be able to fetch updated version, and once it will success we will have the newest MasterFile in cache for a case when restart is issued and initial pull won't work.

[lenisko]

Since we don't provide any MF within repo https://github.com/UnownHash/Golbat/pull/254/files#diff-9a386635a8469bbef0ef8310a20497399a96bd3773f865d237ad6a407f84a53cR202 this should be fatal I think.

[jfberry]

See my comment in gohbem for logic to load a provided master file - it should be in a separate folder from the cache written one to avoid git issues

[Fabio1988]

I don't think we should copy something from different repo into this, maybe a git submodule would work for that case, will have a look tomorrow

[jfberry]

It doesn’t really matter how out of date it is since in the default case it won’t be used, and a separate copy protects against failure / corruption of the (unmaintained now) source.