Add python 3 compatibility #6

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
2 participants
@leonardehrenfried
Contributor

leonardehrenfried commented Oct 5, 2015

Hi,

since I want to use this library with python 3 I added support for it and added py3 to the list of runtimes in tox.ini.

Three fixes were necessary:

  • Imports: mainly to do with the new urllib submodules
  • String/bytes difference: had to add a few encode/decode
  • Import path in the vendor'ed mulitencode module

Please review this and give me feedback. All tests pass.

I'd really like to get this merged!

- for k, v in scheduling_args.items():
- if v is None:
- del scheduling_args[k]
+ scheduling_args = dict((k, v) for k, v in scheduling_args.items() if v != None)

This comment has been minimized.

@gaqzi

gaqzi Oct 26, 2015

Owner

👍 ❤️

@gaqzi

gaqzi Oct 26, 2015

Owner

👍 ❤️

@gaqzi

This comment has been minimized.

Show comment
Hide comment
@gaqzi

gaqzi Oct 26, 2015

Owner

Hi @lenniboy and thank you for this PR!

I'm sorry it has taken so long for me to look at it. It's looking good to me at a quick glance, and it's good motivation for me to get the gocd-cli to support Python 3 as well. :)

I was about to merge this in but hit a snag with VCR and Python 3. It's starting to get a bit too late to get it merged in tonight, but I'll do my best to get it through this week!

The issue I'm seeing for reference is:

___________________________________________________________________________________ TestPipelineGroups.test_pipelines_returns_a_list_of_all_pipeline_names ___________________________________________________________________________________

self = <test_pipeline_groups.TestPipelineGroups object at 0x103395d68>, pipeline_groups = <gocd.api.pipeline_groups.PipelineGroups object at 0x1033959e8>

    @vcr.use_cassette('tests/fixtures/cassettes/api/pipeline_groups/small.yml')
    def test_pipelines_returns_a_list_of_all_pipeline_names(self, pipeline_groups):
>       assert pipeline_groups.pipelines == set(['No-valid-agents'])

tests/api/test_pipeline_groups.py:31:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
gocd/api/pipeline_groups.py:47: in pipelines
    if not self.response:
gocd/api/pipeline_groups.py:35: in response
    self.get_pipeline_groups()
gocd/api/pipeline_groups.py:21: in get_pipeline_groups
    self._response = self._get('/pipeline_groups')
gocd/api/endpoint.py:45: in _get
    return self._request(path, ok_status=ok_status, headers=headers)
gocd/api/endpoint.py:59: in _request
    ok_status=ok_status
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

cls = <class 'gocd.api.response.Response'>, response = <vcr.stubs.VCRHTTPResponse object at 0x103395320>, ok_status = None

    @classmethod
    def _from_request(cls, response, ok_status=None):
        return Response(
            response.code,
>           response.fp,
            response.headers,
            ok_status=ok_status
        )
E       AttributeError: 'VCRHTTPResponse' object has no attribute 'fp'

Owner

gaqzi commented Oct 26, 2015

Hi @lenniboy and thank you for this PR!

I'm sorry it has taken so long for me to look at it. It's looking good to me at a quick glance, and it's good motivation for me to get the gocd-cli to support Python 3 as well. :)

I was about to merge this in but hit a snag with VCR and Python 3. It's starting to get a bit too late to get it merged in tonight, but I'll do my best to get it through this week!

The issue I'm seeing for reference is:

___________________________________________________________________________________ TestPipelineGroups.test_pipelines_returns_a_list_of_all_pipeline_names ___________________________________________________________________________________

self = <test_pipeline_groups.TestPipelineGroups object at 0x103395d68>, pipeline_groups = <gocd.api.pipeline_groups.PipelineGroups object at 0x1033959e8>

    @vcr.use_cassette('tests/fixtures/cassettes/api/pipeline_groups/small.yml')
    def test_pipelines_returns_a_list_of_all_pipeline_names(self, pipeline_groups):
>       assert pipeline_groups.pipelines == set(['No-valid-agents'])

tests/api/test_pipeline_groups.py:31:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
gocd/api/pipeline_groups.py:47: in pipelines
    if not self.response:
gocd/api/pipeline_groups.py:35: in response
    self.get_pipeline_groups()
gocd/api/pipeline_groups.py:21: in get_pipeline_groups
    self._response = self._get('/pipeline_groups')
gocd/api/endpoint.py:45: in _get
    return self._request(path, ok_status=ok_status, headers=headers)
gocd/api/endpoint.py:59: in _request
    ok_status=ok_status
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

cls = <class 'gocd.api.response.Response'>, response = <vcr.stubs.VCRHTTPResponse object at 0x103395320>, ok_status = None

    @classmethod
    def _from_request(cls, response, ok_status=None):
        return Response(
            response.code,
>           response.fp,
            response.headers,
            ok_status=ok_status
        )
E       AttributeError: 'VCRHTTPResponse' object has no attribute 'fp'

@gaqzi

This comment has been minimized.

Show comment
Hide comment
@gaqzi

gaqzi Oct 26, 2015

Owner

Couldn't leave it be and figured it out, all testa are now passing in the develop branch. I'll have to do some manual testing before I feel comfortable clicking the big release button.

…really need to get that integration suite going :P

Owner

gaqzi commented Oct 26, 2015

Couldn't leave it be and figured it out, all testa are now passing in the develop branch. I'll have to do some manual testing before I feel comfortable clicking the big release button.

…really need to get that integration suite going :P

@leonardehrenfried

This comment has been minimized.

Show comment
Hide comment
@leonardehrenfried

leonardehrenfried Oct 27, 2015

Contributor

Good stuff!

If there is anything I can help out with, let me know!

Contributor

leonardehrenfried commented Oct 27, 2015

Good stuff!

If there is anything I can help out with, let me know!

@gaqzi

This comment has been minimized.

Show comment
Hide comment
@gaqzi

gaqzi Nov 2, 2015

Owner

28 days later and this is finally merged 😄

I've tagged and pushed a new release v0.9.0 with Python 3 support. Thanks again @lenniboy!

Owner

gaqzi commented Nov 2, 2015

28 days later and this is finally merged 😄

I've tagged and pushed a new release v0.9.0 with Python 3 support. Thanks again @lenniboy!

@gaqzi gaqzi closed this Nov 2, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment