Mark half of the Orchestrators on Staging as 'untrusted' #1064
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| export async function regionsGetter() { | ||
| const [regions, cursor] = await db.region.find({}, { limit: 100 }); | ||
| export function regionsGetter(halfRegionOrchestratorsUntrusted = false) { | ||
| return async function regionsGetter2() { |
There was a problem hiding this comment.
@livepeer/video-user I feel like this is probably not the optimal way, so would appreciate any pointers on how I should be doing it!
There was a problem hiding this comment.
I think this works fine! One syntax sugar you could use to make this look a little better would be a lambda function, replacing async function regionsGetter2() { with async () => {. Mostly a stylistic choice anyway.
Another alternative, to avoid this helper function that returns a function (which I find a little confusing sometimes) is to curry the new boolean variable on the call site where you need this function with zero arguments instead. That is, instead of returning a function here you could just make this function receive the boolean and return the regions, and in app-router you do:
orchestratorGetters.push(() => regionsGetter(halfUntrusted...)).
This also fixes the issue of using the return value of this function (another function) when we actually want the final list of regions.
This is a totally optional/stylistic change anyway, only raising the possibility since you were a little concerned about the current version here.
Codecov Report
@@ Coverage Diff @@
## master #1064 +/- ##
===================================================
- Coverage 46.12903% 46.11759% -0.01145%
===================================================
Files 64 64
Lines 4030 4031 +1
Branches 679 679
===================================================
Hits 1859 1859
- Misses 1972 1973 +1
Partials 199 199
Continue to review full report at Codecov.
|
| export async function regionsGetter() { | ||
| const [regions, cursor] = await db.region.find({}, { limit: 100 }); | ||
| export function regionsGetter(halfRegionOrchestratorsUntrusted = false) { | ||
| return async function regionsGetter2() { |
There was a problem hiding this comment.
I think this works fine! One syntax sugar you could use to make this look a little better would be a lambda function, replacing async function regionsGetter2() { with async () => {. Mostly a stylistic choice anyway.
Another alternative, to avoid this helper function that returns a function (which I find a little confusing sometimes) is to curry the new boolean variable on the call site where you need this function with zero arguments instead. That is, instead of returning a function here you could just make this function receive the boolean and return the regions, and in app-router you do:
orchestratorGetters.push(() => regionsGetter(halfUntrusted...)).
This also fixes the issue of using the return value of this function (another function) when we actually want the final list of regions.
This is a totally optional/stylistic change anyway, only raising the possibility since you were a little concerned about the current version here.
| @@ -197,7 +198,9 @@ export default async function makeApp(params: CliArgs) { | |||
|
|
|||
| if (returnRegionInOrchestrator) { | |||
| app.use((req, res, next) => { | |||
| req.orchestratorsGetters.push(regionsGetter); | |||
| req.orchestratorsGetters.push( | |||
| regionsGetter(halfRegionOrchestratorsUntrusted) | |||
There was a problem hiding this comment.
Contextualizing the other comment, if regionsGetter is made to return the orchestrators directly instead of a function, here you could have:
() => regionsGetter(halfRegionO...)
Then this function indirection stays localized here, which is where we actually need it. Other places can just use regionGetter (like the API controllers are using now)
There was a problem hiding this comment.
Awesome, that's what I was looking for thanks! Much nicer to curry it at the calling site and keep the actual function clean
What does this pull request do? Explain your changes. (required)
When a new flag is passed, mark 50% of the Orchestrators as "untrusted" to allow us to test Fast Verification on Staging, where there are no community Orchestrators.
Specific updates (required)
How did you test each of these updates (required)
Does this pull request close any open issues?
No
Checklist: