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

Attributes cannot start with underscore #383

Closed
dblanchette opened this issue Apr 7, 2021 · 4 comments
Closed

Attributes cannot start with underscore #383

dblanchette opened this issue Apr 7, 2021 · 4 comments
Labels
🐞bug Something isn't working
Milestone

Comments

@dblanchette
Copy link
Contributor

Describe the bug
If a response attribute in the API starts with _, the client fails when loading the response.

To Reproduce
Steps to reproduce the behavior:

  1. Generate a client for an API with a response that starts with _
  2. Try to call the API
  3. You will get something of the sort:
In [34]: jira_issues_list.sync_detailed(client=c).content
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-34-2bc36d00abc8> in <module>
----> 1 jira_issues_list.sync_detailed(client=c).content

C:\my_api\api\jira_issues\jira_issues_list.py in sync_detailed(client, created_time, created_time__range, in_progress_time, in_progress_time__range, key, key__iexact, page, per_page, web_url, web_url__iexact)
    100     )
    101
--> 102     return _build_response(response=response)
    103
    104

C:\my_api\api\jira_issues\jira_issues_list.py in _build_response(response)
     64         content=response.content,
     65         headers=response.headers,
---> 66         parsed=_parse_response(response=response),
     67     )
     68

C:\my_api\api\jira_issues\jira_issues_list.py in _parse_response(response)
     53 def _parse_response(*, response: httpx.Response) -> Optional[PaginatedJiraIssueList]:
     54     if response.status_code == 200:
---> 55         response_200 = PaginatedJiraIssueList.from_dict(response.json())
     56
     57         return response_200

C:\my_api\models\paginated_jira_issue_list.py in from_dict(cls, src_dict)
     57         _results = d.pop("results", UNSET)
     58         for results_item_data in _results or []:
---> 59             results_item = JiraIssue.from_dict(results_item_data)
     60
     61             results.append(results_item)

C:\my_api\models\jira_issue.py in from_dict(cls, src_dict)
     73             in_progress_time = isoparse(_in_progress_time)
     74
---> 75         jira_issue = cls(
     76             key=key,
     77             web_url=web_url,

TypeError: __init__() got an unexpected keyword argument '_url'

Expected behavior
Be able to have attributes that starts with _

OpenAPI Spec File
I cannot share publicly, but here is the gist of it:

  /v2/jira_issues/{id}/:
    get:
      operationId: jira_issues_retrieve
      parameters:
      - in: path
        name: id
        schema:
          type: integer
        required: true
      tags:
      - jira_issues
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/JiraIssue'
          description: ''
    JiraIssue:
      type: object
      properties:
        web_url:
          type: string
          format: uri
        _url:
          type: string
          format: uri
          readOnly: true
        id:
          type: integer
          readOnly: true
      required:
      - _url
      - id
      - web_url

Desktop (please complete the following information):

  • OS: Windows 10
  • Python Version: 3.8.8
  • openapi-python-client version: 0.8.0

Additional context
This is caused by python-attrs/attrs#391

I can fix it easily in the generated code of the model:

        jira_issue = cls(
            key=key,
            web_url=web_url,
            url=_url,
        )

instead of

        jira_issue = cls(
            key=key,
            web_url=web_url,
            _url=_url,
        )

(this is in from_dict)

But what happens if we have url and _url? I tried creating such a class with attrs and it crashes.

This is probably a tricky one, because of the reliance on attrs which refuses to add an option to disable the behavior of stripping leading underscore.

@dblanchette dblanchette added the 🐞bug Something isn't working label Apr 7, 2021
@dbanty
Copy link
Collaborator

dbanty commented Apr 7, 2021

I actually encountered this same issue in one of our internal generated clients. We're going to have to normalize parameters somehow to make it happy 😞. We already have to rename some parameters automatically to make Python happy so we'll probably just have to do the same here and accept that certain combinations of parameter names won't be viable.

I believe long ago we used @dataclass instead of attrs, I don't remember which features of attrs in particular we're utilizing that the built in can't do. My guess is the main reason we switched was to add support for Python 3.6. Maybe we can switch back whenever we yank 3.6 support? For now though I think stripping leading underscores during generation is probably our best bet.

@dbanty dbanty added this to the 0.9.0 milestone May 2, 2021
@dbanty
Copy link
Collaborator

dbanty commented May 2, 2021

@dblanchette this will get better with 0.9.0 (coming soon) as on main we switched from stringcase to our own parsing/transformation code. A quick test shows that the python_name of _url will be url. I think this resolves the bug... though there will be a limitation that both _url and url can't exist together. I'm not sure how to resolve that easily...

@dbanty dbanty closed this as completed May 3, 2021
@dblanchette
Copy link
Contributor Author

dblanchette commented May 3, 2021

We've abandoned the project to use this library for now, but I think it is promising and would like to be able to use it in the future. For us, we're stuck with both _url and url for retrocompatibility reasons.

Ideas:

  • Adding some sort of canned prefix when it starts with underscore, like object. Not very clean, and then there is the question of what to do if e.g. object_url already exists.
  • Convince the folks at attrs to address option for disable strips the underscores python-attrs/attrs#391 or do a fork of that lib with the option to not strip leading underscores
  • Switch (back) to dataclasses or something else
  • Move the underscore to after, e.g. url_ instead of _url. That would necessitate to add another underscore in case url_ already exists and so on.
  • Add configuration option/some sort of annotation in the spec for a "python friendly" name

Just sharing my thoughts here :)

@dbanty
Copy link
Collaborator

dbanty commented May 3, 2021

Thanks for responding @dblanchette! We probably will move off of attrs next year once Python 3.6 is no longer supported and I no longer feel obligated to keep compatibility for it. I think we still want to avoid leading underscores since they have a "special" meaning in Python but auto-renaming to anything else won't really solve the issue. Allowing an override seems like the best option, and in some places you can already do this by filling in the title attribute. So maybe the answer is to just make sure we support title everywhere as the basis for the python_name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants