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

Fix configuration settings for configuration class. #1130

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

jpwhite4
Copy link
Member

The new configuration API has an odd feature where the default settings
have this behavior:
if the parsed data has a top level json array and there is one entry in the array then the
entry is returned, If there are multiple entries in the array then an
array is returned. This requires the developer to handle the one and
many cases separately.

This lead to a bug because the code assumed that the data was always being returned in
an array. This change sets the configuration class parameter force_array_return
so that the resource information is always returned in an array data structure.

This API design seems bug prone since the defaults settings are almost always not what you want. See issue for follow up: https://app.asana.com/0/342819846538629/1146629814310601

Ensure that the configuration class parameter force_array_return
is set so that the resource information is returned in an array
datastructure.
@ryanrath
Copy link
Contributor

Just an explanation for this behavior is that that’s how The original ‘Config’ class worked. And changing the behavior was outside of the original scope of that particular PR.

@jtpalmer jtpalmer added this to the 8.5.1 milestone Oct 28, 2019
@jtpalmer jtpalmer added bug Bugfixes Category:General General labels Oct 28, 2019
@jpwhite4 jpwhite4 merged commit b5d783a into ubccr:xdmod8.5 Oct 28, 2019
@jpwhite4 jpwhite4 deleted the array_return branch October 28, 2019 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes Category:General General
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants