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 archive_media configuration option #73

Merged
merged 1 commit into from
Aug 23, 2021
Merged

add archive_media configuration option #73

merged 1 commit into from
Aug 23, 2021

Conversation

fobser
Copy link
Contributor

@fobser fobser commented Aug 20, 2021

Styles and docs

  • I have read the contributing guide
  • I have run black on my code
  • My tests are listed in alphabetical order

Tests

  • I have added tests for this code in tests/test_ephemetoot.py
  • I would like assistance to write the required tests
  • I don't know how to write test and would like someone to write them for me

The requests library doesn't support file:// URLs so I have no idea how to write a test for this.
It seems inappropriate to to fetch a random URL from the internet in a test.

What this PR does

Changes in this pull request
With the archive_media option set to true, media attachments are
archived together with the toot.

Related issues

Resolves #72

Copy link
Owner

@hughrun hughrun left a comment

Choose a reason for hiding this comment

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

Hi @fobser

Thanks so much for this PR! It appears to work as advertised.

I can add a test after I pull your final PR, which will help protect against regressions in future.

I have 2 requested changes

  1. Can you please add an archive_media value in example-config.yaml?
  2. Some users will be using ephemetoot with multiple accounts. The media files should therefore ideally be saved to a filepath reflecting their original domain name. Can we update line 335 in ephemetoot.py to read:
media_archive_path = os.path.join(archive_path, url.netloc, dir_name[1:])

This should then save all images to archive_path/server-domain.tld/filepath/filename.extension, avoiding any (admittedly unlikely) filepath name collisions from different accounts.

With the archive_media option set to true, media attachments are
archived together with the toot.
Resolves #72
@fobser
Copy link
Contributor Author

fobser commented Aug 22, 2021

Hi @hughrun
I made the requested changes and force pushed my branch.

Thanks for handling the test, much appreciated. I had
a look at the existing tests but kinda got lost.

@hughrun hughrun added the enhancement New feature or request label Aug 23, 2021
@hughrun hughrun self-requested a review August 23, 2021 05:20
Copy link
Owner

@hughrun hughrun left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks so much, will add tests and issue a new release soon.

If you'd like to be @mentioned by the ephemetoot account when I announce the new release, let me know your mastodon handle.

@hughrun hughrun merged commit 8b44526 into hughrun:master Aug 23, 2021
@fobser
Copy link
Contributor Author

fobser commented Aug 23, 2021

Thanks, I'm @florian@bsd.network

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.

Optionally include media attachments in archives
2 participants