chore: skip orchestrator info requests if service uri not set by orchestrator #3149
Conversation
|
@ad-astra-video Do we still need this now that we've merged into the master branch? Are there any orchestrators that still have their service URI set to an empty string ("") by mistake? I noticed this issue once before, but I'm not sure if any orchestrators are still affected. |
|
The AI service registry is still not fully used by Orchestrators. This is just a thing to ease development starting gateways faster. For production gateways it is not likely noticed much. I have not issue closing this but it also doesn't make sense to try to connect to a URL that is blank. |
|
Counted the empty entries on last test gateway startup. There are still 59 empty urls in the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3149 +/- ##
===================================================
- Coverage 33.72602% 33.71534% -0.01068%
===================================================
Files 141 141
Lines 37434 37434
===================================================
- Hits 12625 12621 -4
- Misses 24088 24091 +3
- Partials 721 722 +1
Continue to review full report in Codecov by Sentry.
|
This commit makes minor updates to comments for better clarity and consistency. No functional code changes are included.
rickstaa
force-pushed
the
ai-video-skip-no-service-uri-orchs
branch
from
ff9da55 to
d4691bc
Compare
This commit refactors the orchestration skip logic by replacing the use of a new data channel with a direct error throw.
There was a problem hiding this comment.
@ad-astra-video I’ve simplified the logic in this commit. If you’re aligned with this change, we can proceed with the merge.
| @@ -278,7 +278,7 @@ func (dbo *DBOrchestratorPoolCache) cacheDBOrchs() error { | |||
| } | |||
| // Do not connect if URI host is not set | |||
| if uri.Host == "" { | |||
| skipped <- 1 | |||
| errc <- fmt.Errorf("skipping orch=%v, URI not set", dbOrch.EthereumAddr) | |||
There was a problem hiding this comment.
@victorges Should we throw this error, or would it be cleaner to simply display the fetched orchestrators? See @ad-astra-video's first solution (i.e. 6667483).
What does this pull request do? Explain your changes. (required)
Update
db_discovery.goto skip starting orchestrator http client if the service uri is not set by the orchestrator. At time of this commit, 71 orchestrators have not registered for the AI subnet so this skips a lot of failed http connections.Docker build is at
docker pull adastravideo/go-livepeer:ai-video-testSpecific updates (required)
Hostexists in the parsed service uri.How did you test each of these updates (required)
Built docker file and started up gateway
startup.log
Checklist:
makeruns successfully./test.shpass