feat: add support for RFC5717 <partial-lock> #135

2026-01-08T17:35:48Z

nemith commented

2026-01-08 17:35:48 +00:00

(Migrated from github.com)

Adds the RPC and to suppoirt RFC5717.

Adds the RPC <partial-lock> and <partial-unlock> to suppoirt RFC5717.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-01-08 17:48:56 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This pull request adds support for RFC 5717 partial-lock operations, enabling fine-grained locking of specific datastore subtrees using XPath expressions. This complements the existing full datastore lock/unlock operations.

Implements PartialLock and PartialUnlock RPC operations with proper XML marshaling
Adds comprehensive unit tests for XML marshaling and execution of both operations
Updates project documentation to reflect RFC 5717 support completion

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
rpc/config.go	Adds PartialLock, PartialLockReply, and PartialUnlock types with XML marshaling and Exec methods
rpc/config_test.go	Adds comprehensive unit tests for both marshal and exec operations of partial-lock and partial-unlock
README.md	Updates RFC support table to mark RFC 5717 as supported
TODO.md	Marks Partial Lock feature as completed in the future work list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This pull request adds support for RFC 5717 partial-lock operations, enabling fine-grained locking of specific datastore subtrees using XPath expressions. This complements the existing full datastore lock/unlock operations. - Implements `PartialLock` and `PartialUnlock` RPC operations with proper XML marshaling - Adds comprehensive unit tests for XML marshaling and execution of both operations - Updates project documentation to reflect RFC 5717 support completion ### Reviewed changes Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments. | File | Description | | ---- | ----------- | | rpc/config.go | Adds PartialLock, PartialLockReply, and PartialUnlock types with XML marshaling and Exec methods | | rpc/config_test.go | Adds comprehensive unit tests for both marshal and exec operations of partial-lock and partial-unlock | | README.md | Updates RFC support table to mark RFC 5717 as supported | | TODO.md | Marks Partial Lock feature as completed in the future work list | --- 💡 <a href="/nemith/netconf/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

TODO.md

					
				@ -21,6 +21,6 @@

				- [ ] Pool/SessionManager for automatic reconnects, retries, etc.

The indentation for the with-defaults item is incorrect. It should be indented at the same level as the Partial Lock item above it (using two spaces for the sub-item indent).

  - [ ] with-defaults

The indentation for the `with-defaults` item is incorrect. It should be indented at the same level as the `Partial Lock` item above it (using two spaces for the sub-item indent). ```suggestion - [ ] with-defaults ```

rpc/config.go

According to RFC 5717, the partial-lock operation requires at least one select element. The MarshalXML method should validate that the Select slice is not empty and return an error if it is, similar to how the Datastore type validates that it's not empty.

The lock-id is required for the partial-unlock operation according to RFC 5717. Consider adding validation in the MarshalXML method to ensure that LockID is not zero (the zero value for uint32), similar to how other required fields are validated in the codebase.

rpc/config.go

					
				@ -369,0 +391,4 @@

						XMLName xml.Name `xml:"urn:ietf:params:xml:ns:netconf:partial-lock:1.0 partial-lock"`

						Select  []selectExpr

					}{

						Select: selects,

The PartialLockReply struct should embed netconf.RPCReply like other reply types in the codebase (e.g., OkReply, GetReply). This embedding provides access to standard RPC reply fields including MessageID, Attributes, and RPCErrors, which are important for proper error handling and response validation.

The PartialLockReply struct should embed `netconf.RPCReply` like other reply types in the codebase (e.g., OkReply, GetReply). This embedding provides access to standard RPC reply fields including MessageID, Attributes, and RPCErrors, which are important for proper error handling and response validation.

rpc/config_test.go

					
				@ -414,6 +414,123 @@ func TestUnlock_Exec(t *testing.T) {

					}

The test loop should wrap each test case in a t.Run call for better test organization and failure reporting, consistent with other test functions in this file (e.g., TestLock_Exec, TestUnlock_Exec). The pattern should be t.Run(tc.name, func(t *testing.T) { ... }).

		tc := tc
		t.Run(tc.name, func(t *testing.T) {
			session, _ := mockSession(t, tc.serverReply)
			reply, err := tc.op.Exec(t.Context(), session)
			if tc.shouldError {
				be.Nonzero(t, err)
			} else {
				be.NilErr(t, err)
				be.Equal(t, tc.lockID, reply.LockID)
			}
		})

The test loop should wrap each test case in a `t.Run` call for better test organization and failure reporting, consistent with other test functions in this file (e.g., TestLock_Exec, TestUnlock_Exec). The pattern should be `t.Run(tc.name, func(t *testing.T) { ... })`. ```suggestion tc := tc t.Run(tc.name, func(t *testing.T) { session, _ := mockSession(t, tc.serverReply) reply, err := tc.op.Exec(t.Context(), session) if tc.shouldError { be.Nonzero(t, err) } else { be.NilErr(t, err) be.Equal(t, tc.lockID, reply.LockID) } }) ```

rpc/config_test.go

					
				@ -417,0 +527,4 @@

							be.Nonzero(t, err)

						} else {

							be.NilErr(t, err)

						}

The test loop should wrap each test case in a t.Run call for better test organization and failure reporting, consistent with other test functions in this file (e.g., TestLock_Exec, TestUnlock_Exec). The pattern should be t.Run(tc.name, func(t *testing.T) { ... }).

		tc := tc
		t.Run(tc.name, func(t *testing.T) {
			session, _ := mockSession(t, tc.serverReply)
			err := tc.op.Exec(t.Context(), session)
			if tc.shouldError {
				be.Nonzero(t, err)
			} else {
				be.NilErr(t, err)
			}
		})

The test loop should wrap each test case in a `t.Run` call for better test organization and failure reporting, consistent with other test functions in this file (e.g., TestLock_Exec, TestUnlock_Exec). The pattern should be `t.Run(tc.name, func(t *testing.T) { ... })`. ```suggestion tc := tc t.Run(tc.name, func(t *testing.T) { session, _ := mockSession(t, tc.serverReply) err := tc.op.Exec(t.Context(), session) if tc.shouldError { be.Nonzero(t, err) } else { be.NilErr(t, err) } }) ```

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-01-08 18:51:15 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments. --- 💡 <a href="/nemith/netconf/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

Sign in to join this conversation.

No reviewers