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

Bulk user fetch for list of IDs #1209

Open
Tracked by #849
david-crespo opened this issue Jun 14, 2022 · 8 comments
Open
Tracked by #849

Bulk user fetch for list of IDs #1209

david-crespo opened this issue Jun 14, 2022 · 8 comments

Comments

@david-crespo
Copy link
Contributor

david-crespo commented Jun 14, 2022

As a kind of workaround for #1207, it would be helpful to be able to fetch info on a specified set of users. We could put a comma-separated list of IDs in the query string.

I was initially concerned about the query string getting too long (UUID v4 is 36 characters), but as far as I can tell browser limits on query string length are pretty high. It's only Edge that's imposing any kind of limit we'd worry about, but I can't tell from various garbagey websites what exactly the limit is and whether it applies to background requests (it probably does). Some places claim it's around 2k chars, others claim it's 4k chars. It's worth looking into this further. If it doesn't work in the query string, we could use a POST.

image

image

@davepacheco
Copy link
Collaborator

Is it even worth doing this in bulk at this point or is that premature optimization?

@david-crespo
Copy link
Contributor Author

Well, so far the number of users is almost always less than the max page size of 1000. But that could change soon and suddenly, couldn't it?

@davepacheco
Copy link
Collaborator

For sure. I was envisioning an API that fetches the details for one user, identified by id. I realize this could get bad in pathological cases. I'm expecting that most resources aren't going to have that many items in the policy, especially if customers are inclined to use groups.

@david-crespo
Copy link
Contributor Author

Yeah, fetching for one user doesn't really help much if we're trying to display the table. I agree that groups are going to be the main way this is done. We also have this problem for group names, though in general there should be a lot fewer of them, so fetching all of them has a much longer lifetime as a temporary solution.

@davepacheco
Copy link
Collaborator

Can't you use the one-at-a-time API the same way you'd use the bulk API, just making separate requests?

Yeah, I think we'd need an analogous ones for groups.

@david-crespo
Copy link
Contributor Author

You can, but browsers limit the number of concurrent requests to 4-8 (source), so that gets sad fast if you're doing 20. It could be ok for groups, though in that case I'd probably just pull them all in one request.

@davepacheco
Copy link
Collaborator

I imagine another prerequisite here is having any useful information about Silo Users. In #1261 they'll only have an id, so this doesn't seem useful. After #1210 they'll have an external_id, but even that may not generally be useful.

@david-crespo
Copy link
Contributor Author

Yes, definitely. That broad problem may be a big deal for role assignment in the console. The UI for picking a user is not usable if you don't have a human-readable display name to choose them by.

leftwo added a commit that referenced this issue Mar 29, 2024
Propolis changes:
Add `IntrPin::import_state` and migrate LPC UART pin states (#669)
Attempt to set WCE for raw file backends
Fix clippy/lint nits for rust 1.77.0

Crucible changes:
Correctly (and robustly) count bytes (#1237)
test-replay.sh fix name of DTrace script (#1235)
BlockReq -> BlockOp (#1234)
Simplify `BlockReq` (#1218)
DTrace, cmon, cleanup, retry downstairs connections at 10 seconds.
(#1231)
Remove `MAX_ACTIVE_COUNT` flow control system (#1217)

Crucible changes that were in Omicron but not in Propolis before this commit.
Return *410 Gone* if volume is inactive (#1232)
Update Rust crate opentelemetry to 0.22.0 (#1224)
Update Rust crate base64 to 0.22.0 (#1222)
Update Rust crate async-recursion to 1.1.0 (#1221)
Minor cleanups to extent implementations (#1230)
Update Rust crate http to 0.2.12 (#1220)
Update Rust crate reedline to 0.30.0 (#1227)
Update Rust crate rayon to 1.9.0 (#1226)
Update Rust crate nix to 0.28 (#1223)
Update Rust crate async-trait to 0.1.78 (#1219)
Various buffer optimizations (#1211)
Add low-level test for message encoding (#1214)
Don't let df failures ruin the buildomat tests (#1213)
Activate the NBD server's psuedo file (#1209)

---------

Co-authored-by: Alan Hanson <alan@oxide.computer>
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

No branches or pull requests

2 participants