Amartus / yang2swagger

Yang to swagger generator
Eclipse Public License 1.0
32 stars 21 forks source link

Update yangtools to 6.0.7 #53

Closed ihrasko closed 2 years ago

ihrasko commented 2 years ago
  1. Update to use Java 11 - yangtools version 6.0.7 requires it.
  2. Update maven plugins to the latest versions.
  3. Bump logback version in order to fix CVE-2021-42550.
  4. Bump yangtools to 6.0.7 and adapt code to use it properly.
bartoszm commented 2 years ago

it looks OK, let me build + run it and check one additional aspect regarding RPC in/out and if there are no problems I will integrate that with the code base. thank you for the contribution

kasingal commented 2 years ago

@ihrasko I built your changes and tried generating swagger - Two items I noticed there: 1) It's not generating paths for /config/ /operational/ 2) It's generating every leaf properties with "default": "" and "description": "" if it's not defined in yang, which is probably ok, but why to keep empty placeholders

ihrasko commented 2 years ago

Q: It's generating every leaf properties with "default": "" and "description": "" if it's not defined in yang, which is probably ok, but why to keep empty placeholders

A: Fixed.

ihrasko commented 2 years ago

Q: It's not generating paths for /config/ /operational/ A: It's because it's generating RFC8040 paths by default. It means you can see /data/ instead of config or operational. To change this behaviour you have to set parameter path-format to odl.

ihrasko commented 2 years ago

I have fixed comments from @kasingal. @bartoszm do you have cycles to make review?

Thank you!

bartoszm commented 2 years ago

hey, I just came back from long vacation - will look at this PR today.

bartoszm commented 2 years ago

Hi, I have noticed that for some time integration tests were not triggered as part of the standard build. I have added them in the master branch and fixed failing tests (due to changes in how paths are interpreted). I have rebased your PR and additional tests in *It are failing. would you mind looking into that?

ihrasko commented 2 years ago

OK, I will check.

ihrasko commented 2 years ago

@bartoszm I will try to rebase this PR on master branch and solve remaining issues - next week or so :D