Add the ability to specify a namespace for tye deploy. #400

brendandburns · 2020-04-23T16:39:09Z

I monkey-patched the path from the CLI down into the generator. Let me know if there is a preferred path to pass this through.

dnfclas · 2020-04-23T16:39:22Z

All CLA requirements met.

src/Microsoft.Tye.Core/GenerateKubernetesManifestStep.cs

src/Microsoft.Tye.Core/KubernetesManifestGenerator.cs

src/tye/Program.DeployCommand.cs

jkotalik · 2020-04-23T17:19:03Z

There are some tests you need to update that check the k8s content with the new namespace mapping.

rynowak

I monkey-patched the path from the CLI down into the generator. Let me know if there is a preferred path to pass this through.

My best idea for this having looked through the code.

Add Namespace as a string? property to ApplicationBuilder - since it applies to all of the services rather than a single one
Set the property in https://github.com/dotnet/tye/pull/400/files#diff-c1c4c07b0626c051d8c7848f451427c8L81
Treat null as "use current context" rather than hardcoded to default
Add the same command-line argument to tye generate following the same pattern

src/Microsoft.Tye.Core/GenerateKubernetesManifestStep.cs

src/Microsoft.Tye.Core/KubernetesManifestGenerator.cs

src/tye/Program.DeployCommand.cs

brendandburns · 2020-04-23T22:48:11Z

Ok, I think I addressed all of the changes, this is much cleaner, thanks!

Please take another look.

brendandburns · 2020-04-23T22:56:53Z

fwiw, the format is flagging on line-endings, but dotnet format on my linux machine shows no problems and makes no changes. I assume that this is a Linux LF vs Windows CRLF thing.

brendandburns · 2020-04-23T22:57:11Z

I would suggest that this project converge on LF instead of CRLF.

rynowak · 2020-04-24T00:33:08Z

fwiw, the format is flagging on line-endings, but dotnet format on my linux machine shows no problems and makes no changes. I assume that this is a Linux LF vs Windows CRLF thing.

Thanks for the feedback, I'm going to follow up on this and some of the other formatting related things. I'm not having the same problem on mac, but it might be due to how I've configured something.

rynowak · 2020-04-24T00:33:53Z

Thanks!

brendandburns · 2020-04-24T00:43:32Z

You can definitely adjust git settings to do the right thing wrt line endings, we just need to decide on what the canonical line ending we want for this repo (I'd suggest LF) and then everyone can configure git to do the conversion.

…

--brendan

________________________________ From: Ryan Nowak <notifications@github.com> Sent: Thursday, April 23, 2020 5:34:05 PM To: dotnet/tye <tye@noreply.github.com> Cc: Brendan Burns <bburns@microsoft.com>; Mention <mention@noreply.github.com> Subject: Re: [dotnet/tye] Add the ability to specify a namespace for tye deploy. (#400) Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Ftye%2Fpull%2F400%23issuecomment-618738877&data=02%7C01%7Cbburns%40microsoft.com%7Ce95a86c4b7a3430464db08d7e7e7321f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637232852476420695&sdata=ACDn7WSdW7Zxp%2BbRVa2XahB2EMwSj7PpXIcB%2Bt3XU7k%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABL4HAULRGFLIIFWQG5FDZ3RODM73ANCNFSM4MPGUHFQ&data=02%7C01%7Cbburns%40microsoft.com%7Ce95a86c4b7a3430464db08d7e7e7321f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637232852476420695&sdata=83%2FM6dgQUtAbIqW0E3RWNiyp1QnoTL5EyBPOQtrwPeA%3D&reserved=0>.

brendandburns requested review from davidfowl, jkotalik and rynowak as code owners April 23, 2020 16:39

brendandburns force-pushed the cli branch from 13bffdc to 5cdb471 Compare April 23, 2020 17:02