-
Notifications
You must be signed in to change notification settings - Fork 186
Replacing google poly integration with Icosa integration #723
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
base: master
Are you sure you want to change the base?
Conversation
|
Hey, running I'm sorry we didn't get our ducks in a row, before your latest work. |
|
Could you please apply this diff file? |
Note: The code for this commit was generated by one or more AI tools: probably Github Copilot (GPT-5 Mini?) and potentially also Claude Code CLI (Sonnet 4.5?).
This reverts commit 76b5e1c.
By @DougReeder Why: Icosa doesn't have the rate limits that YouTube has. The original comment contained useful context.
|
I think you may need to run Again, sorry we didn't have this set up fully before you started work. |
|
With the Icosa API fixed, the rebased code is able to pull object from both archive.org and backblaze (Icosa proper)! The CI won't pass this until files changed in this PR are run through When pulling from backblaze (for example, [info] GET /https://s3.us-east-005.backblazeb2.com/icosa-gallery/poly/0IuRrFSlk5D/GroundDisc.png), but not archive.org, the reticulum log contains the message [warning] Description: ~c"Server authenticity is not verified since certificate path validation is not enabled" That's not a blocker, but let's take a look and see if it's an easy fix. |
|
I just pushed a commit with the results of mix format. Let me know if that does the trick. re: the cert/SSL warning. We can't reproduce this with curl -i https://s3.us-east-005.backblazeb2.com/icosa-gallery/poly/0IuRrFSlk5D/GroundDisc.png or wget https://s3.us-east-005.backblazeb2.com/icosa-gallery/poly/0IuRrFSlk5D/GroundDisc.png - I suspect this is more likely to be an issue with reticulum config. |
|
When I try to merge upstream/master into this PR I get two conflicted files. media_search.ex looks trivial. But page_controller.ex is beyond me. Is this something you can tackle @DougReeder ? |
Yes. I should have linked to #742 which explains the problem and lack of working solution. :-( |
I was able to merge in master — the changes are here: icosa-mirror#3 |
|
Now that archive.org is back up (albeit slowly), the version with master merged is working fine for both archive.org (Poly) and backblaze (Icosa) so far. |
|
absinthe is listed in
and running So, I've updated lint-and-test in the formatting PR to icosa-mirror:master, and that appears to resolve the issue: https://github.com/DougReeder/reticulum/actions/runs/19553649115 |
| |> resolved()} | ||
| end | ||
|
|
||
| # Necessary short circuit around google.com root_host to skip YT-DL check for Poly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there isn't a rate limit for the Icosa API, is this code needed?
|
|
||
| defp resolve_non_video( | ||
| %MediaResolverQuery{ | ||
| url: %URI{host: "api.icosa.gallery", path: "/v1/assets/" <> asset_id} = uri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value assigned to uri is not used. If this is intentional, we should drop = uri
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... or we could rename uri to _uri to indicate it's not used.
| make_reverse_proxy_request(conn, url, body, is_head, opts) | ||
| end | ||
|
|
||
| {:ok, %HTTPoison.Response{status_code: status_code}} -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a success, so we don't care what the exact status code is, so this line should be:
{:ok, %HTTPoison.Response{}} ->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, we could rename status_code to _status_code to show it's not used.
Google Poly integration had been removed in the past, but has been reinstated using the new Open Source project Icosa. These changes support that effort.
Currently, integration centers on the icosa.ixxy.co.uk API implementation.