src(attachment): Use attrs + cattrs #134
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gegnew
force-pushed
the
ge/cattrs-attachment
branch
3 times, most recently
from
a4ba71a to
4d3d608
Compare
gegnew
force-pushed
the
ge/cattrs-attachment
branch
from
4d3d608 to
70eb7e2
Compare
zbjornson
requested changes
Dec 14, 2021
| responses.add( | ||
| responses.GET, | ||
| ENDPOINT_BASE + f"/experiments/{EXP_ID}/attachments", | ||
| json=[attachment.to_dict()], | ||
| ) | ||
| att = Attachment.get(EXP_ID, attachment._id) | ||
| att = client.get_attachment(EXP_ID, attachment._id) |
There was a problem hiding this comment.
Relates to #112 (comment)
It looks like we still have Attachment.get/upload now, but they're no longer tested. Copy these tests to test both?
There was a problem hiding this comment.
Looks like you still need one for Attachment.upload; only client.upload_attachment is tested.
gegnew
force-pushed
the
ge/cattrs2
branch
5 times, most recently
from
46b80e8 to
2d35c99
Compare
gegnew
force-pushed
the
ge/cattrs-attachment
branch
from
70eb7e2 to
13a2dcb
Compare
gegnew
force-pushed
the
ge/cattrs2
branch
3 times, most recently
from
7148f83 to
d77f707
Compare
gegnew
force-pushed
the
ge/cattrs-attachment
branch
from
13a2dcb to
804f6af
Compare
gegnew
force-pushed
the
ge/cattrs-attachment
branch
from
804f6af to
507f981
Compare
|
Looks good % the missing test. Can you rebase on |
gegnew
force-pushed
the
ge/cattrs-attachment
branch
from
507f981 to
8b37037
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #133