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

update prompt for aosw #3233

Merged
merged 4 commits into from
Jul 25, 2024
Merged

update prompt for aosw #3233

merged 4 commits into from
Jul 25, 2024

Conversation

mabezi
Copy link
Contributor

@mabezi mabezi commented Jul 22, 2024

Pre-Request Checklist

  • Passes rubocop code analysis (try rubocop --auto-correct)
  • Tests added or adapted (try rake test)
  • Changes are reflected in the documentation
  • User-visible changes appended to CHANGELOG.md

Description

The aosw template no longer works with newer versions of AOS 8.

With this prompt it should work again. (At least on our devices with AOS 8 it now works).

Should close #3151

@robertcheramy
Copy link
Collaborator

Thank you for the submission. The proposed prompt is not backward compatible. I'm working on it.

@ytti
Copy link
Owner

ytti commented Jul 24, 2024

Maybe we should make test file, that specifically loads prompt from each model, so we could put prompt unit tests in single file? It seems one of the most fragile part of models, for people who don't use exec mode, as people regularly make very poor prompts to both direction, too liberal and getting partial configs and too specific not working outside specific environemnt.

Controlling this seems very hard, without unit test. And unlike rest of the model, it seems quite low hanging fruit to target in testing.

@robertcheramy
Copy link
Collaborator

The unit test is already written, will commit soon ;-)

@mabezi
Copy link
Contributor Author

mabezi commented Jul 24, 2024

Thank you for the submission. The proposed prompt is not backward compatible. I'm working on it.

That was of course unplanned. Since we only use devices with AOS 8, I did not notice this when testing.

- Create an unit test for the aosw prompt
- Update credits in Changelog
- Update prompt regexp to match virtual controllers on AP-515
- Resolve warnings about ambiguity between regexp and two divisions by
wrapping the regexp in parentheses.
@robertcheramy
Copy link
Collaborator

@mabezi - could you test the PR if it works in your setup?
If you think of prompt cases not tested in spec/model/aosw_spec.rb , feel free to add some.

@mabezi
Copy link
Contributor Author

mabezi commented Jul 25, 2024

Works for me. Using ArubaMM-VA with AOS 8.10 and Aruba7210 with AOS 8.10

@robertcheramy
Copy link
Collaborator

Perfect, thank you for the quick test.

@robertcheramy robertcheramy merged commit 5279280 into ytti:master Jul 25, 2024
5 checks passed
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.

ArubaOS8 need update propmt
3 participants