Skip to content

Restconf proxy#58965

Merged
Ch3LL merged 98 commits intosaltstack:masterfrom
ITJamie:restconf-proxy
May 20, 2022
Merged

Restconf proxy#58965
Ch3LL merged 98 commits intosaltstack:masterfrom
ITJamie:restconf-proxy

Conversation

@ITJamie
Copy link
Copy Markdown
Contributor

@ITJamie ITJamie commented Nov 17, 2020

What does this PR do?

New Behavior

Adding a RestConf proxy module for working with network devices like cisco,juniper etc
Issue: #59006

Merge requirements satisfied?

  • Docs
  • Changelog
  • Tests written/updated

Commits signed with GPG?

No

@ITJamie ITJamie requested a review from a team as a code owner November 17, 2020 16:14
@ITJamie ITJamie requested review from dwoz and removed request for a team November 17, 2020 16:14
@welcome
Copy link
Copy Markdown

welcome Bot commented Nov 17, 2020

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at core@saltstack.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@ITJamie ITJamie changed the title Restconf proxy WIP: Restconf proxy Nov 17, 2020
Copy link
Copy Markdown
Contributor

@waynew waynew 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 PR! Just a few things that I noticed while reviewing for test clinic

Comment thread salt/proxy/restconf.py Outdated
Comment thread salt/proxy/restconf.py Outdated
Comment thread salt/proxy/restconf.py Outdated
Comment thread salt/proxy/restconf.py Outdated
Comment thread salt/proxy/restconf.py Outdated
Comment thread salt/proxy/restconf.py Outdated
Comment thread salt/proxy/restconf.py
Comment thread salt/states/restconf.py Outdated
@ITJamie
Copy link
Copy Markdown
Contributor Author

ITJamie commented Nov 24, 2020

@waynew
Per our slack thread error handling is already handled in the salt utils http.
Heres an example of a communication failure being handled as I expected.

bad uri:

salt '*juke*' restconf.get_data ''
jukebox:
    ----------
    error:
        HTTP 404: Not Found
    status:
        404

unreachable host:

salt '*juke*' restconf.get_data ''
jukebox:
    ----------
    error:
        [Errno 111] Connection refused
    status:
        0

The states code has logic written around the status code response.

ITJamie and others added 6 commits February 23, 2022 14:49
Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
@ITJamie ITJamie requested a review from s0undt3ch February 24, 2022 13:56
@ITJamie
Copy link
Copy Markdown
Contributor Author

ITJamie commented Mar 27, 2022

re-run centos-7
re-run amazon-2

@ITJamie
Copy link
Copy Markdown
Contributor Author

ITJamie commented Apr 3, 2022

re-run centos

@ITJamie
Copy link
Copy Markdown
Contributor Author

ITJamie commented Apr 3, 2022

re-run amazon-2
re-run arch
re-run debian-11-arm64

@ITJamie
Copy link
Copy Markdown
Contributor Author

ITJamie commented Apr 4, 2022

re-run amazon-2
re-run arch
re-run debian-11-arm64
re-run centos

@dmurphy18 dmurphy18 added the Phosphorus v3005.0 Release code name and version label Apr 7, 2022
@dmurphy18 dmurphy18 added this to the Phosphorus v3005.0 milestone Apr 7, 2022
@ITJamie
Copy link
Copy Markdown
Contributor Author

ITJamie commented Apr 26, 2022

@dmurphy18 do you think this will get merged in v3005? its finally passing all tests but i see its awaiting re-review still

@dmurphy18
Copy link
Copy Markdown
Contributor

dmurphy18 commented Apr 26, 2022

@ITJamie Yes, we had discussion in the team and have decided to include this PR in the Phosphorous Release (3005). The Team is a little busy at the moment but it is currently scheduled to be included.

@Ch3LL Ch3LL merged commit 4dfb346 into saltstack:master May 20, 2022
@welcome
Copy link
Copy Markdown

welcome Bot commented May 20, 2022

Congratulations on your first PR being merged! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Phosphorus v3005.0 Release code name and version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants