aws / aws-iot-device-sdk-python-v2

Next generation AWS IoT Client SDK for Python using the AWS Common Runtime
Apache License 2.0
408 stars 213 forks source link

Release 1.2.0 and 1.2.1 not backward compatible with 1.1.0 #79

Closed Th3G4mbl3r closed 4 years ago

Th3G4mbl3r commented 4 years ago

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug I've code that works perfectly with SDK version 1.1.0 to manage shadows and send state update requests to the shadow. However, when i upgrade the awsiotsdk to 1.2.0 or 1.2.1, then the program breaks in trying to get the current shadow state with following error:

Requesting current shadow state... Exiting sample due to exception. Traceback (most recent call last): File "valve_code/e2e_valve.py", line 291, in publish_get_future = shadow_client.publish_get_shadow( File "/Users/rohitus/Documents/Projects/mb3-ocatank-iot-build/iot-device-autoregistration/.venv/lib/python3.8/site-packages/awsiot/iotshadow.py", line 104, in publish_get_shadow raise ValueError("request.thing_name is required") ValueError: request.thing_name is required

SDK version number Using awsiotsdk 1.1.0

Platform/OS/Device What are you running the sdk on? MacOS Catalina 10.15.5 and Python 3.8

To Reproduce (observed behavior) Steps to reproduce the behavior (please share code)

        print("Requesting current shadow state...")
        publish_get_future = shadow_client.publish_get_shadow(
            request=iotshadow.GetShadowRequest(thing_name),
            qos=mqtt.QoS.AT_LEAST_ONCE)

Expected behavior Successfully able to return from publish_get_shadow call with an object that can be used to read the current shadow value by making the call publish_get_future.result()

Logs/output

Traceback (most recent call last):
  File "valve_code/e2e_valve.py", line 291, in <module>
    publish_get_future = shadow_client.publish_get_shadow(
  File "/Users/rohitus/Documents/Projects/mb3-ocatank-iot-build/iot-device-autoregistration/.venv/lib/python3.8/site-packages/awsiot/iotshadow.py", line 104, in publish_get_shadow
    raise ValueError("request.thing_name is required")
ValueError: request.thing_name is required
bretambrose commented 4 years ago

thing_name has been a required parameter from the start and the code has checked its presence since the start:

https://github.com/aws/aws-iot-device-sdk-python-v2/blame/master/awsiot/iotshadow.py#L103

Your value for thing_name looks like its now null. How are you populating it?

Th3G4mbl3r commented 4 years ago

thing_name has been a required parameter from the start and the code has checked its presence since the start:

https://github.com/aws/aws-iot-device-sdk-python-v2/blame/master/awsiot/iotshadow.py#L103

Your value for thing_name looks like its now null. How are you populating it?

As i've mentioned the exact same program code works completely fine with sdk version 1.1.0. So the value of thing_name is not null and actually correctly populated.

bretambrose commented 4 years ago

I understand, but the error you posted in the original description comes from that null check. Perhaps the root cause of the issue is occurring earlier and that's what I'm trying to figure out. Is there any information you can give me as to how that parameter is initialized?

Th3G4mbl3r commented 4 years ago

I understand, but the error you posted in the original description comes from that null check. Perhaps the root cause of the issue is occurring earlier and that's what I'm trying to figure out. Is there any information you can give me as to how that parameter is initialized?

I am reading the name from the command line. The command line i am using is as follows:


poetry run python valve_code/e2e_valve.py -i pressure_valve_1 -c 10 -A ./CA/AmazonRootCA1.pem -d ./CA/private/pressure_valve_1.key -D ./CA/certs/pressure_valve_1_ca_cert.crt -e akom112at41zv-ats.iot.us-west-2.amazonaws.com

The actual code base that parses the argument parameters and sets up everything is as follows:


if __name__ == '__main__':
    # Process input args
    args = parser.parse_args()
    thing_name = args.id
    io.init_logging(getattr(io.LogLevel, args.verbosity), 'stderr')

    # Spin up resources
    event_loop_group = io.EventLoopGroup(1)
    host_resolver = io.DefaultHostResolver(event_loop_group)
    client_bootstrap = io.ClientBootstrap(event_loop_group, host_resolver)

    mqtt_connection = mqtt_connection_builder.mtls_from_path(
            endpoint=args.endpoint,
            cert_filepath=args.device_cert,
            pri_key_filepath=args.device_key,
            client_bootstrap=client_bootstrap,
            ca_filepath=args.amazon_ca_cert,
            client_id=args.id,
            clean_session=False,
            keep_alive_secs=6)

    print("Connecting to {} with client ID '{}'...".format(
        args.endpoint, args.id))

    connected_future = mqtt_connection.connect()

    shadow_client = iotshadow.IotShadowClient(mqtt_connection)

    # Wait for connection to be fully established.
    # Note that it's not necessary to wait, commands issued to the
    # mqtt_connection before its fully connected will simply be queued.
    # But this sample waits here so it's obvious when a connection
    # fails or succeeds.
    connected_future.result()
    print("Connected!")

    try:
        # Subscribe to necessary topics.
        # Note that is **is** important to wait for "accepted/rejected" subscriptions
        # to succeed before publishing the corresponding "request".
        print("Subscribing to Delta events...")
        delta_subscribed_future, _ = shadow_client.subscribe_to_shadow_delta_updated_events(
            request=iotshadow.ShadowDeltaUpdatedSubscriptionRequest(thing_name),
            qos=mqtt.QoS.AT_LEAST_ONCE,
            callback=on_shadow_delta_updated)

        # Wait for subscription to succeed
        delta_subscribed_future.result()

        print("Subscribing to Update responses...")
        update_accepted_subscribed_future, _ = shadow_client.subscribe_to_update_shadow_accepted(
            request=iotshadow.UpdateShadowSubscriptionRequest(thing_name),
            qos=mqtt.QoS.AT_LEAST_ONCE,
            callback=on_update_shadow_accepted)

        update_rejected_subscribed_future, _ = shadow_client.subscribe_to_update_shadow_rejected(
            request=iotshadow.UpdateShadowSubscriptionRequest(thing_name),
            qos=mqtt.QoS.AT_LEAST_ONCE,
            callback=on_update_shadow_rejected)

        # Wait for subscriptions to succeed
        update_accepted_subscribed_future.result()
        update_rejected_subscribed_future.result()

        print("Subscribing to Get responses...")
        get_accepted_subscribed_future, _ = shadow_client.subscribe_to_get_shadow_accepted(
            request=iotshadow.GetShadowSubscriptionRequest(thing_name),
            qos=mqtt.QoS.AT_LEAST_ONCE,
            callback=on_get_shadow_accepted)

        get_rejected_subscribed_future, _ = shadow_client.subscribe_to_get_shadow_rejected(
            request=iotshadow.GetShadowSubscriptionRequest(thing_name),
            qos=mqtt.QoS.AT_LEAST_ONCE,
            callback=on_get_shadow_rejected)

        # Wait for subscriptions to succeed
        get_accepted_subscribed_future.result()
        get_rejected_subscribed_future.result()

        # The rest of the sample runs asyncronously.

        # Issue request for shadow's current state.
        # The response will be received by the on_get_accepted() callback
        print("Requesting current shadow state...")
        publish_get_future = shadow_client.publish_get_shadow(
            request=iotshadow.GetShadowRequest(thing_name),
            qos=mqtt.QoS.AT_LEAST_ONCE)

        # Ensure that publish succeeds
        publish_get_future.result()

        # Launch thread to handle user input.
        # A "daemon" thread won't prevent the program from shutting down.
        print("Launching thread to read user input...")
        user_input_thread = threading.Thread(target=user_input_thread_fn, name='user_input_thread')
        user_input_thread.daemon = True
        user_input_thread.start()

    except Exception as e:
        exit(e)

Also, as i mentioned this exact same code file without any code changes works in SDK version 1.1.0 under the same python environment. So if something needs to be changed to accommodate 1.2.0 or 1.2.1, then i would consider that as a breaking change or backward incompatible change. Let me know if you want me to attach the whole python program here as well.

bretambrose commented 4 years ago

I'll be looking into this as my next priority.

Th3G4mbl3r commented 4 years ago

I'll be looking into this as my next priority.

cool, thanks. Let me know if you need me to do anything to help with the debugging process.

bretambrose commented 4 years ago

Is there any other major difference between this and the shadow sample other than the removal of websockets as a connection option? Without the whole file, it looks like it would be easier for me to start from the shadow sample than reconstruct all the missing code.

bretambrose commented 4 years ago

After looking at it, this is indeed a serious breaking change. We're currently evaluating possible options for addressing it.

Th3G4mbl3r commented 4 years ago

Is there any other major difference between this and the shadow sample other than the removal of websockets as a connection option? Without the whole file, it looks like it would be easier for me to start from the shadow sample than reconstruct all the missing code.

hi bret,

no, the differences are not significant from the use of SDK perspective. So yes, you can test using that.

thanks & regards, rohit

Th3G4mbl3r commented 4 years ago

After looking at it, this is indeed a serious breaking change. We're currently evaluating possible options for addressing it.

oh ok... i'll wait for the revised version.

JonathanHenson commented 4 years ago

This was accidental, and we’ll be producing a correction or error report and posting for you here. We’re very sorry we made this mistake and will be implementing measures to make sure it doesn’t happen again.

Th3G4mbl3r commented 4 years ago

This was accidental, and we’ll be producing a correction or error report and posting for you here. We’re very sorry we made this mistake and will be implementing measures to make sure it doesn’t happen again.

hi Jonathan,

thanks for the update. Mistakes can happen, so that's fine. looking forward to receiving the updated fixes.

thanks & regards, rohit

bretambrose commented 4 years ago

The newly-released 1.3.0 version of the SDK should fix this incompatibility as well as prevent anything like it from happening again. Again, we apologize for this mistake.

If you are interested in more details, the release notes for 1.3.0 (https://github.com/aws/aws-iot-device-sdk-python-v2/releases/tag/v1.3.0) contain more information about what happened and how we decided to fix it. Thank you for your patience and investigation into the issue.