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

Bump fideslog to v1.1.3, update usage patterns #453

Merged
merged 6 commits into from
Apr 18, 2022

Conversation

PSalant726
Copy link
Contributor

@PSalant726 PSalant726 commented Apr 5, 2022

Related to ethyca/fideslog#50
Related to ethyca/fideslog#51

Code Changes

  • Bump fideslog to v1.1.3
  • Remove the usage of asyncio.run when sending AnalyticsEvents
  • AnalyticsException --> AnalyticsError
  • Separately print OPT_OUT_COPY and retrieve input using OPT_OUT_PROMPT

Steps to Confirm

  • Run make cli
  • Execute any command that sends analytics
  • Observe no errors, and improved command latency
  • Query the fideslog database to ensure your event was written successfully

Pre-Merge Checklist

  • All CI Pipelines Succeeded

Description Of Changes

Fideslog v1.1.3 includes the following changes

  • The AnalyticsClient.send interface automatically executes the request in a separate event loop. There is no longer a need to manually wrap the call in asyncio.run or similar.
  • AnalyticsException has been renamed to AnalyticsError, to better align with Python convention
  • The SDK's utils.py file now separately exposes OPT_OUT_COPY and OPT_OUT_PROMPT, to better enable non-CLI applications to display OPT_OUT_COPY without automatically assuming a user will respond with their keyboard.

@PSalant726 PSalant726 requested a review from a team April 5, 2022 23:59
@PSalant726 PSalant726 self-assigned this Apr 5, 2022
@ThomasLaPiana ThomasLaPiana marked this pull request as draft April 6, 2022 19:36
@ThomasLaPiana
Copy link
Contributor

@PSalant726 i moved this back to draft while we wait for fideslog 1.1.0

@PSalant726 PSalant726 changed the title Bump fideslog to v1.1.0, send async analytics event requests Bump fideslog to v1.1.0, update usage patterns Apr 7, 2022
Fideslog v1.1.0 changes the `AnalyticsClient.send` interface such that
it automatically executes the request in a separate event loop. There
is no longer a need to manually wrap the call in `asyncio.run` or
similar.

These changes were introduced in ethyca/fideslog#50
@PSalant726 PSalant726 changed the title Bump fideslog to v1.1.0, update usage patterns Bump fideslog to v1.1.1, update usage patterns Apr 15, 2022
@PSalant726 PSalant726 marked this pull request as ready for review April 15, 2022 16:40
@PSalant726 PSalant726 marked this pull request as draft April 15, 2022 16:57
@PSalant726 PSalant726 changed the title Bump fideslog to v1.1.1, update usage patterns Bump fideslog to v1.1.2, update usage patterns Apr 15, 2022
@PSalant726 PSalant726 changed the title Bump fideslog to v1.1.2, update usage patterns Bump fideslog to v1.1.3, update usage patterns Apr 15, 2022
@PSalant726 PSalant726 marked this pull request as ready for review April 15, 2022 18:24
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.

Nice simple implementation updates! Looks great @PSalant726 👍🏽

When first running fidesctl apply demo_resources/ after starting the container, the event was not sent - but I haven't been able to reproduce it since (removing analytics_id, restarting container, etc.). Adding as a comment in case anyone else has seen that and we want to investigate further separately

@PSalant726 PSalant726 changed the base branch from main to release-1.5.3 April 18, 2022 18:55
@PSalant726 PSalant726 merged commit c86c46c into release-1.5.3 Apr 18, 2022
@PSalant726 PSalant726 deleted the async-analytics branch April 18, 2022 19:04
ThomasLaPiana pushed a commit that referenced this pull request Aug 17, 2022
* adds drp action type to db, adds docs, tests

* fix lint, tests

* format

* remove unneeded func arg

* lint

* more lint

* CR changes

* update alembic down migration number

* minor docs updates for drp

* integration tests and updating alembic

* catches integrity error at endpoint level

* lint

Co-authored-by: Cole Isaac <cole@ethyca.com>
ThomasLaPiana pushed a commit that referenced this pull request Sep 26, 2022
* adds drp action type to db, adds docs, tests

* fix lint, tests

* format

* remove unneeded func arg

* lint

* more lint

* CR changes

* update alembic down migration number

* minor docs updates for drp

* integration tests and updating alembic

* catches integrity error at endpoint level

* lint

Co-authored-by: Cole Isaac <cole@ethyca.com>
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.

3 participants