Skip to content
This repository has been archived by the owner on May 31, 2018. It is now read-only.

Automatically split the request for package info into URI lengths les #547

Closed
wants to merge 7 commits into from

Conversation

AerysL
Copy link

@AerysL AerysL commented Aug 26, 2016

…s than 8125 chars

Divides the request into requests at most 8000 characters long, allowing the request to be successful.
Modifies the DownloadJson() function of pacaur.

Potentially addresses #209,#238 , #504

~~ Do note that while this fixes the bug in pacaur, it appears that the YAJL tools (json_verify, json_reformat) have a maximum allowed json character limit of 2^18=262144, and so if the rpc return is larger than this limit, json_verify and json_reformat fail causing pacaur to fail. Although as I have not communicated with the devs it is possible that I just happen to have errors around that location in the files I test. Due to this I cannot completely test the pacaur to successful completion, however in my tests this fixed DownloadJson(). ~~

It appears that my error was caused by the use of sed, and was fixed by running sed twice.

@rmarquis
Copy link
Owner

rmarquis commented Aug 27, 2016

Thanks for this PR.

Can you give an example of why sed is required twice? I won't merge code that I don't understand the purpose.

Don't have time to look in details at this PR yet, but will probably do tomorrow.

@AerysL
Copy link
Author

AerysL commented Aug 28, 2016

I'm not sure why it happens as it does, but in P1.txt json_verify fails, but P2.txt does not fail, and the only difference in source is the second sed. I'm not familiar enough with the code or sed yet to understand why it happens as it does.
(Note. P{1,2}Ex.txt are just the relevant lines extracted from the P{1,2}.txt)

P1.txt
P2.txt

P1Ex.txt
P2Ex.txt

@rmarquis rmarquis added the todo label Sep 8, 2016
@rmarquis rmarquis added this to the 5.0.x - later milestone Sep 8, 2016
@rmarquis
Copy link
Owner

I'm not sure why it happens as it does

Obviously, I cannot merge this PR is the current state. I'll have a look at this issue down the road, but this is very low priority.

@rmarquis
Copy link
Owner

rmarquis commented Oct 6, 2016

Done in ff827ea, based on the code of this PR. This was tricky too debug.

@rmarquis rmarquis closed this Oct 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants