Skip to content

Conversation

@gracelombardi
Copy link
Owner

@gracelombardi gracelombardi commented Jul 25, 2022

Still a WIP. Getting a not implemented error.

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/dist-packages/PyKMIP-0.11.0.dev1-py3.8.egg/kmip/services/server/session.py", line 167, in _handle_message_loop
    request.read(request_data, kmip_version=kmip_version)
  File "/usr/local/lib/python3.8/dist-packages/PyKMIP-0.11.0.dev1-py3.8.egg/kmip/core/messages/messages.py", line 485, in read
    batch_item.read(istream, kmip_version=kmip_version)
  File "/usr/local/lib/python3.8/dist-packages/PyKMIP-0.11.0.dev1-py3.8.egg/kmip/core/messages/messages.py", line 307, in read
    self.request_payload = self.payload_factory.create(
  File "/usr/local/lib/python3.8/dist-packages/PyKMIP-0.11.0.dev1-py3.8.egg/kmip/core/factories/payloads/__init__.py", line 48, in create
    return self._create_add_attribute_payload()
  File "/usr/local/lib/python3.8/dist-packages/PyKMIP-0.11.0.dev1-py3.8.egg/kmip/core/factories/payloads/__init__.py", line 147, in _create_add_attribute_payload
    raise NotImplementedError()
NotImplementedError

@gracelombardi gracelombardi self-assigned this Jul 25, 2022
@gracelombardi gracelombardi requested a review from arp102 July 25, 2022 15:57
@gracelombardi gracelombardi changed the title Added add_attribute operation Issue #661 Added add_attribute operation Jul 25, 2022
Copy link
Collaborator

@arp102 arp102 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good, I just have one note about some of the function documentation.

I would also encourage you to include more information in the commit messages or pull request description, like what file add_attribute.py is based on, why AddAttribute is useful, and links to the KMIP spec.

I'm also wondering if it would have been easier to try sharing code with another similar payload definition, but if all the other payloads are defined separately, then it makes sense to keep doing that for now.

I don't think this PR fully solves the problem in OpenKMIP#661 though.
For that, we would also need to add support for AddAttribute to the PyKMIP server, not just the client.

object.
kmip_version (KMIPVersion): An enumeration defining the KMIP
version with which the object will be decoded. Optional,
defaults to KMIP 1.0.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation doesn't match code.
Also occurs in some other functions below.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants