Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Ensure Stats.CollectAzureCdnLogs gets latest KeyVault secrets on every iteration #679

Closed
wants to merge 5 commits into from

Conversation

xavierdecoster
Copy link
Member

Copy link
Contributor

@skofman1 skofman1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@skofman1
Copy link
Contributor

If we change the FTP password while the job is running it's going to crash and reload the new password when it comes back up, right?

@xavierdecoster
Copy link
Member Author

If we change the FTP password while the job is running it's going to crash and reload the new password when it comes back up, right?

Yep, that's the idea.

@chenriksson
Copy link
Member

@xavierdecoster Didn't realize this included change to JSON configuration... had planned on doing this for job deployment changes (see 2bc5079).

One question - why not use JsonConfigurationJob, like other jobs in this repo?

@xavierdecoster
Copy link
Member Author

xavierdecoster commented Nov 28, 2018

@chenriksson I figured changing to JSON config would make the fix easier, and make the job more consistent at once :)

I was unaware of the JsonConfigurationJob type. I think I based my change off another job that likely duplicates code from this base type (and should be updated to inherit from it) :)

@chenriksson
Copy link
Member

@xavierdecoster As part of SQL AAD work, I moved JsonConfigurationJob from Validation.Common.Job to Jobs.Common, and updated jobs with SQL connections to use it. I believe Orchestrator preceded JsonConfigurationJob, so it's the only outlier.

As part of the work for job deployments, @ryuyu and I want to migrate all jobs in this repo to JSON config and will be moving configurations to NuGetDeployment, with Octopus config replaced in source. So, settings.json will need environment-specific config.

Let me know if you want more details. Otherwise, I'll hold off on my changes to this job and follow your lead.

return value;
}

throw new ArgumentException("Job parameter for Azure CDN Platform is invalid. Allowed values are: HttpLargeObject, HttpSmallObject, ApplicationDeliveryNetwork, FlashMediaStreaming.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string.Join(", ", Enum.GetValues(typeof(AzureCdnPlatform))) instead of listing the values verbatim?

@xavierdecoster
Copy link
Member Author

Closing. New PR #690

@xavierdecoster xavierdecoster deleted the dev-issue1842 branch November 30, 2018 10:41
joelverhagen pushed a commit that referenced this pull request Jul 31, 2020
[ReleasePrep][2019.10.17]RI of dev into master
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
[ReleasePrep][2019.10.17]RI of dev into master
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants