[Merge] lp:~flo-fuchs/mailman/restclient into lp:mailman

[Florian Fuchs]

Review: Resubmit
I did some fixes and improvements like suggested in the last review...

> > +    entry 0:
> > +        ...
> > +        self_link: http://localhost:8001/3.0/lists/test-
> one at example.com/member/jack at example.com
> > +    entry 1:
> > +        ...
> > +        self_link: http://localhost:8001/3.0/lists/test-
> one at example.com/member/meg at example.com
> > +    entry 2:
> > +        ...
> > +        self_link: http://localhost:8001/3.0/lists/test-
> two at example.com/member/jack at example.com
> 
> The client is returning json here, right?

Nope, the client never returns json. Either HTTP status codes or lists/dicts are returned.


> Should we be using httplib2 and urllib2 here?  See the implementation of
> dump_json().

Done.

> > +    def _delete_request(self, path):
> > +        """Send an HTTP DELETE request.
> > +
> > +        :param path: the URL to send the DELETE request to
> > +        :type path: string
> > +        :return: request status code
> > +        :rtype: string
> > +        """
> > +        try:
> > +            self.c.request('DELETE', path, None, self.headers)
> > +            r = self.c.getresponse()
> > +            return r.status
> > +        finally:
> > +            self.c.close()
> 
> I wonder if this duplication can be refactored?

There's only one http request method now.


> > +    def _validate_email_host(self, email_host):
> > +        """Validates a domain name.
> > +
> > +        :param email_host: the domain str to validate
> > +        :type email_host: string
> > +        """
> > +        pat = re.compile('^[-a-z0-9\.]+\.[-a-z]{2,4}$', re.IGNORECASE)
> > +        if not pat.match(email_host):
> > +            raise MailmanRESTClientError('%s is not a valid domain name' %
> email_host)
> 
> Won't the Mailman core refuse to create a domain if it's not valid?  It might
> still be worth doing client-side validation, but I would expect that more in
> some webui JavaScript.  What's the advantage of doing this extra check (which
> might be different than what happens in the core)?

I didn't know if the core does email validation. Also, the django app does some validation. So I removed it. 

> I wonder if this method is necessary.  In general, attributes are preferred
> over accessors, and you've already got a public one right here!  So clients
> can do:
> 
>     >>> my_domain = client.get_domain('example.com')
>     >>> my_domain.domain_info
>     ...
> 
> directly.  In fact, for polymorphism, maybe the attribute should just be
> called 'info'?

Done. 

-- 
https://code.launchpad.net/~flo-fuchs/mailman/restclient/+merge/28522
Your team Mailman Coders is requested to review the proposed merge of lp:~flo-fuchs/mailman/restclient into lp:mailman.