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

IoT Data: delta calculation when desired state adds a new key #8408

Closed
rtandy opened this issue Dec 16, 2024 · 1 comment · Fixed by #8423
Closed

IoT Data: delta calculation when desired state adds a new key #8408

rtandy opened this issue Dec 16, 2024 · 1 comment · Fixed by #8423
Labels

Comments

@rtandy
Copy link

rtandy commented Dec 16, 2024

Hello, thank you very much for working on the IoT Data and shadow emulation. Really appreciate that you are fixing & improving things in this area.

After updating to 5.0.23, I noticed some of my tests failing, I assume due to #8342. On closer inspection, I guess the behaviour wasn't quite correct before either, but fails more loudly now. :) Basically, delta is not emitted correctly when a key is present in desired and absent in reported.

Test script:

import json
import os

import boto3
import moto

os.environ.pop('AWS_PROFILE', None)
os.environ['AWS_ACCESS_KEY_ID'] = 'testing'
os.environ['AWS_SECRET_ACCESS_KEY'] = 'testing'
os.environ['AWS_SESSION_TOKEN'] = 'testing'
os.environ['AWS_SECURITY_TOKEN'] = 'testing'
os.environ['AWS_DEFAULT_REGION'] = 'ca-central-1'


with moto.mock_aws():
    session = boto3.Session()
    iot = session.client('iot')
    iot_data = session.client('iot-data')

    thing_name = 'test'

    iot.create_thing(thingName=thing_name)

    payload = {
        'state': {
            'desired': {
                'new_key': True,
                'existing_key': True
            },
            'reported': {
                'existing_key': True
            }
        }
    }

    iot_data.update_thing_shadow(thingName=thing_name, payload=json.dumps(payload))

    response = iot_data.get_thing_shadow(thingName=thing_name)
    state = json.load(response['payload'])['state']

    print(json.dumps(state, indent=2))

Expected output:

{
  "desired": {
    "new_key": true
  },
  "reported": {
    "existing_key": true
  },
  "delta": {
    "new_key": true
  }
}

Output with 5.0.21 (missing delta):

{
  "desired": {
    "new_key": true
  },
  "reported": {
    "existing_key": true
  }
}

Output with 5.0.22 and 5.0.23:

Traceback (most recent call last):
  File "test.py", line 37, in <module>
    response = iot_data.get_thing_shadow(thingName=thing_name)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[...]
TypeError: keys must be str, int, float, bool or None, not Symbol

In terms of your test suite, I guess the test cases might be something like this (completely untested):

        (
            {"desired": {"enabled": True}},
            {"desired": {"enabled": True}, "delta": {"enabled": True}},
            {"reported": {}},
            {"desired": {"enabled": True}, "delta": {"enabled": True}}
        ),
        (
            {"desired": {"enabled": True}},
            {"desired": {"enabled": True}, "delta": {"enabled": True}},
            {"reported": {"online": True}},
            {"desired": {"enabled": True}, "reported": {"online": True}, "delta": {"enabled": True}}
        )

Thank you!

@bblommers
Copy link
Collaborator

Really appreciate that you are fixing & improving things in this area.

Appreciate the kind words @rtandy! Although I'd argue that introducing TypeError's isn't quite an improvement.. 🙂

The examples that you've provided pass against AWS, so I've added them to our test and opened a PR to simplify the calculation. This can actually be done without the jsondiff dependency, so at least it doesn't blow up anymore, even if we're still missing any edge cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants