search

openshift / openshift-restclient-java

Other
78 stars 112 forks source link

expose IAuthorizationContext#expires (#463) #464

Closed adietish closed 4 years ago

adietish commented 4 years ago

fixes #463

adietish commented 4 years ago

@mhagnumdw could you please test this? Would love to get your feedback. I added IAuthorizationContext.getExpiresIn() & IAuthorizationContext.getExpires(). The prior returns the value that was included in the authorization response while the latter would indicate the moment at which the token expires (adds expiresIn to the creation timestamp).

mhagnumdw commented 4 years ago

@adietish, I ran the test and everything looks fine!

I'll just add a note at one point in the code in case you think it's worth handling a NumberFormatException.

Tks!!

adietish commented 4 years ago

I'll just add a note at one point in the code in case you think it's worth handling a NumberFormatException.

Ideally we should throw an OpenShift exception in this case specifying that we could not determine the expires timestamp bcs the expiresIn wasn't in the correct format. Anything else doesn't seem to make any sense. Agree?

mhagnumdw commented 4 years ago

Ideally we should throw an OpenShift exception in this case specifying that we could not determine the expires timestamp bcs the expiresIn wasn't in the correct format. Anything else doesn't seem to make any sense. Agree?

Sounds good.

ps: I accidentally deleted my review comment

adietish commented 4 years ago

Hi @mhagnumdw

I am now throwing an OpenShiftException when "expiresIn" has an illegal value and cannot be parsed. Can I get your +1 or dissenting opinion, please?

adietish commented 4 years ago

/ok-to-test

adietish commented 4 years ago

merged.