Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3 Service using API instead of .meta file #406

Merged
merged 3 commits into from
Jun 6, 2024

Conversation

Azher2Ali
Copy link
Contributor

@Azher2Ali Azher2Ali commented Jun 5, 2024

This commit refactors the S3DownloadService to utilize the S3 API directly instead of relying on a separate .meta file for object information.

These changes improve the resilience of the S3 download service by-

  • Preventing unexpected service failures due to null values or missing .meta files.
  • Ensuring consistent execution flow by handling different scenarios uniformly.
  • Null Handling: The code now checks if objectSpec is null and handles the case, preventing potential issues encountered earlier.
  • The code is responsible for using the part size (length) from the S3 API. By fetching object metadata and using it to fill part URLs and create the ObjectSpecification, the code ensures that the sizes and offsets match the actual stored parts on S3, rather than solely relying on user-specified values
  • Exception Handling: The exception thrown when the .meta file was missing previously caused service failure. This is now caught and handled by returning null instead.
  • Introduced a try-catch block to handle the IdNotFoundException that occurs when the .meta file is missing. By catching this exception and returning null, the service avoids failure and provides a controlled response, enhancing the overall stability of the application.

Additions to Unit Test Updates in order to validate the new exception handling mechanism. This test case ensures that the service does not fail when the .meta file is missing and that a NullPointerException is appropriately handled.

Overall, this commit modernizes the S3 service by leveraging the S3 API directly.

@Azher2Ali Azher2Ali changed the title Commiting fix related to download of S3 Object Id's S3 Service using API instead of .meta file Jun 5, 2024
Copy link
Contributor

@joneubank joneubank left a comment

Choose a reason for hiding this comment

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

This still uses the length argument from the function definition, instead of reading this from the S3 API.

+ ")");
}
}
if (objectSpec != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This if is duplicate of the same if from line 126. We don't need both of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have removed both the if s

Copy link
Contributor

@joneubank joneubank left a comment

Choose a reason for hiding this comment

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

All good now! Thanks!

@Azher2Ali Azher2Ali merged commit 6a2b312 into develop Jun 6, 2024
2 checks passed
@Azher2Ali Azher2Ali deleted the fix/S3DownloadObjectId branch June 6, 2024 19:37
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