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

Query if projects are in LF #798

Merged

Conversation

myieye
Copy link
Contributor

@myieye myieye commented May 13, 2024

So, this works, but I have a couple concerns:

  1. I don't think it's guarunteed that Project.Type is available, so presumably sometimes we'll query more projects than we want to 🤷
  2. I think the mongo $in operator could get a bit overloaded if we tried to query lots (e.g. 3000) projects. I know we're not planning to, but...it would be kind of cool if we could show this in the project tables. I wonder if it would be smarter to just load all the projects and do the .Contains() in .NET?

E.g. based on this comment:

However, from engineering point of view, if N goes to a big number, it is better to let your business logic server to simulate the $in operation, either by batch or one-by-one.

This is simply because: memory in database servers is way more valuable than the memory in business logic servers.

image
image

image
image

Fixes #687

Copy link

github-actions bot commented May 13, 2024

UI unit Tests

11 tests  ±0   11 ✅ ±0   0s ⏱️ ±0s
 3 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit c2715cb. ± Comparison against base commit 2869a47.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented May 13, 2024

C# Unit Tests

35 tests   35 ✅  4s ⏱️
 8 suites   0 💤
 1 files     0 ❌

Results for commit c2715cb.

♻️ This comment has been updated with latest results.

@myieye myieye requested review from hahn-kev and rmunn May 13, 2024 15:13
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, I do think there's ways we could optimize performance to query all projects, but that's out of scope for now and the performance might be good enough as is. We can easily test performance on production with a custom query once the field exists.

@myieye myieye force-pushed the feat/687-setup-api-to-check-if-a-lf-classic-project-exists branch from 743f115 to c2715cb Compare May 14, 2024 08:26
@myieye myieye merged commit a4395b6 into develop May 14, 2024
14 checks passed
@myieye myieye deleted the feat/687-setup-api-to-check-if-a-lf-classic-project-exists branch May 14, 2024 09:39
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

Successfully merging this pull request may close these issues.

setup api to check if a LF classic project exists
2 participants