Skip to content

Add region property to site and device metadata with automatic propogation#1332

Open
noursaidi wants to merge 1 commit into
faucetsdn:masterfrom
noursaidi:add-region-to-metadata-17728302459280877011
Open

Add region property to site and device metadata with automatic propogation#1332
noursaidi wants to merge 1 commit into
faucetsdn:masterfrom
noursaidi:add-region-to-metadata-17728302459280877011

Conversation

@noursaidi
Copy link
Copy Markdown
Collaborator

This change introduces a new region field to the UDMI metadata schemas. It is added as a top-level property in site_metadata.json and as a property within the location object in model_system.json (used for device metadata).

The Registrar has been updated to automatically propagate this region from the site metadata to any device that does not have its own region explicitly set. This ensures consistency across a site while allowing for per-device overrides.

Java classes were regenerated to include the new fields, and unit tests were added to LocalDeviceTest.java to verify that:

  1. A missing region in device metadata is correctly populated from site metadata.
  2. An existing region in device metadata is preserved and not overwritten.

PR created automatically by Jules for task 17728302459280877011 started by @noursaidi

- Update schema/site_metadata.json to include region.
- Update schema/model_system.json to include region under location.
- Regenerate Java schema classes.
- Update Registrar and LocalDevice to automatically copy region from site metadata to device metadata if unset during normalization.
- Add unit tests in LocalDeviceTest to verify the propagation logic.

Co-authored-by: noursaidi <9341216+noursaidi@users.noreply.github.com>
@noursaidi noursaidi requested review from grafnu and pisuke May 13, 2026 15:16
Copy link
Copy Markdown
Collaborator

@grafnu grafnu left a comment

Choose a reason for hiding this comment

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

I don't understand the reason for this.... ?!?!?

Comment thread schema/model_system.json
}
},
"region": {
"description": "The region according to the site model in which the device is installed in",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this needed in the device model? When is a device ever not in the region for the entire site?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It will shouldn't ever be different, but it needs to be in the device metadata it's describing the device, and we know which region the device is in (the site metadata describes the site). The use cases would be filtering both messages in pubsub where we can add this as an attribute, and also device, where we filter metadata for devices). We can do elaborate data relationship/joins, but it's better and preferable for all the data to be in the device metadata

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok, I think I understand now. My concern is that it ends up in the pre-normalized metadata too, which then makes it redundant info. Can you add a simple check (should just be a line or two) in registrar that generates a warning if it's in the pre-normalized metadata?

Also, should have a test for this in tests/sites somewhre -- doesnt' need to be new, but just add a region into one of the existing sites, so the effects of this are obvious. (Also then can add a field into a metadata.json file somewhere and see the error). Unit tests for things like this aren't always that useful since they're way too spot focused, which is why I'm looking for the site-level tests.

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.

2 participants