openjusticeok / ojodb

OJO's R package for opening the black box of our justice system
https://openjusticeok.github.io/ojodb/
GNU General Public License v3.0
8 stars 3 forks source link

Adds the `.frequency_id` argument required by `rlang::warn` #152

Closed brancengregory closed 1 year ago

brancengregory commented 1 year ago
Error in `rlang::warn()`:
! `.frequency_id` must be supplied with `.frequency`.
Backtrace:
 1. ojodb::ojo_collect(...)
 2. rlang::warn(...)
 3. rlang:::needs_signal(.frequency, .frequency_id, warning_freq_env, "rlib_warning_verbosity")
brancengregory commented 1 year ago

@andrewjbe If the checks pass this is ready. After merge we'll bump the last semver value, i.e. Z in X.Y.Z

andrewjbe commented 1 year ago

GH Actions seem to be failing because of issues with {pak} -- is there something off w/ the lock file or something like that?

andrewjbe commented 1 year ago

The ojo_list_schemas() test is also failing, but it looks like it's because the schemas actually changed. So we should just need to make a new snapshot and it should work, I think

andrewjbe commented 1 year ago

The GitHub actions checks are failing right now for two reasons:

  1. Something about our {renv} / {pak} setup is screwed up; on all platforms except Windows, the workflow hits an error during the setup-r-dependencies@v2 step. The error is Error in remote(function(...) { : Subprocess is busy or cannot start, which is related to {pak} attempting to create a lock file (I think).
  2. The GitHub actions user on our database does not have permission to see the new oscn schema, so it's getting different results than you and I do with our users for ojo_list_schemas() and the testthat tests are failing as a result.

I'm not sure what's going on with 1, but 2 should be easily fixed by giving all_table_reader access to the new oscn schema once I get the go-ahead from @brancengregory

andrewjbe commented 1 year ago

The DB roles are fixed, and the windows runner now works as a result. I think we might want to start from scratch with our renv setup to solve the other outstanding issue.