You can keep each object without the order parameter:
[{"name": "grp1"}, {"name":"grp3"}]
Nevertheless, it sounds like I'm a minority opinion on this one. It's
not that important of an issue, I think. Either way will work.
On Mon, Mar 19, 2018 at 5:50 PM, Rawlin Peters <[email protected]> wrote:
> Yeah, maybe it doesn't make sense to update just one backup entry at a
> time because you'd have to update the order of the rest of the backups
> as well. Perhaps the POST/PUT endpoints are basically the same and
> always take the full list of backups. But I think we should still keep
> each entry as an object with an explicit "order" so that we can extend
> it more easily in the future. I could easily see us adding a "weight"
> to each entry, so that a particular cachegroup could split the
> fallback traffic between multiple cachegroups at a time rather than
> falling back through them sequentially.
>
> - Rawlin
>
> On Mon, Mar 19, 2018 at 2:30 PM, Chris Lemmons <[email protected]> wrote:
>> So, to continue the conversation, it looks like the list of backup
>> groups is stored as a List<String>. It's currently loaded by iterating
>> the elements of the json array in order. That looks great to me.
>>
>> It seems odd to me to have a separate order parameter in the API.
>> Since order has to be unique, it's unlikely that you'd be able to
>> update the order component to do anything other than "move to an end"
>> without updating about half the rows in the db anyway. It just feels
>> like we're asking more of the API user.
>>
>> On Mon, Mar 19, 2018 at 2:04 PM, Chris Lemmons <[email protected]> wrote:
>>> Moving a conversation started in Slack to the mailing list:
>>>
>>> VijayAnand [8:06 AM]
>>> Hi
>>>
>>> This is regarding TR's Backup Cache Group Selection
>>> which is https://github.com/apache/incubator-trafficcontrol/issues/1907
>>> GitHub
>>> Deterministic Cachegroup failover · Issue #1907 ·
>>> apache/incubator-trafficcontrol
>>> Currently, if all caches in a cache group are unavailable Traffic
>>> Router will route clients to the next closest cache group. This works
>>> great for some networks but could cause problems in others. T...
>>> Based on Rawlin's review comments on the PR
>>> https://github.com/apache/incubator-trafficcontrol/pull/1908
>>> GitHub
>>> Changes for Backup Edge Cache Group by Vijay-1 · Pull Request #1908 ·
>>> apache/incubator-trafficcontrol
>>> This PR implements solution for the issue: #1907 It places the backup
>>> policy in the CZF file { "coverageZones": {
>>> "GROUP2": { "backupList": ["GROUP1"],
>>> &quo...
>>> I have the following Schema to support this which is in-sync with
>>> Rawlin's comments
>>> CREATE TABLE cachegroup_fallbacks (
>>> primary_cg bigint,
>>> backup_cg bigint CHECK (primary_cg != backup_cg),
>>> set_order bigint DEFAULT 0,
>>> CONSTRAINT fk_primary_cg FOREIGN KEY (primary_cg) REFERENCES
>>> cachegroup(id) ON DELETE CASCADE,
>>> CONSTRAINT fk_backup_cg FOREIGN KEY (backup_cg) REFERENCES
>>> cachegroup(id) ON DELETE CASCADE,
>>> UNIQUE (primary_cg, backup_cg),
>>> UNIQUE (primary_cg, set_order)
>>> );
>>>
>>> ALTER TABLE cachegroup ADD COLUMN fallback_to_closest BOOLEAN DEFAULT TRUE;
>>> Would like to get your views before i start coding for the same
>>>
>>> Eric Friedrich [8:15 AM]
>>> why does the set_order get a default?
>>>
>>> VijayAnand [8:17 AM]
>>> aah. that is not needed
>>> assuming there is a valid input
>>> for order
>>>
>>> Rawlin Peters [9:15 AM]
>>> yeah a null value there could be interpreted as zero in the API, but
>>> maybe we should add a NOT NULL constraint to the columns as well? I
>>> can't think of any reasons why any column should be optional
>>>
>>> Eric Friedrich [9:18 AM]
>>> any preference in the API between
>>> ```{"list": ["grp1", "grp2"]}```
>>> vs
>>> ```{"list": [{"name": "grp1", "order": 1},
>>> {"name": "grp3", "order: 2}, // or 5 or some other
>>> positive integer
>>> ]
>>> }```
>>>
>>> Rawlin Peters [9:36 AM]
>>> 2nd option would make it easier to extend in the future with weighting
>>> for instance, we could just add another key/value in that object
>>>
>>> Eric Friedrich [9:37 AM]
>>> good point- i hadn’t thought about that
>>>
>>> Chris Lemmons [1:16 PM]
>>> The second is way harder to parse and handle. I strongly prefer the first.
>>> Is there a reason to expect such extension? (edited)
>>>
>>> Jeff Elsloo [1:30 PM]
>>> presumably we’re using a library to handle it, so why is it way harder?
>>>
>>> Dan Kirkwood [1:33 PM]
>>> +1 -- it's all just JSON..
>>>
>>> Rob Butts [1:39 PM]
>>> I don’t agree that we shouldn’t care about the human-readability,
>>> because we have libraries. Someone might want to write an app in a
>>> different language. I wouldn’t say “way” harder, but the second
>>> definitely is harder to read, and unintuitive. I’m +1 on making our
>>> APIs easy to read and work with natively :confused:
>>>
>>> Jeff Elsloo [1:41 PM]
>>> but a list is just a container
>>> it contains “things”
>>> in this case it’s a list of objects with properties
>>>
>>> Dylan Volz [1:41 PM]
>>> agree we shouldn't not care, but most of our apis are lists of json
>>> objects, and if we need to add a key the the second definition is far
>>> more easily extensible
>>>
>>> Chris Lemmons [1:42 PM]
>>> True. Are they actually objects with properties? Or are they string values?
>>>
>>> Jeff Elsloo [1:42 PM]
>>> that’s implied by the structure
>>>
>>> Chris Lemmons [1:42 PM]
>>> Either way, I'd prefer not to overload the order as a parameter,
>>> unless there is a particular reason?
>>>
>>> Jeff Elsloo [1:43 PM]
>>> `{"name": "grp3", "order: 2}` is not a string in JSON
>>>
>>> Chris Lemmons [1:43 PM]
>>> Aye.
>>>
>>> Jeff Elsloo [1:43 PM]
>>> I don’t follow what you mean about order
>>>
>>> Chris Lemmons [1:43 PM]
>>> Are there other parameters (or might there reasonably be in the future)?
>>>
>>> Jeff Elsloo [1:43 PM]
>>> order is just a property and has nothing to do with overloading anything
>>> in this context, if it has to do with Rawlin’s steering work, yes
>>> there certainly could be other properties
>>> which is why the format was suggested
>>>
>>> Chris Lemmons [1:45 PM]
>>> So, for example, using objects:
>>>
>>> I prefer `[{"name": "grp1"}, {"name": "grp3"}]` to `[{"name": "grp1",
>>> "order": 1},{"name": "grp3", "order: 2}]`.
>>> In both cases, you can easily add a new parameter, for example, if
>>> groups had an owner.
>>> But arrays already have orders.
>>>
>>> Jeff Elsloo [1:45 PM]
>>> no
>>> not in JSON
>>>
>>> Chris Lemmons [1:46 PM]
>>> Yes, they do.
>>>
>>> Jeff Elsloo [1:46 PM]
>>> I’m not going to implicitly trust the order of what’s in the source JSON
>>> we don’t have the ability to control that
>>>
>>> Chris Lemmons [1:46 PM]
>>> Arrays in JSON are ordered. Objects in JSON are not ordered.
>>>
>>> Jeff Elsloo [1:46 PM]
>>> no they aren’t
>>> not unless they’re systematically created that way
>>> which in this case they are not
>>> we have a property called order and a value
>>> that could appear in any order of the underlying array
>>> we would have to do a lot more work to build it properly and then just
>>> make the assumption in downstream components that the order is
>>> implicitly correct in JSON
>>> that is a risky assumption
>>>
>>> Chris Lemmons [1:47 PM]
>>> https://tools.ietf.org/html/rfc7159
>>>
>>>> An array is an ordered sequence of zero or more values.
>>>
>>> Jeff Elsloo [1:47 PM]
>>> seriously Chris?
>>> do you even know why or how such things are used in Traffic Ops?
>>> did you know that the orders could be negative?
>>> I’m not arguing about whether or not JSON supports order in its arrays
>>> I’m arguing about how the data is put into JSON and how users
>>> configure the properties
>>> and what the values can be
>>>
>>> Chris Lemmons [1:49 PM]
>>>> do you even know why or how such things are used in Traffic Ops?
>>>
>>> Not in detail, no. Hence my question:
>>>
>>>> unless there is a particular reason?
>>>
>>> Jeff Elsloo [1:49 PM]
>>> having to order that in the JSON adds another layer of complexity that
>>> is unnecessary
>>> I’ve given you some but you dropped an RFC on me in reply which is
>>> implicitly saying you don’t agree and won’t
>>> so for cache group ordering, which is separate from the steering
>>> ordering issue, I think that stating order specifically will give us
>>> more options down the road just as it did with steering
>>> we have multiple ways to order things in the steering case, and we’ve
>>> extended that, so I don’t see why that precedent wouldn’t also apply
>>> here
>>> backup cache group ordering that is
>>>
>>> Chris Lemmons [1:55 PM]
>>> Ah... I think there was a bit of a miscommunication there. To the
>>> point of the discussion, though... I missed which data this is
>>> storing. Yeah, we have to store the ordering data explicitly in the
>>> database, since rows are unordered.
>>>
>>> Jeff Elsloo [1:55 PM]
>>> yes, that’s what I was getting at.. so we’d have to pull the values
>>> out of the database, sort them properly, then ensure they are written
>>> that way to the snapshot table
>>>
>>> Rawlin Peters [1:55 PM]
>>> fwiw the particular JSON in question isn't for the steering stuff I'm
>>> working on, it's for the Cachegroup Failover stuff (i.e. CG1
>>> specifically fails over to CG2, CG3, in that order)
>>>
>>> Jeff Elsloo [1:55 PM]
>>> it’s easier to just have the downstream consumer worry about that, as
>>> they likely would need to worry about it anyway
>>> yeah Rawlin, I saw that eventually :slightly_smiling_face:
>>>
>>> Rawlin Peters [1:56 PM]
>>> sorry just catching up too
>>>
>>> Jeff Elsloo [1:56 PM]
>>> but to me it’s a similar concept as steering
>>> we have an ordering mechanism based on an int value today
>>> maybe that changes in the future
>>> as things in our world tend to
>>> I agree with keeping the content the API spits out as simple as
>>> possible, but not to the extent that it makes us have to do more work
>>> just for sake of readability
>>>
>>> Chris Lemmons [1:58 PM]
>>> So, what happens if ordering is equivalent?
>>>
>>> Rawlin Peters [1:58 PM]
>>> yeah plus we need a way to update the order of a particular relation,
>>> which is why we can't just keep it as a simple list of strings
>>> there's a uniqueness constraint on (primary_cg, order), so a
>>> particular cachegroup can only declare one backup CG at that
>>> particular ordering
>>>
>>> Chris Lemmons [2:01 PM]
>>> Ok. So if you want to change ordering other than "move to end" or
>>> "move to beginning", you'll probably need to update half of the
>>> relations?
>>>
>>> Dave Neuman [2:01 PM]
>>> All of this ^^ should be done on the mailing list (edited)
>>> any of the decisions made here don't count
>>> unless you want to come to some consensus here, put it on the mailing
>>> list, and have everyone +1
>>> which is against the spirit of the mailing list (edited)
