open-horizon / examples

Code examples you can use with Horizon.
Apache License 2.0
40 stars 65 forks source link

"serviceVersionRange" in Hello World deployment.policy.json example is incorrect #492

Closed johnwalicki closed 2 years ago

johnwalicki commented 2 years ago

The https://github.com/open-horizon/examples/blob/master/edge/services/helloworld/policy/deployment.policy.json#L25 file includes an example of a deployment.policy.json with a userInput environment variable definition. It shows a serviceVersionRange key / value pair that is incorrect json. The closing ) should be a ]

"serviceVersionRange": "[0.0.0,INFINITY)",

While it is interesting to show in the example that the serviceVersionRange array can list many versions, to make it relevant to the example, it would be better to specify.

"serviceVersionRange": "[$SERVICE_VERSION]", or "serviceVersionRange": "[$SERVICE_VERSION,INFINITY]",

johnwalicki commented 2 years ago

@t-fine This PR has been open for a while. It fixes a typo in a json file. Would be great if you could review / merge it.

t-fine commented 2 years ago

Hey @johnwalicki sorry for the delay here ... completely slipped my mind after the examples working group meeting, then out last week for thanksgiving ... looking into this now!

t-fine commented 2 years ago

@johnwalicki as far as I can tell when digging into this, there should be no issue including the string "[0.0.0,INFINITY)" ... @linggao I'm going to try to test myself, but is this something you can confirm? We use this syntax in examples in several places so I just want to make sure I'm not missing something

      "serviceVersionRange": "[0.0.0,INFINITY)",

in addition to many exchange return messages on this page... https://github.ibm.com/Edge-Fabric/exchange-api/blob/11cca8b2f6c12628558ad0ea7bec6827acb0931e/doc/openapi-3.json#L1137

t-fine commented 2 years ago

@johnwalicki can you remind me, when you were attempting this initially, did it publish to the exchange with "[0.0.0,INFINITY)" and just not rollback correctly? Or was there an error message when publishing?

linggao commented 2 years ago

@t-fine @johnwalicki The correct syntax is "[0.0.0,INFINITY)". The value for serviceVersionRange is a string, not an array. It uses the semantic version syntax to define a version range. For example:

  1. [1.0.0, 2.0.0] means any version between 1.0.0 to 2.0.0 including 2.0.0 it self.
  2. [1.0.0, 2.0.0) means any version between 1.0.0 to 2.0.0 not including 2.0.0.
  3. (1.0.0, 2.0.0) means any version between 1.0.0 to 2.0.0 not including 1.0.0 and 2.0.0.
  4. [0.0.0,INFINITY) means any version.
johnwalicki commented 2 years ago

Interesting. I use semantic version syntax in several of my projects. I've never heard of "[)" So, not a bug. Suggest we close the issue and reject the PR.

I still like pinning a deployment policy to a specific "serviceVersionRange": "[$SERVICE_VERSION]" but that's just a personal preference.

t-fine commented 2 years ago

Sounds good! Closing.

clementkng commented 2 years ago

Late comment, but I think it would be good to note somewhere that we're using semantic version syntax in that way, since the "[)" looked odd to me as well.