Changes we'd like to try pushing upstream are something like:
[ ] Add a method for calling geoserver's reload route. Maybe in a new class under https://github.com/samvera-labs/geoserver-publish/tree/main/lib/geoserver/publish (but if one of the existing classes there seems like a natural home, go for that). This could then replace the Geoserver::Publish::Connection#post(path: 'reload', payload: nil) call made by our Robots::DorRepo::GisDelivery::ReloadGeoserver with something like Geoserver::Publish::DataStore#reload (just as an example -- DataStore isn't necessarily a better home than e.g. a new ServerCommands class). Though we also might want to turn this into a #get instead of a #post, because the Geoserver doc makes it sound idempotent, and the docs say GET is supported for the route.
[ ] Add the option to geoserver-publish to use the the faraday-retry gem. Maybe this option could be a new field in the config that Geoserver::Publish::Connection uses, whether it gets it via constructor param or fallback to something loaded by (or set on) Geoserver::Publish.config["geoserver"]. See here and here. To keep backwards compatibility, the option could default to false. If the caller does enable the retry option, maybe it's then used by default on idempotent methods? So just #get? Or maybe the retry option's values are none, idempotent, all -- none is the default and disables faraday-retry, idempotent enables retries only on #get, and all enables it on all the connection helper methods? 🤷
[ ] Add the option to configure connection timeout. Also a new field in the Geoserver::Publish::Connectionconfig hash? Also used if present in theFaraday.newcall inConnection#faraday_connection`?
And then depending on which of the above changes are pushed upstream and accepted, upgrade gis-robot-suite to the new gem version and take advantage of the new functionality (e.g. get rid of the retries gem and in-robot retry handling for reload calls, use the #reload method instead of POSTing more directly to the reload path, set a longer timeout for the reload call). For our testing, we could point gis-robot-suite's Gemfile to our WIP branch of the gem, though. Might be a good idea to make some of the gem changes, use them in the robots, see if it makes the gem look more useful and the robots more maintainable, and proceed if both of those things check out?
Totally fine to break spin off new and more specific tickets for subtasks in this ticket, if that feels more reasonable (but please update this ticket to indicate that the subtask is now tracked in the new ticket).
If this works out, this is the longer term fix we'd prefer for the timeout issue that was ticketed by https://github.com/sul-dlss/gis-robot-suite/issues/786 (fixed within this codebase by https://github.com/sul-dlss/gis-robot-suite/pull/791).
Changes we'd like to try pushing upstream are something like:
reload
route. Maybe in a new class under https://github.com/samvera-labs/geoserver-publish/tree/main/lib/geoserver/publish (but if one of the existing classes there seems like a natural home, go for that). This could then replace theGeoserver::Publish::Connection#post(path: 'reload', payload: nil)
call made by ourRobots::DorRepo::GisDelivery::ReloadGeoserver
with something likeGeoserver::Publish::DataStore#reload
(just as an example --DataStore
isn't necessarily a better home than e.g. a newServerCommands
class). Though we also might want to turn this into a#get
instead of a#post
, because the Geoserver doc makes it sound idempotent, and the docs sayGET
is supported for the route.Geoserver::Publish::Connection
uses, whether it gets it via constructor param or fallback to something loaded by (or set on)Geoserver::Publish.config["geoserver"]
. See here and here. To keep backwards compatibility, the option could default to false. If the caller does enable the retry option, maybe it's then used by default on idempotent methods? So just#get
? Or maybe theretry
option's values arenone
,idempotent
,all
--none
is the default and disablesfaraday-retry
,idempotent
enables retries only on#get
, andall
enables it on all the connection helper methods? 🤷config hash? Also used if present in the
Faraday.newcall in
Connection#faraday_connection`?And then depending on which of the above changes are pushed upstream and accepted, upgrade gis-robot-suite to the new gem version and take advantage of the new functionality (e.g. get rid of the
retries
gem and in-robot retry handling forreload
calls, use the#reload
method instead ofPOST
ing more directly to thereload
path, set a longer timeout for thereload
call). For our testing, we could point gis-robot-suite'sGemfile
to our WIP branch of the gem, though. Might be a good idea to make some of the gem changes, use them in the robots, see if it makes the gem look more useful and the robots more maintainable, and proceed if both of those things check out?Totally fine to break spin off new and more specific tickets for subtasks in this ticket, if that feels more reasonable (but please update this ticket to indicate that the subtask is now tracked in the new ticket).