Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Bump fastapi dependencies #614

Merged
merged 3 commits into from
Jun 8, 2022
Merged

Bump fastapi dependencies #614

merged 3 commits into from
Jun 8, 2022

Conversation

sanders41
Copy link
Contributor

Purpose

Bumps FastAPI and FastAPI's dependencies to allow installing fideslib.

There are 2 failing tests, test_generate_field and test_field_collect_matching after updating the dependencies. The dependency causing an issue is Pydantic. Both tests are failing for the same reason. They are comparing two objects with nested objects for equality. In older versions of Pydantic, the nested objects were created with pointers so the equality test would pass because the objects were at the same memory address. Now Pydantic creates the objects by creating new instances of the objects. Because of the, even though the values contained in the objects are the same, the tests fail because the test object id's are no longer the same. Any suggestions on getting around this?

Changes

  • FastAPI bumped and dependencies installed with fastapi[all] removed from requirements.txt

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #

@PSalant726
Copy link
Contributor

even though the values contained in the objects are the same, the tests fail because the test object id's are no longer the same. Any suggestions on getting around this?

I believe we'll have to implement a "deep equality" check, like:

class Something:
    def __eq__(self, other):
        return self.__dict__ == other.__dict__

@sanders41
Copy link
Contributor Author

The version of black that was pinned turned out to be incompatible with the newer version of Click so I'm bumping that also.

@sanders41
Copy link
Contributor Author

Thanks @PSalant726, I tried this initially and it didn't work, but I only did it at the last level and not the nested classes also. I'm thinking now maybe it will work if I take it all the way down the chain.

@sanders41
Copy link
Contributor Author

Adding __eq__ all the way down looks to have worked. All tests passing locally now.

boto3~=1.18.14
click==7.1.2
click==8.1.3
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the secret to getting this to pass? I struggled with this yesterday

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like black was also bumped to v22.3.0 in ada24f8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did have issues with black and the older version of click so that may be it.

@seanpreston seanpreston self-assigned this Jun 8, 2022
@seanpreston seanpreston merged commit 3376c77 into main Jun 8, 2022
@seanpreston seanpreston deleted the fastapi-dependencies branch June 8, 2022 15:17
sanders41 added a commit that referenced this pull request Sep 22, 2022
* Bump fastapi dependencies

* Bump black 21.8b0 to 22.3.0 and format files

* Add __eq__ methods to classes

Co-authored-by: Paul Sanders <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants