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

Slow loading /users and /assets listings #964

Open
Flix6x opened this issue Jan 22, 2024 · 5 comments · May be fixed by #988
Open

Slow loading /users and /assets listings #964

Flix6x opened this issue Jan 22, 2024 · 5 comments · May be fixed by #988

Comments

@Flix6x
Copy link
Contributor

Flix6x commented Jan 22, 2024

I'm noticing the /users (and the /assets) page can take seconds to load if there are many organisation accounts. Internally, the /api/v3_0/users?account_id=<id> endpoint is getting called as many times as there are organisation accounts that the user has access to, so there is great potential to speed this up. Two options that come to mind:

  1. Allow passing account_id multiple times to the users listing endpoint, so we only need to call it once. This would probably require the permission_required_for_context decorator to be enhanced to support a list of accounts.
  2. Alternatively, we stop relying on the internal API so much. For example, instead of users += get_users_by_account(account.id, include_inactive) (which uses the InternalAPI), use users += account.users and then filter out the inactive users if needed.

Arguably, option 2 is easier to implement, but option 1 actually improves API and auth capabilities.

@nhoening nhoening added this to the 0.20 milestone Feb 23, 2024
@victorgarcia98
Copy link
Contributor

I endorse this idea, we need to improve loading times.

In order to have a more informed process, I would like to create a script to load dummy data and perform a benchmark on this endpoint. We can try loading times with different combinations of account/users/assets. What do you think?

Among the two alternatives, I would favor more (1) as it doesn't break with the status quo.

@nhoening
Copy link
Contributor

We had the idea for a benchmark script here but didn't find the right underlying approach yet.

@victorgarcia98
Copy link
Contributor

Thanks for the reference! #949 apparently benchmarks fetching data from the DB (via calling show beliefs command). I was planning to create something that actually hit the endpoint to see the response time. What do you think?

@nhoening
Copy link
Contributor

I agree the approach needs not to call CLI commands, also due to their initial loading time. I thought about calling code directly (and first establishing an app context), then we can call whatever function we like.

But probably simply using the API might be more straightforward.

@Flix6x Flix6x changed the title Slow loading users listing Slow loading /users and /assets listings Feb 23, 2024
@victorgarcia98 victorgarcia98 linked a pull request Feb 26, 2024 that will close this issue
2 tasks
@victorgarcia98
Copy link
Contributor

victorgarcia98 commented Feb 26, 2024

I have create a WIP PR to speed up the loading times of the /users and /assets page. It relies on the fact that the we already propagate authentication to the internal API and we can use the index methods instead of calling get methods on the individual accounts, as suggested by @Flix6x .

Benchmark script

import requests
from bs4 import BeautifulSoup
import time

session = requests.Session()
session.max_redirects = 3

# get CSRF Token
response = session.get("http://localhost:5000")
soup = BeautifulSoup(response.text, "html.parser")
csrf_token = soup.find("input", {"id" : "csrf_token"})["value"]

## Login
response = session.post(
    "http://localhost:5000/login",
    data={
        "email" : "<email>",
        "password" : "<password>",
        "submit" : "Login",
        "next" : "",
        "csrf_token" : csrf_token
    },
)


for _ in range(10):
    t = time.time()
    response = session.get("http://localhost:5000/assets")
    print(time.time() - t)
    soup = BeautifulSoup(response.text, "html.parser")
    print(soup.title.get_text())  # we should see  Asset listing  - FlexMeasures

@Flix6x Flix6x modified the milestones: 0.20.0, 0.21.0 Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants