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

Persist Evaluation objects #196

Merged
merged 6 commits into from
Oct 29, 2021

Conversation

earmenda
Copy link
Contributor

@earmenda earmenda commented Oct 29, 2021

Closes #4

Code Changes

  • Added sql mapping to sql_models.py
  • Added endpoint in fideslang/init.py

Steps to Confirm

  • Test evaluations and call ls evaluation and verify the results are stored

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated

Description Of Changes

  • Added sql mapping to sql_models.py
  • Added endpoint in fideslang/init.py

@earmenda earmenda self-assigned this Oct 29, 2021
@earmenda
Copy link
Contributor Author

OK, i have no idea what I am doing here but it looks like this isnt gonna be too much work

With the above changes the evaluation endpoint looks to kinda work

root@ef7b67ab20b8:/fides/fidesctl# fidesctl ls
Usage: fidesctl ls [OPTIONS] [data_category|data_qualifier|data_subject|data_u
                   se|dataset|organization|policy|registry|system|evaluation]
root@ef7b67ab20b8:/fides/fidesctl# fidesctl ls evaluation
[]

@earmenda
Copy link
Contributor Author

Not able to get the api call to suceed for some reason. Not sure I understand the problem yet:

command:

fidesctl evaluate demo_resources/

evaluate output

Sending the evaluation results to the server...
{"fides_key": "a2c9d753_a014_4b71_ba14_9792a6a0fbc8", "status": "PASS", "details": [], "message": "hello?"}
Internal Server Error

server output

fidesctl_1     | INFO:     172.20.0.3:49356 - "POST /evaluation/ HTTP/1.1" 500 Internal Server Error
fidesctl_1     | ERROR:    Exception in ASGI application
fidesctl_1     | Traceback (most recent call last):
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/uvicorn/protocols/http/httptools_impl.py", line 398, in run_asgi
fidesctl_1     |     result = await app(self.scope, self.receive, self.send)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/uvicorn/middleware/proxy_headers.py", line 45, in __call__
fidesctl_1     |     return await self.app(scope, receive, send)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/fastapi/applications.py", line 208, in __call__
fidesctl_1     |     await super().__call__(scope, receive, send)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/applications.py", line 112, in __call__
fidesctl_1     |     await self.middleware_stack(scope, receive, send)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/middleware/errors.py", line 181, in __call__
fidesctl_1     |     raise exc from None
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/middleware/errors.py", line 159, in __call__
fidesctl_1     |     await self.app(scope, receive, _send)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/exceptions.py", line 82, in __call__
fidesctl_1     |     raise exc from None
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/exceptions.py", line 71, in __call__
fidesctl_1     |     await self.app(scope, receive, sender)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/routing.py", line 580, in __call__
fidesctl_1     |     await route.handle(scope, receive, send)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/routing.py", line 241, in handle
fidesctl_1     |     await self.app(scope, receive, send)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/starlette/routing.py", line 52, in app
fidesctl_1     |     response = await func(request)
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/fastapi/routing.py", line 234, in app
fidesctl_1     |     response_data = await serialize_response(
fidesctl_1     |   File "/usr/local/lib/python3.8/site-packages/fastapi/routing.py", line 137, in serialize_response
fidesctl_1     |     raise ValidationError(errors, field.type_)
fidesctl_1     | pydantic.error_wrappers.ValidationError: 1 validation error for Evaluation
fidesctl_1     | response
fidesctl_1     |   value is not a valid dict (type=type_error.dict)

What is weird is that the data is actually being put in the database but I am not sure why the server can't process the sql object

psql -U fidesctl
fidesctl=#  SELECT * from evaluations
fidesctl-# ;
 id |              fides_key               | status | details | message 
----+--------------------------------------+--------+---------+---------
  1 | a2c9d753_a014_4b71_ba14_9792a6a0fbc8 | PASS   | {}      | hello?
(1 row)

@earmenda
Copy link
Contributor Author

Was able to get past that error with some help from @brentonmallen1

Here is what the evaluation output looks like now:

root@bc54afdcb4f4:/fides/fidesctl# fidesctl evaluate demo_resources/
Loading resource manifests from: demo_resources/
Taxonomy successfully created.
----------
Processing dataset resources...
fidesctlCREATED 1 dataset resources.
UPDATED 0 dataset resources.
SKIPPED 0 dataset resources.
----------
Processing policy resources...
CREATED 1 policy resources.
UPDATED 0 policy resources.
SKIPPED 0 policy resources.
----------
Processing registry resources...
CREATED 1 registry resources.
UPDATED 0 registry resources.
SKIPPED 0 registry resources.
----------
Processing system resources...
CREATED 2 system resources.
UPDATED 0 system resources.
SKIPPED 0 system resources.
----------
Loading resource manifests from: demo_resources/
Taxonomy successfully created.
Evaluating the following policies:
demo_privacy_policy
----------
Checking for missing resources...
Fetching the following missing resources from the server:
- aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified
default_organization
Hydrating the taxonomy...
Executing evaluations...
Sending the evaluation results to the server...
Evaluation passed!

And the evaluation stored in the database:

root@bc54afdcb4f4:/fides/fidesctl# fidesctl ls evaluation
[
  {
    "fides_key": "7db2a909_9fe1_48c5_9df4_ba2452b3e7b5",
    "status": "PASS",
    "details": [],
    "message": "hello?"
  }
]

Some things to figure out still:

  • What should evaluation message be
  • What should evaluation fides_key be
  • Should Evaluation model inherit from FidesModel? This is currenly breaking mypy but it feels weird to attach all the fields in FidesModel to it.

@earmenda
Copy link
Contributor Author

@ThomasLaPiana @NevilleS
I could use some insight on how we see this feature being useful in the short term? At the moment calling fidesctl evaluate and then fidesctl ls evaluation does give you a view into the evaluations but this doesn't feel like a great experience. I'd imagine running a ton of evaluations calling ls evaluation will not be user friendly. It feels like evaluation needs to be treated like a different kind of resource that doesn't get offered like all the FidesModel resources. For the moment I'd like to focus on how this can be useful for our release and if you guys had some thoughts around this already.

@ThomasLaPiana
Copy link
Contributor

this looks good to me!

[
  {
    "fides_key": "ecbde73c_1536_431e_8bc9_ddf80349d965_1635543474",
    "status": "PASS",
    "details": [],
    "message": "first try!"
  },
  {
    "fides_key": "675f2ef0_667c_4648_90be_9d5b056b15b1_1635543604",
    "status": "FAIL",
    "details": [
      "Declaration (Collect data for marketing) of System (demo_marketing_system) failed Rule (Reject Direct Marketing) from Policy (demo_privacy_policy)"
    ],
    "message": "first try!"
  }
]

@ThomasLaPiana
Copy link
Contributor

@ThomasLaPiana @NevilleS I could use some insight on how we see this feature being useful in the short term? At the moment calling fidesctl evaluate and then fidesctl ls evaluation does give you a view into the evaluations but this doesn't feel like a great experience. I'd imagine running a ton of evaluations calling ls evaluation will not be user friendly. It feels like evaluation needs to be treated like a different kind of resource that doesn't get offered like all the FidesModel resources. For the moment I'd like to focus on how this can be useful for our release and if you guys had some thoughts around this already.

this is honestly enough for now. we'll add more functionality as we go!

@earmenda
Copy link
Contributor Author

Merged main in to test with latest evaluation changes as well

@earmenda
Copy link
Contributor Author

Testing with the latest looks like this:

root@bc54afdcb4f4:/fides/fidesctl# fidesctl evaluate demo_resources/ --message "cool system"
Loading resource manifests from: demo_resources/
Taxonomy successfully created.
----------
Processing dataset resources...
CREATED 1 dataset resources.
UPDATED 0 dataset resources.
SKIPPED 0 dataset resources.
----------
Processing system resources...
CREATED 2 system resources.
UPDATED 0 system resources.
SKIPPED 0 system resources.
----------
Processing registry resources...
CREATED 1 registry resources.
UPDATED 0 registry resources.
SKIPPED 0 registry resources.
----------
Processing policy resources...
CREATED 1 policy resources.
UPDATED 0 policy resources.
SKIPPED 0 policy resources.
----------
Loading resource manifests from: demo_resources/
Taxonomy successfully created.
Evaluating the following policies:
demo_privacy_policy
----------
Checking for missing resources...
Executing evaluations...
Sending the evaluation results to the server...
Evaluation passed!
root@bc54afdcb4f4:/fides/fidesctl# fidesctl evaluate test_eduardo/ --message "spooooky"
Loading resource manifests from: test_eduardo/
Taxonomy successfully created.
----------
Processing system resources...
CREATED 1 system resources.
UPDATED 0 system resources.
SKIPPED 0 system resources.
----------
Processing policy resources...
CREATED 1 policy resources.
UPDATED 0 policy resources.
SKIPPED 0 policy resources.
----------
Processing dataset resources...
CREATED 1 dataset resources.
UPDATED 0 dataset resources.
SKIPPED 0 dataset resources.
----------
Loading resource manifests from: test_eduardo/
Taxonomy successfully created.
Evaluating the following policies:
flaskr_policy
demo_privacy_policy
----------
Checking for missing resources...
Executing evaluations...
Sending the evaluation results to the server...
{
  "fides_key": "fa757315_e0f2_433c_8d49_e76e97463c6a_1635545647",
  "status": "FAIL",
  "details": [
    "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy)",
    "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy) for Dataset (dataset_reference_example)",
    "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy) for DatasetCollection (users)",
    "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy) for DatasetField (first_name)"
  ],
  "message": "spooooky"
}
root@bc54afdcb4f4:/fides/fidesctl# fidesctl ls evaluation
[
  {
    "fides_key": "b2a4e7ce_8fcf_4bc2_8afd_b1ddb71d286c_1635545642",
    "status": "PASS",
    "details": [],
    "message": "cool system"
  },
  {
    "fides_key": "fa757315_e0f2_433c_8d49_e76e97463c6a_1635545647",
    "status": "FAIL",
    "details": [
      "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy)",
      "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy) for Dataset (dataset_reference_example)",
      "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy) for DatasetCollection (users)",
      "Declaration (Derive user geographic location) of System (google_analytics_system) failed Rule (Minimize User Identifiable Data) from Policy (flaskr_policy) for DatasetField (first_name)"
    ],
    "message": "spooooky"
  }
]

I'm cool with merging this as is!

@ThomasLaPiana
Copy link
Contributor

shipping it!

Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

LGTM! checked locally and working as expected

@ThomasLaPiana ThomasLaPiana merged commit b6fd951 into main Oct 29, 2021
@ThomasLaPiana ThomasLaPiana deleted the earmenda-branch-persist-evaluation-objects branch October 29, 2021 22:19
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.

Persist Evaluation objects within the DB
2 participants