Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

if constant is optional, don't force to value #952

Merged
merged 7 commits into from
Jun 16, 2021

Conversation

iscai-msft
Copy link
Contributor

@iscai-msft iscai-msft commented Jun 7, 2021

fixes #950, fixes #953

self.client_default_value = client_default_value

@property
def constant(self) -> bool:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

decided to make constant and validation_map readonly properties to unclutter the main__init__ and because we don't set them later


:ivar parameter: Default value: "value1".
:vartype parameter: str
:param parameter:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

swagger. Since this is an optional constant with an x-ms-client-default, we default to the client default.

The change in description shows me our descriptions can be improved, filed a separate issue for it here.


:ivar parameter: Default value: "value1".
:vartype parameter: str
:param parameter:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

swagger. This is an optional constant with no default, so we default to None.

The doc should also be improved here, so we show the one non-None value. This is included in the same docstring issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add docstring fix to this PR

@@ -175,8 +173,8 @@ class FlattenParameterGroup(msrest.serialization.Model):
:type description: str
:param max_product_display_name: Display name of product.
:type max_product_display_name: str
:ivar capacity: Capacity of product. For example, 4 people. Default value: "Large".
:vartype capacity: str
:param capacity: Capacity of product. For example, 4 people.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

swagger. Since current behavior of enum is modelAsString defaults to False, and required defaults to False, this param is an optional constant with no default value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ask tim about whether modelAsString defaults to false or true

@@ -303,8 +297,8 @@ class SimpleProduct(BaseProduct):
:type description: str
:param max_product_display_name: Display name of product.
:type max_product_display_name: str
:ivar capacity: Capacity of product. For example, 4 people. Default value: "Large".
:vartype capacity: str
:param capacity: Capacity of product. For example, 4 people.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same param as above

@@ -114,8 +114,8 @@ class Product(msrest.serialization.Model):
:vartype const_int: int
:ivar const_string: Required. Constant string. Default value: "constant".
:vartype const_string: str
:ivar const_string_as_enum: Constant string as Enum. Default value: "constant_string_as_enum".
:vartype const_string_as_enum: str
:param const_string_as_enum: Constant string as Enum.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

swagger

Not required, no default value

@@ -454,6 +454,7 @@ async def post_flattened_simple_product(
product_id: str,
description: Optional[str] = None,
max_product_display_name: Optional[str] = None,
capacity: Optional[str] = "Large",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as you'll notice, the default for optional constants in the method signature is a value. This has to do with this issue. Still need to find a good non-breaking way around this

@iscai-msft iscai-msft requested a review from lmazuel June 7, 2021 18:49
@iscai-msft
Copy link
Contributor Author

/azp run autorest.python - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@iscai-msft iscai-msft merged commit fb8ad8b into autorestv3 Jun 16, 2021
@iscai-msft iscai-msft deleted the fix_optional_constant_property branch June 16, 2021 21:44
iscai-msft added a commit that referenced this pull request Jun 21, 2021
…into fix_content_type_kwarg

* 'autorestv3' of https://github.com/Azure/autorest.python:
  if constant is optional, don't force to value (#952)
  add support and tests for anything (#946)
  combine method signature function (#942)
  Update operations.md
  Update operations.md
  poll by default for data plane (#936)
  don't split docstrings on hyphens (#931)
  Update initializing.md
iscai-msft added a commit that referenced this pull request Jul 8, 2021
…into prepare_request

* 'autorestv3' of https://github.com/Azure/autorest.python:
  Update ChangeLog.md
  fix lro parameterization with constant params (#968)
  Update package.json
  Update ChangeLog.md
  fix content type kwarg (#956)
  if constant is optional, don't force to value (#952)
  add support and tests for anything (#946)
iscai-msft added a commit that referenced this pull request Jul 15, 2021
…ython into hybrid

* 'prepare_request' of https://github.com/Azure/autorest.python:
  improve send_request docstring, allow send_request to be private
  regen with new show flags
  regen without PipelineTransportHttpRequest import
  multiapi generating
  revert multiapi to version in autorestv3 branch
  all azure vanilla tests passing
  temp
  switch make_request fixture to send_request
  azure tests passing
  Update ChangeLog.md
  fix lro parameterization with constant params (#968)
  temp
  vanilla llc passing
  make azure legacy folder, all but storage generating
  Update package.json
  Update ChangeLog.md
  fix content type kwarg (#956)
  if constant is optional, don't force to value (#952)
  add support and tests for anything (#946)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants