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.

Sign up for GitHub

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

Jump to bottom

Add created_at and updated_at columns to FidesBase model tables #290

Merged
merged 6 commits into from
Dec 20, 2021
Merged

Add created_at and updated_at columns to FidesBase model tables #290

merged 6 commits into from
Dec 20, 2021

Conversation

PSalant726
Copy link
Contributor

@PSalant726 PSalant726 commented Dec 16, 2021 •
edited
Loading

Closes #278

Code Changes

  • Add a created_at column to all FidesBase models
  • Add an updated_at column to all FidesBase models

Steps to Confirm

  • Start the API server, ensure no errors are logged

Pre-Merge Checklist

  • All CI Pipelines Succeeded

Description Of Changes

Fixes some awkward or incorrect logging, and adds "default" created_at and updated_at columns to all FidesBase model tables.

@PSalant726 PSalant726 added the maintenance Refactoring or ongoing maintenance work label Dec 16, 2021
@PSalant726 PSalant726 added this to the fidesctl 1.2 milestone Dec 16, 2021
@PSalant726 PSalant726 requested a review from a team December 16, 2021 21:47
@PSalant726 PSalant726 self-assigned this Dec 16, 2021
@PSalant726 PSalant726 changed the title Ps/add timestamp db columns Add created_at and updated_at columns to FidesBase model tables Dec 16, 2021
@ThomasLaPiana
Copy link
Contributor

@PSalant726 how did you test that the migrations ran successfully and this info is being stored? I checked in the database and I don't see those columns

ThomasLaPiana
ThomasLaPiana suggested changes Dec 18, 2021
View reviewed changes
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.

I don't see those columns in the database, so either the migrations aren't running automatically like they should (i'm pretty sure they are) or the migration itself has some sort of error

ThomasLaPiana
ThomasLaPiana approved these changes Dec 20, 2021
View reviewed changes
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.

Pulled locally and ran, confirmed in the DB that the new columns existed. Great work!

@ThomasLaPiana ThomasLaPiana merged commit 41ddaa8 into main Dec 20, 2021
@ThomasLaPiana ThomasLaPiana deleted the ps/add-timestamp-db-columns branch December 20, 2021 22:18
ThomasLaPiana pushed a commit that referenced this pull request Aug 17, 2022
@pattisdr
Support Grouped Input Fields into a Collection [#290] (#299)
a9bbd0b 
* Allow incoming fields to be specified on a saas config as being dependent on each other, not treated as an independent list of values.

- Update graph_task.pre_process_input_data to be able to optionally separate independent fields from dependent fields when processing incoming data into a collection.

* Add a test for pre_process_input_data when group_dependent_fields is set to True.

- Fix bug where nesting of adding data to output is happening in the wrong place.

* Add validation that grouped_inputs must all reference fields from the same collection.

* Fix bug where empty dict was being added to array.

* Fix bad yaml nesting and the fact that some extra endpoints were adding in the saas config test.

* Fix potential bug where collection name doesn't exist because it didn't pass validation.

* Add a test confirming if no grouped_input fields are specified, "fidesops_grouped_inputs" key just returns an empty list.

* Grouped_inputs fields may not exist.

* Allow grouped inputs to be reference or identity fields.

* Put building the dataset graphs within the try/except because if this fails, this will be swallowed and difficult to debug.

* Remove post-processor item that is being handled by separate PR.

* Responding to CR - when storing grouped_inputs on internal collections, use set representation.

* Set FIDESOPS_GROUPED_INPUTS key regardless.

* Add the fidesops_grouped_inputs keys - they are now included in all outputs.

- Switch the issubset.

* Change grouped_inputs list->set type where we merge collections for saas configs.

* Fix test after merge.
ThomasLaPiana pushed a commit that referenced this pull request Sep 26, 2022
@pattisdr
Support Grouped Input Fields into a Collection [#290] (#299)
37570fd 
* Allow incoming fields to be specified on a saas config as being dependent on each other, not treated as an independent list of values.

- Update graph_task.pre_process_input_data to be able to optionally separate independent fields from dependent fields when processing incoming data into a collection.

* Add a test for pre_process_input_data when group_dependent_fields is set to True.

- Fix bug where nesting of adding data to output is happening in the wrong place.

* Add validation that grouped_inputs must all reference fields from the same collection.

* Fix bug where empty dict was being added to array.

* Fix bad yaml nesting and the fact that some extra endpoints were adding in the saas config test.

* Fix potential bug where collection name doesn't exist because it didn't pass validation.

* Add a test confirming if no grouped_input fields are specified, "fidesops_grouped_inputs" key just returns an empty list.

* Grouped_inputs fields may not exist.

* Allow grouped inputs to be reference or identity fields.

* Put building the dataset graphs within the try/except because if this fails, this will be swallowed and difficult to debug.

* Remove post-processor item that is being handled by separate PR.

* Responding to CR - when storing grouped_inputs on internal collections, use set representation.

* Set FIDESOPS_GROUPED_INPUTS key regardless.

* Add the fidesops_grouped_inputs keys - they are now included in all outputs.

- Switch the issubset.

* Change grouped_inputs list->set type where we merge collections for saas configs.

* Fix test after merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThomasLaPiana ThomasLaPiana ThomasLaPiana approved these changes

Assignees

@PSalant726 PSalant726

Labels
maintenance Refactoring or ongoing maintenance work
Projects
None yet
Milestone
fidesctl 1.2
Development

Successfully merging this pull request may close these issues.

Add created_at and updated_at to default database
2 participants
@PSalant726 @ThomasLaPiana