Hi folks, I know it's a big patch, but it would be really great if someone can take a look at [1]? Specifically, I have added the logic for "continue-on-failure" plus adding old paramters for --load-data that might not be necessary anymore? I even documented them in README.md. This includes flags like: create-constraints, drop-constraints, create-pks, drop-pks and so on. I would like to remove them, but kept them because I'm not sure if people are using them still?
[1] https://issues.apache.org/jira/browse/OFBIZ-9441 On Tue, Jul 11, 2017 at 1:10 PM, Jacques Le Roux <jacques.le.r...@les7arts.com> wrote: > Yes, that's why it's a long task. I have to consider all cases carefully. > > That's also why I added this last comment (quoted below) in OFBIZ-8341 > > "I'll sometimes create subtasks or new Jira issues to differentiate cases > that need to be discussed. > It would be good for instance for a type of exception and a type of file > (service, event, helper/handler/test/etc.) to use and adopt a same type of > exception handling." > > Having patterns would help everybody, when creating, reviewing, refactoring, > etc. > > Jacques > > > > Le 11/07/2017 à 09:47, Taher Alkhateeb a écrit : >> >> This thread is a good example of refactoring. So mass fixing of >> swallowed exceptions is not ideal IMHO because sometimes we want to >> log things, sometimes we want to re-throw, sometimes we want a >> different path. Hence each item should be refactored slowly and in >> isolation because if you just throw a log in there then people would >> think this code is probably okay and doesn't need review. >> >> Anyway, again I appreciate all the help in your reviews, the feature >> is more or less implemented in OFBIZ-9441. >> >> On Tue, Jul 11, 2017 at 12:35 AM, Jacques Le Roux >> <jacques.le.r...@les7arts.com> wrote: >>> >>> I'll refrain to speak about swallowed exceptions ;) >>> >>> I still want to continue on OFBIZ-8341 but accepted to get sidetracked in >>> multi ways :) >>> >>> Jacques >>> >>> >>> >>> Le 10/07/2017 à 21:36, Taher Alkhateeb a écrit : >>>> >>>> Fixed it in the JIRA, the EntitySaxReader (should be the next class to >>>> refactor) is logging an error but suppressing an exception. The logic >>>> for "continue-on-error" had to go 3 classes deep to work correctly. I >>>> always underestimate how much spaghetti code we have :) >>>> >>>> On Mon, Jul 10, 2017 at 8:14 PM, Taher Alkhateeb >>>> <slidingfilame...@gmail.com> wrote: >>>>> >>>>> Quick update, to my surprise an exception is swallowed >>>>> deeeeeeeeeeeeeeeeeeeeeeeeeeeep in the call stack. So getting this >>>>> feature might require some intrusive changes. I'm still working on it >>>>> and will keep you posted, but as of right now, no exception is >>>>> bubbling up to be caught with "continue-on-failure" >>>>> >>>>> On Mon, Jul 10, 2017 at 2:14 PM, Taher Alkhateeb >>>>> <slidingfilame...@gmail.com> wrote: >>>>>> >>>>>> Thank you everyone for your feedback, I will let this discussion >>>>>> continue for a few days before committing anything (testing is going >>>>>> to take some time anyway). >>>>>> >>>>>> Now, I need help, I have a big patch in [1] that does what we >>>>>> discussed in this thread and a whole lot more. If you have the time, I >>>>>> really need your help! Most useful help is testing, there are so many >>>>>> properties and combinations to use, so this requires thorough testing. >>>>>> Also for those familiar with the core framework API, a second look at >>>>>> the code would help. >>>>>> >>>>>> [1] https://issues.apache.org/jira/browse/OFBIZ-9441 >>>>>> >>>>>> On Mon, Jul 10, 2017 at 1:45 PM, Devanshu Vyas >>>>>> <vyas.devansh...@gmail.com> wrote: >>>>>>> >>>>>>> I agree with option #3 and the 'continue-on-failure' flag with >>>>>>> default >>>>>>> value=false. :) >>>>>>> >>>>>>> Thanks & Regards, >>>>>>> Devanshu Vyas. >>>>>>> >>>>>>> On Mon, Jul 10, 2017 at 3:07 PM, Taher Alkhateeb >>>>>>> <slidingfilame...@gmail.com >>>>>>>> >>>>>>>> wrote: >>>>>>>> Hi Rishi, >>>>>>>> >>>>>>>> So my suggestion is that if anything does not load, then immediately >>>>>>>> fail. >>>>>>>> >>>>>>>> Why am I suggesting this? >>>>>>>> - You have to intentionally ignore data failure after being aware of >>>>>>>> it (it does not slip between the cracks) >>>>>>>> - The data will automatically get cleaned by committers because no >>>>>>>> failing data will be committed to the code base. >>>>>>>> >>>>>>>> I suspect we will actually catch some data loading failures that >>>>>>>> exist >>>>>>>> in the code base and we are maybe unaware of. >>>>>>>> >>>>>>>> On Mon, Jul 10, 2017 at 12:04 PM, Rishi Solanki >>>>>>>> <rishisolan...@gmail.com> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> I'm good to go with option #3 and continue-on-failure. >>>>>>>>> >>>>>>>>> Just wanted to mention one thing here; for which type of data we >>>>>>>>> will >>>>>>>>> be >>>>>>>>> failing build. That means, we have several options seed, ext, demo. >>>>>>>>> Do we >>>>>>>>> need to discuss these points or we are fine for all type of data. >>>>>>>>> Like >>>>>>>> >>>>>>>> demo >>>>>>>>> >>>>>>>>> data fails only affect a process for that data set only, and for >>>>>>>>> that >>>>>>>>> failing build is okay or not (as on data load we get logs if any >>>>>>>>> file >>>>>>>>> didn't load). >>>>>>>>> >>>>>>>>> >>>>>>>>> Btw, I'm good with the proposal, just sharing a thought in case we >>>>>>>>> should >>>>>>>>> discuss or may be we can simply ignore if we are good with that. >>>>>>>>> >>>>>>>>> Thaks! >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Rishi Solanki >>>>>>>>> Sr Manager, Enterprise Software Development >>>>>>>>> HotWax Systems Pvt. Ltd. >>>>>>>>> Direct: +91-9893287847 >>>>>>>>> http://www.hotwaxsystems.com >>>>>>>>> >>>>>>>>> On Mon, Jul 10, 2017 at 2:15 PM, Deepak Dixit < >>>>>>>>> deepak.di...@hotwaxsystems.com> wrote: >>>>>>>>> >>>>>>>>>>> Historically the data loader boolean props are false if ommitted >>>>>>>>>>> and >>>>>>>> >>>>>>>> the >>>>>>>>>>> >>>>>>>>>>> code expects that, but you have a point about the double >>>>>>>>>>> negative. >>>>>>>>>>> We >>>>>>>> >>>>>>>> can >>>>>>>>>>> >>>>>>>>>>> instead call it "continue-on-failure" for example. >>>>>>>>>>> >>>>>>>>>> +1 continue-on-failure with default value false >>>>>>>>>> >>>>>>>>>> Thanks & Regards >>>>>>>>>> -- >>>>>>>>>> Deepak Dixit >>>>>>>>>> www.hotwaxsystems.com >>>>>>>>>> www.hotwax.co >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On Jul 10, 2017 3:48 AM, "Paul Foxworthy" <p...@cohsoft.com.au> >>>>>>>> >>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> Hi all, >>>>>>>>>>> >>>>>>>>>>> I agree with option 3. I recall in my own work I once needed to >>>>>>>>>>> add >>>>>>>>>>> a >>>>>>>>>> >>>>>>>>>> throw >>>>>>>>>>> >>>>>>>>>>> where there was none to track down a problem. >>>>>>>>>>> >>>>>>>>>>> However ignore-failure leads to a double negative. How about >>>>>>>>>>> "stop-on-failure", default value true? >>>>>>>>>>> >>>>>>>>>>> Cheers >>>>>>>>>>> >>>>>>>>>>> Paul Foxworthy >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 10 July 2017 at 05:27, Taher Alkhateeb >>>>>>>>>>> <slidingfilame...@gmail.com >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Correction: on item (2) in my post: fail immediately, not after >>>>>>>>>>>> loading all files, otherwise there's no point. >>>>>>>>>>>> >>>>>>>>>>>> On Sun, Jul 9, 2017 at 10:18 PM, Taher Alkhateeb >>>>>>>>>>>> <slidingfilame...@gmail.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Hello Everyone, >>>>>>>>>>>>> >>>>>>>>>>>>> For a long time I was annoyed by something in OFBiz: the build >>>>>>>> >>>>>>>> system >>>>>>>>>>>>> >>>>>>>>>>>>> does not fail if data loading fails for some files. I spend >>>>>>>>>>>>> hours >>>>>>>>>>>>> hunting bugs only to discover that the data simply did not >>>>>>>>>>>>> load. >>>>>>>>>>>>> >>>>>>>>>>>>> Given that I'm working on refactoring the data loading >>>>>>>>>>>>> container, >>>>>>>> >>>>>>>> I >>>>>>>>>>>>> >>>>>>>>>>>>> believe this issue should resolved. However, I'm not sure if >>>>>>>>>>>>> the >>>>>>>>>>>>> community is interested in making such a change. >>>>>>>>>>>>> >>>>>>>>>>>>> So I list below 3 options to select from: >>>>>>>>>>>>> >>>>>>>>>>>>> 1- Leave it as is, do not fail the build if some files do not >>>>>>>>>>>>> load >>>>>>>>>>>>> 2- Continue loading until all files are done and then fail the >>>>>>>> >>>>>>>> build >>>>>>>>>>>>> >>>>>>>>>>>>> 3- Provide a flag e.g. ignore-failure that tells the system >>>>>>>> >>>>>>>> whether >>>>>>>>>> >>>>>>>>>> to >>>>>>>>>>>>> >>>>>>>>>>>>> fail or not with a default value of "false". >>>>>>>>>>>>> >>>>>>>>>>>>> My personal preference is for (3) >>>>>>>>>>>>> >>>>>>>>>>>>> WDYT? >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Coherent Software Australia Pty Ltd >>>>>>>>>>> PO Box 2773 >>>>>>>>>>> Cheltenham Vic 3192 >>>>>>>>>>> Australia >>>>>>>>>>> >>>>>>>>>>> Phone: +61 3 9585 6788 >>>>>>>>>>> Web: http://www.coherentsoftware.com.au/ >>>>>>>>>>> Email: i...@coherentsoftware.com.au >>>>>>>>>>> >
