reference icon indicating copy to clipboard operation
reference copied to clipboard

SubscribeResponse cannot return an error

Open tsuna opened this issue 9 years ago • 17 comments

It's convenient that an Error message was added for UpdateResponse, but a bummer that SubscribeResponse didn't also get it. One common case that has come several times on my end is subscribing to a non-existent path. There is no way to convey to the client that the path didn't exist.

It's also not clear what the implementation is supposed to do in this case. In the EOS implementation, we take the subscription into account anyway, so that if the path starts to exist in the future, notifications will be emitted as soon as it's created. This can be handy in some scenarios.

But I've also seen many first-time users get confused because they made a mistake in the path they subscribe to (e.g. misspell an interface name) and they're confused that no notifications are streamed back to them.

tsuna avatar Nov 07 '16 22:11 tsuna

The problem is that a subscription to a currently nonexistent doesn't mean it won't exist shortly

On Nov 7, 2016 2:14 PM, "Benoit Sigoure" [email protected] wrote:

It's convenient that an Error message was added for UpdateResponse, but a bummer that SubscribeResponse didn't also get it. One common case that has come several times on my end is subscribing to a non-existent path. There is no way to convey to the client that the path didn't exist.

It's also not clear what the implementation is supposed to do in this case. In the EOS implementation, we take the subscription into account anyway, so that if the path starts to exist in the future, notifications will be emitted as soon as it's created. This can be handy in some scenarios.

But I've also seen many first-time users get confused because they made a mistake in the path they subscribe to (e.g. misspell an interface name) and they're confused that no notifications are streamed back to them.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29, or mute the thread https://github.com/notifications/unsubscribe-auth/ALKGm62n41qKhGZRx2uqeeQjs9ei7H0Nks5q76K-gaJpZM4KryVx .

marcushines-zz avatar Nov 07 '16 23:11 marcushines-zz

