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

Connection Objects #1

Merged
merged 3 commits into from
Jan 30, 2024
Merged

Connection Objects #1

merged 3 commits into from
Jan 30, 2024

Conversation

btmonier
Copy link
Member

This PR adds the initial class objects, utilities, and unit tests used to set up connections to BrAPI-compliant PHGv2 endpoints for data retrieval.

@btmonier btmonier requested review from lynnjo and zrm22 January 29, 2024 15:52
@btmonier btmonier self-assigned this Jan 29, 2024

## ----
#' @title Return type of PHG connection
#'
Copy link

Choose a reason for hiding this comment

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

How are phgType and httProtocol different? If phgType is a connection type, does that mean http vs https?

Copy link
Member Author

Choose a reason for hiding this comment

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

phgType is something from v1 since I needed to differentiate between local Postgress/Sqlite connections and the BrAPI connections it returns either local or server depending on the object created. httProtocol simply returns if the server has only http or https encryption. I was thinking maybe further down the road to implement something like this with a direct interface to TileDB.

"SERVER_INFO" = "serverinfo",
"VARIANT_TABLES" = "variantTables",
"VARIANTS" = "variants"
)
Copy link

Choose a reason for hiding this comment

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

Does this list imply that all of these endpoints are supported? What are your allelematrix vs variantTables endpoints? Are these old endpoints from v1? We currently only support sample, serverinfo, variants, and soon variantSets (the latter gives you a link to a multi-sample vcf file)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is another copy from v1. I made this because I (1) wanted a simple data structure to point to all possible BrAPI connections since a lot of these are used in other downstream functions. I can probably remove a majority of these at a later date once we finalize v2 endpoints.

@tcasstevens tcasstevens self-requested a review January 29, 2024 19:46
@@ -1,4 +1,4 @@
Package: rphg2

Choose a reason for hiding this comment

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

Since we are going with phg, should this be rphg?

Copy link
Member Author

Choose a reason for hiding this comment

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

This will have to be rPHG2 to avoid package collisions with the already existing rPHG.

exportMethods(phgType)
exportMethods(port)
exportMethods(readSamples)
exportMethods(serverInfo)

Choose a reason for hiding this comment

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

This is repeated

Choose a reason for hiding this comment

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

Oh one says exportMethods

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes - export() is for exporting function calls into the R namespace and exportMethods() is for exporting any package defined generics that are not in the base libraries.

"METHOD_TABLE" = "allelematrix",
"SAMPLES" = "samples",
"SERVER_INFO" = "serverinfo",
"VARIANT_TABLES" = "variantTables",

Choose a reason for hiding this comment

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

Should variantTables have a capital T?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is something that was transferred over from the v1 ktor service. I will keep it in for now until the v2 service matures.

@btmonier btmonier merged commit bb55327 into main Jan 30, 2024
5 checks passed
@btmonier btmonier deleted the connection-class branch February 6, 2024 13:35
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.

4 participants