Event format compatibility

Development
1

@canton7, other event consumers:

We have this event today:

{
    "id": 8,
    "type": "StateChanged",
    "time": "2014-07-17T13:14:28.697493016+02:00",
    "data": {
        "folder": "default",
        "from": "scanning",
        "duration": 0.19782869900000002,
        "to": "idle"
    }
}

It’s only emitted for folder changes, but it’s very generically named. I’d like to extend it to cover device events (in addition to the existing DeviceConnected / DeviceDisconnected events we already have). E.g.,

{
    "id": 8,
    "type": "StateChanged",
    "time": "2014-07-17T13:14:28.697493016+02:00",
    "data": {
        "device": "373HSRP-QLPNLIE-JYKZVQF-P4PKZ63-R2ZE6K3-YD442U2-JHBGBQG-WWXAHAU",
        "from": "sendingIndex",
        "duration": 0.19782869900000002,
        "to": "syncing"
    }
}

Is this going to break things? The differences compared to the existing one would be

  • Missing key folder
  • New key device
  • New possible values in from and to

The proper reaction to this event if it’s not supported is to ignore it, it just adds information that you’ll be fine without if you were fine without it before. I’m hoping that would be the automatic reaction to the missing folder key if something else specific wasn’t coded…

(By understanding the event you’ll gain more knowledge about what a device is doing, in addition to it’s completion status. Among other things this makes it possible to differentiate between “Syncing (50%)” (it’s working on it) and “Idle (50%)” (it only has half of the files, and seems satisfied with that for the moment).)

2

Maybe. I’ll have to check the code… I’ll try and find time tonight, but it might take until sometime next week when I’m back home.

1 Like
3

Sorry for the delay!

Yeah the missing “folder” key will crash SyncTrayzor currently.

1 Like
4

Okay, it’ll have to be a new event type for now then.

(That said, how about ignoring events you can’t parse rather than crashing? ;))

5

I parse it correctly, but that folder then comes out as a null, which causes an NRE much later. Yay for null.

Yeah if I was doing this project professionally I’d have a layer which does sanitisation on everything I receive. That’s a fair chunk of effort to do comprehensively though so I didn’t bother for something which was never supposed to be popular…

From a purely pretty point of view, I don’t like the fact that the same data can represent two different things, depending on whether or not particular keys/values are present. It’s implied that either folder or device is present, but not neither nor both, which smells. An additional field which days what sort of state change the event represents would be nicer…

We’ve handled breaking changes in the past: give me a little bit to get a new release out and move people across to it…

1 Like
6

I know how that feels :slight_smile:

Oh yes, there’s no rush here, and the proper way may indeed be to have both FolderStateChanged and DeviceStateChanged events with no ambiguity. The real error may be that the existing event is named just StateChanged which makes me want to reuse it. Certainly we’ll not intentionally change things in a way that will break trayzor in a patch release.

1 Like
7

You’ve successfully shamed me into being more resilient to changes in events…

2 Likes
8

That’s probably good, although I didn’t mean to bring that much shame on you. Us breaking the contract the API provides can certainly be a good enough reason to crash.

3 Likes