Stix2.1 wd05 #283
Stix2.1 wd05 #283
Conversation
includes SDO/SRO class updates, but no unit test updates. The class updates broke unit tests, so that still needs to be addressed.
possible library bugs, before I fix the remaining unit tests.
- Removed all plain python base classes (e.g. ValueError, TypeError) - Renamed InvalidPropertyConfigurationError -> PropertyPresenceError, since incorrect values could be considered a property config error, and I really just wanted this class to apply to presence (co-)constraint violations. - Added ObjectConfigurationError as a superclass of InvalidValueError, PropertyPresenceError, and any other exception that could be raised during _STIXBase object init, which is when the spec compliance checks happen. This class is intended to represent general spec violations. - Did some class reordering in exceptions.py, so all the ObjectConfigurationError subclasses were together. Changed how property "cleaning" errors were handled: - Previous docs said they should all be ValueErrors, but that would require extra exception check-and-replace complexity in the property implementations, so that requirement is removed. Doc is changed to just say that cleaning problems should cause exceptions to be raised. _STIXBase._check_property() now handles most exception types, not just ValueError. - Decided to try chaining the original clean error to the InvalidValueError, in case the extra diagnostics would be helpful in the future. This is done via 'six' adapter function and only works on python3. - A small amount of testing was removed, since it was looking at custom exception properties which became unavailable once the exception was replaced with InvalidValueError. Did another pass through unit tests to fix breakage caused by the changed exception class hierarchy. Removed unnecessary observable extension handling code from parse_observable(), since it was all duplicated in ExtensionsProperty. The redundant code in parse_observable() had different exception behavior than ExtensionsProperty, which makes the API inconsistent and unit tests more complicated. (Problems in ExtensionsProperty get replaced with InvalidValueError, but extensions problems handled directly in parse_observable() don't get the same replacement, and so the exception type is different.) Redid the workbench monkeypatching. The old way was impossible to make work, and had caused ugly ripple effect hackage in other parts of the codebase. Now, it replaces the global object maps with factory functions which behave the same way when called, as real classes. Had to fix up a few unit tests to get them all passing with this monkeypatching in place. Also remove all the xfail markings in the workbench test suite, since all tests now pass. Since workbench monkeypatching isn't currently affecting any unit tests, tox.ini was simplified to remove the special-casing for running the workbench tests. Removed the v20 workbench test suite, since the workbench currently only works with the latest stix object version.
constraint checking.
Codecov Report
@@ Coverage Diff @@
## stix2.1 #283 +/- ##
===========================================
+ Coverage 98.66% 99.01% +0.35%
===========================================
Files 122 121 -1
Lines 13060 12907 -153
===========================================
- Hits 12885 12780 -105
+ Misses 175 127 -48
Continue to review full report at Codecov.
|
monkeypatching algorithm. It's no longer needed and I forgot to delete it.
|
Looks good to me... |
additionally required: - Removing the v21 workbench test suite and reinstating the v20 test suite - Fixing up a few v20 unit tests to work with the workbench monkeypatching. - I didn't revert the analogous changes I'd previously made to the v21 unit tests, because I think they make sense even when the workbench monkeypatching isn't happening.
created subclasses, to mention v20 instead of v21.
it doesn't make sense to have a test per STIX version. The workbench only uses the latest supported STIX version. In order to make this work, the test suite was modified to dynamically compute some settings like where to get demo data, based on the value of stix2.DEFAULT_VERSION. Switched stix2.DEFAULT_VERSION back to "2.0", since I figure it should be sync'd up with the 'from .vxx import *' import statement from the top level package.
that turns them on. These have the potential to affect subsequent tests. The side effects include automatically setting property values, and automatically appending additional values to list-valued properties.
autogenerated docstrings: v20/v21 is automatically referenced as appropriate, based on stix2.DEFAULT_VERSION. To avoid duplication, I also moved _STIX_VID from test_workbench.py to workbench.py; the former now imports it from the latter.
|
This looks great! Can we add some deprecation warnings for Observed Data and for its Can we add a test for Also, (though I would merge the PR without this one) for the testing that was removed because it looked at extension properties, could we get back at least a little of it by putting 'extensions' in the EDIT: We also need to update |
Seems to me, the warnings module is primarily for the interpreter and perhaps built-in libraries. It interacts with the interpreter's
You may have misread the commit comment. It said: A small amount of testing was removed, since it was looking at custom exception properties... So this isn't specifically about extensions. I think those changes were regarding exceptions which had previously been TypeError's, and therefore hadn't been caught and replaced with InvalidValueError. Most of the property presence exception classes fall into that category (at least one, extra, missing, etc). |
optional. Add a corresponding unit test.
on ThreatActor.
|
For warnings, I was reading this. Looks like you can set a filter so they show up without the interpreter option, but I'm not sure yet what best practice is for library authors like us. Yes I misspoke (and probably misread too) about the removed tests. I was trying to get at stuff like the removed code above this: https://github.com/oasis-open/cti-python-stix2/pull/283/files#diff-5d546fb7e9a4fb90af64f4ae52b50106R799 I was hoping to be able to check a little more precisely that the correct error was encountered. |
Hmmm... I couldn't find anything that described an intention for users (of Python interpreters and core libraries, i.e. normal developers) to only use UserWarning. I found a Deprecation library, which includes a white paper describing ways other Python libraries deal with deprecation warnings, including well-known libraries like Flask and Django. All signs suggest they use all the warning categories. Ok then, guess we can too. I guess that Deprecation library wouldn't really work for us, since we aren't deprecating a function or class (something we can decorate); we're deprecating a virtual "property" whose definition is buried in a data structure... it's more complicated for us. As far as best practices, I think our libraries should emit the warnings but never set up warning filters. Any commandline tools we create should be free to set filters or give commandline options to allow user control over them.
If you throw out the exception object, you can't test it; that's inherent in the original design. In Python 3, you can chain the original and the new one via a special attribute (__cause__), which is what I chose to do. So on Python 3, you could get the original exception and run some tests. But they would be python version specific. Another idea is to not test cleaning via object creation, as that triggers the whole property checking, exception replacing, mechanism. If you want to test cleaning, directly call clean() on a property. That will raise the exception directly to you and then you can check whatever you want. |
observed-data. Add a unit test to ensure we get the warning.
|
Hi, is there a plan to allow setting deterministic uuidv5 ids for stix sdo objects? See: cti-python-stix2/stix2/properties.py Line 19 in 582ba2b Or is this something that will be included in future PRs? |
|
@cyvs Yes, this is something we are working on and will be in a future PR. |
|
The beginnings are already in the stix2.1 branch here. uuid5's are allowed, but logic for automatically computing one for an SCO from "ID Contributing Properties" is not implemented yet. |
subclass. Changed the unit test to test for that specific warning category, instead of any DeprecationWarning.
bug which seems to be related to how it copes with code which emits warnings.
Major changes include: