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

MySQL test #8

Open
wants to merge 24 commits into
base: dev
Choose a base branch
from
Open

MySQL test #8

wants to merge 24 commits into from

Conversation

Farreeda
Copy link
Collaborator

@Farreeda Farreeda commented Mar 19, 2023

-I edited the function, added the exact arguments to be needed
-created test set but those information are from my mysql server, should I change the host to my ip_address?
For issue #2

@TheCedarPrince TheCedarPrince changed the base branch from main to dev June 25, 2023 18:56
Copy link
Collaborator

@TheCedarPrince TheCedarPrince left a comment

Choose a reason for hiding this comment

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

Lots of small changes as well as points for discussion during our check-in. Would be better to discuss as opposed to me writing it here so we can think through everything that needs to be adjusted/added. Thanks Fareeda!

prettyurls = get(ENV, "CI", "false") == "true",
canonical = "https://github.com/JuliaDatabases/DBConnector.jl"
edit_link = "dev",
footer = "Created by [Jacob Zelko](https://jacobzelko.com) & [Georgia Tech Research Institute](https://www.gtri.gatech.edu). [License](https://github.com/JuliaHealth/OMOPCDMCohortCreator.jl/blob/main/LICENSE)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can remove the footer variable here.

@@ -5,3 +5,6 @@ Example Julia package repo.
```@autodocs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fareeda, could you open an issue about needing to update the home page of DBConnector.jl's documentation? Thanks!

test/runtests.jl Outdated
Comment on lines 14 to 21
@testset "_dbconnect function for MySQL" begin

conn = _dbconnect(MySQL.Connection, "localhost", "debian-sys-maint","HGGsOLypO2LVqq1v", db="mydatabase", port=3306, unix_socket="/var/run/mysqld/mysqld.sock")
@test typeof(conn) == MySQL.Connection
@test isopen(conn)
close(conn)

end
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very great that this is working! Like you said, we'll need to determine how to run these tests somewhere else. I'd like to somehow get this connection made to the GSoC Server for testing over the summer while somehow maintaining secrecy for our server credentials. Let's talk about this at our weekly check-in!

makedocs(modules = [Example], sitename = "Example.jl")
makedocs(;
modules = [DBConnector],
authors = "Jacob Zelko (aka TheCedarPrince) <[email protected]> and Fareeda",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feel free to also add your last name and email too! :D


deploydocs(repo = "github.com/quinnj/Example.jl.git", push_preview = true)
)
deploydocs(repo = "github.com/quinnj/Example.jl.git",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh could you update this to the repo URL as shown above? Don't forget the .git extension that needs to be there. Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am holding off on reviewing this documentation until we chat about tests and further about the API!

@@ -1,5 +1,8 @@
function _dbconnect(conn_obj::Type{MySQL.Connection}, kwargs)
function _dbconnect(conn_obj::Type{MySQL.Connection}, host::String, user::String, password::String; db::String="", port::Integer=3306, unix_socket::Union{Nothing,String}=nothing, client_flag=API.CLIENT_MULTI_STATEMENTS, opts = Dict())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, I am not too sure on this layout here; I think it would be better to have the second argument actually only be a string that is passed to the connection. Let's discuss during our check-in.

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.

2 participants