Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restart schema and capability #411

Merged
merged 33 commits into from
Aug 8, 2022
Merged

Restart schema and capability #411

merged 33 commits into from
Aug 8, 2022

Conversation

grafnu
Copy link
Collaborator

@grafnu grafnu commented Aug 8, 2022

No description provided.

@grafnu grafnu requested a review from noursaidi August 8, 2022 14:01
@@ -3,17 +3,17 @@
"blobset": {
"blobs": {
"arbitrary_manufacturer_id": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Believe I deleted this file and moved the test to more specific filename blobset...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are still in the master branch of faucetsdn -- https://github.com/faucetsdn/udmi/blob/master/tests/config.tests/applied.json

I can delete them, but they haven't been deleted as of yet.

@@ -3,17 +3,17 @@
"blobset": {
"blobs": {
"arbitrary_manufacturer_id": {
"phase": "applied",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but now rename the file too

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you mean by "rename the file" -- so might need to wait until you submit the cleanup PR before any of this will make sense.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phase is now final not applied so

mv blobset_applied.json blobset_final.json

@@ -3,7 +3,7 @@
"blobset": {
"blobs": {
"_firmware_update": {
"phase": "applied",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

"title": "System Blobsets",
"description": "Predefined system blobsets",
"enum": [
"_iot_config"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we can define _firmware_update etc in this enum, reference it with constants, but we aren't using it as a key filter (instead we have open regex on config_blobset.json:15 ) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enums are things that end up in constants in the code -- but I don't know how to make the schema match "this should be an enum or not start with _" -- There is theoretically a "propertyNames" field in the schema spec, but I can't get it to actually do anything so it's either not really implemented, or implemented in a way I don't expect!

pubber/bin/run Outdated

echo $! > $ROOT_DIR/.pubber.pid
while [[ $result == 5 ]]; do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-eq

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

pubber/bin/run Outdated
wait
PUB_PID=$!

echo $PUB_PID > $ROOT_DIR/.pubber.pid
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are we reading this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

publishStateMessage();
System.err.println("Restarting system with extreme prejudice.");
System.err.flush();
System.exit(RESTART_EXIT_CODE);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking -- JVM doesn't reserve any exit status for its own operation, does it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear -- the docs out there are pretty sketchy. Seems like people suggest that it follows the standard BASH rules, which means <128 are internal, 128 + 32 are for Unix signals.... I'll map to 192 and call it a day!

wait $PUB_PID || result=$?
done

echo Done with pubber run, exit code $result
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If wait fails we won't know it versus the process exiting with status 0. Do you care?
If so you could assign wait $PUB_PID; result=$? instead of || and get either wait failure 127 or the exit status.

@@ -3,17 +3,17 @@
"blobset": {
"blobs": {
"arbitrary_manufacturer_id": {
"phase": "applied",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phase is now final not applied so

mv blobset_applied.json blobset_final.json

@johnrandolph johnrandolph self-requested a review August 8, 2022 20:00
@grafnu grafnu merged commit f5d2786 into faucetsdn:master Aug 8, 2022
@grafnu grafnu deleted the restart branch August 8, 2022 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants