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

Implement the new monitor-events CLI for Py35+ users #236

Merged
merged 25 commits into from
Aug 7, 2018

Conversation

jongio
Copy link
Member

@jongio jongio commented Aug 7, 2018

No description provided.

jongio and others added 25 commits April 18, 2018 16:15
Add quickstart video
* add contributing.md and modify readme to point to wiki

* update readme.md

* remove overview section and move resources to bottom

* add overview section with link to wiki at top
@@ -35,6 +35,7 @@ def run(self):
'python-dotenv',
'requests',
'fstrings',
'msrestazure~=0.4.32',
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed for py2 due to an issue with Azure/azure-cli#6973

@jongio jongio merged commit 875bf1c into Azure:master Aug 7, 2018
@jongio jongio deleted the monitor-events branch August 7, 2018 01:35
@@ -304,7 +304,7 @@ def stop_simulator():
help="Specify number of milliseconds to monitor for messages")
def monitor(timeout):
utility = Utility(envvars, output)
ih = IoTHub(envvars, utility, output)
ih = IoTHub(envvars, utility, output, azure_cli)
ih.monitor_events(timeout)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the unit of timeout? Is it still milliseconds? The behavior seems to be inconsistent with Python 2 and Python 3.6. With Python 3.6, I have specified 10 as timeout, but the monitor session ran several minutes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I found out the cause of the inconsistency. The timeout option for iothub-explorer monitor-events specifies "Exit after the given number of seconds". In comparison, the timeout option for az iot hub monitor-events specifies "Maximum seconds to maintain connection without receiving message". Since my Edge device is continuously sending messages to IoT Hub, when monitoring on Python 3.6, the monitor session will never stop.

Copy link
Member

Choose a reason for hiding this comment

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

Yes the extension timeout is in seconds and @LazarusX's comment is right on the timeout behavior for monitor-events

Copy link
Member Author

Choose a reason for hiding this comment

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

@digimaun - I'm trying to add a timeout to process.communicate to force kill the monitor-events process. Can you review this commit and LMK why you think it is not actually killing the process. Also, can you recommend other ways to get this behavior? Thanks!

jongio@ab80d0d

Copy link
Member

@digimaun digimaun Aug 9, 2018

Choose a reason for hiding this comment

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

I sent a PR to your personal repo on the same processtimeout branch. Also please read note about sys.excepthook errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thx. You are a master!

Copy link
Contributor

Choose a reason for hiding this comment

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

@jongio @digimaun Do you plan to merge the processtimeout branch to Azure/iotedgedev:master branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

It didn’t fully work yet. paymaun will have a look this week to figure out why tod is failing. Please feel free to try if you have time. You can pull fork

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.

None yet

5 participants