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

Update block_requests.py to resolve unexpected type error (500 error) after updating to gradio 4.28.3 #5976

Merged
merged 5 commits into from
Jun 24, 2024

Conversation

nero-dv
Copy link
Contributor

@nero-dv nero-dv commented May 3, 2024

Resolves #5975 (comment)

Checklist:

In the original function, file_contents is a bytes object because the file is opened as a binary file and the replace method replaces bytes objects. In the second function, the file_contents is a string object because the file is opened as a text file and the replace method replaces string objects. This is done because when updating the gradio version to the latest version, the following error appears which references the block_requests.py file.

TypeError: replace() argument 1 must be str, not bytes

Additionally a specific iframe script from the content of the file is being removed. This line is commented out in the second function with the comment saying there is no need for it as this effectively renders the request null

file_contents = file_contents.replace('cdnjs.cloudflare.com', b'127.0.0.1') 

@nero-dv nero-dv changed the title Update block_requests.py Update block_requests.py to resolve unexpected type error (500 error) after updating to gradio 4.28.3 May 3, 2024
@nickpotafiy
Copy link

I upgraded to Gradio 4.28.3 and 4.29 and cannot reproduce the error. I don't think it's an issue with newer gradio, might be an issue with older gradio being out of sync with text-generation-webui.

@nero-dv
Copy link
Contributor Author

nero-dv commented May 6, 2024

I upgraded to Gradio 4.28.3 and 4.29 and cannot reproduce the error. I don't think it's an issue with newer gradio, might be an issue with older gradio being out of sync with text-generation-webui.

Could you share which commit you're on so I can replicate?

@nickpotafiy
Copy link

I tested with the most recent one. If I downgrade gradio to 4.25 i also get an error. 4.26 (required version), 4.28, 4.29 all work just fine with this commit. I think the issue happens when someone pulls the latest version of webui without using the update wizard. This is not a flaw with webui but how it's updated.

@nero-dv
Copy link
Contributor Author

nero-dv commented May 6, 2024

I'm concerned about the lack of transparency in package modifications during installation. It's unclear when edits are made and users shouldn't be surprised by changes they didn't consent to.

Without clear information on what changes were made, debugging issues becomes much harder. Developers will struggle to reproduce problems or identify the root cause of issues. also unannounced modifications can introduce unknown security risks.

@nickpotafiy
Copy link

The installation script downloads the necessary packages outlined in various requirements files and installs under installer_files. If you simply let webui install everything for you, there won't be any issues. However, if you git pull from latest webui but your installer_files are outdated, you might get errors because the project requirements are out of sync. This is why update wizard exists. So long as users understand how to install and update webui, these types of issues should not really exist.

@oobabooga
Copy link
Owner

I'm closing this because this error does not happen with the gradio version in requirements.txt. Gradio changes a lot over time and the project is only guaranteed to work with the specified version.

@oobabooga
Copy link
Owner

Happened to me now (randomly). Probably some sub-requirement causes this.

@oobabooga oobabooga merged commit 57119c1 into oobabooga:dev Jun 24, 2024
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.

None yet

3 participants