Skip to content

Media Upload Tools #465

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

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

Media Upload Tools #465

wants to merge 12 commits into from

Conversation

CocoisBuggy
Copy link
Contributor

Working a little bit on #443, and this is where my thinking is taking me so far.

There isn't consensus on this being the best approach - the alternate is discussed in the issue comment chain - but I wanted to take a look and make sure my proposal was feasible

I have written code and tests for both subscription paradigms that google supports, but I guess realistically we need to choose one and curb the other one. @vnugent I'm not really an expert in GCS, but as I see it the choice is down to how we do load balancing.
My initial intuition was that we should just do a web hook since then the requests from google will flow through whatever balancing strategy is in place, but I see some evidence that actually the subscribers get a subset of the messages and that some load balancing is already in place. Do you have any idea? We can go either way since both options are implemented and tested

Next step is just to update the gql mutation and add some tests and then the feature should be done. We can roll it out without affecting any of the existing media upload features in opentacos.

I've put some extra documentation in the relevant README here

The GCS service account key (if present)
(untested) Mongodb alleges it can manage TTL on a document by document
basis so we will be trying this to elide stale media that doesn't get
uploaded to the bucket
I have chosen for the time being to support either the PUSH or PULL
based subscribers, and for now I haven't done anything clever with
deletion events though realistically we could do that to make sure media
gets properly removed - though it would require some authentication
hoops to be jumped through.

Ideally I'd like for us to choose one of these that we would like to
support and then leave the other at the wayside, opting for a mock
service to be used with all other development environments.
The tests related to full integration will skip unless the
environment signals that it is properly instrumented for
integratrion tests.

The full tests require wiring up to a TLS-backed endpoint and all the
GCS infrastructure set up ahead of the test.
@CocoisBuggy CocoisBuggy added the question Further information is requested label Apr 12, 2025
Oops, I left them as example values which causes the instrumentation
tests to run rather than skip
This should indicate that the new features can be safely built on these
interfaces
We leave it to the client to recall its own promised media, because the
alternative involves convuluting the GQL schema and potentially
poisoning the consistency.

In the event that promised media gets dropped, so what? the server will
statelessly deal with the timeout by quietly terminating the pending
media.

This means we need a test case for re-submitting an idential pending
media promise. In this case I imagine we can yield back the existing
pending object - unless the expiry is imminent in which case we can
re-sign a url and send it back.
As far as I can tell these are quite comprehensive, I need to run a
realworld test of the google pull subscriber load balancing so that I
can see if the complexity is needless. That will be the next commit.

This commit contains working code that should be safe to integrate into
the existing services quite easily so long as the infrastructure has the
notifications configured along with it.

I've tried to make the tests catch any funny business, so it should be
safe for any developer to come and poke around without disrupting
anything. The integration tests are the end-all since if they pass, the
code should be totally funcitonal.
The tests were trying to run with full GCS service access when the user
has no credentials.
@CocoisBuggy CocoisBuggy marked this pull request as ready for review April 23, 2025 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant