Brenton interactive generate

[brentonmallen1]

Closes 129

Code Changes

• Added interactive cli for dataset file annotation
• Added -a, --annotate option to the generate_dataset cli command
• Changed visualization for graphs on a single html page
• Updated visualization to allow for both default and total taxonomy to be displayed

Steps to Confirm

• Verify both methods 1 and 2

Method 1:

• Generate a dataset using the generate_dataset functionality and a connection to a test database (i.e. slice) without -a
• Call the fidesctl annotate_dataset <filename> function on the generated dataset output (or any other dataset file)
• Verify the following:
  • The UI allows the user to quit. Upon reloading after quitting, the UI takes the user back to the same field to annotate
  • Upon completion and/or quitting, the dataset yaml file has been edited to have the given data_categories, skipped ones have been left empty
    Method 2:
• Generate a dataset using the generate_dataset functionality and a connection to a test database (i.e. slice) with -a
• Verify that the generate_dataset with -a takes the user into the annotation UI

Pre-Merge Checklist

• All CI Pipelines Succeeded
• Documentation Updated

Description Of Changes

These changes allow the user to better navigate to the resource visualizations and interactively annotate a given dataset yaml file in a guided way without lose of progress if exited before completion.

Things to consider:

• If a dataset.yml file has multiple datasets listed, the annotate_dataset functionality will output multiple resulting dataset files, one per dataset in the list. The resulting files will be in the same location as the unannotated file with new names corresponding to the name key in the dataset description. This is due to the current functionality of manifests.write_manifest to only take in a single dictionary instead of a list of dictionaries.

[ThomasLaPiana]

A few Qs to hash out, overall the flow feels great though!

[ThomasLaPiana]

@brentonmallen1 i did some cleanup here, most notably refactoring crud.py and cleaning up some design things in cli.py

[iamkelllly]

@brentonmallen1 updated the cli documentation in github pages + the CLI inline docs to match the rest of the formatting. actually used it, pretty slick. great work 👍

[ThomasLaPiana]

looking really good here, just a small comments or two left and updating how it writes out files

[ThomasLaPiana]

@brentonmallen1 is this ready for review/merge? if not, can we convert it to a draft PR until its ready for final review? i'll know you're looking for a final review when i see it change to no longer a draft, otherwise if its ready now i'll go ahead and review!

[brentonmallen1]

It’s good for review.

[ThomasLaPiana]

running into this bug, going to look into it:

####
Annotating Dataset: [Demo Users Dataset]
####
Annotating Table: [users]

Field [users.food_preference] has no data categories

Enter comma separated data categories for [food_preference] [Enter: skip, q: quit] [[]]: user.provided.identifiable
[user.provided.identifiable] contains invalid categories, please re-enter!
Enter comma separated data categories for [food_preference] [Enter: skip, q: quit] [[]]: user          
[user] contains invalid categories, please re-enter!
Enter comma separated data categories for [food_preference] [Enter: skip, q: quit] [[]]: user.provided.identifiable
[user.provided.identifiable] contains invalid categories, please re-enter!
Enter comma separated data categories for [food_preference] [Enter: skip, q: quit] [[]]: 

[brentonmallen1]

Hmm, looks like it might not be seeing that category in the list of valid categories. Maybe it’s only grabbing leaf nodes

[brentonmallen1]

That's not it, I see all the paths in the valid categories list

[ThomasLaPiana]

@brentonmallen1 i got the bugs i found cleaned up, can you review the tweaks i made and lmk how they look?

[brentonmallen1]

did we want to make the validation false by default?

Looks good. I only see two minor potential issues:
http://github.com/ethyca/fides/blob/c7dfce05e62cbf89dcaf85bcba6799e090df4dd0/fidesctl/src/fidesctl/core/annotate_dataset.py#L59-L59 uses input as a variable name though it's a builtin function

With the change in the loop, current_dataset here: http://github.com/ethyca/fides/blob/c7dfce05e62cbf89dcaf85bcba6799e090df4dd0/fidesctl/src/fidesctl/core/annotate_dataset.py#L169-L169 could be reference before it's assigned.

Minor things that probably aren't really an issue but just wanted to point them out. If you're good with it, I say ship it.

[ThomasLaPiana]

did we want to make the validation false by default?

Looks good. I only see two minor potential issues: http://github.com/ethyca/fides/blob/c7dfce05e62cbf89dcaf85bcba6799e090df4dd0/fidesctl/src/fidesctl/core/annotate_dataset.py#L59-L59 uses input as a variable name though it's a builtin function

With the change in the loop, current_dataset here: http://github.com/ethyca/fides/blob/c7dfce05e62cbf89dcaf85bcba6799e090df4dd0/fidesctl/src/fidesctl/core/annotate_dataset.py#L169-L169 could be reference before it's assigned.

Minor things that probably aren't really an issue but just wanted to point them out. If you're good with it, I say ship it.

good catches!

not really worried about either, we're not using input in this function (since we're relying on click) and technically it would only hit that if the dataset list was empty, which would get caught earlier i believe?

either way i'm going to go ahead and merge as-is, thank you for your work on this huge feature!

[ThomasLaPiana]

for the default validation as false, i would've had to rename it to no-validation and then completely redo how that bool gets passed down the pipeline, so for now i just made it false and the most feature-rich command is fidesctl annotate-dataset <path> -a -v

[brentonmallen1]

Sounds good!