On Thu, Jun 19, 2014 at 09:58:43AM -0400, Abhishek Koneru wrote:
Please review the attached patch which adds methods that allow users
to
pass the profile data in a file.
Also attached two sample xml files, original.xml and modified.xml.
Place them in /tmp before running the profile.py module.
Thanks,
Abhishek
Hi Abhishek,
Patch applied and __main__ example run. Seems to do what it says on
the tin; the usual "haven't used it in anger" caveat ^_^. Review
comments inline below.
Fraser
+ @pki.handle_exceptions()
def modify_profile(self, profile_data):
"""
Modify an existing profile.
"""
- if profile_data is None:
- raise ValueError("No ProfileData specified")
+ return self._send_profile_request(profile_data, 'modify')
+
+ def _send_request_in_file(self, path_to_file, data_format, operation):
+
+ if path_to_file is None:
+ raise ValueError("File path must be specified.")
+
+ if data_format not in ['xml', 'json']:
+ raise ValueError("Unsupported data type: " + str(data_format))
+
+ if operation not in ['create', 'modify']:
+ raise ValueError("Invalid operation specified: " +
str(operation))
+
+ data = None
+ with open(path_to_file) as input_file:
+ data = input_file.read()
+
+ if data_format == 'xml':
+ self.headers['Content-type'] = 'application/xml'
1)
Overwriting self.headers['Content-type'] is problematic. For
example, if the data cannot be parsed or lacks an 'id' key, an
exception will be raised and the ProfileClient will be stuck with
the wrong Content-Type header.
Possible solutions:
- use try/finally or a context manager to ensure the header gets
reset to 'application/json' even if an exception is raised.
- Modify the _put and _post methods to include an optional argument
to override the content-type header. To avoid special-cases too
many things, this arg could even be a dict that can be merged with
the default headers, e.g.:
def _post(self, url, payload=None, headers=None):
self.account_client.login()
headers = dict(self.headers, **(headers or {}))
r = self.connection.post(url, payload, headers, query_params)
...
Then callers can supply custom headers for a single request
without potentially affecting other requests.
+
+ r = None
+ # Sending the data to the server.
+ if operation == 'create':
+ r = self._post(self.profiles_url, data)
+ else:
+ profile_id = None
+ if data_format == 'xml':
+ profile_id = etree.fromstring(data).get('id')
+ else:
+ profile_id = json.loads(data)['id']
+
+ if profile_id is None:
+ raise ValueError('Profile Id is not specified.')
+ url = self.profiles_url + '/' + profile_id
+ r = self._put(url, data)
+
+ # Reset the Content-type header to json(As used by other methods).
+ if data_format == 'xml':
+ self.headers['Content-type'] = 'application/json'
- url = self.profiles_url + '/' + str(profile_data.profile_id)
- profile_object = json.dumps(profile_data, cls=encoder.CustomTypeEncoder,
- sort_keys=True)
- r = self._put(url, profile_object)
return Profile.from_json(r.json())
+ @pki.handle_exceptions()
+ def create_profile_using_file_input(self, path_to_file,
data_format="xml"):
+ """
+ Create a new profile from a profile object stored in a file.
+ Acceptable data formats - json, xml.
+ """
+ return self._send_request_in_file(path_to_file, data_format, 'create')
+
2)
Default ``data_format="xml"`` makes it too easy for people to misuse
the API, e.g. a path to a JSON file, but no ``data_format`` kwarg
given, resulting in default "xml" being used. I suggest either
making it a compulsory argument, or making its default ``None`` and,
if no explicit ``data_format`` arg given, using the file extension.
+ @pki.handle_exceptions()
+ def modify_profile_using_file_input(self, path_to_file,
data_format="xml"):
+ """
+ Modify a profile from a profile object stored in a file.
+ Acceptable data formats - json, xml.
+ """
+ return self._send_request_in_file(path_to_file, data_format, 'modify')
+
+ @pki.handle_exceptions()
def delete_profile(self, profile_id):
"""
Delete a profile.
@@ -1185,6 +1253,23 @@ def main():
# pylint: disable-msg=W0703
except Exception as e:
print str(e)
+ print
+
+ # Creating a profile from file
+ print('Creating a profile using file input.')
+ print('------------------------------------')
+ original = profile_client.create_profile_using_file_input(
+ '/tmp/original.xml')
+ print(original)
+ print
+
+ # Modifying a profile from file
+ print('Modifying a profile using file input.')
+ print('------------------------------------')
+ modified = profile_client.modify_profile_using_file_input(
+ '/tmp/modified.xml')
+ print(modified)
+ print
3)
Nit-pick: could you delete the created profile here so that the
example can be run multiple times?