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

[BUG] - JHubApps Create App Page Selects only First ~100 Conda Environments #2599

Open
kenafoster opened this issue Jul 30, 2024 · 11 comments
Open
Assignees

Comments

@kenafoster
Copy link
Contributor

kenafoster commented Jul 30, 2024

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

@kenafoster kenafoster added the needs: triage 🚦 Someone needs to have a look at this issue and triage label Jul 30, 2024
@kcpevey kcpevey added area: user experience 👩🏻‍💻 type: bug 🐛 Something isn't working and removed needs: triage 🚦 Someone needs to have a look at this issue and triage labels Jul 30, 2024
@aktech
Copy link
Member

aktech commented Jul 31, 2024

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

@aktech aktech transferred this issue from nebari-dev/jhub-apps Jul 31, 2024
@aktech
Copy link
Member

aktech commented Jul 31, 2024

For anyone picking this one, we just need to add:

/conda-store/api/v1/environment/?page=1&size=100

here:

and then iterate over all the pages until it runs out of all the pages.

@krassowski
Copy link
Member

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.

@aktech
Copy link
Member

aktech commented Jul 31, 2024

Is this endpoint documented somewhere? conda-store API reference does not seem to mention it: conda.store/conda-store/references/api

I didn't saw it in the docs. Docs are mostly incomplete. I saw pagination API in the code:

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.

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).

@krassowski
Copy link
Member

The results are sorted by namespace and name.

I don't think I follow you here though.

Say we have five environments A, B, D, E, F, and page size equals 2.
User performs two actions:

  • requested creating environment called "C", and then
  • opens the form in the other page

If the database commit happens after the second page is fetched, this could lead to the form getting replies:

  • A, B - first page (ok)
  • D, E - second page (ok at the time)
  • E, F - third page - oops!

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):

  • A, B
  • D, E
  • F, C

Does it make sense?

@aktech
Copy link
Member

aktech commented Aug 1, 2024

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:

Screenshot 2024-08-01 at 12 51 06 pm

Pagination in the frontend would require design and backend support, which we're not ready for at the moment.
In the race condition of an environment not showing up yet (while it's being committed to db or still building), we do expect the user to know the environment needs to be built completely to show up in the list and user would refresh the page to get the latest list at that point.

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.

@krassowski
Copy link
Member

krassowski commented Aug 1, 2024

we're not planning to do pagination in the frontend 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 current plan is to fetch all the pages here and return all of them in a single call in this UI

The linked fragment is executed when the frontend hits the API as this function is passed down here:

c.JAppsConfig.conda_envs = get_conda_store_environments
c.JAppsConfig.jupyterhub_config_path = (

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-forge-store would be opened asking for ability to sort by creation date to prevent this issue.

@aktech
Copy link
Member

aktech commented Aug 1, 2024

I did not mean pagination on the frontend.

Ah I see

I mean that while backend is fetching the pages to concatenate them, the state may change.

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.

it would remove this problem, but then the environments would not refresh as user creates them.

Yep agree.

Sorry, I do not mean to be an annoyance here,

I would not say that. :) It's an extremely useful discussion and apologies for the delayed response.

Though since we already had this conversation I think that it is fine to proceed with the naive implementation

Do we have an alternative here, without changing the conda-store API?

with the caveat that ideally an issue on conda-forge-store would be opened asking for ability to sort by creation date to prevent this issue.

Yes agreed.

@krassowski
Copy link
Member

Do we have an alternative here, without changing the conda-store API?

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.

@kcpevey
Copy link
Contributor

kcpevey commented Aug 2, 2024

@krassowski can you open a conda-store issue and summarize the functionality that would be helpful here?

@krassowski
Copy link
Member

@krassowski can you open a conda-store issue and summarize the functionality that would be helpful here?

Done - conda-incubator/conda-store#859.

BrianCashProf added a commit to MetroStar/nebari that referenced this issue Aug 5, 2024
Naive approach that was mentioned in:
nebari-dev#2599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New 🚦
Development

No branches or pull requests

5 participants