CORE-1144: added support for incoming CPU, memory and storage requests.

[slr71]

No description provided.

[ianmcorvidae]

Looks pretty straightforward to me. I guess the only potential change would be if we wanted to have these resource calculations split out into their own functions like the limits are, just for consistency.

I'm fine either way, though.

[slr71]

Thanks for the review!

I don't quite understand the suggestion. As far as I could tell, the code that I added pretty closely matched the code to calculate the limit. These functions extracted the limit requests from the incoming job submission: https://github.com/cyverse-de/app-exposer/blob/master/internal/deployments.go#L112-L124. This code built the limit object to use when creating the deployment: https://github.com/cyverse-de/app-exposer/blob/master/internal/deployments.go#L212-L227. Am I missing something?

[ianmcorvidae]

... yeah, my brain apparently transposed some code while reading it. Disregard me, heh.

[cyverse-org]

No worries. I had the same thing happen to me when I was reviewing code yesterday. I was staring directly at some sprintf calls and seeing only the last argument to those calls. Heh, some days are like that.

…

On Tue, Oct 20, 2020 at 6:49 PM Ian McEwen ***@***.***> wrote: ... yeah, my brain apparently transposed some code while reading it. Disregard me, heh. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#7 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEAFZ46RIMOB636LB7M5HADSLY4Y5ANCNFSM4SZB7X7Q> . _______________________________________________ Core-sw mailing list ***@***.*** https://maillist.cyverse.org/mailman/listinfo/core-sw

[slr71]

Thanks again for the review! The check failures aren't in the code that was modified, so I'm going to merge this now.