Skip to content

Conversation

swartzn
Copy link
Contributor

@swartzn swartzn commented Jul 4, 2025

What does this PR do / why do we need it?

Required for all PRs.

I got a little carried away one day and these changes just seemed to work. When I found LocalStack which is an AWS emulator, I was curious about Azure and found Azurite. Since Azure is not S3-compliant so this required a couple modifications to protobuf.

  • Implements RST provider for Azure blobs with the exception of multi-part upload/download

Note: Requires protobuf branch swartzn/add-azure-support

Are there any specific topics we should discuss before merging?

Not required.

What are the next steps after this PR?

Not required.

Checklist before merging:

Required for all PRs.

When creating a PR these are items to keep in mind that cannot be checked by GitHub actions:

  • Documentation:
    • Does developer documentation (code comments, readme, etc.) need to be added or updated?
    • Does the user documentation need to be expanded or updated for this change?
  • Testing:
    • Does this functionality require changing or adding new unit tests?
    • Does this functionality require changing or adding new integration tests?
  • Git Hygiene:

For more details refer to the Go coding standards and the pull request process.

@swartzn swartzn requested a review from a team as a code owner July 4, 2025 11:14
@swartzn swartzn force-pushed the swartzn/add-azure-support branch from c05c7b9 to a394dad Compare July 7, 2025 10:04
@swartzn swartzn self-assigned this Aug 4, 2025
@swartzn
Copy link
Contributor Author

swartzn commented Aug 4, 2025

@swartzn Squash commits before merging if needed

@swartzn swartzn force-pushed the swartzn/add-azure-support branch from a394dad to c48250b Compare August 25, 2025 13:18
}

if !IsFileLocked(lockedInfo) {
if lockedInfo, writeLockSet, _, err = GetLockedInfo(ctx, r.mountPoint, mappings, cfg, cfg.Path); err != nil {

Check warning

Code scanning / CodeQL

Impossible interface nil check Warning

This value can never be nil, since it is a wrapped interface value.

Copilot Autofix

AI about 2 months ago

To fix the issue, change the assignment on line 374 from direct assignment to the outer err variable (of type error interface) to a short variable declaration, capturing the returned error value as its concrete type. This allows the subsequent nil check (if err != nil) to work as intended. Only assign to the outer return variable err after the check and if necessary (as in the error-handling block).

  • Specifically, replace:
    if lockedInfo, writeLockSet, _, err = GetLockedInfo(...); err != nil {
    
    with:
    var getLockedErr error
    if lockedInfo, writeLockSet, _, getLockedErr := GetLockedInfo(...); getLockedErr != nil {
        err = fmt.Errorf(..., getLockedErr.Error())
        return
    }
    

This avoids the interface nil problem entirely.

Only the region inside common/rst/blob.go at line 374 requires changing, introducing a new error variable for this block-scoped assignment.


Suggested changeset 1
common/rst/blob.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/common/rst/blob.go b/common/rst/blob.go
--- a/common/rst/blob.go
+++ b/common/rst/blob.go
@@ -371,8 +371,9 @@
 	}
 
 	if !IsFileLocked(lockedInfo) {
-		if lockedInfo, writeLockSet, _, err = GetLockedInfo(ctx, r.mountPoint, mappings, cfg, cfg.Path); err != nil {
-			err = fmt.Errorf("%w: %s", ErrJobFailedPrecondition, fmt.Sprintf("failed to acquire lock: %s", err.Error()))
+		var getLockedErr error
+		if lockedInfo, writeLockSet, _, getLockedErr := GetLockedInfo(ctx, r.mountPoint, mappings, cfg, cfg.Path); getLockedErr != nil {
+			err = fmt.Errorf("%w: %s", ErrJobFailedPrecondition, fmt.Sprintf("failed to acquire lock: %s", getLockedErr.Error()))
 			return
 		}
 		cfg.SetLockedInfo(lockedInfo)
EOF
@@ -371,8 +371,9 @@
}

if !IsFileLocked(lockedInfo) {
if lockedInfo, writeLockSet, _, err = GetLockedInfo(ctx, r.mountPoint, mappings, cfg, cfg.Path); err != nil {
err = fmt.Errorf("%w: %s", ErrJobFailedPrecondition, fmt.Sprintf("failed to acquire lock: %s", err.Error()))
var getLockedErr error
if lockedInfo, writeLockSet, _, getLockedErr := GetLockedInfo(ctx, r.mountPoint, mappings, cfg, cfg.Path); getLockedErr != nil {
err = fmt.Errorf("%w: %s", ErrJobFailedPrecondition, fmt.Sprintf("failed to acquire lock: %s", getLockedErr.Error()))
return
}
cfg.SetLockedInfo(lockedInfo)
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant