Skip to content

Fixed bug where sum returns wrong values with long data type #1

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

Draft
wants to merge 272 commits into
base: integ-sum-fix
Choose a base branch
from

Conversation

matthewryanwells
Copy link
Owner

Description

Fixes a bug regarding sum returning an incorrect value when working with large long values because the current code uses the double data type. In addition we also extracted code for checking null source into it's own class so it doesn't need to be checked for/

Issues Resolved

opensearch-project#5537
opensearch-project/sql#1052

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Comment on lines 9 to 10
/*
* Licensed to Elasticsearch under one or more contributor

Choose a reason for hiding this comment

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

I think you can omit this header

Copy link
Owner Author

Choose a reason for hiding this comment

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

I do not know if this is something that I should omit as every other file in the server section of the code shares the same header, I think it might be some legal thing.

Comment on lines 9 to 10
/*
* Licensed to Elasticsearch under one or more contributor

Choose a reason for hiding this comment

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

The same

@GumpacG
Copy link

GumpacG commented Apr 3, 2023

Could you add tests to verify?

reta and others added 25 commits April 4, 2023 16:00
…st (opensearch-project#6979)

* [Segment Replication] Fix flaky testReplicaHasDiffFilesThanPrimary test

Signed-off-by: Suraj Singh <[email protected]>

* Use existing doc assertion methods

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Suraj Singh <[email protected]>
…#6968)

* Remove LinkedDeque and replace with LinkedHashMap

After the recent changes the usage of the LinkedDeque fits quite well
with the insertion order semantics of LinkedHashMap, which also allows
for constant time additions and removals.

Signed-off-by: Andrew Ross <[email protected]>

* Use class member reference now that lock is final

Signed-off-by: Andrew Ross <[email protected]>

---------

Signed-off-by: Andrew Ross <[email protected]>
…versions (opensearch-project#6730)

* compatCheck

Signed-off-by: Poojita Raj <[email protected]>

* refactor

Signed-off-by: Poojita Raj <[email protected]>

---------

Signed-off-by: Poojita Raj <[email protected]>
…hards. (opensearch-project#6850)

* Add new background task to fail stale replica shards.

Signed-off-by: Rishikesh1159 <[email protected]>

* Add condition to check if backpressure is enabled.

Signed-off-by: Rishikesh1159 <[email protected]>

* Fix failing tests.

Signed-off-by: Rishikesh1159 <[email protected]>

* Fix failing tests by adding manual refresh.

Signed-off-by: Rishikesh1159 <[email protected]>

* Address comments on PR.

Signed-off-by: Rishikesh1159 <[email protected]>

* Addressing comments on PR.

Signed-off-by: Rishikesh1159 <[email protected]>

* Update background task logic to fail stale replicas of only one shardId's in a single iteration of background task.

Signed-off-by: Rishikesh1159 <[email protected]>

* Fix failing import.

Signed-off-by: Rishikesh1159 <[email protected]>

* Address comments.

Signed-off-by: Rishikesh1159 <[email protected]>

* Add code doc to SEGMENT_REPLICATION_INDEXING_PRESSURE_ENABLED setting.

Signed-off-by: Rishikesh1159 <[email protected]>

---------

Signed-off-by: Rishikesh1159 <[email protected]>
…/repository-azure (opensearch-project#6942)

* Bump com.azure:azure-storage-blob in /plugins/repository-azure

Bumps [com.azure:azure-storage-blob](https://github.com/Azure/azure-sdk-for-java) from 12.20.0 to 12.21.1.
- [Release notes](https://github.com/Azure/azure-sdk-for-java/releases)
- [Commits](Azure/azure-sdk-for-java@azure-storage-blob_12.20.0...azure-storage-blob_12.21.1)

---
updated-dependencies:
- dependency-name: com.azure:azure-storage-blob
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

* Change the location of adding changelog from Unreleased 3.0 to 2.x

Signed-off-by: Tianli Feng <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Tianli Feng <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tianli Feng <[email protected]>
…16.12 (opensearch-project#6940)

* Bump com.avast.gradle:gradle-docker-compose-plugin

Bumps [com.avast.gradle:gradle-docker-compose-plugin](https://github.com/avast/gradle-docker-compose-plugin) from 0.16.11 to 0.16.12.
- [Release notes](https://github.com/avast/gradle-docker-compose-plugin/releases)
- [Commits](avast/gradle-docker-compose-plugin@0.16.11...0.16.12)

---
updated-dependencies:
- dependency-name: com.avast.gradle:gradle-docker-compose-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…cation opensearch-project#6644 (opensearch-project#6765)

* Segment Replication - PIT/Scroll compatibility.

This change makes updates to make PIT/Scroll queries compatibile with Segment Replication.
It does this by refcounting files when a new reader is created, and discarding those files after
a reader is closed.

Signed-off-by: Marc Handalian <[email protected]>

* Fix broken test.

Signed-off-by: Marc Handalian <[email protected]>

* Fix test bug with PIT where snapshotted segments are queried instead of current store state.

Signed-off-by: Marc Handalian <[email protected]>

* Address review comments and prevent temp file deletion during reader close

Signed-off-by: Suraj Singh <[email protected]>

* Fix precommit failure

Signed-off-by: Suraj Singh <[email protected]>

* Use last committed segment infos reference from replication engine

Signed-off-by: Suraj Singh <[email protected]>

* Clean up and prevent incref on segment info file copied from primary

Signed-off-by: Suraj Singh <[email protected]>

* Fix failing test

Signed-off-by: Suraj Singh <[email protected]>

---------

Signed-off-by: Marc Handalian <[email protected]>
Signed-off-by: Suraj Singh <[email protected]>
Co-authored-by: Marc Handalian <[email protected]>
…-codecs from sandbox/modules to sandbox/plugins (so it won't be installed automatically) (opensearch-project#7014)

Signed-off-by: Andriy Redko <[email protected]>
…ins/ingest-attachment (opensearch-project#6946)

* Bump org.apache.commons:commons-compress in /plugins/ingest-attachment

Bumps org.apache.commons:commons-compress from 1.22 to 1.23.0.

---
updated-dependencies:
- dependency-name: org.apache.commons:commons-compress
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
… /plugins/repository-hdfs (opensearch-project#6945)

* Bump org.apache.commons:commons-configuration2

Bumps org.apache.commons:commons-configuration2 from 2.8.0 to 2.9.0.

---
updated-dependencies:
- dependency-name: org.apache.commons:commons-configuration2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…0 in /buildSrc (opensearch-project#6943)

* Bump com.netflix.nebula:nebula-publishing-plugin in /buildSrc

Bumps [com.netflix.nebula:nebula-publishing-plugin](https://github.com/nebula-plugins/nebula-publishing-plugin) from 19.2.0 to 20.2.0.
- [Release notes](https://github.com/nebula-plugins/nebula-publishing-plugin/releases)
- [Changelog](https://github.com/nebula-plugins/nebula-publishing-plugin/blob/main/CHANGELOG.md)
- [Commits](nebula-plugins/nebula-publishing-plugin@v19.2.0...v20.2.0)

---
updated-dependencies:
- dependency-name: com.netflix.nebula:nebula-publishing-plugin
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…fs-fixture (opensearch-project#6944)

* Bump net.minidev:json-smart in /test/fixtures/hdfs-fixture

Bumps [net.minidev:json-smart](https://github.com/netplex/json-smart-v2) from 2.4.9 to 2.4.10.
- [Release notes](https://github.com/netplex/json-smart-v2/releases)
- [Commits](netplex/json-smart-v2@2.4.9...2.4.10)

---
updated-dependencies:
- dependency-name: net.minidev:json-smart
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…ribution/packages (opensearch-project#6941)

* Bump com.netflix.nebula.ospackage-base in /distribution/packages

Bumps com.netflix.nebula.ospackage-base from 11.0.0 to 11.2.0.

---
updated-dependencies:
- dependency-name: com.netflix.nebula.ospackage-base
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
* Bump version of protobuf-java to 3.22.2

Signed-off-by: Tianli Feng <[email protected]>
…vailable=true (opensearch-project#6839)

* Fix bug for Get Snapshot API to return correct response when getting a non-existing snapshot (opensearch-project#6820)

Signed-off-by: Gao Binlong <[email protected]>

* modify change log

Signed-off-by: Gao Binlong <[email protected]>

* Modify changelog

Signed-off-by: Gao Binlong <[email protected]>

---------

Signed-off-by: Gao Binlong <[email protected]>
…feature flag. (opensearch-project#7006)

* Add new SEGMENT_REPLICATION_CLUSTER_SETTING feature flag.

Signed-off-by: Rishikesh1159 <[email protected]>

* Remove entry from changelog as the cluster setting introduced is noy ready for users to use.

Signed-off-by: Rishikesh1159 <[email protected]>

* Fix failing unit tests.

Signed-off-by: Rishikesh1159 <[email protected]>

* Address comments on PR.

Signed-off-by: Rishikesh1159 <[email protected]>

* Address comments on PR and chnage feature flag to SEGMENT_REPLICATION_EXPERIMENTAL.

Signed-off-by: Rishikesh1159 <[email protected]>

---------

Signed-off-by: Rishikesh1159 <[email protected]>
* Add FlatObject FieldMapper

Signed-off-by: Mingshi Liu <[email protected]>

* resolve import package for HttpHost

Signed-off-by: Mingshi Liu <[email protected]>

* Dynamic Create FlatObjectFieldType for dotpath field

Signed-off-by: Mingshi Liu <[email protected]>

* Rename flat-object to flat_object and fix CI tests

Signed-off-by: Mingshi Liu <[email protected]>

* Organized package

Signed-off-by: Mingshi Liu <[email protected]>

* resolved compile error

Signed-off-by: Mingshi Liu <[email protected]>

* organize package

Signed-off-by: Mingshi Liu <[email protected]>

* Add integration tests and remove benchmark

Signed-off-by: Mingshi Liu <[email protected]>

* fix IT tests

Signed-off-by: Mingshi Liu <[email protected]>

* Skip IT tests before 2.7.0

Signed-off-by: Mingshi Liu <[email protected]>

* Revert "Skip IT tests before 2.7.0"

Signed-off-by: Mingshi Liu <[email protected]>

* Add more IT tests for supported queries

Signed-off-by: Mingshi Liu <[email protected]>

* Removed license head and add tests for wildcard query

Signed-off-by: Mingshi Liu <[email protected]>

* Add tests for array, nested-arrary, number and float

Signed-off-by: Mingshi Liu <[email protected]>

* Upgrade FlatObjectFieldMapperTests to  MapperTestCase

- Upgrading `FlatObjectFieldMapperTests` from `MapperServiceTestCase` to `MapperTestCase`. The `MapperTestCase` explicitly forces us to:
	- Test parameter updates (empty now)
	- Explicitly specify if the field supports Meta and Boost (if not, relevant tests are automatically skipped)
- Test also the substring fields
- Add new test `testMapperServiceHasParser` to verify the new `flat_object` field mapper is present in mapper service registry
- Remove duplicated test and assertions methods
- Removed `testExistsQueryDocValuesDisabledWithNorms` as this test was not adding much. We shall reintroduce similar test later if we decide that we want to support ExistsQuery and what to do if DocValue are disabled and Norms enabled.

Signed-off-by: Lukáš Vlček <[email protected]>

* Add exist query in FlatObjectFieldMapperTests and FlatObjectFieldDataTests

Signed-off-by: Mingshi Liu <[email protected]>

* Add IT tests for painless query in testDocValues

Signed-off-by: Mingshi Liu <[email protected]>

* Add filter script (Painless) test for flat_object

While it is not possible to use flat_object field in scripting filter context to access doc values (like `doc[<flat_object>.<field_x>]`) it is possible to call `doc[<flat_object>].size()` to get number of fields inside the flat_object field.

- Reorganize flat_object yaml tests into sections:
  - Mappings
  - Supported
  - Unsupported
- Scripting (Painless) yamlRest tests need to go into lang-painless module

Signed-off-by: Lukáš Vlček <[email protected]>

* Removed Normalizer

Signed-off-by: Mingshi Liu <[email protected]>

* removed unused codes

Signed-off-by: Mingshi Liu <[email protected]>

* Remove non-relevant Javadoc from DynamicKeyFieldMapper

Signed-off-by: Lukáš Vlček <[email protected]>

* Improve flat_object scripting test

Make it more obvious what the `doc[<flat_field>].size()` number represents.

Signed-off-by: Lukáš Vlček <[email protected]>

* Add test for mapping parameters

Mapping parameters are not allowed in the initial version. This commit adds a test to demonstrate that trying to specify index/search analyzer for the flat_object field will fail.

Signed-off-by: Lukáš Vlček <[email protected]>

* remove IndexAnalyzer from Builder

Signed-off-by: Mingshi Liu <[email protected]>

* remove IndexAnalyzer from Builder

Signed-off-by: Mingshi Liu <[email protected]>

---------

Signed-off-by: Mingshi Liu <[email protected]>
Signed-off-by: Lukáš Vlček <[email protected]>
Co-authored-by: Lukáš Vlček <[email protected]>
dependabot bot and others added 27 commits May 29, 2023 15:51
* Bump org.gradle.test-retry from 1.5.2 to 1.5.3

Bumps org.gradle.test-retry from 1.5.2 to 1.5.3.

---
updated-dependencies:
- dependency-name: org.gradle.test-retry
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…ion/tools/plugin-cli (opensearch-project#7808)

* Bump com.google.guava:guava in /distribution/tools/plugin-cli

Bumps [com.google.guava:guava](https://github.com/google/guava) from 31.1-jre to 32.0.0-jre.
- [Release notes](https://github.com/google/guava/releases)
- [Commits](https://github.com/google/guava/commits)

---
updated-dependencies:
- dependency-name: com.google.guava:guava
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…ion/tools/upgrade-cli (opensearch-project#7807)

* Bump com.google.guava:guava in /distribution/tools/upgrade-cli

Bumps [com.google.guava:guava](https://github.com/google/guava) from 31.1-jre to 32.0.0-jre.
- [Release notes](https://github.com/google/guava/releases)
- [Commits](https://github.com/google/guava/commits)

---
updated-dependencies:
- dependency-name: com.google.guava:guava
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update changelog

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
Refactoring Build to core library broke the build metadata. Test
coverage was insufficient to catch this bug. This commit fixes the
build metadata and improves test coverage such that any unexpected
metadata changes are caught.

Signed-off-by: Nicholas Walter Knize <[email protected]>
Co-authored-by: Nicholas Walter Knize <[email protected]>
Co-authored-by: Thomas Farr <[email protected]>
…ory-hdfs (opensearch-project#7812)

* Bump net.minidev:json-smart in /plugins/repository-hdfs

Bumps [net.minidev:json-smart](https://github.com/netplex/json-smart-v2) from 2.4.10 to 2.4.11.
- [Release notes](https://github.com/netplex/json-smart-v2/releases)
- [Commits](netplex/json-smart-v2@2.4.10...2.4.11)

---
updated-dependencies:
- dependency-name: net.minidev:json-smart
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
This commit adds a WriteableRegistry to register generic read / write
serialization methods for StreamInput.readGeneric and
StreamOutput.writeGeneric. This enables modules, plugins, and libraries
to register their own type serialization logic without forcing the class
implementation to be in the :server module.

Decoupling this logic also further decouples the StreamInput /
StreamOutput transport logic from the server module such that it can be
refactored to the core library to support cloud native and serverless
implementations.

Signed-off-by: Nicholas Walter Knize <[email protected]>
* 1. reduce memory copy in zstd compression

Signed-off-by: luyuncheng <[email protected]>

* Add Change log

Signed-off-by: luyuncheng <[email protected]>

---------

Signed-off-by: luyuncheng <[email protected]>
* Add average,min,max and thread info to tasks response

Signed-off-by: Jay Deng <[email protected]>

* Update server/src/main/java/org/opensearch/tasks/TaskResourceStats.java

Signed-off-by: Jay Deng <[email protected]>

---------

Signed-off-by: Jay Deng <[email protected]>
Changes:

- Added ZSTD compressor for snapshotting
- 2 JSON repository settings:
  - readonly
  - compression
were moved into the BlobStoreRepository class
and removed from other repos classes where they
were used.

Signed-off-by: Andrey Pleskach <[email protected]>
* AWS SDK v2.
* Migrating com.amazonaws.sdk.ec2MetadataServiceEndpointOverride to aws.ec2MetadataServiceEndpoint per https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/migration-client-changes.html.
* Fetch host address for EC2 metadata service from SdkSettings.
* Deprecations of running with partial keys were removed in AWS SDK v2.
* Underscore is not a valid host name, thus URI.create doesn't treat it as a host.
* Added missing JARs, all tests pass.
* Setup a default AWS profile under config, don't read ~/.aws.
* Re-enable availability zone node attribute implementation.
* Cleaning up privileged calls using SocketAccess
* Consistently setup and teardown AWS settings in repository-s3.
* Using SdkException instead of RuntimeException in S3BlobContainer
* Removing AwsSessionCredentials object from S3BasicSesstionCredentials
* Removing S3BasicCredentials and S3BasicSessionCredentials

Signed-off-by: dblock <[email protected]>
Co-authored-by: Raghuvansh Raj <[email protected]>
…earch-project#7889)

Signed-off-by: Andriy Redko <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…verse segment read (opensearch-project#7244)" (opensearch-project#7892)

This reverts commit 4c98b3d.

Reverting due to issue reported in opensearch-project#7878.

Signed-off-by: Andrew Ross <[email protected]>
Add TokenManager to IdentityPlugin

Provide an mechanism for identity plugins to manage tokens, lays the groundwork for OnBehalfOf authentication

Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Co-authored-by: Andriy Redko <[email protected]>
Signed-off-by: Matthew Wells <[email protected]>
…' for normal or high precision but slower summation

Signed-off-by: Matthew Wells <[email protected]>
@Yury-Fridlyand
Copy link

Probably you need to rebase the base branch to clean up commit history.

@@ -93,6 +101,7 @@ protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBu
*/
public SumAggregationBuilder(StreamInput in) throws IOException {
super(in);
method = in.readString();

Choose a reason for hiding this comment

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

if (!hasMethod)

}

public static void registerAggregators(ValuesSourceRegistry.Builder builder) {
SumAggregatorFactory.registerAggregators(builder);
}

private String method = null;

Choose a reason for hiding this comment

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

why not a enum?

…ional parameter, added tests, set correct default value, changed method to enum

Signed-off-by: Matthew Wells <[email protected]>
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.