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

Feature/add working directory override #59

Merged

Conversation

IliasP91
Copy link
Contributor

Description

  • Added a WorkingDirectory option to allow us specify another directory to execute daprd.exe in. The requirement for this is to allow some components use relative file paths.
  • Updated one of the example projects to start a sidecar earlier in the startup flow in order to be able to use secretstores components during startup to load secrets or configurations from them.

Issue reference

#57
#58

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation where possible

@badgeratu
Copy link
Contributor

Many thanks for the contribution! The one thing I'd request is rather than change the default ControllerSample I think the custom secrets/startup example should be in a new sample project, specifically named to indicate it represents a pattern for loading secrets at startup and with a README alongside it. In the future we will look at supporting secrets pre-loading natively in the AddDaprSidekick() method rather than using the builder, as the latter is really intended for non-ASP.NET Core apps.

@IliasP91
Copy link
Contributor Author

IliasP91 commented Oct 2, 2023

Many thanks for the contribution! The one thing I'd request is rather than change the default ControllerSample I think the custom secrets/startup example should be in a new sample project, specifically named to indicate it represents a pattern for loading secrets at startup and with a README alongside it. In the future we will look at supporting secrets pre-loading natively in the AddDaprSidekick() method rather than using the builder, as the latter is really intended for non-ASP.NET Core apps.

not a problem at all, ill update it in the next couple of days

as for your suggestion that would be better, however probably a new internal process or component would be required as it needs to happen a lot earlier than the hosted service

regardless ill post back when i update this PR

@IliasP91 IliasP91 requested a review from badgeratu October 3, 2023 20:31
@IliasP91 IliasP91 requested a review from badgeratu October 4, 2023 20:23
@badgeratu badgeratu merged commit 8a00cc7 into man-group:main Oct 9, 2023
4 checks passed
@IliasP91 IliasP91 deleted the feature/add-working-directory-override branch October 9, 2023 09:17
@IliasP91
Copy link
Contributor Author

IliasP91 commented Oct 9, 2023

@badgeratu thanks for approving and merging this
QQ i there a plan for the next release? thanks

@badgeratu
Copy link
Contributor

badgeratu commented Oct 9, 2023 via email

@IliasP91
Copy link
Contributor Author

IliasP91 commented Oct 9, 2023

It’s released now 😊 https://github.com/man-group/dapr-sidekick-dotnet/releases/tag/v1.2.2 From: Ilias @.> Sent: Monday, October 9, 2023 10:19 AM To: man-group/dapr-sidekick-dotnet @.> Cc: Simon Jones @.>; Mention @.> Subject: Re: [man-group/dapr-sidekick-dotnet] Feature/add working directory override (PR #59) @badgeratu https://github.com/badgeratu thanks for approving and merging this QQ i there a plan for the next release? thanks — Reply to this email directly, view it on GitHub <#59 (comment)> , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG36I6TVR6QG3GNDSHGWILX6O6OTAVCNFSM6AAAAAA5KCGVOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJSGYZDKNZSHE . You are receiving this because you were mentioned. https://github.com/notifications/beacon/ABG36IZWGYYPGLQJ5PETW43X6O6OTA5CNFSM6AAAAAA5KCGVOCWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTIO3ZEC.gif Message ID: @.*** @.***> >

Legend!
FYI I had a quick look to see if I could relatively easily kick off the svc earlier, didn't go too far, I'll have another look next weekend, otherwise the approach in the sample has been working fine for us for a bout a week or so now

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