Add Netty Restconf endpoint#2364
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
4cf04e5 to
20bb157
Compare
|
/gemini |
|
It looks like you tried to invoke Gemini Code Assist with just Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new Netty-based RESTCONF endpoint and an example application demonstrating its use. The changes are well-structured. My review includes a few points for improvement:
- In
Main.java, there's a redundant call to start the RESTCONF module which should be removed. - In the
RestClienttest utility, theExecutorServiceis static, which could cause issues in parallel test runs; it should be an instance field. Also, an explicit charset should be used when converting strings to bytes. - In
NettyRestConf.java, there are a couple of incorrect log messages in the shutdown procedure that need to be corrected or removed.
Overall, these are good additions. Addressing these points will improve the robustness and clarity of the new code.
...-community-netty-restconf-app/src/main/java/io/lighty/controllers/nettyrestconfapp/Main.java
Outdated
Show resolved
Hide resolved
...etty-restconf-app/src/test/java/io/lighty/controllers/nettyrestconfapp/tests/RestClient.java
Outdated
Show resolved
Hide resolved
...etty-restconf-app/src/test/java/io/lighty/controllers/nettyrestconfapp/tests/RestClient.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/lighty/modules/northbound/netty/restconf/community/impl/NettyRestConf.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/lighty/modules/northbound/netty/restconf/community/impl/NettyRestConf.java
Outdated
Show resolved
Hide resolved
20bb157 to
fc0e354
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new Netty-based RESTCONF endpoint as an alternative module, along with a community example application demonstrating its use. The changes are well-structured, adding new modules and examples clearly. My review focuses on improving code efficiency, maintainability, and adherence to Java best practices. I've suggested refactoring for better file handling, improving method naming conventions and reducing code duplication in the test client, making fields final where appropriate, and enhancing exception handling and code comments for clarity.
...-community-netty-restconf-app/src/main/java/io/lighty/controllers/nettyrestconfapp/Main.java
Show resolved
Hide resolved
...etty-restconf-app/src/test/java/io/lighty/controllers/nettyrestconfapp/tests/RestClient.java
Show resolved
Hide resolved
...in/java/io/lighty/modules/northbound/netty/restconf/community/impl/NettyRestConfBuilder.java
Outdated
Show resolved
Hide resolved
...java/io/lighty/modules/northbound/netty/restconf/community/impl/util/NettyRestConfUtils.java
Show resolved
Hide resolved
0a91132 to
2a1a355
Compare
introduce alternative module (lighty-restconf-netty-nb-community) and community RESTCONF Netty example (lighty-community-netty-restconf-netconf-app) using org.opendaylight.restconf.server.NettyEndpoint. JIRA: LIGHTY-333 Signed-off-by: tobias.pobocik <[email protected]>
Add tests which will verify configuration, writing to datastore and authorization. JIRA: LIGHTY-333 Signed-off-by: tobias.pobocik <[email protected]>
2a1a355 to
1f816ce
Compare
|
Ready for review |
introduce alternative module (lighty-restconf-netty-nb-community) and community RESTCONF Netty example (lighty-community-netty-restconf-netconf-app) using org.opendaylight.restconf.server.NettyEndpoint.
JIRA: LIGHTY-333