Skip to content

Conversation

@kweav
Copy link
Contributor

@kweav kweav commented Aug 11, 2025

Was using the new ottrpal::borrow_chapter in place of the old cow::borrow_chapter and got an error because it expected a token. Hadn't passed one because the documentation made me think I only needed to if I was grabbing content from a private repo (and typically my use cases for borrow_chapter don't). So tried to adjust the logic in the check_git_repo function that borrow_chapter was using so that if the token was NULL and the repo was private, then it would look for a token.

To test if a repo was private, I was using the GItHub API (gh) and grabbing the repo_info. I tried to add in some error handling (wrapping the gh request in a try) in case for some reason we didn't have high enough permissions to be looking into whatever repo it was or the repo name had a typo or what have you. So that's the nested if looking at the repo_info output. If it failed, I knew the repo_info$private wouldn't be accessible and would fail when I added that to the condition checking if the token is NULL.

When testing this, I found that the grepl line errored because if it was a public repo, then the !grepl... was logical(0). So added an isTRUE to maintain the expected function.

Now, my tests are failing with a sink() issue.
image

I tried to add debugging messages with the message function to the overall borrow_chapter function to see where it is failing, but they didn't display in the logs in my latest test.

@kweav
Copy link
Contributor Author

kweav commented Aug 12, 2025

I'm wondering if we can simplify this check_git_repo function to use the github api gh function fully and just process its output codes for failures? Because the goal of the function seems to be to check and report if the repo exists. And the API should be able to do that, right? I'm sure there are reasons for not using the API, and perhaps you've got something else in mind for seeing if the repo is private @cansavvy

@kweav kweav marked this pull request as draft August 12, 2025 03:52
@kweav
Copy link
Contributor Author

kweav commented Aug 12, 2025

I should also note that I ran the unit test locally for borrow chapter and it seemed to work. That's why I've been working in a branch and with a dev version on a dev docker image. Related: jhudsl/Adv_Reproducibility_in_Cancer_Informatics#72

@cansavvy
Copy link
Contributor

I'm wondering if we can simplify this check_git_repo function to use the github api gh function fully and just process its output codes for failures? Because the goal of the function seems to be to check and report if the repo exists. And the API should be able to do that, right? I'm sure there are reasons for not using the API, and perhaps you've got something else in mind for seeing if the repo is private @cansavvy

Yes it should be simplified. I wrote it a long time ago and never updated it. But really using gh would be better.

@kweav
Copy link
Contributor Author

kweav commented Dec 16, 2025

Closing because #19 from @acoffman addresses the issue

@kweav kweav closed this Dec 16, 2025
@kweav kweav deleted the tokenHandling branch December 16, 2025 16:23
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.

3 participants