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

Add configuration property defaultPageSize #2235

Merged
merged 3 commits into from
Apr 12, 2021

Conversation

csandersdev
Copy link
Contributor

@csandersdev csandersdev commented Apr 9, 2021

This address #2229 by creating a configuration property
fhirServer/core/defaultPageSize that controls the number of records
retrieved by search and history when no _count parameter is specified in the
request. This parameter honors SearchConstants.MAX_PAGE_SIZE (1000). If
the server configuration specifies a value greater than MAX_PAGE_SIZE, a
warning is logged and MAX_PAGE_SIZE is used. When no configuration
property is specified the previous default
FHIRConstants.FHIR_PAGE_SIZE_DEFAULT (10) will be used.

Signed-off-by: Corey Sanders corey.thecolonel@gmail.com

This address LinuxForHealth#2229 by creating a configuration property
fhirServer/core/defaultPageSize that controls the number of records
retrieved by search when no _count parameter is specified in the
request. This parameter honors SearchConstants.MAX_PAGE_SIZE (1000). If
the server configuration specifies a value greater than MAX_PAGE_SIZE, a
warning is logged and MAX_PAGE_SIZE is used. When no configuration
property is specified the previous default
FHIRConstants.FHIR_PAGE_SIZE_DEFAULT (10) will be used.

Signed-off-by: Corey Sanders <corey.thecolonel@gmail.com>
Copy link
Contributor

@prb112 prb112 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Corey Sanders <corey.thecolonel@gmail.com>
@@ -1953,6 +1953,7 @@ This section contains reference information about each of the configuration prop
|`fhirServer/core/capabilityStatementCacheTimeout`|integer|The number of minutes that a tenant's CapabilityStatement is cached for the metadata endpoint. |
|`fhirServer/core/extendedCodeableConceptValidation`|boolean|A boolean flag which indicates whether extended validation is performed by the server during object construction for code, Coding, CodeableConcept, Quantity, Uri, and String elements which have required bindings to value sets.|
|`fhirServer/core/disabledOperations`|string|A comma-separated list of operations which are not allowed to run on the IBM FHIR Server, for example, `validate,import`. Note, do not include the dollar sign `$`|
|`fhirServer/core/defaultPageSize`|integer|Sets the pageSize to use in search when no _count parameter is specified in the request. If a user-specified value exceeds the max page size (1000), then a warning is logged and max page size will be used. If not provided, the default page size (10) is used.|
Copy link
Member

Choose a reason for hiding this comment

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

I assume it applies to history interactions as well

Suggested change
|`fhirServer/core/defaultPageSize`|integer|Sets the pageSize to use in search when no _count parameter is specified in the request. If a user-specified value exceeds the max page size (1000), then a warning is logged and max page size will be used. If not provided, the default page size (10) is used.|
|`fhirServer/core/defaultPageSize`|integer|Sets the pageSize to use in search and history interactions when no _count parameter is specified in the request. If a user-specified value exceeds the max page size (1000), then a warning is logged and max page size will be used. If not provided, the default page size (10) is used.|

Copy link
Member

Choose a reason for hiding this comment

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

Also, its a bit confusing, but we have 3 separate tables in section 5.1. You added this property to the table that has the description. However, 5.1.2 has the default value and 5.1.3 indicates whether the parameter is tenant-specific (different values allowed for different tenants) and dynamic (changes take affect without a server restart).
We used to have all of this in a single table, but it was too wide for just about any documentation format.
For 5.1.2, just add a row and indicate the default is 10.
For 5.1.3, since you're using FHIRConfigHelper, this property will be dynamic and tenant-specific, so thats just Y|Y.

- doc updates
- copyright fixes
- remove log guards on warning messages

Signed-off-by: Corey Sanders <corey.thecolonel@gmail.com>
@lmsurpre lmsurpre self-requested a review April 12, 2021 14:08
Copy link
Member

@lmsurpre lmsurpre left a comment

Choose a reason for hiding this comment

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

LGTM

@lmsurpre lmsurpre merged commit eaba471 into LinuxForHealth:main Apr 12, 2021
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