bedatadriven / activityinfo-R

ActivityInfo R Language Client
https://www.activityinfo.org/support/docs/R/
17 stars 12 forks source link

improve messaging from `queryAuditLog()` #41

Closed Ryo-N7 closed 1 year ago

Ryo-N7 commented 1 year ago
queryAuditLog(
  databaseId = "c6swoa5kymocnz02",
  before = end_date,
  after = start_date,
  resourceId = NULL,
  typeFilter = "RECORD",
  limit = 10000 ## ... or more!
)

Sending POST request to https://www.activityinfo.org/resources/databases/c6swoa5kymocnz02/audit
Reading username:password from C:/Users/ryo90/Documents/.activityinfo.credentials...
OK (HTTP 200).
Received 83 events...
Sending POST request to https://www.activityinfo.org/resources/databases/c6swoa5kymocnz02/audit
OK (HTTP 200).
Received 110 events...
Sending POST request to https://www.activityinfo.org/resources/databases/c6swoa5kymocnz02/audit
OK (HTTP 200).
Received 56 events...
Sending POST request to https://www.activityinfo.org/resources/databases/c6swoa5kymocnz02/audit
OK (HTTP 200).
Received 63 events...
Sending POST request to https://www.activityinfo.org/resources/databases/c6swoa5kymocnz02/audit
OK (HTTP 200).
Received 81 events...
Sending POST request to https://www.activityinfo.org/resources/databases/c6swoa5kymocnz02/audit
OK (HTTP 200).
Received 83 events...
Sending POST request to https://www.activityinfo.org/resources/databases/c6swoa5kymocnz02/audit
OK (HTTP 200).
Received 74 events...

... etc. esp. when trying to grab 10,000s of audit log records so it can get rather annoying. I don't necessary mind it giving updates but maybe less verbosely (we don't need to see Sending POST request to --url-- every single new request for example)

nickdickinson commented 1 year ago

I wasn't sure about the best way to address this. In the last version 4.30 the messages already changed a bit as we use the task messages but there are no less:

Query audit log returned with status 200: success
Received 15 events...

For large requests, it is already possible to cancel the task messages using the option activityinfo.verbose.tasks = FALSE. The following that will eliminate these messages for just the audit log query:

withr::with_options(list(activityinfo.verbose.tasks = FALSE), {
  results <- queryAuditLog(database$databaseId, limit = 1000)
})

It is also possible to suppress the "Received XX events..." by suppressing all messages with suppressMessages().

Are these methods ok or do you feel we should universally limit the log messages only to the Received XX events messages?

Ryo-N7 commented 1 year ago

yep, i think that looks much better.

the 'received x events' and the 'query audit log returned with...' are separate messages right?

maybe it'll be better to have them combined into one line instead?

also we should have the database ID appear in the 'query audit log....' message in line with the messages from other functions

nickdickinson commented 1 year ago

What do you think if we remove the task message with status code? So then it would read (without a http status code): Successfully received X query audit log events for database XXXX

The difficulty is that the first message is coming from the internal postResource function. And the second message from inside the queryAuditLog() function. So I cannot easily combine them without writing a custom message handler which seems like a lot of work.

For debugging we may want to include an argument with default queryAuditLog(verbose = FALSE) in case we want to see the rest messages. Any warnings or errors will bubble up anyway.

Ryo-N7 commented 1 year ago

What do you think if we remove the task message with status code? So then it would read (without a http status code): Successfully received X query audit log events for database XXXX

yep, this makes sense to me!

akbertram commented 1 year ago

@nickdickinson Can we close this?