Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use included SQL files instead of those included in etn #49

Open
wants to merge 22 commits into
base: live-test
Choose a base branch
from

Conversation

PietrH
Copy link
Member

@PietrH PietrH commented Oct 2, 2024

I accidentally introduced a circular dependency, etnservice was dependent on etn and visa versa. While etnservice should be standalone. It used some sql files included in inst, now these are included in etnservice.

This PR includes a change to connect_to_etn() in an attempt to return a better error when something goes wrong when connecting to the database (eg. wrong credentials).

Todo

  • Pass R CMD Check
  • Resolve merge conflict
  • Pass unit tests with {etn} uninstalled
  • test error for connect_to_etn()
  • check test coverage

Failing tests

Failing tests: are they covered in etn ? -> This PR is pre sync between etn main and etnservice, thus some failing tests/functions might have changed in the meantime. To avoid duplication of effort, it'll be more practical to solve these failing tests in a seperate PR. I've created issues and added skip()'s for them.

Failure (test-get_acoustic_detections.R:36:3): get_acoustic_detections() returns unique detection_id
nrow(df) not equal to nrow(df %>% distinct(detection_id)).
1/1 mismatches
[1] 100 - 42 == 58

Failure (test-list_receiver_ids.R:11:3): list_receiver_ids() returns unique list of values
all(!is.na(vector)) is not TRUE

`actual`:   FALSE
`expected`: TRUE 

@PietrH PietrH linked an issue Oct 2, 2024 that may be closed by this pull request
@PietrH PietrH changed the title 47 remove dependency on etn Use included SQL files instead of those included in etn Oct 2, 2024
@PietrH PietrH added the bug Something isn't working label Oct 22, 2024
@PietrH PietrH self-assigned this Oct 22, 2024
@PietrH
Copy link
Member Author

PietrH commented Oct 22, 2024

test coverage for utils is low, need to merge some tests from etn v2.3 into etnservice: #53

@PietrH PietrH marked this pull request as ready for review October 22, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove dependency on etn in sql file calls
1 participant