feat: add functionality to optionally disable path encoding

[ashutoshkrris]

Description

Before this PR, this library would encode all paths passed to either create_url() or create_srcset(). This was a great default in a lot of scenarios, but this proved to be irritating when integrating with some data sources that provided already encoded URL paths.

After this PR, the library would allow users to optionally pass a options parameter containing disable_path_encoding having boolean value.

Steps to test

• Testing the create_url() method

  >>> from imgix import UrlBuilder
  >>> ub = UrlBuilder("sdk-test.imgix.net")
  >>> ub.create_url(" <>[]{}|^%.jpg", params={'w': 100, 'h': 100}, options={'disable_path_encoding': True})  
  'https://sdk-test.imgix.net/ <>[]{}|^%.jpg?h=100&w=100'

  Normally this would output a source URL like https://sdk-test.imgix.net/%20%3C%3E%5B%5D%7B%7D%7C%5E%25.jpg?h=100&2=100, but since path encoding is disabled, it will output a source URL like https://sdk-test.imgix.net/ <>[]{}|^%.jpg?h=100&w=100.

• Testing the create_scrset() method

  >>> from imgix import UrlBuilder
  >>> ub = UrlBuilder("sdk-test.imgix.net")
  >>> ub.create_srcset("image<>[]{} 123.png", widths=[100], options={'disable_path_encoding': True})
  'https://sdk-test.imgix.net/image<>[]{} 123.png?&w=100 100w'

  Normally this would output a source URL like https://sdk-test.imgix.net/image%3C%3E%5B%5D%7B%7D%20123.png?&w=100 100w, but since path encoding is disabled, it will output a source URL like https://sdk-test.imgix.net//image<>[]{} 123.png?&w=100 100w.

Checklist

• Read the contributing guidelines.
• Each commit follows the Conventional Commit spec format.
• Update the readme (if applicable).
• Update or add any necessary API documentation (if applicable)
• All existing unit tests are still passing (if applicable).

• Add some steps so we can test your bug fix or feature (if applicable).
• Add new passing unit tests to cover the code introduced by your PR (if applicable).
• Any breaking changes are specified on the commit on which they are introduced with BREAKING CHANGE in the body of the commit.
• If this is a big feature with breaking changes, consider opening an issue to discuss first. This is completely up to you, but please keep in mind that your PR might not be accepted.

[commit-lint]

Features

• urlbuilder: add path encoding to be disabled optionally (51f15e2)
• urlbuilder: add path encoding to be disabled optionally in create_url() and create_srcset() functions (2a5dd2b)

Tests

• urlbuilder: add more testcases to test urlbuilder with srcset (de9dee6)
• urlbuilder: add more testcases to test creation of url (af6ad36)
• urlbuilder: add more testcases to test creation of srcset (d45f864)

Documentation

• readme: add demo for disabled path encoding (a9233cd)
• readme: fix demo for disabled path encoding (284903e)

Code Refactoring

• urlbuilder: remove comments (cdde746)

Contributors

ashutoshkrris

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR:

• @Commit-Lint merge patch will merge dependabot PR on "patch" versions (X.X.Y - Y change)
• @Commit-Lint merge minor will merge dependabot PR on "minor" versions (X.Y.Y - Y change)
• @Commit-Lint merge major will merge dependabot PR on "major" versions (Y.Y.Y - Y change)
• @Commit-Lint merge disable will desactivate merge dependabot PR
• @Commit-Lint review will approve dependabot PR
• @Commit-Lint stop review will stop approve dependabot PR

[luqven]

Hey @ashutoshkrris, thanks again for taking the time to clean up those commits and open a new PR. This is looking great!

Unfortunately, CI is failing because our linter, flake8, is taking issues with some of the syntaxes. That's not your fault, our tests don't run flake8 locally so you wouldn't have known. We need to document that better.

Do you think you could address the issues flake8 is raising on CI? Namely:

imgix/urlbuilder.py:41:80: E501 line too long (87 > 79 characters)
tests/test_url_builder.py:30:80: E501 line too long (86 > 79 characters)
tests/test_url_builder.py:178:1: E302 expected 2 blank lines, found 1
tests/test_url_builder.py:178:80: E501 line too long (82 > 79 characters)
tests/test_url_builder.py:185:80: E501 line too long (85 > 79 characters)
tests/test_srcset.py:137:80: E501 line too long (90 > 79 characters)
tests/test_srcset.py:141:11: E275 missing whitespace after keyword
tests/test_srcset.py:145:80: E501 line too long (90 > 79 characters)
tests/test_srcset.py:149:11: E275 missing whitespace after keyword

One quick way to do this would be to use black, a python auto-formatter. In the root of your project fork, could do run something like:

# install black
pip install black
# run black on the files that have linting issues with max line-length set to 79
# ie, black --line-length 79 {source_file_or_directory}
black --line-length 79 tests/test_url_builder.py 
# make sure you have no more flake8 issues
tox -e flake8

[ashutoshkrris]

Thanks for the feedback @luqven
I have formatted the code using black and made sure that flake8 doesn't raise any more issues.

[luqven]

Hey @ashutoshkrris,

Thanks for your prompt work on this, it's coming along nicely!

I did get some feedback from the team, and we're leaning toward doing this functionally rather than in a class setting. IE, the options accepted as an optional parameter in create_url.

def create_url(self, path="", params={}, options={}):

Where options would accept disablePathEncoding.

I apologize for asking you to change the approach here after all your hard work. I failed to consider the inconsistency that creating a class setting would create with our other SDKs, where this is handled functionally.

If it's not too much trouble, would you mind refactoring your approach to instead add disablePathEncoding to create_url? I'm happy to clarify that further if it's unclear why I'm asking for this change.

I want to be respectful of your time so if you'd rather one of the team members handle this change you can always opt to give us push permissions to your fork and we'll take this the rest of the way.

Again thank you for the work you've put into this, it's been very helpful.

[ashutoshkrris]

Hi @luqven
As per the above-requested change, I have made the required changes in the code.
I have also updated the description of the PR

[luqven]

Things are looking great!

I left you some small questions and will take this back to the team. I'll come back and review the PR after we've had a chance to discuss it.

[luqven]

LGTM