Skip to content

Allow omitted rule constraints to match any value#101

Merged
OrangeTux merged 3 commits into
AdvancedClimateSystems:masterfrom
rgov:rzg/optional-args
Jul 27, 2020
Merged

Allow omitted rule constraints to match any value#101
OrangeTux merged 3 commits into
AdvancedClimateSystems:masterfrom
rgov:rzg/optional-args

Conversation

@rgov

@rgov rgov commented Jul 24, 2020

Copy link
Copy Markdown
Contributor

The definition of umodbus.server.route gives default values of None to the arguments, suggesting they can be omitted. However, if you actually provide None, it will fail later when evaluating DataRule.match.

This change makes it possible to omit any rule constraint to match on any value.

@coveralls

coveralls commented Jul 24, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.005%) to 95.857% when pulling 5dc1dbe on rgov:rzg/optional-args into dcf35a1 on AdvancedClimateSystems:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.005%) to 95.857% when pulling 3937df2 on rgov:rzg/optional-args into dcf35a1 on AdvancedClimateSystems:master.

@OrangeTux OrangeTux left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Again, many thanks for your work!

Comment thread dev_requirements.txt
@@ -1,5 +1,6 @@
-r requirements.txt

mock==3.0.5;python_version<"3.3"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was a bit to quick with approving the PR. I'm wondering why you added mock as development dependency. I don't see it being used in your PR.

@rgov rgov Jul 25, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You cannot run tests on Python < 3.3 without it, which blocked my adding unit tests that go with this change.

try:
# Mock has been added to stdlib in Python 3.3.
from unittest.mock import MagicMock, call
except ImportError:
from mock import MagicMock, call

In the future when you target only newer versions of Python (#86), you ought to remove the requirement and the check from the above code.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah I see. Thanks!

@OrangeTux OrangeTux self-requested a review July 25, 2020 10:23
@OrangeTux OrangeTux merged commit 63c7679 into AdvancedClimateSystems:master Jul 27, 2020
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.

3 participants