Skip to content

feat(rst): add microsoft azure blob support#213

Open
swartzn wants to merge 1 commit into
mainfrom
swartzn/add-azure-support
Open

feat(rst): add microsoft azure blob support#213
swartzn wants to merge 1 commit into
mainfrom
swartzn/add-azure-support

Conversation

@swartzn

@swartzn swartzn commented Jul 4, 2025

Copy link
Copy Markdown
Contributor

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

swartzn commented Aug 4, 2025

Copy link
Copy Markdown
Contributor Author

@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
Comment thread common/rst/blob.go
}

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 11 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.

2 participants