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 zgw-consumers to 0.35.1, commonground-api-common to 2.0.1, OAF to 0.9.0 #478

Merged
merged 4 commits into from
Nov 29, 2024

Conversation

annashamray
Copy link
Collaborator

@annashamray annashamray commented Nov 25, 2024

Copy link
Contributor

@SonnyBA SonnyBA left a comment

Choose a reason for hiding this comment

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

The recent changes in the client behavior should be updated in the tests.

@@ -1,4 +1,5 @@
open-api-framework
git+https://github.com/maykinmedia/commonground-api-common.git@issue/44-migration-error
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

For now I think this can be removed and you can bump it by running bin/compile_dependencies.sh -P commonground-api-common==2.1.0

@@ -47,7 +47,7 @@ def test_kanaal_create_with_name(self, mock_get_client):
Test is request to create kanaal is send with specified kanaal name
"""
client = mock_get_client.return_value
client.list.return_value = []
client.get.return_value = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the client here is an APIClient that is a simple wrapper around requests's Session class (link). The client class now only returns requests's Response objects whereas previously the zds_client.Client also was responsible for parsing the responses (link).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've replaced patch with requests_mock.Mocker for tests, now it should be correct

@@ -72,27 +72,27 @@ def test_kanaal_create_without_name(self, mock_get_client):
Test is request to create kanaal is send with default kanaal name
"""
client = mock_get_client.return_value
client.list.return_value = []
client.get.return_value = []
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed

Copy link
Collaborator

@stevenbal stevenbal left a comment

Choose a reason for hiding this comment

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

just the comment about the dependency, looks good otherwise

@annashamray annashamray requested a review from SonnyBA November 29, 2024 15:09
"""
Test is request to create kanaal is send with default kanaal name
"""
client = mock_get_client.return_value
client.list.return_value = []
m.get(f"{NOTIFICATIONS_API_ROOT}kanaal", json=[])
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍 , not having to know the exact patch location anymore this way.

@annashamray annashamray merged commit 259c9a6 into master Nov 29, 2024
14 checks passed
@annashamray annashamray deleted the deps/zgw-consumers branch November 29, 2024 15:13
SonnyBA pushed a commit that referenced this pull request Jan 8, 2025
bump zgw-consumers to 0.35.1, commonground-api-common to 2.0.1, OAF to 0.9.0
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.

ZGW Consumers must be updated in all projects
3 participants