Add provisioning filters#171
Conversation
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Awesome work here! I have a few notes about code organization. Nothing here seems like a blocker for internal release
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Looks like its ready for some UTs and IT's, let me know if you need help writing them.
Please also attach a screenshot of all existing UT's and IT's passing.
|
|
| def dps_populate_essential_info(self, dps_info, device_status): | ||
| error = { | ||
| "provisioned": "None", | ||
| "registered": "Device it not yet provisioned.", |
| device_status=None, | ||
| ): | ||
| provider = CentralDeviceProvider(cmd=cmd, app_id=app_id, token=token) | ||
| provider = CentralDeviceProvider(cmd=cmd, app_id=app_id, token=token,) |
There was a problem hiding this comment.
We use an auto-formatter called "Black", that is adding this additional comma at the end
There was a problem hiding this comment.
Though in this particular case it should probably be removed
There was a problem hiding this comment.
I would not worry about this one. This is standard python black behavior, in this case adding a comma after the last kwarg element. If you remove it now (which is fine), the next change to this module and a re-run of the formatter would re-introduce it.
python black is an opinionated formatter with minimal configuration. Idea for using Black is to save time and mental energy for more important matters. By using it, you agree to cede control over minutiae of hand-formatting. Also Black will check that the reformatted code still produces a valid AST that is equivalent to the original. It's a relatively newer Python tool that has immense popularity in the open source world.
| if not device_id: | ||
| return provider.get_all_registration_info(central_dns_suffix=central_dns_suffix) | ||
| return provider.get_all_registration_info( | ||
| central_dns_suffix=central_dns_suffix, device_status=device_status |
There was a problem hiding this comment.
What pattern are we following, new line for parameter or on the same line?
There was a problem hiding this comment.
We use an auto-formatter called "Black", that tool is deciding the spacing/line breaks for us
| def dps_populate_essential_info(self, dps_info, device_status): | ||
| error = { | ||
| "provisioned": "None", | ||
| "registered": "Device it not yet provisioned.", |
There was a problem hiding this comment.
nit: The registered value string has a period at the end . It would be good to standardize on ending with a period or not.
There was a problem hiding this comment.
I'd suggest that cleaning up error messages is a P2 since I have a task in our backlog to clean up all error messages once we are ready to ship.
Let me know if this is something PR blocking and I can work with you to resolve
| ) | ||
|
|
||
| result = self.cmd( | ||
| "iot central app device registration-info --app-id {} --ds {}".format( |
There was a problem hiding this comment.
Do we have the necessary foundation in place to exercise assertions against devices with other possible device status values?
There was a problem hiding this comment.
Unfortunately we don't, iot central creating a device doesn't actually provision it so its somewhat difficult to get the device into the other states programatically during an IT.
I'll look into this as a follow up and see if theres anything we can do
There was a problem hiding this comment.
One option is to create an app with existing devices in different states and then adding validations. IT will take dependency on external environment. Will explore other options as well.
digimaun
left a comment
digimaun
left a comment
There was a problem hiding this comment.
Merging with the assumption @prbans will follow up at some point with the error message clean-up backlog task.
ghost
deleted the
* Int test updates and documentation * Updated azure login to v2 * Updated integration test documentation * Refactored variables to consolidate references * Only run tox int tests on default init feature set
3 participants
This project has adopted the Microsoft Open Source Code of Conduct. For more information see the Code of Conduct FAQ or contact opencode@microsoft.com with any additional questions or comments.
Thank you for contributing to the IoT extension!
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
pytest <project root> -vv