Skip to content

chore(maintenance): avoid attaching two middlewares to ua #1582

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

Closed
wants to merge 6 commits into from
Closed

chore(maintenance): avoid attaching two middlewares to ua #1582

wants to merge 6 commits into from

Conversation

am29d
Copy link
Contributor

@am29d am29d commented Jul 5, 2023

Description of your changes

This PR adds a check before adding user agent middleware, to avoid attaching two or more middlewares and fire a warning in the catch block. I have also refactored the tests, to make it a bit more readable.

Related issues, RFCs

Issue number: closes #1578

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my change is effective and works
  • The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@am29d am29d requested a review from a team July 5, 2023 14:13
@boring-cyborg boring-cyborg bot added commons This item relates to the Commons Utility parameters This item relates to the Parameters Utility tests PRs that add or change tests labels Jul 5, 2023
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Jul 5, 2023
@am29d am29d requested review from dreamorosi and removed request for a team July 5, 2023 14:13
@am29d am29d self-assigned this Jul 5, 2023
@am29d am29d added internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) and removed parameters This item relates to the Parameters Utility labels Jul 5, 2023
@dreamorosi
Copy link
Contributor

@mergify rebase

@mergify
Copy link

mergify bot commented Jul 5, 2023

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/5)
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

    git commit --allow-empty

Otherwise, please use 'git rebase --skip'
interactive rebase in progress; onto 5eba149
Last command done (1 command done):
   pick e2aaa83 add user agent to appconfig parameters
Next commands to do (4 remaining commands):
   pick 5817243 add user agent to dynamodb provider
   pick b7a3c01 add user agent to secrets provider
  (use "git rebase --edit-todo" to view and edit)
You are currently rebasing branch '1578-avoid-attaching-two-middlewares-to-ua' on '5eba149'.
  (all conflicts fixed: run "git rebase --continue")

nothing to commit, working tree clean
Could not apply e2aaa83... add user agent to appconfig parameters

err-code: 29980

@dreamorosi
Copy link
Contributor

Could you please rebase the branch? I see changes from a previous one in the diff and want to make sure we are not overwriting things.

I tried to do it with @ mergify but it doesn't work if the branch is on a fork.

@am29d am29d closed this Jul 5, 2023
@am29d am29d deleted the 1578-avoid-attaching-two-middlewares-to-ua branch July 5, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commons This item relates to the Commons Utility internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) size/L PRs between 100-499 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maintenance: avoid attaching two middlewares when appending UA to AWS SDK
2 participants