Skip to content

Conversation

@GarrettBeatty
Copy link
Contributor

@GarrettBeatty GarrettBeatty commented Oct 23, 2025

Stacked PRs:


Add GetObjectResponse to TransferUtilityDownloadResponse Mapping

Motivation and Context

To copy all fields to the Transfer Utility Response so we ensure completeness. this is per the SEP compliance design doc

Testing

  1. ran test locally
  2. ffeb46dc-8a09-4558-a777-fb2778601370 - pass

Types of changes

  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/stacked/7 branch from a75c7d4 to 8fab5ec Compare October 23, 2025 17:37
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/stacked/8 branch from 0e777fb to f5b6bfa Compare October 23, 2025 17:37
@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 23, 2025 17:39
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/stacked/8 branch from f5b6bfa to 2c12068 Compare October 23, 2025 17:39
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 23, 2025 17:40
@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 23, 2025 17:52
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/stacked/8 branch from 2c12068 to 7caf946 Compare October 23, 2025 17:52
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 23, 2025 17:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 23, 2025 23:53
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/stacked/8 branch from 7caf946 to bef9eae Compare October 23, 2025 23:53
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 23, 2025 23:54
@GarrettBeatty GarrettBeatty requested a review from Copilot October 24, 2025 00:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 24, 2025 13:48
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/stacked/8 branch from bef9eae to be12e1d Compare October 24, 2025 13:48
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 24, 2025 13:49
@GarrettBeatty GarrettBeatty marked this pull request as ready for review October 24, 2025 13:49
using Amazon.Runtime;
using Amazon.S3.Model;

namespace Amazon.S3.Transfer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eventually this class will be used as the return type of transferUtility.DownloadWithResponseAsync(req)

// Determine the correct property type for test value generation
Type propertyTypeForTestValue = sourceProperty?.PropertyType;

// If direct property doesn't exist but we use headers collection, check Headers collection property type
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is needed because we need to write the test value into both the headers and the property value. previously we were writing into the property value and since we didnt write into headers there was a mismatch

@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 24, 2025 14:57
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 24, 2025 14:57
@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 24, 2025 15:17
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 24, 2025 15:17
@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 24, 2025 15:46
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 24, 2025 15:46
@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/stacked/7 to feature/transfermanager October 24, 2025 16:19
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/stacked/7 October 24, 2025 16:19
Base automatically changed from GarrettBeatty/stacked/7 to feature/transfermanager October 24, 2025 22:21
stack-info: PR: #4075, branch: GarrettBeatty/stacked/8
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/stacked/8 branch from be12e1d to 86890ec Compare October 24, 2025 22:24
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