[Core feature] Enforce Keyword-Only Arguments in SecretsManager.get Method and Add Deprecation Warning

[10sharmashivam]

Tracking issue

Reference Issue

Why are the changes needed?

The changes are needed to enforce keyword-only arguments in the SecretsManager.get method, addressing a common usability issue where users inadvertently use positional arguments, which could lead to confusion or errors in secret retrieval. Additionally, a deprecation warning will be raised whenever positional arguments are passed.

What changes were proposed in this pull request?

1. Updated the SecretsManager.get method to require keyword arguments by adding * to the method signature. This change enforces that all arguments must be provided as keywords.
2. Introduced a deprecation warning for the use of positional arguments. If the method is called with positional arguments, a Deprecation Warning is raised to inform users of the impending removal of support for this usage.

How was this patch tested?

• test_get_with_keyword_arguments: Verified that calling the get method with keyword arguments works as expected. • test_get_with_positional_arguments: Confirmed that calling the get method with positional arguments raises the appropriate Deprecation Warning.

Check all the applicable boxes

• [ ] I updated the documentation accordingly.
• [ X] All new and existing tests passed.
• [ X] All commits are signed off.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.05%. Comparing base (6bf6f8e) to head (8d5f23f). Report is 25 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #2878       +/-   ##
===========================================
+ Coverage   76.79%   92.05%   +15.25%     
===========================================
  Files         196       28      -168     
  Lines       20546     1598    -18948     
  Branches     2646        0     -2646     
===========================================
- Hits        15779     1471    -14308     
+ Misses       4068      127     -3941     
+ Partials      699        0      -699     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[10sharmashivam]

@10sharmashivam , can you add a few unit tests just to confirm the behavior?

Hi @eapolinario

I’ll add the unit tests to confirm the decorator’s behavior and will update the PR soon.

[10sharmashivam]

@10sharmashivam , can you add a few unit tests just to confirm the behavior?

Hi @eapolinario @thomasjpfan,

I’ve implemented the decorator based on the logic and approach suggested by @thomasjpfan, referencing sklearn. The goal is to ensure backward compatibility while issuing a FutureWarning for positional arguments as discussed.

To validate the behavior, I temporarily added debug print statements to _deprecate_positional_args to confirm it is being triggered during tests. I’ve also included unit tests (@eapolinario as suggested), but despite these efforts, the tests are not fully passing, as the FutureWarning is not being captured as expected. I also used the decorator logic in the unit test file, instead of importing it, but still no luck.

Any feedback or guidance on fine-tuning the decorator or identifying potential issues with the tests would be a great help!

Thanks in advance!

Regards