In our commercial GUI, we’ve created the “porcelain” for DS Types. I personally find it easier to use. I don’t need to go to the docs to pick the right type from the dropdown
Screenshot: https://cisco.box.com/s/agpflh82j6nv3wusukjdfk22cpgxbbbt —Eric On Jun 27, 2018, at 5:24 PM, Rawlin Peters <[email protected]<mailto:[email protected]>> wrote: replies inline On Wed, Jun 27, 2018 at 2:17 PM Robert Butts <[email protected]<mailto:[email protected]>> wrote: I'm inclined to agree with not adding new DS types. Yeah, we'll need code for `if go_direct is undefined...`, but I'm pretty sure we'd need equal or more code, across all our apps, to handle the new types. We have a ton of code that checks `if dstype == HTTP_LIVE { ... } else if dstype == HTTP_LIVE_NATNL { ... } else if dstype.StartsWith("DNS") && !dstype.Contains("NATNL") {`, etc. I know we have a bunch of code checks like that, but in this case we're dealing with changing the generation of one ATS config file: parent.config. So I imagine it would be pretty well constrained to just that function, making it check for 'BYPASS' in the type name. Everywhere else that checks for 'NATNL', 'LIVE', etc should be constrained to ATS config generation and should do the right thing with the new types (just adding '_BYPASS' to 4 of the existing type names). Some code does string searches for the concatenated parts (http, live, etc), while some checks the full string, e.g. "HTTP_LIVE_NATNL". It's going to be risky searching all the code in all our components for those string checks, and making sure they're all doing the right thing with the new types. Something will get missed. I've had to deal with a lot of that code in TM and TO, and it always feels super-dangerous. IMO we should really be moving away from DS "types" and make different fields for each configuration property: protocol, disk, mid. Ideally, with normalized tables for existing fields that only apply to certain configurations. But I'm not suggesting we do that today. I agree, that probably would be better than DS types, but DS types is what we have today, and I don't think mixing the two is a great idea. So until we can break the API to make it better, new DS types is better IMO. However, if we do decide on a "go_direct" field, it should be more generic than a boolean because there are at least 3 different mid strategies: no mids at all, mids but bypass to origin when all mids are down, and mids with no bypass. I'd be more on board with this if the DS-type no longer determined ATS config for those strategies and was just used to select the proper default for this "go_direct/mid" field which in turn would determine the ATS config. Then at least we could make the code cleaner and hopefully do away with the whole "you can't do this if the type is FOO_LIVE" stuff. But, in the long term, I think using string concatenation and enums for every combination of multiple properties is going to be more code, and more bug-prone, than dealing with a nullable field. It's also worth observing, there's no reason for the UI to be tied to the implementation. If we want the Portal to present users with a "DS Type" dropdown with "HTTP Live National", "HTTP Live Bypass", etc, we can still do that, and set the correct fields in the API and database. There's nothing wrong with UI porcelains. That's what we'll have to do with the new "go_direct" field for sure. We're really debating which kind of denormalization is the lesser evil. Both are dangerous and bug-prone, but IMO the nullable field is a little less so than the multi-property field.
