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

Update yangtools to 6.0.7 #53

Merged
merged 12 commits into from
Sep 25, 2022
Merged

Update yangtools to 6.0.7 #53

merged 12 commits into from
Sep 25, 2022

Conversation

ihrasko
Copy link
Contributor

@ihrasko ihrasko commented Apr 26, 2022

  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.

@ihrasko ihrasko changed the title Update to 6.0.7 Update yangtools to 6.0.7 Apr 26, 2022
@ihrasko ihrasko mentioned this pull request Apr 26, 2022
Closed
@bartoszm bartoszm self-assigned this Apr 27, 2022
@bartoszm
Copy link
Owner

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

bartoszm
bartoszm requested changes Apr 29, 2022
View reviewed changes
Copy link
Owner

@bartoszm bartoszm left a comment

Choose a reason for hiding this comment

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

Looks good - the only thing I would really want to understand is in AbstractDataObjectBuilder before I integrate that into main. Thank you for the contribution

swagger-generator/src/main/java/com/mrv/yangtools/codegen/impl/path/PutOperationGenerator.java Outdated Show resolved Hide resolved
swagger-generator/src/main/java/com/mrv/yangtools/codegen/impl/path/PostOperationGenerator.java Outdated Show resolved Hide resolved
...ger-generator/src/main/java/com/mrv/yangtools/codegen/impl/path/PatchOperationGenerator.java Outdated Show resolved Hide resolved
swagger-generator/src/main/java/com/mrv/yangtools/codegen/impl/path/GetOperationGenerator.java Outdated Show resolved Hide resolved
...er-generator/src/main/java/com/mrv/yangtools/codegen/impl/path/DeleteOperationGenerator.java Outdated Show resolved Hide resolved
swagger-generator/src/main/java/com/mrv/yangtools/codegen/impl/AbstractDataObjectBuilder.java Outdated Show resolved Hide resolved
@ihrasko ihrasko force-pushed the yangtools-update branch 2 times, most recently from f20b1ce to 0d96b43 Compare May 5, 2022 15:38
@ihrasko ihrasko requested a review from bartoszm May 5, 2022 15:39
@kasingal
Copy link

@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
Copy link
Contributor Author

ihrasko commented Jun 22, 2022

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
Copy link
Contributor Author

ihrasko commented Jun 28, 2022

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 ihrasko force-pushed the yangtools-update branch 3 times, most recently from 0f6542e to bd0dbd9 Compare June 29, 2022 11:36
@ihrasko
Copy link
Contributor Author

ihrasko commented Jun 29, 2022

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

Thank you!

@bartoszm
Copy link
Owner

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

@bartoszm
Copy link
Owner

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
Copy link
Contributor Author

ihrasko commented Jul 22, 2022

OK, I will check.

@ihrasko
Copy link
Contributor Author

ihrasko commented Sep 13, 2022

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

@ihrasko
Switch to use Java 11
08543a1 
Enable Java 11 source and target compatibility.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Bump maven plugins
42802b1 
Bump maven-compiler-plugin to 3.10.1
and maven-source-plugin to 3.2.1.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Bump logback to 1.2.11
e9a065a 
Bump logback to 1.2.11 which contains fix
for https://cve.report/CVE-2021-42550.

See: https://logback.qos.ch/news.html

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Bump yangtools to 6.0.7
68dbba4 
Update yangtools version from 1.2.1 to 6.0.7.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Update the code to yangtools 6.0.7
c3334d8 
Adapt the code to use new classes, statements, utilities
and features of yangtools 6.0.7.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Move from SchemaContext to EffectiveModelContext
b0d65ab 
Replace SchemaContext with more recent EffectiveModelContext.
SchemaContext is planned to be removed in the future.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Correct response message
83835e9 
The original message got lost during migration to yangtools 6.0.7.
Revert it back.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Use unique grouping names
655068e 
Use unique grouping name in test to ensure YANG model
is valid according to:
https://datatracker.ietf.org/doc/html/rfc7950#section-6.2.1

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@ihrasko
Replace joda time with java time
09f7122 
Replace joda time with Java 11 java.time package classes.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
ihrasko and others added 3 commits September 23, 2022 16:38
@ihrasko
Quick fix for testBug15
d4ebe4a 
RegularListEffectiveStatement cannot find equal object similar way
as ListEffectiveStatementImpl.

This is not a complete fix. It just points to where the problem is.

Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
@bartoszm
fix for RPC augmentation test
c26bc58
@bartoszm
fix for RPC augmentation test (part2)
a6f47c0
bartoszm
bartoszm approved these changes Sep 25, 2022
View reviewed changes
Copy link
Owner

@bartoszm bartoszm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good now. I have fixed augmentations bit and modified a bit tests covering this functionality. I hope you don't mind. Thanks for your contribution

@bartoszm bartoszm merged commit a567007 into bartoszm:master Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartoszm bartoszm bartoszm approved these changes

Assignees

@bartoszm bartoszm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ihrasko @bartoszm @kasingal