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 tootctl preview_cards remove #11320

Merged
merged 5 commits into from
Jul 28, 2019

Conversation

mayaeh
Copy link
Contributor

@mayaeh mayaeh commented Jul 15, 2019

This changes will be change the remove thumbnails of previews from automatic execution by the Sidekiq scheduler to manual execution by the tootctl command.

After #11304, object storage’s rate-limit is triggered when Scheduler::PreviewCardsCleanupScheduler is executed in some object storage environments.

スクリーンショット 2019-07-15 22 43 31

スクリーンショット_2019-07-15_22_46_12

This is similar to #8187.

So I made tootctl preview_cards remove command referring to tootctl media remove.

There are multiple if statements.
I think that it is not a good writing style.

Please let me know if there is a better way of writing.


TODO

  • Remove unused file
  • Fix if/elsif blocks
  • Add output to confirm when the number of days is less than 2 weeks

@mayaeh mayaeh changed the title Add tootctl preview-cards remove Add tootctl preview_cards remove Jul 15, 2019
@mayaeh mayaeh force-pushed the feature_tootctl_preview_cards branch from 3997dc3 to a4d4dd5 Compare July 15, 2019 15:32
lib/mastodon/preview_cards_cli.rb Outdated Show resolved Hide resolved
config/sidekiq.yml Show resolved Hide resolved
@Gargron
Copy link
Member

Gargron commented Jul 15, 2019

After #11304, object storage’s rate-limit is triggered when Scheduler::PreviewCardsCleanupScheduler is executed in some object storage environments.

It might be possible to add a rate-limit system to Sidekiq jobs. While Scheduler::PreviewCardsCleanupScheduler is an obvious culprit here, at certain number of Sidekiq threads/workers and certain workloads (a lot of incoming posts with media?) the object-storage rate-limit could be triggered by other jobs as well. Something to think about.

@mayaeh mayaeh changed the title Add tootctl preview_cards remove [WIP] Add tootctl preview_cards remove Jul 16, 2019
@mayaeh
Copy link
Contributor Author

mayaeh commented Jul 16, 2019

While Scheduler::PreviewCardsCleanupScheduler is an obvious culprit here, at certain number of Sidekiq threads/workers and certain workloads (a lot of incoming posts with media?) the object-storage rate-limit could be triggered by other jobs as well.

I think that it isn't happening on my server so far.
But, as you say, I think it can happen.

@mayaeh mayaeh changed the title [WIP] Add tootctl preview_cards remove Add tootctl preview_cards remove Jul 16, 2019

if time_ago > 2.weeks.ago
prompt.say "\n"
prompt.say('The preview cards less than the past two weeks will not be re-acquired even when needed.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this text easy to understand? I do not have any confidence.

Copy link
Member

Choose a reason for hiding this comment

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

Media remove does not ask for confirmation. Do you think it's necessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think it is necessary.

In preview cards, I think that only those older than 2 weeks will be re-acquired.
https://github.com/tootsuite/mastodon/blob/master/app/services/fetch_link_card_service.rb#L25

On the other hand, I can not find the date limit for re-acquisition of attached media.

Therefore, I think that it is necessary to output a warning if the number of days less than 2 weeks is specified in this command.

@mayaeh mayaeh requested a review from Gargron July 17, 2019 16:01
@Gargron Gargron merged commit 0d80f68 into mastodon:master Jul 28, 2019
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
* Add `tootctl preview_cards remove`

* fix code style

* Remove `Scheduler::PreviewCardsCleanupScheduler` file

* fix code style again
Add exclude case where image_file_name is blank

* Added a function to output confirmation if the specified number of days is less than 2 weeks
option :link, type: :boolean, default: false
desc 'remove', 'Remove preview cards'
long_desc <<-DESC
Removes locally thumbnails for previews.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does locally thumbnails means?

@mayaeh mayaeh deleted the feature_tootctl_preview_cards branch October 29, 2021 01:02
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.

None yet

3 participants