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

Update DbConnectionExtensions.cs #753

Closed
wants to merge 5 commits into from
Closed

Update DbConnectionExtensions.cs #753

wants to merge 5 commits into from

Conversation

PeteDuncanson
Copy link
Contributor

Add a log message to stop database start up code from swallowing exception messages, now stash them in the logs to give developers a chance ;)

Add a log message to stop database start up code from swallowing exception messages, now stash them in the logs to give developers a chance ;)
@@ -77,6 +77,8 @@ public static bool IsAvailable(this IDbConnection connection)
}
catch (DbException)
{
// Don't swallow this error, the exception is super handy for knowing "why" its not available
LogHelper.Info<DatabaseContext>("Configured database is reporting as not being available!", DbException );
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps you should log this as an WarnWithException, and then pass in the exception... you also need to define the exception variable... this will not build ;)

@nul800sebastiaan
Copy link
Member

Just make a new commit in this branch and push it, the PR then updates automatically!

Fixing by ham fisted cut and paste job on my log message and changing the method used to log due to Shannons feedback
Third times a charm. Lesson learn don't try to code in a RTE!
{
// Don't swallow this error, the exception is super handy for knowing "why" its not available
LogHelper.WarnWithException<DbConnectionExtensions>("Configured database is reporting as not being available!", exc );
Copy link
Member

Choose a reason for hiding this comment

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

You're missing using Umbraco.Core.Logging; 🙌

Comedy of errors. I've started doing this in the browser github editor I'm going to finish it!
@PeteDuncanson
Copy link
Contributor Author

I'm finding this quite funny to do...sorry for littering the commit history

@PeteDuncanson
Copy link
Contributor Author

I'm giving up...and never doing this in browser again :(

@nul800sebastiaan
Copy link
Member

Well, this was the perfect opportunity for me to try and learn how to squash commits (which involved using VIM for the very first time.. whut?! 🔫)

Anyway, your shame is hidden, it's all one commit now: https://github.com/umbraco/Umbraco-CMS/commits/dev-v7
Unfortunately that didn't gel with github and it didn't accept this PR, but it's in the core now, all done!

@nul800sebastiaan
Copy link
Member

Duh.. was I too hasty?
Eep

haha, I'll fix it up. Browsers are for fixing spelling errors.. ;-)

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.

None yet

3 participants