Skip to content

Add get_acoustic_deployment_logs() - #517

Open
PietrH wants to merge 161 commits into
mainfrom
254-add-receiver-logs-view
Open

Add get_acoustic_deployment_logs()#517
PietrH wants to merge 161 commits into
mainfrom
254-add-receiver-logs-view

Conversation

@PietrH

@PietrH PietrH commented Mar 26, 2026

Copy link
Copy Markdown
Member

meeting conclusion:

  • rename and reduce arguments: get_acoustic_deployment_logs(deployment_id, limit = FALSE)
  • change on service side
  • deploy opencpu to production
  • document
  • check if tests need reducing/rewriting
  • rerun tests
  • set minimum httr2 version to v1.0.0 for parallel requests
  • consider http response caching

under construction!

Notes for the reviewer:

get_receiver_logs() was implemented with speed in mind, but compromises were made for maintainability and to not take extra dependencies (yyjsonr, data.table).

  • You will need to install the most recent version of etnservice, this should be handled automatically outside of the testing context.
  • Until etnservice 0.6.0+ is deployed to the production deployment of OpenCPU, you will need to set the ETN_TEST_API environmental variable to the test api url. Contact me if you don't have this value handy. VLIZ has requested we not make it public.
  • deployment_id is required, getting all deployments should not be done likely as it'll take a while.
  • station_name was requested by Lotte
  • I opted to always include argument values in the output tibble.
  • start_date and end_date were implemented to mirror other etn functions, getRad does it better with lubridate support but that is a development for another day.
  • Currently I don't know what fields I'm going to get back from a receiver log, so I implemented name repair to ensure all returned fields are unique and R friendly (no spaces etc.). This also avoids collisions with station_name.
  • Because the returned object is a tidy, (rectangular) tibble, most fields will almost always be empty. This is because a single measurement value can add a whole column even if it's unique within a deployment. Returning as a list is possible, but not currently supported (it would be easy as it could just be an early return). Let me know if you think this is a good idea.
  • I chose not to implement {vcr} HTTP response caching at the moment because as of writing the production {etnservice} deployment is lagging, this way I can ensure that tests will continue to fail until production has caught up.
  • All naming and documentation is open for debate
  • Do you think my tests have enough coverage?
  • Are my failure modes defensive enough?

Before merging

  • switch to production deployment
  • rerun tests

@PietrH

PietrH commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Hi Pieter! Finally took a look at the function. I can get some values out! But to me it's not clear how I can know which deployment (or which receiver) has a log and which does not. Do you know anything about the status of documentation of the logs?

In my example below I asked for logs of 10 random deployments in the BPNS and got out 6 logs...Already nice of course, but wondering how I can know why I didn't get 10 logs. Also knowing more about the record_type would be very handy in the future. This is very exciting!!

Cheers, Lotte

Thanks for your review Lotte!

Figuring out if there are deployment logs for an id

I've added a warning message when one or more of the requested deployments doesn't have log data available. I suppose I could also add a second function has_acoustic_deployment_logs() that you could use to check if a deployment has logs. But it wouldn't be much faster than just running get_acoustic_deployment_logs I'm afraid. We could modify the query behind get_acoustic_deployments() for an extra column that checks for the presence of logs. But this might slow down the query.

Personally I think the warning is enough, if you think we need another way for people to figure this out, we can investigate and tackle that in a separate development!

Documentation of fields/logs

I'm afraid I don't know much about these fields. I suppose they might be documented in a separate database table? I'd certainly be in favor of adding a help page with more information about these fields. But currently I don't have much to go on.

I don't even really know what fields I'm going to be returning, and there are quite a few different ones. See this comment: #254 (comment), I found 243 different fields at the time.

@PietrH
PietrH deployed to manual-approval August 21, 2026 10:03 — with GitHub Actions Active
Comment thread DESCRIPTION Outdated
@PietrH
PietrH deployed to manual-approval August 21, 2026 11:41 — with GitHub Actions Active
@PietrH
PietrH deployed to manual-approval August 21, 2026 12:19 — with GitHub Actions Active
@PietrH
PietrH marked this pull request as ready for review August 21, 2026 12:46
@PietrH
PietrH requested a review from peterdesmet August 21, 2026 12:47
@PietrH

PietrH commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Addressed reviewer comments. Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Use of bare predicate functions in get_acoustic_deployment_logs() Add get_acoustic_deployment_logs() function

3 participants