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

Code challenge attempt #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

technight
Copy link

Don't mind me - Just a Delegate employee curios if my Kotlin implementation will pass 😄

@sjkp
Copy link
Member

sjkp commented Mar 25, 2019

Are you exposing port 80 from the container? It crashes when I try to start it, and bind to port 80.

@technight
Copy link
Author

Hi, thanks 😄
I have exposed 80 in the original dockerfile:


I can start it locally like this:

@sjkp
Copy link
Member

sjkp commented Mar 25, 2019

I see - I'm able to start the container just fine on a ubuntu box (which the tests also runs on) just seems that you API is not behaving so nicely it returns 500...

@technight
Copy link
Author

Yeah, I'll have a look, thanks for investigating!
If you don't mind, the API works for me using my local tests, when using the example data from the readme. Do you see any immediate mistakes here in regards to how I call the API compared to in the tests?

Otherwise never mind, I was just curios 😄

@thehabbos007
Copy link
Contributor

thehabbos007 commented Mar 27, 2019

Hey @technight !
I've looked at your solution and tested it against the private tests. Seems like the problem lies in the way your web framework deserializes JSON. Since our private tester is made in dotnet, it has a tendency to use UpperCamelCase on keys in the serialized JSON.

I'd say either change your payload classes to use UpperCamelCase, or see if the framework supports case insensitive deserilization, perhaps that will work!

I tried it out in python too against the container
Lowercase
img1

Capped
img2

@technight
Copy link
Author

Oh thanks @thehabbos007, didn't realize I got a response. I just tried to update, still didn't work, but I suspect I didn't update probably. Thanks for the advice, I will have a second look 😄

@thehabbos007
Copy link
Contributor

thehabbos007 commented Apr 3, 2019

It seems like our tests also wrap some junk data in the request. I will investigate further and comment when it's fixed! @technight

Ninja edit: We love those green checks :)

@technight
Copy link
Author

@thehabbos007 Oh awesome, thank you! Now I just have to figure out the actual errors 😄

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.

3 participants