Sure, but

  1. I was using this as just one example. There could be other errors that we might want to return as well (for example certain paths may be unavailable via subscription, only readable via one-shot get requests, and we'd like to return an error indicating that is the case)
  2. I still think returning an error saying the path doesn't exist, even though we keep the subscription active. So far this little problem has been the number one source of confusion for people toying with this interface, because it turns out that people tyop paths a lot. :-/

tsuna avatar Nov 07 '16 23:11 tsuna

Hi Benoit,

In the specification that we're just working through the last stages on publishing (see #31), we added an error field to the SubscribeResponse message. The accompanying specification discusses the use of this field. Particularly, Section 3.5.1.3 covers exactly the case you discussed:

There is no requirement for the path that is specified within the message to exist within the current data tree on the server. Whilst the path within the subscription SHOULD be a valid path within the set of schema modules that the target supports, subscribing to any syntactically valid path within such modules MUST be allowed. In the case that a particular path does not (yet) exist, the target MUST NOT close the channel, and instead should continue to monitor for the existence of the path, and transmit telemetry updates should it exist in the future. The target MAY send a SubscribeResponse message populating the error field with NotFound (5) to inform the client that the path does not exist at the time of subscription creation

My colleagues are just reviewing this PR, it will be merged soon.

Cheers, r.

robshakir avatar Nov 08 '16 04:11 robshakir

On Mon, Nov 7, 2016 at 3:54 PM, Benoit Sigoure [email protected] wrote:

Sure, but

  1. I was using this as just one example. There could be other errors that we might want to return as well (for example certain paths may be unavailable via subscription, only readable via one-shot get requests, and we'd like to return an error indicating that is the case)

Can you provide an example of something that could not be retrieved via a subscribe?

  1. I still think returning an error saying the path doesn't exist, even though we keep the subscription active. So far this little problem has been the number one source of confusion for people toying with this interface, because it turns out that people tyop paths a lot. :-/

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259002308, or mute the thread https://github.com/notifications/unsubscribe-auth/ARfIL3NRxP-HMEXmYFVxwUTTfZHXbbIJks5q77pCgaJpZM4KryVx .

gcsl avatar Nov 08 '16 04:11 gcsl

For paths that can never be represented by the schema (i.e., they are syntactically valid but cannot be parsed via the schema), an implementation should indicate this to the subscriber, though not necessarily fail the request. For example, if container "foobar" has only 2 elements, "foo" and "bar", and a subscription is made for ".../foobar/abc", it would be sensible to indicate this path will never be matched under any circumstance with the schema used by the device. The caller would then decide if the subscription should be closed. It is easy to imagine providing a fixed set of paths to every device, even though not every device supports every path.

I think there needs to be both a "not found ever" and "not found now" message that are distinguishable by the caller.

On Mon, Nov 7, 2016 at 8:47 PM, gcsl [email protected] wrote:

On Mon, Nov 7, 2016 at 3:54 PM, Benoit Sigoure [email protected] wrote:

Sure, but

  1. I was using this as just one example. There could be other errors that we might want to return as well (for example certain paths may be unavailable via subscription, only readable via one-shot get requests, and we'd like to return an error indicating that is the case)

Can you provide an example of something that could not be retrieved via a subscribe?

  1. I still think returning an error saying the path doesn't exist, even though we keep the subscription active. So far this little problem has been the number one source of confusion for people toying with this interface, because it turns out that people tyop paths a lot. :-/

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259002308>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ARfIL3NRxP- HMEXmYFVxwUTTfZHXbbIJks5q77pCgaJpZM4KryVx>

.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259044922, or mute the thread https://github.com/notifications/unsubscribe-auth/AE4QHVgrso7Nl9hCr4EhZZ_GoJfJIDZHks5q7_7ogaJpZM4KryVx .

pborman avatar Nov 08 '16 15:11 pborman

@pborman I think this could be achieved with what we have today - using NotFound to indicate that there is a temporary reason that the path does not exist (valid parent in the tree not yet created, non-existent list key) and then InvalidArgument for a path that is never a valid schema path within a module. The latter is also aligned with the definition of InvalidArgument in the gRPC error codes where it states that InvalidArguments indicates that the arguments are invalid regardless of the state of the system.

Thoughts?

r.

robshakir avatar Nov 08 '16 15:11 robshakir

A single subscribe of at least 4 paths could have 4 different "errors" all at the same time:

  1. None - path is valid and currently known
  2. Not found - path is valid, currently not known, but might appear in the future
  3. Impossible - path is valid, but does not match the schema and will never produce a result
  4. Invalid - path is not valid

I would be fine that if any path is of case 4 that the subscription immediately fail and you only receive a notice about the first invalid path (ignoring cases 1, 2, and 3). If no paths are of case 4, I would want to receive a notice for each case 2 and case 3 path with the ability to distinguish between them and know which path causes which error.

So does InvalidArgument apply to case 3, case 4, or both?

On Tue, Nov 8, 2016 at 7:31 AM, Rob Shakir [email protected] wrote:

@pborman https://github.com/pborman I think this could be achieved with what we have today - using NotFound to indicate that there is a temporary reason that the path does not exist (valid parent in the tree not yet created, non-existent list key) and then InvalidArgument for a path that is never a valid schema path within a module. The latter is also aligned with the definition of InvalidArgument in the gRPC error codes https://godoc.org/google.golang.org/grpc/codes where it states that InvalidArguments indicates that the arguments are invalid regardless of the state of the system.

Thoughts?

r.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259168148, or mute the thread https://github.com/notifications/unsubscribe-auth/AE4QHWPJDH2nAe8nWRNjN2qt-PYxMKiWks5q8JXHgaJpZM4KryVx .

pborman avatar Nov 08 '16 18:11 pborman

You are hitting on my general objection to this error to me either the subscription is fundamentally invalid which means it errors on creation and then is closed or the subscription is successful then you just may or may not return data

On Nov 8, 2016 10:09 AM, "pborman" [email protected] wrote:

A single subscribe of at least 4 paths could have 4 different "errors" all at the same time:

  1. None - path is valid and currently known
  2. Not found - path is valid, currently not known, but might appear in the future
  3. Impossible - path is valid, but does not match the schema and will never produce a result
  4. Invalid - path is not valid

I would be fine that if any path is of case 4 that the subscription immediately fail and you only receive a notice about the first invalid path (ignoring cases 1, 2, and 3). If no paths are of case 4, I would want to receive a notice for each case 2 and case 3 path with the ability to distinguish between them and know which path causes which error.

So does InvalidArgument apply to case 3, case 4, or both?

On Tue, Nov 8, 2016 at 7:31 AM, Rob Shakir [email protected] wrote:

@pborman https://github.com/pborman I think this could be achieved with what we have today - using NotFound to indicate that there is a temporary reason that the path does not exist (valid parent in the tree not yet created, non-existent list key) and then InvalidArgument for a path that is never a valid schema path within a module. The latter is also aligned with the definition of InvalidArgument in the gRPC error codes https://godoc.org/google.golang.org/grpc/codes where it states that InvalidArguments indicates that the arguments are invalid regardless of the state of the system.

Thoughts?

r.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259168148>, or mute the thread <https://github.com/notifications/unsubscribe- auth/AE4QHWPJDH2nAe8nWRNjN2qt-PYxMKiWks5q8JXHgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259213392, or mute the thread https://github.com/notifications/unsubscribe-auth/ALKGm0EpRAi-jWsTYME-aiExjRrqathBks5q8LrJgaJpZM4KryVx .

marcushines-zz avatar Nov 08 '16 18:11 marcushines-zz

On Tue, Nov 8, 2016 at 10:35 AM, marcushines [email protected] wrote:

You are hitting on my general objection to this error to me either the subscription is fundamentally invalid which means it errors on creation and then is closed or the subscription is successful then you just may or may not return data

I want to be careful that we don't Close a session if there is any possibility that some data will be returned. I would hate to have the protocol restrict the ability to make schema changes in a non-backward compatible way. Ideally we would like to be able to subscribe to devices running different schemas using the same protocol, potentially requesting a superset of what either actually supports in the schema. Reporting an error is fine, but it should be up to the client whether the error is acceptable or not. A device should only ever terminate the subscription if there is no possible way it will ever send any (further - in the case of a once) data.

  1. What defines invalid? il-formatted character encoding? unmatched brackets?

On Nov 8, 2016 10:09 AM, "pborman" [email protected] wrote:

A single subscribe of at least 4 paths could have 4 different "errors" all at the same time:

  1. None - path is valid and currently known
  2. Not found - path is valid, currently not known, but might appear in the future
  3. Impossible - path is valid, but does not match the schema and will never produce a result
  4. Invalid - path is not valid

I would be fine that if any path is of case 4 that the subscription immediately fail and you only receive a notice about the first invalid path (ignoring cases 1, 2, and 3). If no paths are of case 4, I would want to receive a notice for each case 2 and case 3 path with the ability to distinguish between them and know which path causes which error.

So does InvalidArgument apply to case 3, case 4, or both?

On Tue, Nov 8, 2016 at 7:31 AM, Rob Shakir [email protected] wrote:

@pborman https://github.com/pborman I think this could be achieved with what we have today - using NotFound to indicate that there is a temporary reason that the path does not exist (valid parent in the tree not yet created, non-existent list key) and then InvalidArgument for a path that is never a valid schema path within a module. The latter is also aligned with the definition of InvalidArgument in the gRPC error codes https://godoc.org/google.golang.org/grpc/codes where it states that InvalidArguments indicates that the arguments are invalid regardless of the state of the system.

Thoughts?

r.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259168148>, or mute the thread <https://github.com/notifications/unsubscribe- auth/AE4QHWPJDH2nAe8nWRNjN2qt-PYxMKiWks5q8JXHgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259213392>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALKGm0EpRAi-jWsTYME- aiExjRrqathBks5q8LrJgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259220321, or mute the thread https://github.com/notifications/unsubscribe-auth/ARfIL-0hA8PwfgV0Hi0-iyNMAp9cC_zVks5q8MDYgaJpZM4KryVx .

gcsl avatar Nov 09 '16 00:11 gcsl

Invalid would include things like control characters in an element name as well as malformed brackets ("container]name=foo]" would be invalid.

It seems that there are three tests that need to be made:

  1. Is the path syntactically valid?
  2. Does the path name something possible in the schema?
  3. Is there something at the requested path?

Failure on #1 should be universal. In theory, for any given path, all implementations should answer question 1 the same.

Failure on #2 is interesting as it would help catch typos. Clearly the answer to #2 is schema dependent. I don't think failure on #2 should kill a subscription.

Failure on #3 is trying to prove the negative. The answer is implied once the sync notification comes in.

It seems rather than having subscribe handle these failures, other than #1, which should cancel the subscription, there should be a way to validate a path and get back one of:

  • Exists
  • Does not exist (yet)
  • Cannot exist
  • Invalid

A client who cares can make a call to validate the paths, getting no data back, prior to calling subscribe. Note there is a race on the first two.

On Tue, Nov 8, 2016 at 4:09 PM, gcsl [email protected] wrote:

On Tue, Nov 8, 2016 at 10:35 AM, marcushines [email protected] wrote:

You are hitting on my general objection to this error to me either the subscription is fundamentally invalid which means it errors on creation and then is closed or the subscription is successful then you just may or may not return data

I want to be careful that we don't Close a session if there is any possibility that some data will be returned. I would hate to have the protocol restrict the ability to make schema changes in a non-backward compatible way. Ideally we would like to be able to subscribe to devices running different schemas using the same protocol, potentially requesting a superset of what either actually supports in the schema. Reporting an error is fine, but it should be up to the client whether the error is acceptable or not. A device should only ever terminate the subscription if there is no possible way it will ever send any (further - in the case of a once) data.

  1. What defines invalid? il-formatted character encoding? unmatched brackets?

On Nov 8, 2016 10:09 AM, "pborman" [email protected] wrote:

A single subscribe of at least 4 paths could have 4 different "errors" all at the same time:

  1. None - path is valid and currently known
  2. Not found - path is valid, currently not known, but might appear in the future
  3. Impossible - path is valid, but does not match the schema and will never produce a result
  4. Invalid - path is not valid

I would be fine that if any path is of case 4 that the subscription immediately fail and you only receive a notice about the first invalid path (ignoring cases 1, 2, and 3). If no paths are of case 4, I would want to receive a notice for each case 2 and case 3 path with the ability to distinguish between them and know which path causes which error.

So does InvalidArgument apply to case 3, case 4, or both?

On Tue, Nov 8, 2016 at 7:31 AM, Rob Shakir [email protected] wrote:

@pborman https://github.com/pborman I think this could be achieved with what we have today - using NotFound to indicate that there is a temporary reason that the path does not exist (valid parent in the tree not yet created, non-existent list key) and then InvalidArgument for a path that is never a valid schema path within a module. The latter is also aligned with the definition of InvalidArgument in the gRPC error codes https://godoc.org/google.golang.org/grpc/codes where it states that InvalidArguments indicates that the arguments are invalid regardless of the state of the system.

Thoughts?

r.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259168148>, or mute the thread <https://github.com/notifications/unsubscribe- auth/AE4QHWPJDH2nAe8nWRNjN2qt-PYxMKiWks5q8JXHgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259213392>, or mute the thread <https://github.com/notifications/unsubscribe- auth/ALKGm0EpRAi-jWsTYME- aiExjRrqathBks5q8LrJgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259220321>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ARfIL-0hA8PwfgV0Hi0- iyNMAp9cC_zVks5q8MDYgaJpZM4KryVx> .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259299861, or mute the thread https://github.com/notifications/unsubscribe-auth/AE4QHT1a0ymerw_iwm_g8ewqwdjXgtFJks5q8Q8tgaJpZM4KryVx .

pborman avatar Nov 09 '16 01:11 pborman

On Tue, Nov 8, 2016 at 5:08 PM, pborman [email protected] wrote:

Invalid would include things like control characters in an element name as well as malformed brackets ("container]name=foo]" would be invalid.

It seems that there are three tests that need to be made:

  1. Is the path syntactically valid?
  2. Does the path name something possible in the schema?
  3. Is there something at the requested path?

Failure on #1 should be universal. In theory, for any given path, all implementations should answer question 1 the same.

Failure on #2 is interesting as it would help catch typos. Clearly the answer to #2 is schema dependent. I don't think failure on #2 should kill a subscription.

Failure on #3 is trying to prove the negative. The answer is implied once the sync notification comes in.

It seems rather than having subscribe handle these failures, other than #1, which should cancel the subscription,

This is the one point that I contend. If a subscription contains two separate paths, one that meets criteria in #1 and one that does not, I argue the subscription should not be cancelled. Having the device report the error per path allows the client to decide whether the request should be fatal.

If all paths fall into #1, then there is no data that the client might receive, and thus it is fine to close the RPC.

there should be a way to validate a path and get back one of:

  • Exists
  • Does not exist (yet)
  • Cannot exist
  • Invalid

A client who cares can make a call to validate the paths, getting no data back, prior to calling subscribe. Note there is a race on the first two.

On Tue, Nov 8, 2016 at 4:09 PM, gcsl [email protected] wrote:

On Tue, Nov 8, 2016 at 10:35 AM, marcushines [email protected] wrote:

You are hitting on my general objection to this error to me either the subscription is fundamentally invalid which means it errors on creation and then is closed or the subscription is successful then you just may or may not return data

I want to be careful that we don't Close a session if there is any possibility that some data will be returned. I would hate to have the protocol restrict the ability to make schema changes in a non-backward compatible way. Ideally we would like to be able to subscribe to devices running different schemas using the same protocol, potentially requesting a superset of what either actually supports in the schema. Reporting an error is fine, but it should be up to the client whether the error is acceptable or not. A device should only ever terminate the subscription if there is no possible way it will ever send any (further - in the case of a once) data.

  1. What defines invalid? il-formatted character encoding? unmatched brackets?

On Nov 8, 2016 10:09 AM, "pborman" [email protected] wrote:

A single subscribe of at least 4 paths could have 4 different "errors" all at the same time:

  1. None - path is valid and currently known
  2. Not found - path is valid, currently not known, but might appear in the future
  3. Impossible - path is valid, but does not match the schema and will never produce a result
  4. Invalid - path is not valid

I would be fine that if any path is of case 4 that the subscription immediately fail and you only receive a notice about the first invalid path (ignoring cases 1, 2, and 3). If no paths are of case 4, I would want to receive a notice for each case 2 and case 3 path with the ability to distinguish between them and know which path causes which error.

So does InvalidArgument apply to case 3, case 4, or both?

On Tue, Nov 8, 2016 at 7:31 AM, Rob Shakir <[email protected]

wrote:

@pborman https://github.com/pborman I think this could be achieved with what we have today - using NotFound to indicate that there is a temporary reason that the path does not exist (valid parent in the tree not yet created, non-existent list key) and then InvalidArgument for a path that is never a valid schema path within a module. The latter is also aligned with the definition of InvalidArgument in the gRPC error codes https://godoc.org/google.golang.org/grpc/codes where it states that InvalidArguments indicates that the arguments are invalid regardless of the state of the system.

Thoughts?

r.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259168148>, or mute the thread <https://github.com/notifications/unsubscribe- auth/AE4QHWPJDH2nAe8nWRNjN2qt-PYxMKiWks5q8JXHgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259213392>, or mute the thread <https://github.com/notifications/unsubscribe- auth/ALKGm0EpRAi-jWsTYME- aiExjRrqathBks5q8LrJgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259220321>, or mute the thread <https://github.com/notifications/unsubscribe- auth/ARfIL-0hA8PwfgV0Hi0- iyNMAp9cC_zVks5q8MDYgaJpZM4KryVx> .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/openconfig/reference/issues/ 29#issuecomment-259299861>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE4QHT1a0ymerw_iwm_ g8ewqwdjXgtFJks5q8Q8tgaJpZM4KryVx> .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259309101, or mute the thread https://github.com/notifications/unsubscribe-auth/ARfILyeMc7hB902XlclWtKRAbHX3ZJRhks5q8R0qgaJpZM4KryVx .

gcsl avatar Nov 09 '16 19:11 gcsl

This is the one point that I contend. If a subscription contains two separate paths, one that meets criteria in #1 and one that does not, I argue the subscription should not be cancelled. Having the device report the error per path allows the client to decide whether the request should be fatal.

The request is invalid and could never be valid at any point in time, since the request itself is malformed. The fact that only part of the request is malformed does not make a valid request. In truth, I expect this case to be extremely rare. More typical is one or more paths reference a value that will never exist, or does not yet exist. In both of those cases I agree the subscription should not be canceled (but I would like someway to ask if a path can ever be valid or not).

pborman avatar Nov 09 '16 19:11 pborman

On Wed, Nov 9, 2016 at 11:55 AM, pborman [email protected] wrote:

This is the one point that I contend. If a subscription contains two separate paths, one that meets criteria in #1 https://github.com/openconfig/reference/pull/1 and one that does not, I argue the subscription should not be cancelled. Having the device report the error per path allows the client to decide whether the request should be fatal.

The request is invalid and could never be valid at any point in time, since the request itself is malformed. The fact that only part of the request is malformed does not make a valid request. In truth, I expect this case to be extremely rare. More typical is one or more paths reference a value that will never exist, or does not yet exist. In both of those cases I agree the subscription should not be canceled (but I would like someway to ask if a path can ever be valid or not).

My worry is two-fold, first that imprecision on specifying what is valid, might cause some implementations to be too heavy handed and include what you or I might think falls into one of the other categories, or second, we might hem ourselves in going forward limiting our ability to grow the interface to accommodate additions to the schema. I think both could be dangerous.

Either we are too vague in our communication of what constitutes a catastrophic error and leave it up to individual implementations, or we today must decide very explicitly where the fences will be built that could be an incredible pain point in the future. I think having a more flexible, non-RPC terminating approach is a middle-ground that gives the power to a client to determine what is and is not catastrophic allowing the same client to talk to a wider number of implementations as they evolve over time.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259509918, or mute the thread https://github.com/notifications/unsubscribe-auth/ARfIL4snjvw_nRuUjCSJjfEdKVLQkmxmks5q8iU_gaJpZM4KryVx .

gcsl avatar Nov 09 '16 20:11 gcsl

I think you misunderstand what I mean by invalid. I mean a path that cannot be parsed. If it can be parsed it is valid even if nonsensical. This is why I enumerated 4 different possibilities for a path, only only one is invalid. Further, all implementations will recognized an invalid path as invalid. The only paths which cause the subscription to fail are paths that are syntactically invalid paths, not misspelled paths, not paths that don't exist, not paths that are just plain weird. Only paths that are syntactically invalid will cause a subscription to fail.

pborman avatar Nov 09 '16 21:11 pborman

On Wed, Nov 9, 2016 at 1:13 PM, pborman [email protected] wrote:

I think you misunderstand what I mean by invalid. I mean a path that cannot be parsed. If it can be parsed it is valid even if nonsensical. This is why I enumerated 4 different possibilities for a path, only only one is invalid. Further, all implementations will recognized an invalid path as invalid. The only paths which cause the subscription to fail are paths that are syntactically invalid paths, not misspelled paths, not paths that don't exist, not paths that are just plain weird. Only paths that are syntactically invalid will cause a subscription to fail.

:) I do not misunderstand and am not just trying to be difficult. Can you provide a precise definition in code that identifies all invalid paths? If it is difficult to provide one implementation, I argue it will be intractable to ask N vendors to each provide an implementation and have them be comparable.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/29#issuecomment-259528093, or mute the thread https://github.com/notifications/unsubscribe-auth/ARfIL6c3gryPwMtdqZSTYxZjO3JIVAx-ks5q8jdkgaJpZM4KryVx .

gcsl avatar Nov 09 '16 21:11 gcsl

I don't have the exact syntax that was decided on, so modify this as appropriate:

A path is made up of a repeated list of path elements, each path element is a string of printable characters. When representing a list, the element name may be suffixed with a series of key value pairs, for example: interface[name="eth0"]. If a [ appears in the path element, it must not be the first character and there must be a matching ]. The matching ] must either terminate the string or must be followed by a [, which in turn must have a matching ]. Any element that contains an unmatched [ or ] is invalid. The value between [ and ] must be of the form NAME="VALUE" (it is possible we ended up without quotes, I do not recall, but I think we kept them). NAME must not contain non-printable characters or the characters ", [, ] or =. VALUE must not contain an unescaped ".

The following are all invalid elements:

[foo="bar"]     // starts with a [
a[foo="bar"     // missing trailing ]
a[foo="bar"]b   // b following ]
a[foo=bar]      // missing quotes
a[foo="bar]     // unmatched quote
a[foo="ba"r]    // closing " not followed by ]
a[foo="bar"[    // second [ is invalid
a]foo="bar"]    // first [ is invalid
a=b[foo="bar"]  // = in name
"a"[foo="bar"]  // " in name

I don't recall how we quote in a value, but it should be apparent how this would affect parsing.

So, I think it should not be difficult to specify what is valid and what is not valid such that any string will be marked the same (valid or invalid) by any implementation. It is sort of like saying

double f = 1.2.3;

has a syntax error on the second ..

pborman avatar Nov 09 '16 22:11 pborman

On the path validity discussion - I started some library work in response to an internal bug that Marcus raised; let's try and get precise definitions in code therein, and we can determine how best then to ensure that there is a single implementation that we can get consistency of "syntactically broken" paths in. @pborman's list is a good start for testdata :-)

robshakir avatar Dec 02 '16 16:12 robshakir