Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Add support for amqp+ssl URL's and configured username and password #17

Merged
merged 7 commits into from
Aug 2, 2018

Conversation

maruno
Copy link
Contributor

@maruno maruno commented Jul 31, 2018

No description provided.

@maruno maruno requested a review from matyaskuti July 31, 2018 09:21
Copy link
Contributor

@matyaskuti matyaskuti left a comment

Choose a reason for hiding this comment

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

@maruno Could you extend the README as well with the (new) configuration possibilities?

"""
if urls:
return [url.strip() for url in urls.split(',')]
splited_url = urlsplit(url.strip())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicking: split_url

VERSION Outdated
@@ -1 +1 @@
1.0.2
1.0.3
Copy link
Contributor

@matyaskuti matyaskuti Jul 31, 2018

Choose a reason for hiding this comment

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

I'm not sure if this is only a patch version bump, since we are adding functionality in a backwards compatible manner, I might lean towards 1.1.0

(https://semver.org/)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's pretty minor, but sure.

@@ -15,7 +15,7 @@
author = 'Bynder B.V.'

version = '1.0' # The short X.Y version.
release = '1.0.2' # The full version, including alpha/beta/rc tags.
release = '1.0.3' # The full version, including alpha/beta/rc tags.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here about the versioning

'username' in config and 'password' in config):
user_pass = f"{config['username']}:{config['password']}@"
new_netloc = user_pass + splited_url.netloc
splited_url = splited_url._replace(netloc=new_netloc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why _replace? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a form of namedtuple, eventhough it's mangled with the _ this is officially documented as the way to create a new one with changes: https://docs.python.org/3/library/collections.html#collections.somenamedtuple._replace

@matyaskuti
Copy link
Contributor

Tests failed in Travis!

@coveralls
Copy link

coveralls commented Jul 31, 2018

Pull Request Test Coverage Report for Build 28

  • 73 of 74 (98.65%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.5%) to 53.635%

Changes Missing Coverage Covered Lines Changed/Added Lines %
test/test_config.py 54 55 98.18%
Totals Coverage Status
Change from base Build 24: 2.5%
Covered Lines: 782
Relevant Lines: 1458

💛 - Coveralls

@maruno maruno requested review from condemil and mircea-cosbuc and removed request for condemil July 31, 2018 10:45
@matyaskuti matyaskuti merged commit 8f2d5a7 into Bynder:master Aug 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants