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

Add reflector utility #408

Merged
merged 14 commits into from
Aug 4, 2022
Merged

Add reflector utility #408

merged 14 commits into from
Aug 4, 2022

Conversation

grafnu
Copy link
Collaborator

@grafnu grafnu commented Aug 4, 2022

This adds a basic utility that can be used to inject messages into the system through the reflector. This is used by testing scripts to, for example, reset or modify a device config.

bin/reset_config Outdated
cd $ROOT

device_config=/tmp/${device_id}_config.json
cp $site_dir/devices/${device_id}/out/generated_config.json $device_config
Copy link
Collaborator

Choose a reason for hiding this comment

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

Be consistent and ${var} everywhere?

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

bin/reset_config Outdated

device_config=/tmp/${device_id}_config.json
cp $site_dir/devices/${device_id}/out/generated_config.json $device_config
now_date=$(date -Ins -u | sed -E 's/.{6}\+.*/Z/' | tr , .)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love shell manipulation but we have to worry about OSX compat (if we target that) and one needs to run it to see what it does.

We already depend on python. What do you think of this oneliner? We could throw it into bin/util_output_utcnow_8601 or whatever.

print(datetime.datetime.utcnow().isoformat())
2022-08-04T13:44:21.327511

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

bin/reset_config Outdated
cp $site_dir/devices/${device_id}/out/generated_config.json $device_config
now_date=$(date -Ins -u | sed -E 's/.{6}\+.*/Z/' | tr , .)
echo Setting config timestamp $now_date
jq .timestamp=\"$now_date\" < $device_config | sponge $device_config
Copy link
Collaborator

Choose a reason for hiding this comment

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

OSX, if we care.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Update: Just comment sponge is a Linuxism or whatever

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added runtime check and warning message, would have to fix actual OSX compat in another round (with QA)

jarfile=validator/build/libs/validator-1.0-SNAPSHOT-all.jar
mainclass=com.google.daq.mqtt.validator.Reflector

cmd="java -cp $jarfile $mainclass -p $project_id -s $site_dir -d $device_id $*"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just invoke directly and "$*" on end?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there was supposed to be another diagnostic "echo" in there... added!

}

private void reflect(String topic, String data) {
client.publish(deviceId, topic, data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this going to verify that the json input is well-formed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No -- it's not actually even strictly JSON. This was meant as a reasonably low-level utility that wasn't really opinionated about formats. Maybe future iterations could add a "validate schema" flag or something... E.g. this tool can be used to validate that a borked garbage config does not tank the system.


if [[ $# != 3 ]]; then
echo Usage: $0 site_dir project_id device_id
false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want exec false? or just exit 1?

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 avoid "exit 1" because sometimes I'll end up doing "source" on a script and then "exit 1" will totally bork the shell session... the behavior of "false" is only that if the -e flag is specified. So, in this particular case they are functionally equivalent, but I'm not sure if there's really a compelling reason for one over the other in the big picture.

bin/reset_config Outdated
cp $site_dir/devices/${device_id}/out/generated_config.json $device_config
now_date=$(date -Ins -u | sed -E 's/.{6}\+.*/Z/' | tr , .)
echo Setting config timestamp $now_date
jq .timestamp=\"$now_date\" < $device_config | sponge $device_config
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update: Just comment sponge is a Linuxism or whatever

if [[ $# < 4 ]]; then
echo Usage: $0 site_dir project_id device_id directive [directives...]
echo " Directive is something like update/config:sites/udmi_site_model/devices/AHU-1/out/generated_config.json"
false
Copy link
Collaborator

Choose a reason for hiding this comment

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

exit 1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same as other comment... not sure if there's a clear reason for one over the other because they're just "different" in how they handle different edge use cases...

@grafnu grafnu merged commit 6754f61 into faucetsdn:master Aug 4, 2022
@grafnu grafnu deleted the reflect branch August 4, 2022 23:58
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