Skip to content

[Python] adds required tag, allows null values in models#3923

Merged
wing328 merged 3 commits intoswagger-api:masterfrom
vishalsanfran:swagger-3922
Oct 7, 2016
Merged

[Python] adds required tag, allows null values in models#3923
wing328 merged 3 commits intoswagger-api:masterfrom
vishalsanfran:swagger-3922

Conversation

@vishalsanfran
Copy link
Copy Markdown

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Swagger ignores 'required' tag in model.mustache in python.
The setter function for these properties does not allow null values, unlike ruby and other languages

Issue: #3922

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Oct 5, 2016

@vishalsanfran thanks for the PR. Would you please run ./bin/python-petstore.sh to update the Python Petstore API client so that the CI can test the changes?

@vishalsanfran
Copy link
Copy Markdown
Author

pr is updated

{{#hasValidation}}

{{#required}}
if not {{name}}:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be an issue if {{name}} is 0 (integer) or False (boolean)

What about using if {{name}} is None to check instead?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, makes sense, i'll update

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This particular issue was blocking me, since I got the invalid value due to a 0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can merge this PR as @vishalsanfran has fixed the issue related to 0 and False but I found other issues about validation for optional properties and doing "required" check only if there are other validation rules (#hasValidation = true). I'll file another PR with the fix to the issues I mentioned.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vishalsanfran @jdevera FYI. I've filed #3944 with the fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#3944 merged into master. Please pull the latest master to give it a try.

@vishalsanfran
Copy link
Copy Markdown
Author

updated

@wing328 wing328 merged commit 0ca6035 into swagger-api:master Oct 7, 2016
acramatte added a commit to comerge/swagger-codegen that referenced this pull request Oct 10, 2016
* upstream/master:
  [aspnet] Fix .sln/.xproj guids
  [Python][Flask] fix python2 support in Flask (swagger-api#3952)
  Bugfix/3929 do not set multipart (swagger-api#3932)
  fix python required property check and validation for optional properties
  required tag is used in model, allows null values (swagger-api#3923)
  fix number format for dart model
  support number enum for swift
  [Objc] Added support for lower case discriminator (swagger-api#3927)
  [Android][Volley] add serializeModel support to Android (swagger-api#3933)
  add more info about test with latest master
  add back java okhttp petstore client
  fix pom duplicated id issue
  update pom to test java okhttp-gson parcelable models
  fix bug with parcelable
  [Java] Fix bug in generated code if parcelableModel and serializableModel are both true.
  [Java] Make generated models Parcelable for okhttp-gson if the -DparcelableModel=true option is provided.
  add http://onedata.org
  validate(s)_presence_of Migration should create pluralized table names Change controller filenames
  Issue#3829. Objective-C client code, discriminator generated in both base and child.
@wing328 wing328 changed the title required tag is used in model, allows null values [Python] adds required tag, allows null values in models Feb 20, 2017
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.

3 participants