Closed ghost closed 7 years ago
Thanks for the pull request.
Looking at all the updates, I think the SymUser object should be updated with the additional detail attributes vs creating a new model. This just makes the attributes more portable with everything else in the SJC.
On the AuthorizationClient changes, please see my latest commit renaming that class to AuthenticationClient. More importantly, since version 1.0.1 Authorization/AuthenticationClient supports a custom HTTP Client through a constructor. Please review.
I'm not going to commit this, but rather update the existing code base to incorporate all the changes in a slightly different way.
Happy to make these changes - want us to handle it?
We are going to address all the comments and hope to have an updated PR today
We will also remove the authentication changes and make a separate pull request. We did try injecting the AuthenticationClient via the constructor but when running services internally you need a proxy for the pod and not for the KM, etc. but we can use that PR to review more.
The changes look good, but you are merging onto the master vs develop branch which is ahead right now.
There will be a number minor conflicts on develop branch, so we can continue with this update and I can merge back into develop.
In the future, use develop branch for changes.
Allows us to set a separate Http client for the KM - and retains backward compatibility,