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

SendValidationTakingTooLongNotice method with tests #5402

Merged
merged 4 commits into from
Feb 6, 2018
Merged

Conversation

agr
Copy link
Contributor

@agr agr commented Feb 5, 2018

Added a method to CoreMessageService that sends notification about validation taking too much time.

Part of the changes for the https://github.com/NuGet/Engineering/issues/1144

@karann-msft
Copy link
Contributor

@agr what is "too much time"? where is that defined and how easy is to modify that?

@agr
Copy link
Contributor Author

agr commented Feb 5, 2018

@karann-msft, it's what previously was "failure timeout" in the orchestrator configuration file:

https://github.com/NuGet/NuGet.Jobs/blob/0cf53771820b35306f12ddd9e730aff12a30a338/src/NuGet.Services.Validation.Orchestrator/settings.json#L6

Another PR is coming with Orchestrator changes once this one is merged.

@agr
Copy link
Contributor Author

agr commented Feb 6, 2018

@karann-msft modification would consist of configuration update and service redeployment.

@skofman1
Copy link
Contributor

skofman1 commented Feb 6, 2018

@agr, what will be this value configured to? 1 hour? 24 hours?
IMO 1 hour is the right direction. @karann-msft ?

@agr
Copy link
Contributor Author

agr commented Feb 6, 2018

Right now it "inherited" the previous "failAfter" value of 1 day, reducing it seems like a right thing. Would need to update our alerting, too, once we settle on a value.

string body = "It is taking longer than expected for your package [{1} {2}]({3}) to get published.\n\n" +
"We are looking into it and there is no action on you at this time. We’ll send you an email notification when your package has been published.\n\n" +
"Thank you for your patience.\n\n" +
"NuGet Team";
Copy link
Member

Choose a reason for hiding this comment

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

Should we use something like GalleryOwner, instead of NuGet Team?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the signature completely as we don't have it in other emails.

@karann-msft
Copy link
Contributor

karann-msft commented Feb 6, 2018

I agree with @skofman1, 1 hour seems right. In addition to sending the email, we should create an alert for us to investigate why it is taking that long.
cc: @anangaur

@agr
Copy link
Contributor Author

agr commented Feb 6, 2018

We already have an alert

Removed the unused argument from the `SendValidationTakingTooLongNotice` method
@agr agr merged commit 93d5307 into dev Feb 6, 2018
@agr agr deleted the dev-long-val-msg branch February 6, 2018 20:58
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.

5 participants