Remove un-necessary exception trace in logs

[ankurjuneja]

Rationale

There is an un-necessary exception object getting logged in logs.

[bbimber]

@ankurjuneja: the original intent on that was to capture the stacktrace.

[bbimber]

Also, I dont know anything about your use-case, but if this warning is being triggered for a valid usage, should we be more declarative with this warning? Maybe let the table opt-out of this validation?

Whether an exception is logged or not, is the data that is hitting this warning a valid use-case for the pattern of that center, or not?

[ankurjuneja]

@bbimber

The warning is not a valid use-case for the pattern of TNPRC, they don't record timestamps in housing entries.

[ankurjuneja]

there is an existing option to opt-out of this warning but how does logging stack call trace help here?

[bbimber]

@ankurjuneja - see below:

1. I'm not saying the stacktrace is mission critical here. The use case would be any normal reason why seeing a stacktrace is helpful over one line of logging. I took a quick look and I see that there is currently only one method calling this (AFAIK), so I agree it's kinda moot from that lens.

2. The far more interesting question is why is this warning being hit at all, which is what I was trying to get at. You say there's a valid use-case upstream of this, so it seems like the question we ought to be asking is why it's being called at all, or how to fix it. If we stop calling it, the whole question about stacktrace or not becomes moot.

I thought you mentioned this but I dont see it in the thread above. Anyway, if TNPRC doesnt is treating housing as date-only, it seems like there are more direct ways to address that. You probably know that TriggerScriptHelper has this:

public void closeHousingRecords(List<Map<String, Object>> records) throws Exception
{
    closePreviousDatasetRecords("housing", records, false, false);
}

Seems like a simply solution is to:

a) Overload that method to add a variant that lets the caller supply the value of dateOnly. The JS code calling this probably should grab the value of removeTimeFromDate and pass this. This way we only have one place in the code controls that behavior.

b) I dont see any code actually calling TriggerScriptHelper.closeHousingRecords(); however, I dont have all the NPRC modules locally. If only one or two files are actually calling it, maybe we could just modify the existing method instead of overloading.

But the general point is that if TNPRC's housing is going to behave as though it is date-only, you ought to just make the table declare that, and if any EHR code doesnt respect this (which is very possible for something like housing which is assumed to be datetime everywhere else), then fix that.