Make getPeersV1 meta element optional

[dapplion]

No one likes the meta element, but there's no appetite either for the effort to remove it

• See discussion in Add peers v2 without meta #384 (comment)

According to @mcdee review of clients seems that support of the meta element is poor. If no-one has complained should mean that no consumer is dependent on it.

To address this one-off oddity we could do a breaking change on route v1 and mark the meta property as optional. Then only Nimbus needs to fix the route to be compliant, which @arnetheduck is already keen on doing.

Closes #366

[rolfyone]

We just need to be careful here because ultimately required -> optional == breaking change to a published interface...

[dapplion]

We just need to be careful here because ultimately required -> optional == breaking change to a published interface...

Yes indeed, this PR intends to gauge support if a one-off breaking change to deal with this oddity is ok or not

[rolfyone]

As a rule, i'd be against this, it really doesn't accomplish anything useful, and is a breaking change by definition, so that's my reasoning.

[dapplion]

As a rule, i'd be against this, it really doesn't accomplish anything useful, and is a breaking change by definition, so that's my reasoning.

• Ok, let's leave this open for a bit if there's not more input close this PR and Inconsistency in /eth/v1/node/peers #366 going for option 2

[nflaig]

do a breaking change on route v1 and mark the meta property as optional

The change in this PR makes the data property required and not the meta property optional as all model fields are optional by default unless they are part of required field in schema.

It does not seem like the spec uses the required field a lot but I think it would be good practice to use it to be technically correct (relevant for schema validation) and also to get a visual hint when looking at the explorer as it adds a * next to the property.

[nflaig]

We just need to be careful here because ultimately required -> optional == breaking change to a published interface...

I would generally agree with this statement but should we consider meta being part of the response a bug and hence it would be acceptable to remove it? Seems like it is just a leftover from previous considerations adding pagination to the API, see #101 (comment).

I think if this was not 3+ years ago we could definitely just remove it altogether, but at this point, consumers might rely on the bug, although unlikely because bad client support / compatiblity between clients.

[rolfyone]

I would generally agree with this statement but should we consider meta being part of the response a bug and hence it would be acceptable to remove it? Seems like it is just a leftover from previous considerations adding pagination to the API, see #101 (comment).

Whether we make a breaking change in fixing a bug or make a breaking change in an enhancement seems inconsequential... It's a breaking change, it's going to likely cause problems...

On that basis, is it important enough to do, and if we do it, we should make it a new version and deprecate / remove the old one... - Still breaking but breaking nicely? We can easily define a new endpoint the way we want it as a new version, that would be probably cleaner.

[nflaig]

if we wanna clean this up, might be better to add a v2 api and get rid of this one at some point

[rolfyone]

closing for now. we can re-visit later.