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

Auto-parse the DB URL and add parseTime=true if the backend is mysql #1150

Merged
merged 7 commits into from
May 9, 2017

Conversation

ashfall
Copy link
Contributor

@ashfall ashfall commented May 8, 2017

Closes #328

Also updates the mysql driver to a newer release

Signed-off-by: Ashwini Oruganti [email protected]

if err != nil {
return nil, err
}
if !url_config.ParseTime {
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: We can probably just set url_config.ParseTime to true here, since if it was true before, setting it to true again doesn't hurt anything. :)

Would be awesome if we could add a test for this though!

Also updates the mysql driver to a newer release

Signed-off-by: Ashwini Oruganti <[email protected]>
truie either way without any issues

Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
Signed-off-by: Ashwini Oruganti <[email protected]>
@endophage
Copy link
Contributor

Re-iterating @cyli, we need to add a test :-)

Signed-off-by: Ashwini Oruganti <[email protected]>
Copy link
Contributor

@endophage endophage left a comment

Choose a reason for hiding this comment

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

LGTM pending CircleCI/Codecov results (looking for the new error case in ParseSQLStorage to be covered)

@cyli
Copy link
Contributor

cyli commented May 9, 2017

LGTM! Thanks for fixing this!

@cyli cyli merged commit 23eceed into notaryproject:master May 9, 2017
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.

3 participants