> On Dec. 3, 2016, 8:36 p.m., Anand Mazumdar wrote: > > src/slave/containerizer/mesos/io/switchboard.cpp, line 885 > > <https://reviews.apache.org/r/54297/diff/2/?file=1575106#file1575106line885> > > > > Can you add a comment here for posterity as to why these are not > > invariant checks i.e., the agent does not validate if the next message > > should be `PROCESS_IO`?
Why doesn't the server validate this (it could, and maybe should)? First message is `CONTAINER_ID` type, all others are `PROCESS_IO` type. > On Dec. 3, 2016, 8:36 p.m., Anand Mazumdar wrote: > > src/slave/containerizer/mesos/io/switchboard.cpp, line 916 > > <https://reviews.apache.org/r/54297/diff/2/?file=1575106#file1575106line916> > > > > hmm, this message is a bit mis-leading given that you only start > > processing the control message from L953. This is just validating the > > control message itself. > > > > How about you introduce validation overloads. This would simplify your > > call sites a lot i.e., you won't have to manually set `response` for every > > error case. An example here: > > > > ```cpp > > // Validate the call. > > ...... error = validate(...); > > if (error.isSome()) { > > return BadRequest(error.get()); > > } > > > > // Validate the 'PROCESS_IO' message. > > .... > > > > // Validate the control message. > > .... > > > > // Process the control message. > > ... > > > > // Validate the data message. > > ... > > > > // Process the data message. > > > > ``` > > > > What do you think? Where would I put these? In `src/slave/validation.cpp` namespaced under `io_switchboard::call`, etc. Or maybe just local to my class? - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54297/#review157901 ----------------------------------------------------------- On Dec. 3, 2016, 2:07 a.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54297/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2016, 2:07 a.m.) > > > Review request for mesos, Anand Mazumdar and Jie Yu. > > > Bugs: MESOS-6467 > https://issues.apache.org/jira/browse/MESOS-6467 > > > Repository: mesos > > > Description > ------- > > Added support for ATTACH_CONTAINER_INPUT to the io switchboard. > > > Diffs > ----- > > src/slave/containerizer/mesos/io/switchboard.cpp > 594c37e531c5b26599989a126aede56954d460fa > > Diff: https://reviews.apache.org/r/54297/diff/ > > > Testing > ------- > > This is not tesedt yet. I plan to write a test and update this patch soon. > Just wanted to get a preliminary patch out there for review. > > > Thanks, > > Kevin Klues > >
