Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Added better error handling when a DB driver is wrong defined (fix #10) #11

Merged
merged 1 commit into from
Sep 26, 2016

Conversation

daria-lewandowska
Copy link
Contributor

Fixes #10
Summary of changes:
-better error handling

Testing done:
-integration test

@daria-lewandowska daria-lewandowska force-pushed the fix_for_#10 branch 4 times, most recently from 7ef8299 to b83fade Compare September 12, 2016 08:20
@@ -150,4 +150,73 @@ var (
}
]
}`)
// FileContCorr is a mocked content of incorrect setfile
FileContCorr = []byte(`{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider moving json content as separate file and read it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@marcin-krolik
Copy link
Collaborator

There is still incorrectly handled error in closeDBs()

@daria-lewandowska
Copy link
Contributor Author

daria-lewandowska commented Sep 15, 2016

I'm not sure how should I handle this error in closeDBs(),
you think it will be ok to do:
return fmt.Errorf("Cannot close connection to database %+s\n", dbs[i].DBName)
instead of:
fmt.Fprintf(os.Stdout, "Cannot close connection to database %+v, err=%+v\n", dbs[i].DBName, err)

@daria-lewandowska daria-lewandowska force-pushed the fix_for_#10 branch 2 times, most recently from 2bae8c8 to 60a4681 Compare September 15, 2016 09:19
@marcin-krolik
Copy link
Collaborator

@daria-lewandowska yes, I'd suggest to add error as returned type to closeDBS() function, but would not return early from loop. I think you should try to close all existing DB connections, and if any fails, return an error

@daria-lewandowska daria-lewandowska force-pushed the fix_for_#10 branch 3 times, most recently from c39fbe1 to a7fe16c Compare September 23, 2016 13:27
@marcin-krolik
Copy link
Collaborator

LGTM

@daria-lewandowska daria-lewandowska merged commit 657ecea into master Sep 26, 2016
@IzabellaRaulin IzabellaRaulin deleted the fix_for_#10 branch April 27, 2017 09:24
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.

2 participants