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

mark 'export' CLI command as deprecated #3264

Merged
merged 2 commits into from
May 10, 2023

Conversation

adamsachs
Copy link
Contributor

@adamsachs adamsachs commented May 10, 2023

partially closes https://github.com/ethyca/fidesplus/issues/879

Code Changes

  • add deprecation flag to fides export CLI command

Steps to Confirm

  • ran fides export datamap --csv -d . in a fides container shell and got the following output
image
  • verified using 2.12.2a1 tag that in fidesplus we can override this command and route it to the fidesplus version that is not deprecated and is based on the migrated backing code in fidesplus

Pre-Merge Checklist

Description Of Changes

Initially we were going to remove the command wholesale. That PR can stay open for another release or two as we allow the command to be "phased out" more smoothly by being marked as deprecated. The fides export command in fidesplus will be able to override this deprecated command here and use all the code in fidesplus. This code should be effectively frozen until it is officially removed and phased out in the near future: #3265

@adamsachs adamsachs force-pushed the fidesplus-879-deprecate-export-command branch from 2f81243 to e30055e Compare May 10, 2023 11:01
@adamsachs adamsachs marked this pull request as ready for review May 10, 2023 11:10
@cypress
Copy link

cypress bot commented May 10, 2023

Passing run #1892 ↗︎

0 4 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 8ce596a into 8dd6881...
Project: fides Commit: 96a809168c ℹ️
Status: Passed Duration: 01:10 💡
Started: May 10, 2023 5:55 PM Ended: May 10, 2023 5:56 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (c063274) 87.39% compared to head (e30055e) 87.39%.

❗ Current head e30055e differs from pull request most recent head 8ce596a. Consider uploading reports for the commit 8ce596a to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3264   +/-   ##
=======================================
  Coverage   87.39%   87.39%           
=======================================
  Files         305      305           
  Lines       18265    18267    +2     
  Branches     2384     2384           
=======================================
+ Hits        15962    15964    +2     
  Misses       1866     1866           
  Partials      437      437           
Impacted Files Coverage Δ
src/fides/cli/commands/export.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adamsachs adamsachs self-assigned this May 10, 2023
Copy link
Contributor

@SteveDMurphy SteveDMurphy left a comment

Choose a reason for hiding this comment

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

Validated the deprecation command looks good, left a suggestion or topic of conversation around the response text is all

@adamsachs adamsachs force-pushed the fidesplus-879-deprecate-export-command branch 2 times, most recently from fba7b04 to 0214aaa Compare May 10, 2023 17:28
@ethyca ethyca deleted a comment from SteveDMurphy May 10, 2023
@adamsachs adamsachs force-pushed the fidesplus-879-deprecate-export-command branch from 0214aaa to 8ce596a Compare May 10, 2023 17:35
@adamsachs adamsachs merged commit 56e4b71 into main May 10, 2023
@adamsachs adamsachs deleted the fidesplus-879-deprecate-export-command branch May 10, 2023 18:41
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.

2 participants