fix: aligns base Device type with commonalities #70
Conversation
caubut-charter
requested review from
benhepworth,
clundie-CL and
mayur007
as code owners
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports MegaLinter is graciously provided by |
caubut-charter
force-pushed
the
46-base-device-type-off-commonalities
branch
5 times, most recently
from
58d3782 to
360e755
Compare
caubut-charter
force-pushed
the
46-base-device-type-off-commonalities
branch
from
360e755 to
c62ec93
Compare
| BaseDevice: | ||
| description: | | ||
| End-user equipment able to connect to a mobile network. Examples of devices include smartphones or IoT sensors/actuators. | ||
| The developer can choose to provide the below specified device identifiers: | ||
| * `ipv4Address` | ||
| * `ipv6Address` | ||
| * `phoneNumber` | ||
| * `networkAccessIdentifier` | ||
| NOTE1: the network operator might support only a subset of these options. The API invoker can provide multiple identifiers to be compatible across different network operators. In this case the identifiers MUST belong to the same device. | ||
| NOTE2: as for this Commonalities release, we are enforcing that the networkAccessIdentifier is only part of the schema for future-proofing, and CAMARA does not currently allow its use. After the CAMARA meta-release work is concluded and the relevant issues are resolved, its use will need to be explicitly documented in the guidelines. | ||
| type: object | ||
| properties: | ||
| phoneNumber: | ||
| $ref: "#/components/schemas/PhoneNumber" | ||
| networkAccessIdentifier: | ||
| $ref: "#/components/schemas/NetworkAccessIdentifier" | ||
| ipv4Address: | ||
| $ref: "#/components/schemas/DeviceIpv4Addr" | ||
| ipv6Address: | ||
| $ref: "#/components/schemas/DeviceIpv6Address" | ||
| # minProperties: 1 # NOTE: not used currently |
There was a problem hiding this comment.
Does it make sense to have something like this attached to BaseDevice:
example: &base-device-model
phoneNumber: "+123456789"
networkAccessIdentifier: "123456789@example.com"
ipv4Address:
publicAddress: "84.125.93.10"
publicPort: 59765
ipv6Address: "2001:db8:85a3:8d3:1319:8a2e:370:7344"
in order to update examples like this:
BaseDevice:
summary: Base Device
description: |
Output with the following scope(s):
- **one of**
- `network-access-management:devices:read`
- `network-access-management:devices:write`
value: &base-device
- *base-device-model
- id: *device-id
name: *device-name
description: *device-description
clundie-CL
left a comment
clundie-CL
left a comment
There was a problem hiding this comment.
Overall LGTM, the only thing I noticed is that there's possibly opportunity to update how the rendered examples display the new BaseDevice schema fields, but wasn't convinced that was critical to hold up the PR.
|
@clundie-CL BaseDevice is just Device from commonalities. As it changes I just copy-paste over the last version. I try to minimize customization and do all that in the models that inherit from it. In this case I had to change the name due to the name clash, but otherwise its 1-to-1 with commonalities. BaseDevice shouldn't be seen in the wild. Rather it should be looked at like a abstract class or mix-in of sorts. Since it is just a building block to concrete models, I'm good skipping the example for it to make it easier to cut and paste updates. We aren't using any of the new fields either, this was just a commonalities alignment that those fields exist but are not used. In the future, we should drive anything we need that may not be unique to NAM into commonalities. This update is a baby step in that direction. |
What type of PR is this?
What this PR does / why we need it:
Needed to align with commonalities.
Which issue(s) this PR fixes:
Fixes #46
Does this PR introduce a breaking change?
Special notes for reviewers:
Extra fields are not really used at this stage.
Changelog input
Additional documentation