You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Taher Alkhateeb <sl...@gmail.com> on 2017/07/09 19:18:12 UTC
[Discussion] Failing the build if data loading fails
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?
Re: [Discussion] Failing the build if data loading fails
Posted by Scott Gray <sc...@hotwaxsystems.com>.
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 Alkhateeb <sl...@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?
>
Re: [Discussion] Failing the build if data loading fails
Posted by Rishi Solanki <ri...@gmail.com>.
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 continue-on-failure with default false, with no
confusion in thread from my side. :-)
--
Rishi Solanki
Sr Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com
On Mon, Jul 10, 2017 at 3:07 PM, Taher Alkhateeb <slidingfilaments@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 <ri...@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.dixit@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" <pa...@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 <slidingfilaments@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
> >> > > <sl...@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: info@coherentsoftware.com.au
> >> >
> >>
>
Re: [Discussion] Failing the build if data loading fails
Posted by Jacques Le Roux <ja...@les7arts.com>.
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 times on the mailing list is not CTR.
>
> On Fri, Aug 4, 2017 at 10:21 AM, Jacques Le Roux
> <ja...@les7arts.com> wrote:
>> 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
>> https://demo-trunk.ofbiz.apache.org/webtools/control/view/checkdb
>>
>> It should not be (something else is used I guess) but I did not check
>>
>> Jacques
>>
>>
>>
>> Le 04/08/2017 à 08:52, Taher Alkhateeb a écrit :
>>> 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
>>> be going smooth. If anyone wants to review before I commit please go
>>> ahead and raise your hand!
>>>
>>> Cheers,
>>>
>>> Taher Alkhateeb
>>>
>>> On Thu, Jul 20, 2017 at 9:34 AM, Taher Alkhateeb
>>> <sl...@gmail.com> wrote:
>>>> 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
>>>> <ja...@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
>>>>>> <ja...@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
>>>>>>>> <sl...@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
>>>>>>>>> <sl...@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
>>>>>>>>>> <vy...@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
>>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>>> <ri...@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.dixit@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"
>>>>>>>>>>>>>>> <pa...@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
>>>>>>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>>>>>>>>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
<ja...@les7arts.com> wrote:
> 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
> https://demo-trunk.ofbiz.apache.org/webtools/control/view/checkdb
>
> It should not be (something else is used I guess) but I did not check
>
> Jacques
>
>
>
> Le 04/08/2017 à 08:52, Taher Alkhateeb a écrit :
>>
>> 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
>> be going smooth. If anyone wants to review before I commit please go
>> ahead and raise your hand!
>>
>> Cheers,
>>
>> Taher Alkhateeb
>>
>> On Thu, Jul 20, 2017 at 9:34 AM, Taher Alkhateeb
>> <sl...@gmail.com> wrote:
>>>
>>> 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
>>> <ja...@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
>>>>> <ja...@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
>>>>>>> <sl...@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
>>>>>>>> <sl...@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
>>>>>>>>> <vy...@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
>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>> <ri...@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.dixit@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"
>>>>>>>>>>>>>> <pa...@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
>>>>>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>>>>>>>
>
Re: [Discussion] Failing the build if data loading fails
Posted by Jacques Le Roux <ja...@les7arts.com>.
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
https://demo-trunk.ofbiz.apache.org/webtools/control/view/checkdb
It should not be (something else is used I guess) but I did not check
Jacques
Le 04/08/2017 à 08:52, Taher Alkhateeb a écrit :
> 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
> be going smooth. If anyone wants to review before I commit please go
> ahead and raise your hand!
>
> Cheers,
>
> Taher Alkhateeb
>
> On Thu, Jul 20, 2017 at 9:34 AM, Taher Alkhateeb
> <sl...@gmail.com> wrote:
>> 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
>> <ja...@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
>>>> <ja...@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
>>>>>> <sl...@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
>>>>>>> <sl...@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
>>>>>>>> <vy...@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
>>>>>>>>> <slidingfilaments@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
>>>>>>>>>> <ri...@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.dixit@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" <pa...@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
>>>>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>>>>>>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
be going smooth. If anyone wants to review before I commit please go
ahead and raise your hand!
Cheers,
Taher Alkhateeb
On Thu, Jul 20, 2017 at 9:34 AM, Taher Alkhateeb
<sl...@gmail.com> wrote:
> 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
> <ja...@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
>>> <ja...@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
>>>>> <sl...@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
>>>>>> <sl...@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
>>>>>>> <vy...@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
>>>>>>>> <slidingfilaments@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
>>>>>>>>> <ri...@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.dixit@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" <pa...@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
>>>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>>>>>
>>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
<ja...@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
>> <ja...@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
>>>> <sl...@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
>>>>> <sl...@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
>>>>>> <vy...@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
>>>>>>> <slidingfilaments@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
>>>>>>>> <ri...@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.dixit@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" <pa...@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
>>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>>>>
>
Re: [Discussion] Failing the build if data loading fails
Posted by Jacques Le Roux <ja...@les7arts.com>.
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
> <ja...@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
>>> <sl...@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
>>>> <sl...@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
>>>>> <vy...@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
>>>>>> <slidingfilaments@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
>>>>>>> <ri...@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.dixit@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" <pa...@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
>>>>>>>>>> <slidingfilaments@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
>>>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>>>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
<ja...@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
>> <sl...@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
>>> <sl...@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
>>>> <vy...@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
>>>>> <slidingfilaments@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
>>>>>> <ri...@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.dixit@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" <pa...@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
>>>>>>>>> <slidingfilaments@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
>>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>>
>
Re: [Discussion] Failing the build if data loading fails
Posted by Jacques Le Roux <ja...@les7arts.com>.
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
> <sl...@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
>> <sl...@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
>>> <vy...@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 <slidingfilaments@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 <ri...@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.dixit@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" <pa...@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 <slidingfilaments@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
>>>>>>>>> <sl...@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: info@coherentsoftware.com.au
>>>>>>>>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
<sl...@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
> <sl...@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
>> <vy...@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 <slidingfilaments@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 <ri...@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.dixit@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" <pa...@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 <slidingfilaments@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
>>>> >> > > <sl...@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: info@coherentsoftware.com.au
>>>> >> >
>>>> >>
>>>>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
<sl...@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
> <vy...@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 <slidingfilaments@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 <ri...@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.dixit@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" <pa...@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 <slidingfilaments@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
>>> >> > > <sl...@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: info@coherentsoftware.com.au
>>> >> >
>>> >>
>>>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
<vy...@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 <slidingfilaments@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 <ri...@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.dixit@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" <pa...@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 <slidingfilaments@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
>> >> > > <sl...@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: info@coherentsoftware.com.au
>> >> >
>> >>
>>
Re: [Discussion] Failing the build if data loading fails
Posted by Devanshu Vyas <vy...@gmail.com>.
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 <slidingfilaments@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 <ri...@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.dixit@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" <pa...@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 <slidingfilaments@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
> >> > > <sl...@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: info@coherentsoftware.com.au
> >> >
> >>
>
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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 <ri...@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.dixit@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" <pa...@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 <sl...@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
>> > > <sl...@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: info@coherentsoftware.com.au
>> >
>>
Re: [Discussion] Failing the build if data loading fails
Posted by Rishi Solanki <ri...@gmail.com>.
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.dixit@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" <pa...@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 <sl...@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
> > > <sl...@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: info@coherentsoftware.com.au
> >
>
Re: [Discussion] Failing the build if data loading fails
Posted by Deepak Dixit <de...@hotwaxsystems.com>.
> 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" <pa...@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 <sl...@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
> > <sl...@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: info@coherentsoftware.com.au
>
Re: [Discussion] Failing the build if data loading fails
Posted by Jacques Le Roux <ja...@les7arts.com>.
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
> instead call it "continue-on-failure" for example.
>
> On Jul 10, 2017 3:48 AM, "Paul Foxworthy" <pa...@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 <sl...@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
>> <sl...@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: info@coherentsoftware.com.au
>
Re: [Discussion] Failing the build if data loading fails
Posted by Jacopo Cappellato <ja...@hotwaxsystems.com>.
On Mon, Jul 10, 2017 at 7:03 AM, Taher Alkhateeb <slidingfilaments@gmail.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
Re: [Discussion] Failing the build if data loading fails
Posted by Michael Brohl <mi...@ecomify.de>.
+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 Foxworthy" <pa...@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 <sl...@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
>> <sl...@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: info@coherentsoftware.com.au
>
Re: [Discussion] Failing the build if data loading fails
Posted by Paul Foxworthy <pa...@cohsoft.com.au>.
On 10 July 2017 at 15:03, Taher Alkhateeb <sl...@gmail.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.
>
Hi Taher,
I'm happy with that.
Cheers
Paul
--
Coherent Software Australia Pty Ltd
PO Box 2773
Cheltenham Vic 3192
Australia
Phone: +61 3 9585 6788
Web: http://www.coherentsoftware.com.au/
Email: info@coherentsoftware.com.au
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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" <pa...@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 <sl...@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
> <sl...@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: info@coherentsoftware.com.au
Re: [Discussion] Failing the build if data loading fails
Posted by Paul Foxworthy <pa...@cohsoft.com.au>.
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 <sl...@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
> <sl...@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: info@coherentsoftware.com.au
Re: [Discussion] Failing the build if data loading fails
Posted by Taher Alkhateeb <sl...@gmail.com>.
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
<sl...@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?