<div dir="ltr"><div>Reply in-line.</div><div><br></div><div>Thanks for raising this discussion Steve!</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 30, 2020 at 1:27 AM Wang, Jerry A <<a href="mailto:jerry.a.wang@intel.com">jerry.a.wang@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">



<div dir="auto">
<br>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
Hi Steve:</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
<br>
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
Thanks for your great efforts for WSME replacement.</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
<br>
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
I had a quick review for your code changes, see my comments below:</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
<br>
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
<table width="" cellspacing="0" cellpadding="1" style="border-collapse:collapse">
<tbody>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">Review<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">Feedback<br>
</td>
<td style="border:1px solid rgb(171,171,171)">Reason<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">704490<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">Has concern<br>
</td>
<td style="border:1px solid rgb(171,171,171)">move wsme code to ironic, types.py<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">704487<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">Has concern<br>
</td>
<td style="border:1px solid rgb(171,171,171);width:715px">move wsme code to ironic, node.py<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">704489<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">has concern<br>
</td>
<td style="border:1px solid rgb(171,171,171)">move wsme code to ironic, args.py<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">704486<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">has concern <br>
</td>
<td style="border:1px solid rgb(171,171,171)">move wsme code to ironic expose.py<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">703898<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">Both positive and concern <br>
</td>
<td style="border:1px solid rgb(171,171,171)">Positive, better code structure than original code, concern, more difficult to locate WSME code<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">704488<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">has concern<br>
</td>
<td style="border:1px solid rgb(171,171,171)">move wsme code to ironic<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">704485<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">has concern<br>
</td>
<td style="border:1px solid rgb(171,171,171)">Add pecan code, pecan would be replaced by flask<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">703897<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">Positive<br>
</td>
<td style="border:1px solid rgb(171,171,171)">Removed some WSME code with this change<br>
</td>
</tr>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">703723<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">Positive<br>
</td>
<td style="border:1px solid rgb(171,171,171)">Removed some WSME code with this change<br>
</td>
</tr>
</tbody>
</table>
<table width="" style="border-collapse:collapse">
<tbody>
<tr style="background-color:rgb(255,255,255)">
<td style="width:120px;border:1px solid rgb(171,171,171)">703695<br>
</td>
<td style="width:120px;border:1px solid rgb(171,171,171)">Positive<br>
</td>
<td style="border:1px solid rgb(171,171,171)">Removed some WSME code with this change<br>
</td>
</tr>
</tbody>
</table>
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
<br>
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
Firstly, I appreciated 3 changes which definately removed some wsme code, especially the change 703897.</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
<br>
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
But I have some concerns for changes 704490, 704487, 704489, ...,  these changes seemed move WSME code into ironic, that would make ironic code base become a bit large, from my personal view, to use python built-in feature or other 3-party lib to replace WSME
 function would be better.  I like the way that code change 703897 did. <br>
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif"><br></div></div></blockquote><div><br></div><div>I guess I have two concerns.</div><div><br></div><div>1) Attribution and detailing the original source and licensing since the import of code does seem to meet the word "substantial", least to my perception.</div><div>2) I share the concern of importing code into ironic, and even adding substantial amount of code that we would need to potentially maintain. If my memory is recalling correctly, we wanted to move away from WSME in order to reduce maintenance overhead since it is not being actively developed.</div><div><br></div><div>In the grand scheme of the universe, I am for us achieving our goals, and if built-in tooling  or another third party library does not meet our needs, then I feel like it makes sense.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
Thanks</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
Jerry</div>
<div style="font-size:12pt;font-family:Calibri,Helvetica,sans-serif">
<br>
</div>
<div dir="ltr">发自我的iPhone</div>
<div dir="ltr"><br>
<blockquote type="cite">在 2020年1月30日，上午8:53，Steve Baker <<a href="mailto:sbaker@redhat.com" target="_blank">sbaker@redhat.com</a>> 写道：<br>
<br>
</blockquote>
</div>
<blockquote type="cite">
<div dir="ltr"><span>I've put together a set of changes for removing WSME which involves copying just enough of it into ironic, and I think we need to have the conversation about whether this approach is desirable :) This git branch[1] finishes with WSME removed
 and existing tests passing.</span><br>
<span></span><br>
<span>Here are some stats about lines of code (not including unit tests, calculated with cloc):</span><br>
<span></span><br>
<span>4500 wsme/wsme</span><br>
<span></span><br>
<span>6000 ironic/ironic/api master</span><br>
<span></span><br>
<span>7000 ironic/ironic/api story/1651346</span><br>
<span></span><br>
<span>In words, we need 1000 out of 4500 lines of WSME source in order to support 6000 lines of ironic specific API code.</span><br>
<span></span><br>
<span>Switching to a replacement for WSME would likely touch a good proportion of that 6000 lines of ironic specific API code. If we eventually replace it with a new library I think it would be easier if the thing being replaced is inside the ironic source
 tree to allow for a gradual transition. So this approach could be an end in itself, or it could be a step towards the final goal.</span><br>
<span></span><br>
<span>My strategy for copying in code was to pull in chunks of required logic while removing some unused features (like request pass-through and date & time type serialization). I also replaced py2/3 patterns with pure py3. There is likely further things which
 can be removed or refactored for simplicity, but what exists currently works.</span><br>
<span></span><br>
<span>If there is enough positive feedback for this approach I'll start on unit test coverage for the new code.</span><br>
<span></span><br>
<span>[1] <a href="https://review.opendev.org/#/q/topic:story/1651346" target="_blank">https://review.opendev.org/#/q/topic:story/1651346</a></span><br>
<span></span><br>
<span></span><br>
</div>
</blockquote>
</div>

</blockquote></div></div>