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

Add ability to save resources #1272

Merged
merged 17 commits into from
Jan 1, 2023
Merged

Conversation

parafoxia
Copy link
Contributor

@parafoxia parafoxia commented Aug 26, 2022

Summary

This PR adds the ability to save attachments (and more generally, resources) to disk.

  • Adds the Resource.save() method
  • Adds tests for Resource.save() and Resource.read()

Checklist

  • I have run nox and all the pipelines have passed.
  • I have made unittests according to the code I have added/modified/deleted.

Related issues

None

@parafoxia parafoxia marked this pull request as ready for review August 26, 2022 19:27
hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Show resolved Hide resolved
hikari/files.py Outdated Show resolved Hide resolved
tests/hikari/test_files.py Outdated Show resolved Hide resolved
@davfsa davfsa added the enhancement New feature or request label Sep 5, 2022
hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Show resolved Hide resolved
tests/hikari/test_files.py Outdated Show resolved Hide resolved
@parafoxia
Copy link
Contributor Author

Honestly I think I'm just gonna have to leave the tests up to somebody else to take care of cos I can't figure this out. I've emulated what you've done, and I'm getting all sorts of errors and have absolutely no idea what I'm doing wrong.

@davfsa
Copy link
Member

davfsa commented Sep 26, 2022

Honestly I think I'm just gonna have to leave the tests up to somebody else to take care of cos I can't figure this out. I've emulated what you've done, and I'm getting all sorts of errors and have absolutely no idea what I'm doing wrong.

@parafoxia mind pushing what you have (can be with broken tests too, or just skip them entirely).

@parafoxia
Copy link
Contributor Author

parafoxia commented Sep 26, 2022

There's not much extra there, but that's where I got up to. The error it's giving is that the async context manager mock can't be used in async contexts.

@davfsa davfsa enabled auto-merge (squash) September 26, 2022 20:19
@parafoxia
Copy link
Contributor Author

@FasterSpeeding, just bumping this as it's ready for review.

hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Show resolved Hide resolved
hikari/files.py Outdated Show resolved Hide resolved
hikari/files.py Outdated Show resolved Hide resolved
@davfsa davfsa requested a review from FasterSpeeding January 1, 2023 20:07
@FasterSpeeding
Copy link
Collaborator

Test cases for the specialisations?

@davfsa
Copy link
Member

davfsa commented Jan 1, 2023

Test cases for the specialisations?

Working on those atm :)

@davfsa
Copy link
Member

davfsa commented Jan 1, 2023

@FasterSpeeding tests done. I also found some bugs in the code, so some parts are update too :)

hikari/files.py Outdated Show resolved Hide resolved
@davfsa davfsa force-pushed the task/save-attachments branch from 5b603a4 to 9544f2d Compare January 1, 2023 21:58
FasterSpeeding
FasterSpeeding previously approved these changes Jan 1, 2023
hikari/files.py Outdated Show resolved Hide resolved
@davfsa davfsa disabled auto-merge January 1, 2023 22:03
@FasterSpeeding FasterSpeeding self-requested a review January 1, 2023 22:03
Co-authored-by: Lucina <lucina@lmbyrne.dev>
Signed-off-by: davfsa <davfsa@gmail.com>
@davfsa davfsa merged commit f9e883d into hikari-py:master Jan 1, 2023
@parafoxia parafoxia deleted the task/save-attachments branch January 1, 2023 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants