Implemented get-data #54
Conversation
senderic
commented
Oct 30, 2021
senderic
commented
- RFC-8526: https://datatracker.ietf.org/doc/html/rfc8526#section-3.1.1
- Also, updated filter variable name to be specific to xpathfilter
| if (netconfSession == null) { | ||
| throw new IllegalStateException("Cannot execute RPC, you need to " + | ||
| "establish a connection first."); | ||
| } | ||
| return this.netconfSession.getRunningConfigAndState(filter); | ||
| if (datastore != null) switch (datastore.trim().toLowerCase(Locale.ROOT)) { |
There was a problem hiding this comment.
I can't help but think datastore should be an enum rather than a free-format string that only has four valid options.
There was a problem hiding this comment.
@GregDThomas - I agree in general, but for small numbers of options (which are strings anyway), I wasn't sure if an enum was necessary.
Instead, a localized string (that's why I threw in Locale.ROOT). I figured for this situation, its more of a stylistic thing. There are a small finite number of these states, I wasn't sure if programming in an enum was wanted.
That said, I can easily put together an enum here. I have no problem doing that. As I think about it, I probably souldn't have used Locale.ROOT and used Locale.US
There was a problem hiding this comment.
a) I'm not an approved committer, just a user of the library, so feel free to ignore!
b) "There are a small finite number of these states" - that's exactly the sort of use case for an Enum, IMHO. Also makes it quite clear to the caller of the API what values would be valid.
If it were me, I'd use something like ...
@Getter
@RequiredArgsConstructor
public enum Datastore {
RUNNING("running"),
CANDIDATE("candidate"),
STARTUP("startup"),
INTENDED("intended");
public final String tag;
}
Then you could use datastore.getTag()
There was a problem hiding this comment.
@GregDThomas - Yeah I agree. Its stylistic but I will fleshout a Datastore enum.
There was a problem hiding this comment.
Okay, Datastore enum is made. I put it in the element package as it seemed aligned with the other classes in there, in that its based off RFC defined objects (such as RpcReply, Hello, etc.
senderic
force-pushed
the
implement-get-data
branch
2 times, most recently
from
b11f677 to
2045774
Compare
- RFC-8526: https://datatracker.ietf.org/doc/html/rfc8526#section-3.1.1 - Also, updated filter variable name to be specific to xpathfilter - Per PR discussion, created a Datastore enum. Overriding toString in enum to ensure lowercase.
senderic
force-pushed
the
implement-get-data
branch
from
2045774 to
4d6d480
Compare
- Per discussion on github, using Datastore enum: Juniper#54 (comment) - Using enum pass to function call too
