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.

Sign up for GitHub

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

Jump to bottom

#657 Ability to set UseDefaultCredentials per route #1521

Open
wants to merge 6 commits into
base: release/24.0
Choose a base branch
from
Open

#657 Ability to set UseDefaultCredentials per route #1521

wants to merge 6 commits into from

Conversation

Sacrilege
Copy link

@Sacrilege Sacrilege commented Sep 22, 2021 •
edited by raman-m

Closes #657

Support setting WebClient.UseDefaultCredentials per route if needed.

Docs

This was referenced Sep 22, 2021
Closed
Open
@clintonbale
Copy link

Would be nice to get this merged in. Very useful PR.

@Sacrilege
Copy link
Author

Sacrilege commented Aug 8, 2022 •
edited

What will it take to get this merged in to main?
@TomPallister

stefancruz added a commit to stefancruz/Ocelot that referenced this pull request Dec 22, 2022
@stefancruz
Merge branch 'develop@Sacrilege' into develop
f16dc0b 
Cherry picked from ThreeMammals#1521
@raman-m
Copy link
Member

raman-m commented May 16, 2023

@Sacrilege Hi Troy!
Could you resolve merge conflicts please?
And what issue is this PR related to?

@raman-m raman-m added invalid Not actually an issue waiting Waiting for answer to question or feedback from issue raiser labels May 16, 2023
@raman-m raman-m marked this pull request as draft May 16, 2023 17:33
@raman-m raman-m marked this pull request as ready for review July 11, 2023 11:53
@raman-m raman-m added proposal Proposal for a new functionality in Ocelot and removed invalid Not actually an issue waiting Waiting for answer to question or feedback from issue raiser labels Jul 11, 2023
@raman-m raman-m self-requested a review July 11, 2023 11:57
@raman-m
Copy link
Member

raman-m commented Aug 28, 2023

The feature branch has been rebased onto ThreeMammals:develop successfully!
Welcome to code review!

@raman-m
Copy link
Member

raman-m commented Aug 28, 2023

@Sacrilege commented on Aug 8, 2022

I hope, merging will happen soon... 🤣

@raman-m raman-m changed the title added ability to set UseDefaultCredentials per route Ability to set UseDefaultCredentials per route Aug 28, 2023
@raman-m raman-m requested review from omdahl, Z-byte-prog, binarymash, jps, geffzhang and RaynaldM and removed request for Z-byte-prog and omdahl August 28, 2023 17:07
@ggnaegi
Copy link
Member

ggnaegi commented Apr 3, 2024 •
edited

@raman-m Mocking UseDefaultCredentials in an acceptance test on linux hosts could be quite a pain...

@raman-m
Copy link
Member

raman-m commented Apr 5, 2024 •
edited

@ggnaegi commented on Apr 3

Give me the link to the problematic code plz?!
Now I don't understand the problem... Impossible to write tests?
Will current tests for the feature fail in Linux environment on CircleCI?

Would you like to mentor this PR? If Yes, assign this PR to yourself!

@raman-m
Copy link
Member

raman-m commented Apr 5, 2024

@ggnaegi I've reran new PR build...
Check the status here: pull/1521
The build: 3794

@raman-m raman-m force-pushed the develop branch 2 times, most recently from 7d70134 to ce2add3 Compare April 7, 2024 13:18
@raman-m
Copy link
Member

raman-m commented Apr 16, 2024

The branch has been rebased onto release/24.0 with top commit 59b63ea !

@raman-m raman-m self-assigned this Apr 25, 2024
@raman-m
Copy link
Member

raman-m commented Apr 26, 2024 •
edited

@ggnaegi commented on April 3:

Mocking UseDefaultCredentials in an acceptance test on Linux hosts could be quite a pain...

Indeed, it can be challenging. However, we could set up a Windows build machine as a Docker image and direct CircleCI to execute the Windows build by downloading the image with the Windows OS from Docker Hub.
Have you encountered similar issues in your development?

Roadmap

  • Write all tests and verify them locally using the Visual Studio IDE on Windows OS. Ensure all tests pass.
  • Conduct initial builds on CircleCI using our existing ocelot2/circleci-build image with Linux OS aka Alpine 3.xx. It's likely that some tests will fail.
  • Develop a new Docker image based on Windows OS and upload it to the ocelot2 account. All three .NET SDKs must be preinstalled.
  • Modify the existing build.cake script and/or CircleCI configuration in the repository to detect Windows build scenarios and dynamically create the necessary Windows OS image container. This may present a DevOps challenge, but CircleCI workflows should offer some functionality to select the desired Docker image based on a condition.
  • Achieve a successful build on CircleCI with all checks passing.

Links

@raman-m
Copy link
Member

raman-m commented Apr 26, 2024

TODO

  • Expand unit testing, particularly for the MessageInvokerPool class
  • Conduct acceptance testing, including trials on a Windows machine

@ggnaegi
Copy link
Member

ggnaegi commented Apr 26, 2024

TODO

  • Expand unit testing, particularly for the MessageInvokerPool class
  • Conduct acceptance testing, including trials on a Windows machine

So, you're expecting that quality level from our side and you merged some stuff this week out of the blue...

Ok fine then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raman-m raman-m raman-m requested changes

@RaynaldM RaynaldM RaynaldM approved these changes

@Z-byte-prog Z-byte-prog Z-byte-prog approved these changes

@omdahl omdahl omdahl approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@Sacrilege Sacrilege

@raman-m raman-m

Labels
2023 Annual 2023 release feature A new feature high High priority proposal Proposal for a new functionality in Ocelot
Projects
None yet
Milestone
Annual 2023
Development

Successfully merging this pull request may close these issues.

Ocelot not working with a Windows Authentication
9 participants
@Sacrilege @clintonbale @raman-m @pank789 @ggnaegi @tobiashoeft @RaynaldM @Z-byte-prog @omdahl