akretion
Refactore roulier to be able to choose between multiple implementation for one carrier
For some carrier, we may want to manage different services/implementation. Gls France for instance propose multiple solutions, like GLS-BOX, which return zpl, or GLS Web API. Laposte fr also have multiple solutions (SOAP XML, the current implementation, or a REST API, not implemented yet) DPD is also changing its api, the current one should be abandoned soon, etc...
It seems it could be quite usefull to ba able to choose among mulltipe implementation of a service/action. For example to enjoy new functionalities or during the transition from one system to another, etc...
We currently have the case for GLS, where present implementation is the GLS-BOX and we want to also implementent the GLS Web API one : https://github.com/akretion/roulier/pull/138.
Since one carrier may have a propose different solution depending on the country (typlically, the implemented GLS-BOX is only for France and won't work for other countries), we had previously decided to add a suffix depending the country to be able to manage the carrier in multiple countries. So, the gls_eu carrier type proposed in https://github.com/akretion/roulier/pull/138 is really confusing (unless it work for multiple european countries, but I doubt it..not sure though.
I'd like then to introduce a system allowing to choose between 2 implementation, while having a default one (so the user that do not care about which implementation is used to get a label don't need to care about that).
For this, we could modify the factory sytem to have something like that :
class RoulierFactory(object):
def __init__(self):
self._carrier_action = {}
def register_builder(self, carrier_type, action, Carrierclass, ttype="default"):
self._carrier_action[(carrier_type, action)] = {ttype: Carrierclass}
def get(self, carrier_type, action, ttype="default, **kwargs):
carrierclass = self._carrier_action.get((carrier_type, action), {}).get(ttype)
if not carrierclass:
raise ValueError((carrier_type, action))
return carrierclass(carrier_type, action, **kwargs)
def get(carrier_type, action, *args, **kwargs):
carrier_obj = factory.get(carrier_type, action)
ttype = kwargs.pop('ttype', 'default')
return getattr(carrier_obj, action)(carrier_type, action, *args, **kwargs)
And on carrier side :
class GlsFrBoxGetLabel(CarrierGetLabel):
...
factory.register_builder("gls_fr", "get_label", GlsFrBoxGetLabel, ttype="gls-box")
class GlsWebGetLabel(CarrierGetLabel):
...
factory.register_builder("gls_fr", "get_label", GlsWebGetLabel)
This way, calling roulier.get('gls_fr', 'get_label', payload) will return a gls label using the WEB api
and calling roulier.get('gls_fr', 'get_label', payload, ttype="gls-box") will return a gls label using the GLS-BOX solution.
Any comment about such a change ? @DylannCordel I'd appreciate your opinion about this
cc @hparfr @bealdav
Hi,
I think it's a good idea but the "default" could be problematic. IMHO, user should explicitly chose the solution he wants to use because data are not always the same (credentials for GLS REST are not the same as for GLS BOX for eg.). But it means it won't be backward-compatible if we force the user to chose the solution.
+1 to have a naming convention as <carrier>_<countrycode>_<api_type> : gls_fr_rest, gls_fr_glsbox etc.
In this way, if we want to force the choice of the solution, there is no changes to do except renaming current carriers folders (and their registration).
With this change, it could be great to split those subdirectories into external repositories to be able to:
- only install what we need
- avoid dead code in the master branch
eg: akretion/roulier, akretion/roulier-gls-fr-rest, akretion/roulier-gls-fr-glsbox etc.
setup.py could be configured to have optionnal dependency to easily choose which carriers (officially supported/maintained) by akretion) we want to auto-install.
Nice to see gls_fr_rest, gls_fr_glsbox convention
setup.py could be configured to have optionnal dependency to easily choose which carriers we want to auto-install.
Don't you think it could be confusing to have so many configuration cases ? @DylannCordel
What do you think about a lib by carrier with plugin management ?
@DylannCordel The idea of the 'default', was not to add complexity for the majority of the carrier fr which we have only one implementation. But yes the developper of the app using roulier should know what implementation he is using so maybe it is clearer and easier the way you propose (which does not require change in base roulier actually)
So I guess for a start, we can use the convention gls_fr_rest, gls_fr_rest.
@bealdav I thought to update the setup.py to inform pip there are some extra:
extras = {
'gls-fr': ['roulier-gls-fr', 'andSomeOthersDepsIfNeeded'],
'laposte-fr': ['roulier-laposte-fr', 'andSomeOthersDepsIfNeeded'],
'dpd-fr': ['roulier-dpd-fr', 'andSomeOthersDepsIfNeeded'],
'some-uk': ['roulier-some-uk', 'andSomeOthersDepsIfNeeded'],
}
extras['all'] = list(set(dep for opt_deps in extras.values() for dep in opt_deps))
extras['all-fr'] = list(set(dep for opt, opt_deps in extras.items() for dep in opt_deps if opt.endswith('-fr')))
setup(
# ...
extras_require=extras,
)
When you install roulier you can choose to install only what you need. For exemple, only GLS and DPD:
pip install roulier[gls-fr,dpd-fr]
And if we want all french "officially" supported carriers:
pip install roulier[all-fr]
Or all "officially" supported carriers (fr, uk etc.):
pip install roulier[all]
What do you think about a lib by carrier with plugin management ?
I'm not sure to understand your proposal. Do you mean to have a shared / common base for a carrier (the API and eventually some tests) and have the technical solution (transport, encoding, decoding) as "plugin" ?
Use extras_require can be a good step, thanks. @florian-dacosta ? It's sufficient to cover that I thought by plugin, no need to more decouple.