-
Notifications
You must be signed in to change notification settings - Fork 94
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
[BUG] - JHubApps Create App Page Selects only First ~100 Conda Environments #2599
Comments
Thanks for reporting this @kenafoster The issue will need to be moved to Nebari. Fetching from conda-store API happens on Nebari. Currently pagination is not used at all: https://github.com/nebari-dev/nebari/blob/f2f5a5b67a7e8eccacf1a8b2d77f03a9441ea26b[…]s/kubernetes/services/jupyterhub/files/jupyterhub/02-spawner.py |
For anyone picking this one, we just need to add:
here: Line 29 in f2f5a5b
and then iterate over all the pages until it runs out of all the pages. |
Is this endpoint documented somewhere? conda-store API reference does not seem to mention it: https://conda.store/conda-store/references/api A native pagination may be sub-optimal if users are adding environments while pages are being fetched, unless the list is guaranteed to be ordered in the order of environment creation in which case it is safe. |
I didn't saw it in the docs. Docs are mostly incomplete. I saw pagination API in the code:
The results are sorted by namespace and name. I don't think I follow you here though. The user could say on the page for 5 minutes and 100 environments could be created in that time theoretically by other users, but when the user is at that page, they already know which environment they are looking for and that should have already been created before they come on that page (Create App page). |
Say we have five environments A, B, D, E, F, and page size equals 2.
If the database commit happens after the second page is fetched, this could lead to the form getting replies:
If many users are creating environments, the admin would be randomly missing some environments. If the results were sorted by date of environment was created (/last modified if environments can be renamed), then this is not a problem, because the replies could be (assuming A, B, D, E, F were created in this order):
Does it make sense? |
Thanks for the explanation, yes it does makes sense, but in this case it's not a problem as we're not planning to do pagination in the frontend yet. The current plan is to fetch all the pages here and return all of them in a single call in this UI: Pagination in the frontend would require design and backend support, which we're not ready for at the moment. If there are thousands (or any large number) of environments to fetch and that causes issues with the UI component to become unusable, then we might prioritise pagination in the frontend or "pagination + search by name", but most likely not yet. |
I did not mean pagination on the frontend. I mean that while backend is fetching the pages to concatenate them, the state may change.
The linked fragment is executed when the frontend hits the API as this function is passed down here: Lines 59 to 60 in f2f5a5b
and gets executed from this route via an util helper here. If you were to replace: - c.JAppsConfig.conda_envs = get_conda_store_environments
+ c.JAppsConfig.conda_envs = get_conda_store_environments() it would remove this problem, but then the environments would not refresh as user creates them. Sorry, I do not mean to be an annoyance here, I just wanted to help by preventing a common pagination issue which could be very hard to debug later. Though since we already had this conversation I think that it is fine to proceed with the naive implementation with the caveat that ideally an issue on conda |
Ah I see
Yes, totally agree it would, but what I meant is user can always refresh the page to get the latest state at that point in the current implementation, since it's single API call.
Yep agree.
I would not say that. :) It's an extremely useful discussion and apologies for the delayed response.
Do we have an alternative here, without changing the conda-store API?
Yes agreed. |
No good ideas for alternatives; fetching all environments seem to be needed universally across different components, I think that conda-store should have an officially documented and fool-proof API for it. |
@krassowski can you open a conda-store issue and summarize the functionality that would be helpful here? |
Done - conda-incubator/conda-store#859. |
Naive approach that was mentioned in: nebari-dev#2599
Context
The Software Environment dropdown on the create page truncates after the first (apparently 98) environments in alpha order
This first came up when a user reported to that he couldn't see his conda store envs when he tried to create apps on a nebari deployment. Further testing verified this (username beginning with 'z' can not see their apps in the same env. Creating another env with a username starting with 'k' bumped a 't...' environment off the list in the dropdown)The Software Environment dropdown on the create page truncates after the first (apparently 98) environments in alpha order
Value and/or benefit
This is a bug - we can't assume that there will only be <100 valid options in the environment selector even after #2193 is addressed
Anything else?
No response
The text was updated successfully, but these errors were encountered: