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

Enhancement/doc #39

Merged
merged 3 commits into from
Aug 4, 2023
Merged

Enhancement/doc #39

merged 3 commits into from
Aug 4, 2023

Conversation

yhan818
Copy link
Contributor

@yhan818 yhan818 commented Aug 1, 2023

Description

See #36
It is a dependency upgraded to the latest version. Requests is a simple HTTP library.

Screenshots or additional context

Testing (if applicable)

All redata codes shall be performed as usual, because requests' upgrade to the latest version (2.31.0) shall have no impact to redata.

@yhan818 yhan818 requested a review from zoidy August 1, 2023 00:43
@zoidy
Copy link
Collaborator

zoidy commented Aug 3, 2023

This PR needs downstream testing before being merged into main. From the requests library changelog :

  • 2.29 introduces a change to how chunked downloads are handled
  • 2.30 introduces urllib3 2.0 which itself may have breaking changes

LD-Cool-P uses the urllib library directly as well as chunked downloads. ReBACH uses chunked downloads. Therefore, before merging this PR, the following tests are needed. With the latest requests library, test

  • LD-Cool-P: test qualtrics link generation (or verify that no affected functions are used from urllib)
  • LD-Cool-P: test qualtrics API report downloads (or verify that no affected functions are used from urllib)
  • LD-Cool-P: test downloading large files (anything more than a few tens of MB)
  • ReBACH: test downloading large files (anything more than a few tens of MB)

Copy link
Collaborator

@zoidy zoidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests needed for approval. See above

@zoidy
Copy link
Collaborator

zoidy commented Aug 4, 2023

Tests needed for approval. See above

After thinking about it, I think this can be merged now and the testing done at the time the update is applied to each dependent repo. LD-Cool-P and ReBACH are pinned to v0.42 and 0.41 of redata commons respectively so updating this code won't affect them right away. I'll create an issue in those repos to update this dependency and test it.

@zoidy zoidy merged commit a755b68 into main Aug 4, 2023
@yhan818
Copy link
Contributor Author

yhan818 commented Aug 4, 2023

Tests needed for approval. See above

After thinking about it, I think this can be merged now and the testing done at the time the update is applied to each dependent repo. LD-Cool-P and ReBACH are pinned to v0.42 and 0.41 of redata commons respectively so updating this code won't affect them right away. I'll create an issue in those repos to update this dependency and test it.

Both shall be tested and applied. In addition, dependencies (pandas and tabulate) new versions were released. Probably these shall be upgraded at the same time.

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