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

Add handling subclassed evidence support #80

Merged
merged 5 commits into from Nov 5, 2020

Conversation

alfinkel
Copy link
Contributor

@alfinkel alfinkel commented Nov 2, 2020

What

  • Add support for check decorators and context manager to return subclassed framework evidence objects.
  • Replace os.path with pathlib.PurePath in the evidences module
  • Add a content_as_json property to evidence base class

Why

  • The act of casting of subclassed raw or external evidence should be handled by the framework instead of having to be handled by individual check logic.
  • Using PurePath or Path from pathlib whenever possible over os.path is a cleaner more generic solution.
  • Adding content_as_json is also helpful and a common enough need that it should be contained within the framework for evidences with JSON content

How

  • Add FromEvidence named tuple
  • Add passing FromEvidence to with_*_evidences decorators
  • Add passing FromEvidence to evidences context manager
  • Replace os.path usage with pathlib.PurePath
  • Updated docs

Test

  • Present behavior of with_*_evidences decorators and evidences context manager is preserved
  • Passing a combination of FromEvidence named tuples and string paths to decorators and context manager yields the expected results for evidences returned. Evidences specified as FromEvidence are returned to the test_ function/method as the evidence specified as the ev_class otherwise the evidence is returned as either raw or external as appropriate.

Context

N/A

@alfinkel alfinkel force-pushed the add-evidence-casting-to-decorators-and-ctx-mgr branch from 79cccd3 to cd53b75 Compare November 2, 2020 22:30
- Add FromEvidence named tuple
- Add passing FromEvidence to with_*_evidences decorators
- Add passing FromEvidence to evidences context manager
- Replace os.path usage with pathlib.PurePath
- Add content_as_json property to all Evidence objects
- Updated docs
@alfinkel alfinkel force-pushed the add-evidence-casting-to-decorators-and-ctx-mgr branch from cd53b75 to dff8b54 Compare November 2, 2020 22:33
compliance/evidence.py Outdated Show resolved Hide resolved
@alfinkel
Copy link
Contributor Author

alfinkel commented Nov 3, 2020

@cletomartin as discussed over Slack, at your suggestion, I've changed the FromEvidence named tuple functionality to be part of a class method (lazy_load) now found on all Evidence classes. This removes the burden of having to import FromEvidence every time you'd want to use it. I did however change things a bit in that FromEvidence is now LazyLoader and it is at the evidence module level rather than inside the base evidence class as we had originally discussed. The reason for that is the with_*_evidences decorators and evidences context manager need to reference it in order to ensure proper processing.

- A dictionary where a key is an evidence short name and a value
is either string path or a LazyLoader namedtuple

Using the LazyLoader namedtuple tells this context manager to cast
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

ev_class = None
if isinstance(from_ev, LazyLoader):
path = from_ev.path
if issubclass(from_ev.ev_class, get_evidence_class(type_str)):
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 this could use an explanatory comment - if I'm reading it right, it'll make an instance of the class defined in the lazy loader if that class is a subclass of the type_str, otherwise will make the evidence be of the type in type_str.

How about:

Suggested change
if issubclass(from_ev.ev_class, get_evidence_class(type_str)):
ev_class = get_evidence_class(type_str)
if isinstance(from_ev, LazyLoader):
path = from_ev.path
if issubclass(from_ev.ev_class, get_evidence_class(type_str)):
ev_class = from_ev.ev_class

and save an if.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in a96efa7

path = from_ev.path
if issubclass(from_ev.ev_class, get_evidence_class(type_str)):
ev_class = from_ev.ev_class
if not path.startswith(prefix):
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if pathlib could be used here to avoid the if: & the fstring.

Copy link
Contributor Author

@alfinkel alfinkel Nov 4, 2020

Choose a reason for hiding this comment

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

Done in a96efa7 and 70751fd

@alfinkel alfinkel merged commit 729c21d into main Nov 5, 2020
@alfinkel alfinkel deleted the add-evidence-casting-to-decorators-and-ctx-mgr branch November 5, 2020 16:28
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

3 participants