Skip to content

fix(api): delete invalid 'project-runner get' command - #1628

Merged
nejch merged 3 commits into
python-gitlab:mainfrom
lgatellier:fix/delete-project-runner-get
Oct 17, 2021
Merged

fix(api): delete invalid 'project-runner get' command#1628
nejch merged 3 commits into
python-gitlab:mainfrom
lgatellier:fix/delete-project-runner-get

Conversation

@lgatellier

@lgatellier lgatellier commented Oct 9, 2021

Copy link
Copy Markdown
Contributor

Deletes the 'project-runner get' command which does'nt match any existing Gitlab API resource.

Closes #1608.

@codecov-commenter

codecov-commenter commented Oct 9, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1628 (d989c9b) into main (4170dbe) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1628   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files          74       74           
  Lines        4295     4295           
=======================================
  Hits         3948     3948           
  Misses        347      347           
Flag Coverage Δ
cli_func_v4 81.93% <100.00%> (ø)
py_func_v4 80.95% <100.00%> (ø)
unit 83.74% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gitlab/v4/objects/runners.py 98.18% <100.00%> (ø)

@nejch nejch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you very much @lgatellier!

You are right that the group runner should only inherit from ListMixin. Whether you'd like to add this here - or open a new PR - is up to you :)

Comment thread gitlab/v4/objects/runners.py
@lgatellier
lgatellier force-pushed the fix/delete-project-runner-get branch from 3514b2b to 06ef25d Compare October 9, 2021 12:47
@nejch

nejch commented Oct 9, 2021

Copy link
Copy Markdown
Member

Funny, looks like there were actually some unit tests written that mocked POST/DELETE responses to the group runner. I'll check if this might be an undocumented API in gitlab already..

@nejch

nejch commented Oct 9, 2021

Copy link
Copy Markdown
Member

Funny, looks like there were actually some unit tests written that mocked POST/DELETE responses to the group runner. I'll check if this might be an undocumented API in gitlab already..

@lgatellier I think you can safely delete the following to make it pass:

https://github.com/python-gitlab/python-gitlab/blob/master/tests/unit/objects/test_runners.py#L255-L256

and

https://github.com/python-gitlab/python-gitlab/blob/master/tests/unit/objects/test_runners.py#L266-L270

as real GitLab servers would not work with this.

@lgatellier

Copy link
Copy Markdown
Contributor Author

I removed the 2 test cases.
I think mocked responses related to enabling/disabling runner should be changed to remove "groups" from URI pattern.
Can you confirm that ?

@pytest.fixture
def resp_runner_enable():
    with responses.RequestsMock() as rsps:
        pattern = re.compile(r".*?(projects|groups)/1/runners")

@pytest.fixture
def resp_runner_disable():
    with responses.RequestsMock() as rsps:
        pattern = re.compile(r".*?/(groups|projects)/1/runners/6")

@nejch

nejch commented Oct 10, 2021

Copy link
Copy Markdown
Member

I removed the 2 test cases. I think mocked responses related to enabling/disabling runner should be changed to remove "groups" from URI pattern. Can you confirm that ?

@pytest.fixture
def resp_runner_enable():
    with responses.RequestsMock() as rsps:
        pattern = re.compile(r".*?(projects|groups)/1/runners")

@pytest.fixture
def resp_runner_disable():
    with responses.RequestsMock() as rsps:
        pattern = re.compile(r".*?/(groups|projects)/1/runners/6")

That's right @lgatellier, thank you!

@nejch

nejch commented Oct 14, 2021

Copy link
Copy Markdown
Member

@lgatellier would you mind rebasing this onto the main branch?

@lgatellier
lgatellier force-pushed the fix/delete-project-runner-get branch from dffa8a3 to d989c9b Compare October 17, 2021 17:01
@lgatellier

Copy link
Copy Markdown
Contributor Author

@nejch all done, mocked responses are fixed, and branch is rebased !

@nejch
nejch merged commit 905781b into python-gitlab:main Oct 17, 2021
@nejch

nejch commented Oct 17, 2021

Copy link
Copy Markdown
Member

Thanks again @lgatellier!

@lgatellier
lgatellier deleted the fix/delete-project-runner-get branch October 17, 2021 17:21
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.

'project-runner get' command should not exist

3 participants