Hi Markus, On Thu, 2009-10-22 at 23:30 -0700, Markus Roberts wrote: > > > This is not the right fix, but more a hackish workaround. > > Still, overall, it's pretty clean for a hack. A few thoughts: > > 1. Would it make sense to make it optional instead of obligatory?
The only issue is if you have a short facts list, in which case the compression/base64 itself will take more place than the textual representation. Another (small) issue is that now you can't read anymore the facts contents in the various access logs. But yes, this is simple to make optional, the cost is to add yet another configuration settings, which we could call: facts_format It could have the following values "yaml", "pson", "b64_zlib_yaml". > 2. If it is to be obligatory, we should probably update the comments > as well. What comments? > 3. Would it make sense to use JSON (i.e. PSON) instead? I don't know. The facts were serialized as YAML beforehand. If there is a desire to move all our serialization to pson, then I guess this one should move too. > 4. What are you thinking for a less hackish solution? POST? The issue is that POST doesn't fit the REST model. We're asking for a rest resource of type catalog. But to be able to process it we need the facts. Before that, the operation was done in 2 distinct operations: * we were POSTing the facts, which were saved on the master * we were GETting the catalog The issue was that in a multiple master system you could well post the facts to master1 but ask the catalog to master2 which had no way to fetch those facts. Hence it was decided (although at that time I objected that the request size would be an issue) to merge those two operations. Of course we still can break the REST model, by returning the catalog when saving the facts, but that feels ugly. > 5. Do you know if there's a way to have a format handler inherit from > another, so we don't have so much code duplication? That was my first concern too, I hate copy/pasting code. Unfortunately there's no possibility without changing the way the format_handlers are working which I wasn't ready to do for a 5 minutes hack. I'm wondering if a proper solution would be to have a format (ie the serialization system) and an encoding chain system (ie how we transport the values). This way we could chose for every entity we transport: * how it is serialized * how it is transformed to be transported (ie compressed/uncompressed, base64 encoded, nothing,...). Those transformations could be chained so that we could compress, then encode and son... > 6. Should this work on 1.8.1? For that matter, why didn't my PSON > patch break fact-passing on 1.8.1? I don't know. -- Brice Figureau Follow the latest Puppet Community evolutions on www.planetpuppet.org! --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
