sassoftware / saspy

A Python interface module to the SAS System. It works with Linux, Windows, and Mainframe SAS as well as with SAS in Viya.
https://sassoftware.github.io/saspy
Other
373 stars 150 forks source link

Don't report an ERROR if there is none #448

Closed saspyrate closed 2 years ago

saspyrate commented 2 years ago

The warning Noticed 'ERROR:' in LOG, you ought to take a look and see if there was a problem should only be displayed in case ERROR: appears at the beginning of a line in the SAS log.

For example, the following code produces the warning:

sas = saspy.SASsession()
sas.submit("data _null_; if 0 then put 'ERROR: no error here'; run;")
tomweber-sas commented 2 years ago

Haha, are you looking for a job as a tester? If I had a budget I'd hire you :) :) :) Well, first, look here: https://sassoftware.github.io/saspy/advanced-topics.html#automatic-checking-for-error-in-the-log-and-the-warnings-module Then, and I agree - I don't try to parse the log for the word error and make assumptions about what may or may not have happened. But, the other user wanted a way to be signaled in cases where that did show up in the log. Thus the implementations of warnings into saspy from that issue. You can easily, with one line in your config, keep those from coming out. But, I can't deterministically know whether that string showing up in the log caused a failure or not for anything that happened to be submitted. So, I added the feature, yet documented how to disable it or even configure it to be used in various ways, depending upon how you wanted to deal with it (that's mostly features of the warnings module itself). For this, you can just disable it in your config and not see these warnings.

Thoughts? Tom

saspyrate commented 2 years ago

Thanks again for the quick reply! :)

Yeah, I really like the feature. To reduce the number of false positives, I was merely suggesting to only print the warning in case \nERROR: appears in the log (note the newline character at the beginning).

In my experience, for all SAS errors the keyword ERROR is immediately preceded by a newline character, isn't it?

tomweber-sas commented 2 years ago

Oh, it's just about adding in a newline in the search. Sure, I think that would be reasonable. Let me investigate that and run it through it's paces, just to be sure. I don't have a problem with that if it pans out. In the back of my head, I feel I may have coded it that way first, but opened it up a little just in case. Let me do some testing and see about adding in the newline. Thanks, Tom

saspyrate commented 2 years ago

Great, thank you!

tomweber-sas commented 2 years ago

Hey, finally getting back to this little one, which wasn't exactly little (not for STDIO anyway). I've address this by looking for ERROR: as in the first column of the log; which isn't always '\nERROR:', not in STDIO anyway. But it's handled to be 1st column which is what prepending the NL means in other access methods..

I also added in the wait(0.001) into the submit code, like you had in the PR with all of that other rework using queue and thread for stdin. Since that causes a regression on Win, and it doesn't actually fix the other issue w/ abort cancel (if that was in there too; kinda all blends together), I just added the wait in here since it's just a couple lines out of all the other changes in the PR.

I've pushed this (these) to main, so they are there and will be built into the next release. Given that, I think we can close this one, but I'll let you make that determination.

Thanks! Tom

saspyrate commented 2 years ago

Thanks also for this fix. I appreciate it very much.

Everything works smoothly here, too. There's just a small inconsistency I noticed: In STDIO mode an error will be reported if a line starts with ERROR while in the other modes it has to start with ERROR: (note the colon). Namely, in STDIO running

sas.submit("%put ERROR;")

will produce the following error message:

UserWarning: Noticed 'ERROR:' in LOG, you ought to take a look and see if there was a problem
tomweber-sas commented 2 years ago

Oh, good catch., Yep, I forgot the ':' when I reworked the STDIO version. Just added it and pushed it to main. Thanks! Tom

saspyrate commented 2 years ago

Wow, that was fast. I'll close the issue then. Thanks again!