aws / sagemaker-sparkml-serving-container

This code is used to build & run a Docker container for performing predictions against a Spark ML Pipeline.
Apache License 2.0
50 stars 25 forks source link

update mleap dependency version with 0.13.0 #4

Closed ancasarb closed 5 years ago

ancasarb commented 5 years ago

Description of changes: Updating the mleap version to mleap 0.13.0. We can make use of the included LeapFrameSupport utility to remove the need for JavaConverters.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

orchidmajumder commented 5 years ago

Hi @ancasarb thanks for the PR, really appreciate it. The changes look just fine, however there is one small thing I'd like you to add so that I can directly merge this PR - please change the build configuration file to update the tag to 2.3 here - https://github.com/aws/sagemaker-sparkml-serving-container/blob/master/ci/buildspec.yml

Reason

While experimenting with the library, I realized that anything beyond MLeap 0.9.6 does not work with Spark 2.2 ML models if the model contains OneHotEncoder. Hence for Spark 2.2, we can not go beyond MLeap 0.9.6 and to ensure backward compatibility I want to generate a new Docker image with the tag 2.3 to indicate that it should be used with Spark 2.3 (with MLeap 0.13).

ancasarb commented 5 years ago

Thanks @orchidmajumder, I've updated buildspec.yml and a couple of other places to reference 2.3 instead of 2.2 now. Could you please take another look? Thank you!

orchidmajumder commented 5 years ago

Thanks @ancasarb . Sorry to bother you about one small thing again, please revert the changes for 2.3 in the README.md file. I'll change that file once I generate all the Docker images with the latest tag and push to ECR repositories in all these regions. Otherwise, at this point, people may try to download those images which are yet not built via our internal pipeline.

ancasarb commented 5 years ago

No worries, I've removed the changes to the README.