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

Collect pandoc urls from github api #372

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

nstrmx
Copy link

@nstrmx nstrmx commented Aug 23, 2024

No description provided.

@JessicaTegner
Copy link
Owner

Hey @nstrmx thanks for the PR.

Can you take a look at the failing checks. Do you think it's a PR issue, or something else?

@nstrmx nstrmx force-pushed the master branch 2 times, most recently from fe9e4ad to 188d739 Compare August 26, 2024 01:41
@nstrmx
Copy link
Author

nstrmx commented Aug 26, 2024

Fixed it (hopefully).

@JessicaTegner
Copy link
Owner

Interestingly @nstrmx it seems that the single test failing, is actually because of a 403 rate limit.
What would you say we could do about that? Asking because Idon't know that much about GHs api, when used in this way.

@nstrmx
Copy link
Author

nstrmx commented Oct 20, 2024

If we add support for the GITHUB_TOKEN as an environment variable to be used in requests, will that solve this?

@JessicaTegner
Copy link
Owner

yes, that would be acceptable. But I feel like that if that key isn't set, we should fall back to the old behaviour.
Due to the nature of pypandoc, where it's used a lot in CI/CD workflows, I do think that if it's not set, we should fall back to old behaviour so stuff doesn't break

@nstrmx
Copy link
Author

nstrmx commented Oct 26, 2024

Should work now.

@JessicaTegner
Copy link
Owner

Great work. Lastly, since we now should have 2 download behaviors, we should probably make a test casefor each behavior

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.

2 participants