add support for configurable entities limit for the connections browser#1403
add support for configurable entities limit for the connections browser#1403GnsP wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request makes the wrangler browse entities limit configurable by introducing the ui.wrangler.connections.browse.entities.limit setting. The reviewer recommends keeping the default fallback limit at 1000 instead of increasing it to 2000 to maintain backward compatibility and prevent performance degradation. Additionally, the reviewer points out that evaluating the configuration at module load time in apiHelpers.ts could lead to initialization issues, suggesting retrieving the value dynamically instead.
|
|
||
| export const ENTITY_TRUNCATION_LIMIT = 1000; | ||
| export const ENTITY_TRUNCATION_LIMIT_SETTING = 'defaultWranglerBrowseEntitiesLimit'; | ||
| export const ENTITY_TRUNCATION_LIMIT = getCdapConfig(ENTITY_TRUNCATION_LIMIT_SETTING, 2000); |
There was a problem hiding this comment.
The default limit was previously 1000. Increasing the default fallback to 2000 might lead to performance degradation or larger payloads for users who have not explicitly configured this setting. It is recommended to keep the default fallback as 1000 to maintain backward compatibility.\n\nAdditionally, evaluating ENTITY_TRUNCATION_LIMIT at module load time means its value is resolved once when the file is imported. If window.CDAP_CONFIG is not yet initialized (which is common in unit tests or early bootstrap phases), it will permanently use the fallback value. Consider retrieving this value dynamically inside exploreConnection (or using a getter function) to ensure it always reflects the active configuration.
| export const ENTITY_TRUNCATION_LIMIT = getCdapConfig(ENTITY_TRUNCATION_LIMIT_SETTING, 2000); | |
| export const ENTITY_TRUNCATION_LIMIT = getCdapConfig(ENTITY_TRUNCATION_LIMIT_SETTING, 1000); |
| hstsPreload: cdapConfig['hsts.preload'], | ||
| runRecordsTtl: cdapConfig['app.run.records.ttl.days'], | ||
| defaultPollIntervalMs: parseInt(cdapConfig['ui.default.poll.interval.millis'], 10) || 10000, | ||
| defaultWranglerBrowseEntitiesLimit: parseInt(cdapConfig['ui.wrangler.connections.browse.entities.limit'], 10) || 2000, |
There was a problem hiding this comment.
To maintain consistency with the previous default limit of 1000 and avoid potential performance issues for unconfigured environments, consider keeping the fallback value as 1000 instead of 2000.
| defaultWranglerBrowseEntitiesLimit: parseInt(cdapConfig['ui.wrangler.connections.browse.entities.limit'], 10) || 2000, | |
| defaultWranglerBrowseEntitiesLimit: parseInt(cdapConfig['ui.wrangler.connections.browse.entities.limit'], 10) || 1000, |
| "hsts.preload": "true", | ||
| "ui.default.poll.interval.millis": "10000" | ||
| "ui.default.poll.interval.millis": "10000", | ||
| "ui.wrangler.connections.browse.entities.limit": "2000" |
There was a problem hiding this comment.
Consider setting this to 1000 to align with the default limit, unless 2000 is specifically desired as the default for the development environment.
| "ui.wrangler.connections.browse.entities.limit": "2000" | |
| "ui.wrangler.connections.browse.entities.limit": "1000" |
radhikav1
left a comment
radhikav1
left a comment
There was a problem hiding this comment.
Nit: Please check if we need to document this change anywhere like guides or How-Tos etc..
3 participants
configurable rows limit for the connections browser
PR Type
Links
Jira: Jira issue #
Test Plan
Screenshots