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

add charset encoding to SPARQLConnector.update() request. #2112

Merged
merged 3 commits into from Sep 15, 2022

Conversation

robcast
Copy link
Contributor

@robcast robcast commented Sep 9, 2022

Summary of changes

Add encoding "charset=UTF-8" to Content-Type header in SPARQLConnector.update() request.

Fixes #2095

I added a separate test which is basically a copy of the existing test_graph_update(). The assertion could be integrated in the existing test instead.

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Added tests for any changes that have a runtime impact.
  • Checked that all tests and type checking passes.
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

@aucampia
Copy link
Member

Change looks good, I will double check why test is failing when I have time.

The content type check should not be doing string equality on whole value of
the header, as it can include encoding.
@aucampia aucampia requested a review from a team September 10, 2022 17:55
@aucampia
Copy link
Member

pre-commit.ci autofix

Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @robcast - the change and the test looks good.

@coveralls
Copy link

coveralls commented Sep 10, 2022

Coverage Status

Coverage remained the same at 90.634% when pulling 9bccda6 on robcast:issue-2095-set_charset into bcd05e9 on RDFLib:main.

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.

SPARQLConnector.update should set ContentType charset
3 participants