Skip to content

fix: change return value to "None" in case getattr returns None#2430

Merged
nejch merged 1 commit into
python-gitlab:mainfrom
pinxue:main
Dec 19, 2022
Merged

fix: change return value to "None" in case getattr returns None#2430
nejch merged 1 commit into
python-gitlab:mainfrom
pinxue:main

Conversation

@pinxue

@pinxue pinxue commented Dec 19, 2022

Copy link
Copy Markdown
Contributor

It is quite similar to issue #1425 , even though getattr call already provides default value.

It fixed following error:

gitlab -c gitlab.cfg user-event list --user-id 292
target_title: iozone

Traceback (most recent call last):
  File "~/miniforge3/bin/gitlab", line 8, in <module>
    sys.exit(main())
  File "~/miniforge3/lib/python3.9/site-packages/gitlab/cli.py", line 376, in main
    gitlab.v4.cli.run(
  File "~/miniforge3/lib/python3.9/site-packages/gitlab/v4/cli.py", line 549, in run
    printer.display_list(data, fields, verbose=verbose)
  File "~/miniforge3/lib/python3.9/site-packages/gitlab/v4/cli.py", line 512, in display_list
    self.display(get_dict(obj, fields), verbose=verbose, obj=obj)
  File "~/miniforge3/lib/python3.9/site-packages/gitlab/v4/cli.py", line 494, in display
    value = value.replace("\r", "").replace("\n", " ")
AttributeError: 'NoneType' object has no attribute 'replace'

@pinxue pinxue changed the title change return value to "None" in case getattr returns None fix: change return value to "None" in case getattr returns None Dec 19, 2022
@nejch

nejch commented Dec 19, 2022

Copy link
Copy Markdown
Member

Thanks for catching this and for contributing the fix @pinxue!

Looks like a regression in #2010. I'll merge this to get it released, but if you feel like adding a unit test for this, I'd be happy to take a look at follow-up PRs! 😊

@nejch nejch merged commit 3f86d36 into python-gitlab:main Dec 19, 2022
@pinxue

pinxue commented Dec 21, 2022

Copy link
Copy Markdown
Contributor Author

Thanks a lot for your great effort to maintain this project and quick response! It is great idea to keep the coverage, let me find a time later.

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.

2 participants