feat(metadata): implement Unit of Measure (UoM) ADR #4119

chr1shung · 2022-08-08T08:46:34Z

Signed-off-by: Chris Hung chris@iotechsys.com

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
I am not introducing a new dependency (add notes below if you are)
I have added unit tests for the new feature or bug fix (if not, why?)
I have fully tested (add details below) this the new feature or bug fix (if not, why?)
I have opened a PR for the related docs change (if not, why?)
feat: document core metadata units of measure capability edgex-docs#834

Testing Instructions

Run metadata from this branch with WRITABLE_UOM_VALIDATION: "true"
add/update device porfile and compare the result with res/uom.toml

Also,
I updated the sample uom TOML in ADR from

[Uom]
    Source="reference to source for all UoM if not specified below"
    [Uom.temperature]
    Source="www.weather.com"
    Values="C,F,K"
    [Uom.weights]
    Source="www.usa.gov/federal-agencies/weights-and-measures-division"
    Values="lbs,ounces,kilos,grams"

to

[Uom]
Source="reference to source for all UoM if not specified below"
  [Uom.Units]
    [Uom.Units.temperature]
    Source="www.weather.com"
    Values=["C","F","K"]
    [Uom.Units.weights]
    Source="www.usa.gov/federal-agencies/weights-and-measures-division"
    Values=["lbs","ounces","kilos","grams"]

so that it can be correctly unmarshaled

New Dependency Instructions (If applicable)

cloudxxx8 · 2022-08-10T01:37:14Z

@hahattan we merged a PR which should resolved the snap test issue. please rebase it.

lenny-goodell · 2022-08-10T15:47:12Z

internal/core/metadata/application/deviceprofile.go

+	if container.ConfigurationFrom(dic.Get).Writable.UoM.Validation {
+		uom := container.UnitsOfMeasureFrom(dic.Get)
+		for _, dr := range d.DeviceResources {
+			if ok := uom.Validate(dr.Properties.Units); !ok {
+				return errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("DeviceResource %s units %s is invalid", dr.Name, dr.Properties.Units), nil)
+			}
+		}
+	}


Create helper function to reduce duplicated code

internal/core/metadata/uom/bootstrap.go

snap/snapcraft.yaml

lenny-goodell

@hahattan , please complete PR check list with link to PR for the Doc updates for this new capability. THX!

internal/core/metadata/application/deviceprofile.go

internal/core/metadata/uom/bootstrap.go

internal/core/metadata/uom/uom.go

snap/snapcraft.yaml

Signed-off-by: Chris Hung <chris@iotechsys.com>

lenny-goodell

LGTM

lenny-goodell · 2022-08-11T23:32:18Z

@hahattan , FYI I have escalated the snap testing failure to @farshidtz

chr1shung · 2022-08-11T23:51:23Z

document PR updated: edgexfoundry/edgex-docs#834

cloudxxx8

LGTM

farshidtz · 2022-08-12T09:19:24Z

snap/snapcraft.yaml

+          "core-metadata")
+              install -DT "./cmd/core-metadata/res/configuration.toml" "$SNAPCRAFT_PART_INSTALL/config/core-metadata/res/configuration.toml"
+              install -DT "./cmd/core-metadata/res/uom.toml" "$SNAPCRAFT_PART_INSTALL/config/core-metadata/res/uom.toml"
+          ;;


This is good as it copies also uom.toml to the snap package (readonly, SNAP env var). But we also need to add code to copy it over to the snap's writable area (SNAP_DATA env var) on install time. The given path UOM_UOMFILE: $SNAP_DATA/config/core-metadata/res/uom.toml expects it to be under the writable area.

Currently only configuration.toml files get copied:

edgex-go/snap/local/hooks/cmd/install/install.go

Lines 118 to 143 in e3ca38a

destDir := hooks.SnapDataConf + "/"

srcDir := hooks.SnapConf + "/"

// handle exceptions (i.e. config in non-std dirs)

if v == "security-bootstrap-redis" {

destDir = destDir + "security-bootstrapper/res-bootstrap-redis"

srcDir = srcDir + "security-bootstrapper/res-bootstrap-redis"

} else if v == "app-service-configurable" {

destDir = destDir + v + "/res/rules-engine"

srcDir = srcDir + "/res/rules-engine"

} else {

destDir = destDir + v + "/res"

srcDir = srcDir + v + "/res"

}

if err = os.MkdirAll(destDir, 0755); err != nil {

return err

}

srcPath := srcDir + "/configuration.toml"

destPath := destDir + "/configuration.toml"

err = hooks.CopyFile(srcPath, destPath)

if err != nil {

return err

}

This is really an unnecessary complication in how the snap is packaged. Maybe we should refactor and simply copy the whole res directory.

For now, a quick fix would be to update the referenced code to copy uom.toml from SNAP to SNAP_DATA for core-metadata. I'll work on it and commit to the same PR (since the permission has been granted by the author during PR submission.)

