Uploaded image for project: 'Libcloud'
  1. Libcloud
  2. LIBCLOUD-399

Creating instances of AzureBlobsStorageDriver should not mutate the host attribute of AzureBlobsConnection

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.13.1
    • Fix Version/s: 0.14.0-beta3
    • Component/s: Storage
    • Labels:
      None

      Description

      Creating instances of AzureBlobsStorageDriver should not mutate the host attribute of AzureBlobsConnection.

      The host attribute is dependent on the key of the azure blob storage account. It should be possible to have 2 drivers to do distinct Azure storage accounts in the same Python program without having one driver instance affect the state of another.

      Please followup for a patch.

      1. LIBCLOUD-399_20130915_1.diff
        3 kB
        Olivier Grisel
      2. LIBCLOUD-399_20130915.diff
        2 kB
        Olivier Grisel

        Activity

        Hide
        ogrisel Olivier Grisel added a comment -

        Here is a patch with a fix + test for this issue. The patch is also available on this branch of my github fork:

        https://github.com/ogrisel/libcloud/compare/LIBCLOUD-399

        There might be a cleaner way to do it but I don't see how without resorting to metaclass which I think would make the code even more complex.

        Show
        ogrisel Olivier Grisel added a comment - Here is a patch with a fix + test for this issue. The patch is also available on this branch of my github fork: https://github.com/ogrisel/libcloud/compare/LIBCLOUD-399 There might be a cleaner way to do it but I don't see how without resorting to metaclass which I think would make the code even more complex.
        Hide
        kami Tomaz Muraus added a comment -

        Thanks, that's a good catch.

        Another and preferred way to do that would be to use `ex_connection_class_kwargs`, similar as it's done in this pull request - https://github.com/apache/libcloud/pull/138 (https://github.com/apache/libcloud/pull/138/files#L0R168).

        Show
        kami Tomaz Muraus added a comment - Thanks, that's a good catch. Another and preferred way to do that would be to use `ex_connection_class_kwargs`, similar as it's done in this pull request - https://github.com/apache/libcloud/pull/138 ( https://github.com/apache/libcloud/pull/138/files#L0R168 ).
        Hide
        ogrisel Olivier Grisel added a comment -

        Thanks Tomaz, I will have a look an update my patch ASAP if I can make it work.

        Show
        ogrisel Olivier Grisel added a comment - Thanks Tomaz, I will have a look an update my patch ASAP if I can make it work.
        Hide
        ogrisel Olivier Grisel added a comment -

        Here is a new version of the patch. I also updated my LIBCLOUD-399 branch on github.

        Show
        ogrisel Olivier Grisel added a comment - Here is a new version of the patch. I also updated my LIBCLOUD-399 branch on github.
        Hide
        kami Tomaz Muraus added a comment -

        Olivier Grisel Thanks, the patch looks good.

        Can you please re-generate it with git format-patch so I can preserve the commit author when merging it?

        Thanks

        Show
        kami Tomaz Muraus added a comment - Olivier Grisel Thanks, the patch looks good. Can you please re-generate it with git format-patch so I can preserve the commit author when merging it? Thanks
        Hide
        kami Tomaz Muraus added a comment -

        Actually, ignore my last comment. This pull request only has a single commit so I can just use patch from that.

        Show
        kami Tomaz Muraus added a comment - Actually, ignore my last comment. This pull request only has a single commit so I can just use patch from that.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 79444117414be2aba91bd8c44d7abc9bd66dc37d in branch refs/heads/trunk from Olivier Grisel
        [ https://git-wip-us.apache.org/repos/asf?p=libcloud.git;h=7944411 ]

        LIBCLOUD-399: avoid mutating AzureBlobsConnection at driver instance time

        Signed-off-by: Tomaz Muraus <tomaz@apache.org>

        Show
        jira-bot ASF subversion and git services added a comment - Commit 79444117414be2aba91bd8c44d7abc9bd66dc37d in branch refs/heads/trunk from Olivier Grisel [ https://git-wip-us.apache.org/repos/asf?p=libcloud.git;h=7944411 ] LIBCLOUD-399 : avoid mutating AzureBlobsConnection at driver instance time Signed-off-by: Tomaz Muraus <tomaz@apache.org>
        Hide
        ogrisel Olivier Grisel added a comment -

        Thanks. I think this issue can be closed now. There is just a typo in the Changes entry: IBCLOUD-399 instead of LIBCLOUD-399.

        Show
        ogrisel Olivier Grisel added a comment - Thanks. I think this issue can be closed now. There is just a typo in the Changes entry: IBCLOUD-399 instead of LIBCLOUD-399 .
        Hide
        kami Tomaz Muraus added a comment -

        No problem.

        I have fixed a typo and pushed another change to only return "host" in the "_ex_connection_class_kwargs" method if the "host" argument is not passed to the driver constructor (https://git-wip-us.apache.org/repos/asf?p=libcloud.git;a=commitdiff;h=5888d45c000ba3045ba03cb845a8191a822409e6). The reason is that "host" argument, if present, has precedence over "key".

        This way users can, for example, also use it with other installations which expose Azure API.

        Show
        kami Tomaz Muraus added a comment - No problem. I have fixed a typo and pushed another change to only return "host" in the "_ex_connection_class_kwargs" method if the "host" argument is not passed to the driver constructor ( https://git-wip-us.apache.org/repos/asf?p=libcloud.git;a=commitdiff;h=5888d45c000ba3045ba03cb845a8191a822409e6 ). The reason is that "host" argument, if present, has precedence over "key". This way users can, for example, also use it with other installations which expose Azure API.

          People

          • Assignee:
            kami Tomaz Muraus
            Reporter:
            ogrisel Olivier Grisel
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development