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

Use storage in ads uploads #185

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

rubencm
Copy link

@rubencm rubencm commented Dec 25, 2024

No description provided.

@iMattPro iMattPro closed this Dec 26, 2024
@iMattPro iMattPro reopened this Dec 26, 2024
@rubencm rubencm closed this Dec 26, 2024
@rubencm rubencm reopened this Dec 26, 2024
@rubencm rubencm force-pushed the storage branch 3 times, most recently from 8c7b872 to 5d35d50 Compare December 26, 2024 21:06
@rubencm rubencm force-pushed the storage branch 2 times, most recently from 078bfd4 to 050c175 Compare December 26, 2024 23:37
continue;
}

$file_tracker->track_file('phpbb_ads', $file, filesize($dir . '/' . $file));
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me wonder if the $file_tracker class should have track_files and untrack_files methods for handling multiple entries in 1 query?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it can be updated to support more than one file.
I add it to the todo list

Copy link
Contributor

Choose a reason for hiding this comment

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

Great! (Will be a lot less queries)

phpbb_ads_storage_banner:
path: /ads_download/{file}
defaults:
_controller: phpbb.ads.controller.banner:handle
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll probably want to add stateless: true here eventually, re: https://github.com/phpbb/phpbb/compare/master...marc1706:phpbb:feature/stateless_sessions?expand=1

Copy link
Author

Choose a reason for hiding this comment

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

Well, this can be done in the future, there is not even a pull request for this and can change in the future

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.

2 participants