Dependency xml2js changed radically, breaking awssum's users

[danfuzz]

See https://npmjs.org/package/xml2js for details, but the short story is that the output of converting XML to JS objects is now very different, and this breaks existing awssum use cases.

I think what the right thing to do is to update awssum's package.json to refer more specifically to xml2js at version ~0.1.14, and publish that. That will fix existing users.

Eventually I suspect you'll want to move to the new xml2js, but maybe that will involve changing awssum internally? Not sure, but even if not, I'd guess a major version bump would be in order, assuming the API that gets passed through to awssum users will be different.

[danfuzz]

Oh, upon further reading, it looks like there's a compatibility shim, which may work well enough. Quoting the docs:

---

As version 0.2 will change the default parsing settings version 0.1.14 introduced the default settings for version 0.2.

var xml2js = require('xml2js');
var parser = new xml2js.Parser(xml2js.defaults["0.2"]);

To get the 0.1 defaults in version 0.2 you can just use xml2js.defaults["0.1"] in the same place. This provides you with enough time to migrate to the saner way of parsing in xml2js 0.2. We try to make the migration as simple and gentle as possible, but some breakage cannot be avoided.

[danfuzz]

Ok, it looks like awssum is already passing in explicit options to the xml2js.Parser constructor, so I guess just xmljs.defaults["0.1"] won't quite do. For reference, here are the two sets of defaults from the xml2js source:

  exports.defaults = {
    "0.1": {
      explicitCharkey: false,
      trim: true,
      normalize: true,
      attrkey: "@",
      charkey: "#",
      explicitArray: false,
      ignoreAttrs: false,
      mergeAttrs: false,
      explicitRoot: false,
      validator: null
    },
    "0.2": {
      explicitCharkey: false,
      trim: false,
      normalize: false,
      attrkey: "$",
      charkey: "_",
      explicitArray: true,
      ignoreAttrs: false,
      mergeAttrs: false,
      explicitRoot: true,
      validator: null
    }
  };

It looks like awssum does not currently override any of explicitArray attrkey charkey, all of which changed between the two sets of defaults.

[chilts]

Hi Dan,

Yes, it seems a bit of a change in terms of what xml2js is doing.

I was thinking about the data structure generation the other day ... and I think this issue would be great to fully work out what we should be doing (and use this xml2js change as the initiator of that). From the start, I used xml2js in there since that was easy and I could concentrate on other things, however there are other things to consider.

e.g. when lists come back, sometimes they might be empty (and give {}), sometimes one element (which gives { ... }) and sometimes two or more which gives [ { ...}, { ... } ]. All of this needs to be tidied up and figured out.

One reason I didn't do it from the start was because I tried to do it in my Perl version 4 years ago and it was a lot of work. I think for this project it will be worth it.

Thanks for your initial investigation. I've pulled 78 and released v0.10.1 onto NPM.

Cheers, Andy

[danfuzz]

Glad to be of service. Thanks for pushing out the fix so quickly.

And totally agreed that it'd be super to unify the way 0/1/many results are returned. I've gotten bitten by the differences on a couple occasions.

[chilts]

I think there is some specification which defines how to properly turn XML into a data structure, so I'll look around for that. Maybe looking at the existing sdk tools will give us some pointers.

Cheers, Andy

P.S. Would it be easy to get a list of those 0/1/many results you've had to make fixes for? It's going to be a big job to go through all the operations so anything to help will be gratefully received. :)

[danfuzz]

I meant literally "a couple." The two places I ended up writing code to check are for the results of Route53.ListResourceRecordSets() and Route53.ListHostedZones().

Sorry I don't have anything more than that!

[chilts]

No worries, that's perfect. That means I know I can test in S3, EC2 and Route53. There will be plenty of others but I'll focus on those first.

Cheers, Andy

[chilts]

AwsSum is now deprecated in favour of the offical aws-sdk from Amazon. Thanks for opening this originally.

/me waves at @danfuzz

[danfuzz]

/me waves back at @chilts.