The SoftLayer_Ticket::addAttachedFile method fails when tried to attach images #506 by gordonjm · Pull Request #507 · softlayer/softlayer-python · GitHub
Skip to content

The SoftLayer_Ticket::addAttachedFile method fails when tried to attach images #506#507

Closed
gordonjm wants to merge 3 commits into
softlayer:masterfrom
gordonjm:master
Closed

The SoftLayer_Ticket::addAttachedFile method fails when tried to attach images #506#507
gordonjm wants to merge 3 commits into
softlayer:masterfrom
gordonjm:master

Conversation

@gordonjm

Copy link
Copy Markdown

A proposed fix for the "No authentication headers found" problem in issue #506.

@landscape-bot

Copy link
Copy Markdown

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 85.75% when pulling 7822b24 on gordonjm:master into ef030ec on softlayer:master.

Comment thread SoftLayer/transports.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

to tackle too many local variable restriction of landscape .. you could may be direclty compare
if sys.version_info[0] <3

not sure who/why the limit of 15 local variables is/was enforced

@gordonjm

Copy link
Copy Markdown
Author

I eliminated "interpreter_version" in favor of "sys.version_info[0]" to get to the local variables limit.

@landscape-bot

Copy link
Copy Markdown

Code Health
Code quality remained the same when pulling dc524a3 on gordonjm:master into ef030ec on softlayer:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 85.75% when pulling dc524a3 on gordonjm:master into ef030ec on softlayer:master.

@sudorandom

Copy link
Copy Markdown
Contributor

Are we sure that it works this way with Python 3? From a library standpoint with no knowledge of the server's API endpoints I don't know how it would ever assume that an arbitrary parameter needs to be base64 encoded without using a user specifying the xmlrpc_client.Binary type or making assumptions (which is what this PR is making). Any other endpoint that takes an object that has a "data" key is assumed to be base64, which has the following problems:

  • Objects with a 'data' attribute may have a type other than base64
  • Attributes of the base64 type aren't always named 'data'

To me, it seems like the user should give the base64 argument the base64ed value. This should work today with no code change to the library required and fits more in line with SLDN documentation. When it says it expects a base64 encoded value, you should give it a base64 encoded value. Example:

attachedFile = { 'data': base64.b64encode(filecontents), 'filename': 'image.png'}
result = client['Ticket'].addAttachedFile(attachedFile, id=ticket_id)

or potentially use a xmlrpc.Binary value:

attachedFile = { 'data': xmlrpc.Binary(filecontents), 'filename': 'image.png'}
result = client['Ticket'].addAttachedFile(attachedFile, id=ticket_id)

or as a SoftLayer-client type (so that other transports like REST will be able to support this):

attachedFile = { 'data': softlayer.Base64(filecontents), 'filename': 'image.png'}
result = client['Ticket'].addAttachedFile(attachedFile, id=ticket_id)

@gordonjm

Copy link
Copy Markdown
Author

@gordonjm gordonjm closed this Mar 26, 2015
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.

5 participants