Skip to content

Fix for Python client upload list of files Issue #2036#2046

Closed
scottrw93 wants to merge 6 commits intoswagger-api:masterfrom
scottrw93:master
Closed

Fix for Python client upload list of files Issue #2036#2046
scottrw93 wants to merge 6 commits intoswagger-api:masterfrom
scottrw93:master

Conversation

@scottrw93
Copy link
Copy Markdown
Contributor

Issue #2036 A fix to allow a list of files to be uploaded. The main change concerns the type of form_params being changed from a dict() to nested tuples in the form (key, (value)) to allow files with the same key not to be overwritten each iteration.

@wing328 wing328 added this to the v2.1.6 milestone Feb 5, 2016
@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

@scottrw93 thanks for the PR. We'll review and let you know if we've any questions.

Do you mind sharing the spec (with array of file as parameter) for repeating the issue?

Also I notice the commits are not linked to your Github account and therefore your contribution won't count toward https://github.com/swagger-api/swagger-codegen/graphs/contributors.

@scottrw93
Copy link
Copy Markdown
Contributor Author

I never knew that's why my avatar is blank! The spec is available at https://github.com/scottrw93/swagger-python-test/blob/master/swagger.json. I also have a spring server and the before and after python client if you want to test in the same repo

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

@scottrw93 thanks for the spec. Will use it for testing this PR and other PRs that fix similar issue in other API clients (e.g. PHP, Ruby, etc)

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 7, 2016

You will need to fix your local git setting: user.email

Ref: http://alvinalexander.com/git/git-show-change-username-email-address

Then push again and Github should be able to link the commits correctly to your Github account.

@scottrw93
Copy link
Copy Markdown
Contributor Author

Ok, I have done that and the latest push is linked to my account. Thanks for your help, I'll bear it in mind for future PR's.

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 7, 2016

@scottrw93 in the "Commits" tab, I still see previous commits not linked to your account.

(I'm ok with that but just want to make sure you get the credits you deserved for your contribution/PR, which is reflected here: https://github.com/swagger-api/swagger-codegen/graphs/contributors)

One possible workaround is to close this PR and submit a new one.

@scottrw93
Copy link
Copy Markdown
Contributor Author

Ok, I have created a new PR shown above. I think it might still be in the same situation but I'm happy for you to go ahead though and merge.

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 7, 2016

@scottrw93
Copy link
Copy Markdown
Contributor Author

Thanks, I have done that. It has created conflicts though so I can close it and reopen this one if you like

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 7, 2016

It has a lot of commits so it seems to me it's not updated correctly.

I'll give it a try in my end.

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 7, 2016

I submitted #2060 with commits correctly linked to your Github account.

@scottrw93
Copy link
Copy Markdown
Contributor Author

Thanks very much.

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 7, 2016

FYI. All test cases passed:

-----------------------------------------------------------------
TOTAL                                1338    528    61%   
----------------------------------------------------------------------
Ran 29 tests in 29.580s

OK
_____________________________________________________ summary ______________________________________________________
  py27: commands succeeded
  py34: commands succeeded
  congratulations :)

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 7, 2016

PR merged. Thanks again for the contribution.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants