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

feat: include application name in subscription el #10651

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dev-marek
Copy link
Contributor

@dev-marek dev-marek commented Feb 13, 2025

Issue

https://gravitee.atlassian.net/browse/APIM-5979

Description

include application name in subscription el

  • Remember to update gateway-api version before merging!

Additional context

gravitee-io/gravitee-gateway-api#268


📚  View the storybook of this branch here

@dev-marek dev-marek force-pushed the apim-5979-application-name-in-subscription branch from 3c40670 to 39b3784 Compare February 13, 2025 12:26
String applicationId = subscriptionDoc.get("application").toString();
return new SubscriptionApplicationPair(
subscriptionDoc,
getCollection("applications").find(Filters.eq("_id", applicationId)).first()
Copy link
Contributor

Choose a reason for hiding this comment

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

Another way to go about it could be:

  1. get a set of all the application ids from the subscriptions that you found
  2. search applications to recuperate all the applications with those ids
  3. create a HashMap in your class with the key as the application Id and the value as the application name or application
  4. for each subscription in your list, create an update model with the subscription id + application name, found via the HashMap with the subscription's application id

I'm not sure if this would be more "efficient", but it would be less network calls instead of calling the DB for every subscription that you find. WDYT?

Copy link
Contributor Author

@dev-marek dev-marek Feb 13, 2025

Choose a reason for hiding this comment

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

Being inspired by your suggestion, I decided to execute 1 bulkAction per 1 application, so that we have flat number of updates. What do you think now?

@jhaeyaert can you think about any further optimization/is this approach good enough for prod?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the way it is done is ok. I would add a split into bulks of, let's say 1000, instead of having 1 single bulk to avoid loading all in memory. We already saw > 90k applications in some hard cases.

@dev-marek dev-marek force-pushed the apim-5979-application-name-in-subscription branch 2 times, most recently from 8da4cf3 to 73c7496 Compare February 14, 2025 12:38
@dev-marek dev-marek force-pushed the apim-5979-application-name-in-subscription branch from ec18973 to c30694a Compare February 14, 2025 15:10
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.

3 participants