Re: [Discussion] Failing the build if data loading fails

2017-08-04 Thread Jacques Le Roux
Yes, I meant that for such next endeavours you might commit directly. Especially if you can commit smaller pieces... Anyway it's up to you... Jacques Le 04/08/2017 à 09:35, Taher Alkhateeb a écrit : Starting a JIRA more than a month ago, putting 5 patches and asking for reviews multiple

Re: [Discussion] Failing the build if data loading fails

2017-08-04 Thread Taher Alkhateeb
Starting a JIRA more than a month ago, putting 5 patches and asking for reviews multiple times on the mailing list is not CTR. On Fri, Aug 4, 2017 at 10:21 AM, Jacques Le Roux wrote: > Hi Taher, > > My last reviews of your work (previous commits) let me think that

Re: [Discussion] Failing the build if data loading fails

2017-08-04 Thread Jacques Le Roux
Hi Taher, My last reviews of your work (previous commits) let me think that we can go in a CTR mode :) I'll try to do a review today though... About the "old paramters for --load-data" did you check that they are not indirectly be used by webtools I mean

Re: [Discussion] Failing the build if data loading fails

2017-08-04 Thread Taher Alkhateeb
Hello everyone, So I didn't get feedback for quite a while, probably because the patch is large. However, I think given that this is only a refactoring / cleanup exercise (plus the feature in this thread) I will go ahead and commit this work soon. I've tested it on my machine and things seem to

Re: [Discussion] Failing the build if data loading fails

2017-07-20 Thread Taher Alkhateeb
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

Re: [Discussion] Failing the build if data loading fails

2017-07-11 Thread Jacques Le Roux
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

Re: [Discussion] Failing the build if data loading fails

2017-07-11 Thread Taher Alkhateeb
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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Jacques Le Roux
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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Taher Alkhateeb
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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Taher Alkhateeb
Quick update, to my surprise an exception is swallowed dp 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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Taher Alkhateeb
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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Devanshu Vyas
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 wrote: > Hi Rishi, > > So my suggestion is that if anything does not load, then immediately

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Rishi Solanki
Thanks Taher for your reply. I was just pushing the types to set some type of data as in ignoring. But, now I am completely agree with you on point "The data will automatically get cleaned by committers because no failing data will be committed to the code base". Again +1 for #3 with option

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Michael Brohl
+1 Michael Am 10.07.17 um 07:03 schrieb Taher Alkhateeb: 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. On Jul 10, 2017 3:48 AM, "Paul

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Taher Alkhateeb
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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Rishi Solanki
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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Deepak Dixit
> 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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Jacopo Cappellato
On Mon, Jul 10, 2017 at 7:03 AM, Taher Alkhateeb 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

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Paul Foxworthy
On 10 July 2017 at 15:03, Taher Alkhateeb 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. > Hi Taher,

Re: [Discussion] Failing the build if data loading fails

2017-07-10 Thread Jacques Le Roux
Option 3 with with a default value of "false" for "continue-on-failure is fine with me Jacques Le 10/07/2017 à 07:03, Taher Alkhateeb a écrit : 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

Re: [Discussion] Failing the build if data loading fails

2017-07-09 Thread Taher Alkhateeb
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. On Jul 10, 2017 3:48 AM, "Paul Foxworthy" wrote: Hi all, I agree with

Re: [Discussion] Failing the build if data loading fails

2017-07-09 Thread Scott Gray
I don't mind if the behavior changes, but it's never bothered me since the information is only a few lines above the build's success/fail message. Or at least, it was with the ant build system, I haven't tried with gradle recently enough to recall. Regards Scott On 10 July 2017 at 07:18, Taher

Re: [Discussion] Failing the build if data loading fails

2017-07-09 Thread Paul Foxworthy
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

Re: [Discussion] Failing the build if data loading fails

2017-07-09 Thread Taher Alkhateeb
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 wrote: > Hello Everyone, > > For a long time I was annoyed by something in OFBiz: the build system > does

[Discussion] Failing the build if data loading fails

2017-07-09 Thread Taher Alkhateeb
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