sorry I wasn't aware you're working on it.
I've added a quick fix: https://github.com/edgexfoundry/edgex-go/compare/4c4951218dd81a254464f99d357ca8617bed2473..3aaec565c9b77e4c31d71bd8720703868403a2fd

Thanks for the fix. I was wondering why I couldn't push. I was testing locally so it took some time.

You force-pushed so it isn't clear what exactly you changed. But the install.go part looks good.

Signed-off-by: Chris Hung <chris@iotechsys.com>

sonarqubecloud · 2022-08-12T09:33:23Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

lenny-goodell requested changes Aug 10, 2022

View reviewed changes

chr1shung requested a review from lenny-goodell August 11, 2022 05:47

lenny-goodell requested changes Aug 11, 2022

View reviewed changes

internal/core/metadata/application/deviceprofile.go Outdated Show resolved Hide resolved

internal/core/metadata/uom/bootstrap.go Outdated Show resolved Hide resolved

internal/core/metadata/uom/uom.go Show resolved Hide resolved

snap/snapcraft.yaml Show resolved Hide resolved

feat(metadata): implement Unit of Measure (UoM) ADR

d9427cb

Signed-off-by: Chris Hung <chris@iotechsys.com>

lenny-goodell previously approved these changes Aug 11, 2022

View reviewed changes

lenny-goodell mentioned this pull request Aug 11, 2022

build: Add convenient alias for each services build and docker target #4123

Merged

5 tasks

cloudxxx8 previously approved these changes Aug 12, 2022

View reviewed changes

farshidtz requested changes Aug 12, 2022

View reviewed changes

fix(snap): fix PR snap test error

3aaec56

Signed-off-by: Chris Hung <chris@iotechsys.com>

chr1shung dismissed stale reviews from cloudxxx8 and lenny-goodell via 3aaec56 August 12, 2022 09:32

farshidtz approved these changes Aug 12, 2022

View reviewed changes

cloudxxx8 merged commit 03487ec into edgexfoundry:main Aug 12, 2022

farshidtz mentioned this pull request Aug 15, 2022

fix(snap): Install Unit of Measure config file on upgrade #4125

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(metadata): implement Unit of Measure (UoM) ADR #4119

feat(metadata): implement Unit of Measure (UoM) ADR #4119

chr1shung commented Aug 8, 2022 •

edited

Loading

cloudxxx8 commented Aug 10, 2022

lenny-goodell Aug 10, 2022

lenny-goodell left a comment •

edited

Loading

lenny-goodell left a comment

lenny-goodell commented Aug 11, 2022

chr1shung commented Aug 11, 2022

cloudxxx8 left a comment

farshidtz Aug 12, 2022

chr1shung Aug 12, 2022

farshidtz Aug 12, 2022

sonarqubecloud bot commented Aug 12, 2022

	destDir := hooks.SnapDataConf + "/"
	srcDir := hooks.SnapConf + "/"

	// handle exceptions (i.e. config in non-std dirs)
	if v == "security-bootstrap-redis" {
	destDir = destDir + "security-bootstrapper/res-bootstrap-redis"
	srcDir = srcDir + "security-bootstrapper/res-bootstrap-redis"
	} else if v == "app-service-configurable" {
	destDir = destDir + v + "/res/rules-engine"
	srcDir = srcDir + "/res/rules-engine"
	} else {
	destDir = destDir + v + "/res"
	srcDir = srcDir + v + "/res"
	}

	if err = os.MkdirAll(destDir, 0755); err != nil {
	return err
	}

	srcPath := srcDir + "/configuration.toml"
	destPath := destDir + "/configuration.toml"

	err = hooks.CopyFile(srcPath, destPath)
	if err != nil {
	return err
	}

feat(metadata): implement Unit of Measure (UoM) ADR #4119

feat(metadata): implement Unit of Measure (UoM) ADR #4119

Conversation

chr1shung commented Aug 8, 2022 • edited Loading

PR Checklist

Testing Instructions

New Dependency Instructions (If applicable)

cloudxxx8 commented Aug 10, 2022

lenny-goodell Aug 10, 2022

Choose a reason for hiding this comment

lenny-goodell left a comment • edited Loading

Choose a reason for hiding this comment

lenny-goodell left a comment

Choose a reason for hiding this comment

lenny-goodell commented Aug 11, 2022

chr1shung commented Aug 11, 2022

cloudxxx8 left a comment

Choose a reason for hiding this comment

farshidtz Aug 12, 2022

Choose a reason for hiding this comment

chr1shung Aug 12, 2022

Choose a reason for hiding this comment

farshidtz Aug 12, 2022

Choose a reason for hiding this comment

sonarqubecloud bot commented Aug 12, 2022

chr1shung commented Aug 8, 2022 •

edited

Loading

lenny-goodell left a comment •

edited

Loading