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

OpenStack BaseConnection does not allow GET parameters as a key-value pairing

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Core
    • Labels:
      None

      Description

      For GETs, OpenStack BaseConnection assigns the cache-busting attribute on the parameters to a random number.

      This assignment is done as a dictionary assignment, meaning it is not possible to pass in a value of key-value pairs (the main reason to do this is to pass the same key in with different values, for example, as in the batch delete at http://docs.rackspace.com/loadbalancers/api/v1.0/clb-devguide/content/Remove_Nodes-d1e2675.html

      1. libcloud-153.patch
        5 kB
        Dave King
      2. libcloud-153-attempt-2.patch
        4 kB
        Dave King

        Activity

        Hide
        tildedave Dave King added a comment -

        First attempt at this.

        Show
        tildedave Dave King added a comment - First attempt at this.
        Hide
        tildedave Dave King added a comment -

        I've attached a file with my first attempt at this.

        I think that libcloud not supporting params as a tuple list makes the utility of the connection objects much lower.

        I think long term we would want to get away from params-as-a-dict. I'm not sure if it's better to just allow the OpenStackBaseConnection to have this special behavior for now or allow users to pass parameters in as a dict of lists (but that breaks a lot of things too).

        Let me know what you think.

        Show
        tildedave Dave King added a comment - I've attached a file with my first attempt at this. I think that libcloud not supporting params as a tuple list makes the utility of the connection objects much lower. I think long term we would want to get away from params-as-a-dict. I'm not sure if it's better to just allow the OpenStackBaseConnection to have this special behavior for now or allow users to pass parameters in as a dict of lists (but that breaks a lot of things too). Let me know what you think.
        Hide
        kami Tomaz Muraus added a comment -

        I would be OK with only supporting this in the OpenStack driver for now, but I would also like to fix it inside the base connection class and other drivers ASAP.

        Also keep in mind that there are many hooks and places where the request parameters are modified (add_default_params, etc.) which means we need to be careful when modifying existing code and add a bunch of tests so we can be sure that we didn't break something.

        Show
        kami Tomaz Muraus added a comment - I would be OK with only supporting this in the OpenStack driver for now, but I would also like to fix it inside the base connection class and other drivers ASAP. Also keep in mind that there are many hooks and places where the request parameters are modified (add_default_params, etc.) which means we need to be careful when modifying existing code and add a bunch of tests so we can be sure that we didn't break something.
        Hide
        tildedave Dave King added a comment -

        Cool. I'll open another JIRA for that.

        I think the right pattern for a general fix is to make a _extend_parameters in the Connection superclass that does the right thing if you pass in a dict or a list. That should alleviate most of the danger in making this change, unless there's something I'm missing.

        Show
        tildedave Dave King added a comment - Cool. I'll open another JIRA for that. I think the right pattern for a general fix is to make a _extend_parameters in the Connection superclass that does the right thing if you pass in a dict or a list. That should alleviate most of the danger in making this change, unless there's something I'm missing.
        Hide
        tildedave Dave King added a comment -

        Oh, what is the pattern for getting the pydoc correct here? params is no longer a dict but I'm not clear how to update the pydoc to show it can be a dict or a tuple list.

        Show
        tildedave Dave King added a comment - Oh, what is the pattern for getting the pydoc correct here? params is no longer a dict but I'm not clear how to update the pydoc to show it can be a dict or a tuple list.
        Hide
        kami Tomaz Muraus added a comment -

        Sounds good to me.

        Also please update your repository and attach a new patch, because this one doesn't apply cleanly to the current trunk (probably related to the Brad's latest changes).

        Show
        kami Tomaz Muraus added a comment - Sounds good to me. Also please update your repository and attach a new patch, because this one doesn't apply cleanly to the current trunk (probably related to the Brad's latest changes).
        Hide
        kami Tomaz Muraus added a comment -

        I would make the docstring look like this:

        @type params: C

        {dict}

        or C

        {list}

        of C

        {tuple}
        Show
        kami Tomaz Muraus added a comment - I would make the docstring look like this: @type params: C {dict} or C {list} of C {tuple}
        Hide
        tildedave Dave King added a comment -

        I'll change the docstring on the base Connection class to that with LIBCLOUD-154, thanks.

        Attached a patch of changes against the most recent trunk, sorry about that.

        Show
        tildedave Dave King added a comment - I'll change the docstring on the base Connection class to that with LIBCLOUD-154 , thanks. Attached a patch of changes against the most recent trunk, sorry about that.

          People

          • Assignee:
            Unassigned
            Reporter:
            tildedave Dave King
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development