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.

Sign up for GitHub

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

Jump to bottom

pincushion reserved names #281

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

pincushion reserved names #281

wants to merge 2 commits into from

Conversation

canozyurt
Copy link
Contributor

Fixes #275

I am not sure if you would like to fix this server side or any other way but until then, I took Gavin's idea to give it a try.

https://github.com/maas/python-libmaas/blob/69c51b5e64e7ffa09ad35cf14cbaa1bd27e77bbc/maas/client/bones/testing/desc.py#L52-L61

Name import_ works as intended.

>>> client.SSHKeys.import_(keysource='gh:canozyurt')
[{'key': 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDmtjegTY011FoyM5u1wQABWWg1bwEzQ8CoOG0DLcqATLt4wVK3+MaxzIVZElcQG9M4H7ghekqRaaV+vVt/OnM6p97qyQ40urqco+H5fBkVuygFXtZeQngL2QUSUhB8nm2pVHu4eI1P/ObcIejFSfmDcAKRgcKZLoLqY86CD7e4dBOOGBfMw5cWMsuB2derRt1gimAMcnGDh9DapaAX9Qu7oLwWwQc5qWYntyhRtLpAEXcWuptN+zbJGz2ydzCVqD9SqPhsTemdItUrSMngPYxThzvuxVCgr1PuTwtpYNF0FGOJ5JI0T1E3b3hTI+BB0ckyuSzJR5GSCYMWWAm8/vp6bzWsoFXgJUW8fvLbH6pIboSg5uQ1Toic9YgT1qWdt2u/0Uii+Y6FUQ5HqfK1BcmdqdjMPwkgmAiAKCD5y6hfb88ElF5JiZaxNvH/YgH5LMJzO6XAzShvayqvoXu+EUXGhfZRBvfTHzzlbw7+MjoH+he0nSNlVPp7gJrOsrhkWNE=', 'keysource': 'gh:canozyurt', 'id': 7, 'resource_uri': '/MAAS/api/2.0/account/prefs/sshkeys/7/'}]

I look forward to your comments to make this fix more appealing for you.

cpg1111
cpg1111 requested changes Jul 21, 2023
View reviewed changes
Copy link

@cpg1111 cpg1111 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, can you please add tests?

@codecov-commenter
Copy link

Codecov Report

Merging #281 (d7993da) into master (1ff28d5) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

❗ Current head d7993da differs from pull request most recent head 35fd3c8. Consider uploading reports for the commit 35fd3c8 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #281      +/-   ##
==========================================
- Coverage   69.16%   69.08%   -0.09%     
==========================================
  Files          73       72       -1     
  Lines        5958     5938      -20     
  Branches     1336     1337       +1     
==========================================
- Hits         4121     4102      -19     
+ Misses       1607     1606       -1     
  Partials      230      230
Files Changed Coverage Δ
maas/client/bones/__init__.py 79.15% <100.00%> (+0.16%) ⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@SK1Y101
Copy link
Member

SK1Y101 commented Aug 21, 2023

Hey @canozyurt, Thanks for the contribution!
Could you add the tests that @cpg1111 requested, then we can look into merging this? That would be great!

@SK1Y101 SK1Y101 added the bug label Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpg1111 cpg1111 cpg1111 requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ActionAPIs with name "import" are broken due to using a reserved keyword
4 participants
@canozyurt @codecov-commenter @SK1Y101 @cpg1111