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

Submission: virtuoso #271

Closed
11 of 25 tasks
cboettig opened this issue Dec 4, 2018 · 23 comments
Closed
11 of 25 tasks

Submission: virtuoso #271

cboettig opened this issue Dec 4, 2018 · 23 comments

Comments

@cboettig
Copy link
Member

cboettig commented Dec 4, 2018

Submitting Author: Carl Boettiger (@cboettig)
Repository: https://github.com/cboettig/virtuoso
Version submitted: 0.1.1 (tagged)
Editor: Melina Vidoni (@melvidoni)
Reviewer 1: Ildiko Czeller (@czeildi)
Reviewer 2: Edgar Ruiz (@edgararuiz)
Archive: TBD
Version accepted: TBD


  • Paste the full DESCRIPTION file inside a code block below:
Package: virtuoso
Type: Package
Title: R interface to Virtuoso using ODBC
Version: 0.1.1
Authors@R: c(person("Carl", "Boettiger", 
                  email = "cboettig@gmail.com", 
                  role = c("aut", "cre", "cph"),
                  comment = c(ORCID = "0000-0002-1642-628X")),
             person("Bryce", "Mecum", 
                    role = "ctb", 
                    email = "brycemecum@gmail.com",
                    comment = c(ORCID = "0000-0002-0381-3766")))
Description: Virtuoso is a high-performance "universal server," which can act
             as both a relational database (supporting standard SQL queries),
             and an Resource Description Framework (RDF) triplestore, supporting 
             SPARQL queries and semantic reasoning. The virtuoso package R provides
             R users with a DBI-compatible connection to the Virtuoso database. 
             The package also provides helper routines to install, launch, and manage
             a Virtuoso server locally on Mac, Windows and Linux platforms using
             the standard interactive installers from the R command-line.  By 
             automatically handling these setup steps, the package can make Virtuoso
             considerably faster and easier for a most users to deploy in a local
             environment. While this can be used as a normal dplyr backend, Virtuoso 
             excels when used as a RDF triplestore.  Managing the bulk import of triples
             from common serializations with a single intuitive command is another key
             feature of the Virtuoso R package.  Bulk import performance can be tens to
             hundreds of times faster than the comparable imports using existing R tools,
             including rdflib and redland packages.  
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
Imports: 
    odbc,
    processx,
    DBI,
    utils,
    ini,
    rappdirs,
    curl,
    fs,
    digest
RoxygenNote: 6.1.1
Suggests: 
    knitr,
    rmarkdown,
    nycflights13,
    testthat,
    covr,
    jsonld,
    rdftools,
    dplyr,
    spelling
VignetteBuilder: knitr
Remotes: cboettig/rdftools
Language: en-US

Scope

  • Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

    • data retrieval
    • data extraction
    • database access
    • data munging
    • data deposition
    • reproducibility
    • geospatial data
    • text analysis
  • Explain how the and why the package falls under these categories (briefly, 1-2 sentences):

R users confronted with large dump of triples (e.g. nquad, owl, or other file) currently have few ways of reading in this data,
and no performant option that can handle the huge file sizes frequently involved that do not fit into memory. This package provides
a relatively convenient way to import this data into an RDF-capable database and query that data directly from R.

  • Who is the target audience and what are scientific applications of this package?

Researchers working with RDF / semantic data.

This package overlaps with ropensci package rdflib (and thus redland, which is rdflib uses under the hood.), which primarily provides an in-memory model for working with RDF data, which fails with large triplestores. rdflib & redland do have a pluggable backend that can connect to Virtuoso and other databases, but this is not only very complicated to set up (not only does redland R package need to be built from source, but so does the redland C library in some cases) but is also much slower. This package handles the installation easily in a user-friendly and more performant way, and the resulting Virtuoso server can then be used as a backend to rdflib (though there is usually little reason to do so since Virtuoso can be called directly though this package already).

  • If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

Technical checks

Confirm each of the following by checking the box. This package:

  • does not violate the Terms of Service of any service it interacts with.
  • has a CRAN and OSI accepted license.
  • contains a README with instructions for installing the development version.
  • includes documentation with examples for all functions.
  • contains a vignette with examples of its essential functions and uses.
  • has a test suite.
  • has continuous integration, including reporting of test coverage using services such as Travis CI, Coveralls and/or CodeCov.

Publication options

JOSS Options
  • The package has an obvious research application according to JOSS's definition.
    • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
    • The package is deposited in a long-term repository with the DOI:
    • (Do not submit your package separately to JOSS)
MEE Options
  • The package is novel and will be of interest to the broad readership of the journal.
  • The manuscript describing the package is no longer than 3000 words.
  • You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
  • (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
  • (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
  • (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

@sckott sckott added the package label Dec 4, 2018
@sckott
Copy link
Contributor

sckott commented Dec 4, 2018

thanks for your submission - we're discussing

@melvidoni
Copy link
Contributor

melvidoni commented Dec 9, 2018

Editor checks:

  • Fit: The package meets criteria for fit and overlap
  • Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
  • License: The package has a CRAN or OSI accepted license
  • Repository: The repository link resolves correctly
  • Archive (JOSS only, may be post-review): The repository DOI resolves correctly
  • Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

Editor comments

Thank you for the submission @cboettig! Here is the output from goodpractice. There is no need to address them now, as this is information for the reviewers.

── GP virtuoso ──────────────────────────────────────────────────────────────────

It is good practice to

  ✖ write unit tests for all functions, and all package code
    in general. 74% of code lines are covered by test cases.

    R/odbc.R:55:NA
    R/odbc.R:105:NA
    R/odbc.R:106:NA
    R/odbc.R:107:NA
    R/odbc.R:108:NA
    ... and 95 more lines

  ✖ add a "URL" field to DESCRIPTION. It helps users find
    information about your package online. If your package does not
    have a homepage, add an URL to GitHub, or the CRAN package package
    page.
  ✖ add a "BugReports" field to DESCRIPTION, and point it to
    a bug tracker. Many online code hosting services provide bug
    trackers for free, https://github.com, https://gitlab.com, etc.
  ✖ avoid long code lines, it is bad for readability. Also,
    many people prefer editor windows that are about 80 characters
    wide. Try make your lines shorter than 80 characters

    inst\examples\docker.R:13:1
    inst\examples\json.R:9:1
    R\virtuoso.R:32:1
    R\virtuoso.R:42:1
    R\virtuoso.R:44:1
    ... and 6 more lines

  ✖ fix this R CMD check ERROR: Packages suggested but not
    available: 'nycflights13' 'jsonld' 'rdftools' 'spelling' The
    suggested packages are required for a complete check. Checking can
    be attempted without them by setting the environment variable
    _R_CHECK_FORCE_SUGGESTS_ to a false value. See section 'The
    DESCRIPTION file' in the 'Writing R Extensions' manual.
─────────────────────────────────────────────────────────────────────────────────

I'll be seeking for reviewers now, and update the information.


Reviewers: @edgararuiz @czeildi
Due date: 2019-01-17

@melvidoni
Copy link
Contributor

Reviewers assigned!

Reviewers: @edgararuiz @czeildi
Due date: 2019-01-17

We are extending a little bit the due date because of the holidays.

@czeildi
Copy link

czeildi commented Jan 6, 2019

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).
For packages co-submitting to JOSS

The package contains a paper.md matching JOSS's requirements with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 6


Review Comments

In general the package has extensive documentation and easy-to-follow code, it is a pleasure to review!
I included checkboxes for the comments which I feel are relatively important. The other comments are rather for consideration and would not block my recommendation for approval.

I tried the package on my mac.

Documentation

  • Add examples to function help pages. They should be probably in \dontrun{} as they have heavy dependencies. Even if the example is just restating how the function can be called I think they are useful, many users first scroll to the examples section in the documentation.
  • Add package documentation, skeleton could be added by usethis::use_package_doc().
  • vos_count_triples() should be removed from pkgdown reference until it is implemented
  • fix jsonlite::read_json("https://raw.githubusercontent.com/ropensci/roregistry/ex/codemeta.json") in datalake article, related comment which I found is here.
  • For me the following line did not run in the datalake vignette: system.time( vos_import(con, c("flights.nq", "planes.nq", "airlines.nq")) ). I have not yet investigated further, it is possibly a problem specific to my machine, but please confirm that it is working for you with a clean install.
  • For first time users it can be helpful to group functions for the pkgdown reference site. I could imagine 3 groups: getting started, working with data and advanced usage / troubleshooting. For guidance see the Grouping functions in the reference section in the ropensci guide.
  • Links do not work in function help pages, this is fixed by adding the following line to the DESCRIPTION file: Roxygen: list(markdown = TRUE)
  • TRUE, FALSE should be marked as code in vos_destroy_all() documentation, in vos_log() in just_errors true should be capital and shown as code.
  • In details section of vos_import() after list of all extensions there should be a newline.
  • Remove one ` from double ` in vos_import.R and vos_status.R
  • vos_odbcinst(): ODBC Uses a: uses should be lower case
  • In DESCRIPTION file: The virtuoso package provides or the virtuoso R package provides
  • spell check errors in README.Rmd line 80: Remember, because, convenient
  • Add a getting started vignette (called virtuoso so that it shows up as Getting Started in pkgdown website): this could have the same content as the second part of the README.
  • In datalake article: Change sentence to this: 'First, we must represent any primary or foreign keys in any table as URIs and not literal integers or strings:'
  • In vos_install() if I see correctly, this is a general function and the documentation title is incorrect that it is only for mac. This is in conflict with readme on what platforms are fully supported.

Usage

  • In vos_install() if Virtuoso is already installed, for me the message could be slightly cleaner with sg like this: Virtuoso installation already present so no installation happens.
  • I think vos_kill should not throw error if there is nothing to kill.
  • In vos_status() I would not give a warning, only a message or plain return value if no process. A warning indicates sg that should be fixed, I might just want to know whether I should start or kill virtuoso.
  • In my experience NULL is more widespread and thus easier to understand as default parameter value than NA in vos_kill(). Reference here.
  • vos_delete_db() fails with a non-informative error message in non-interactive environment or if ask = FALSE.

Code

  • For installation and other code, hard coding the version of Virtuoso open source does not seem future proof. Consider either installing always the latest version or making the version a parameter or environment variable. Or at least make this clear in README that the code works with a specific version if local server is considered.
  • react to your own FIXME comments in code if relevant ( :) )
  • linux branch in vos_uninstall(): consider converting to message (consistent with vos_uninstall_osx()) or error (consistent with vos_install_linux())
  • I see that in a recent commit you removed rdftools from Suggests. For someone rebuilding articles locally it would be nice to add sg like this to the article: if (!requre('rdftools')) stop('please install...'). Although this might only be relevant for this review process: until I found your issue already discussing this my first thought was that this package should be added to Suggests.
  • I am not sure about this but would it make sense to include addons from travis as SystemRequirements in DESCRIPTION?
  • Use testthat::setup and teardown for vos_start and vos_kill in test-vos_query as it is more transparent and it will ensure vos_kill is run even if there is some error during running the tests.
testthat::teardown(vos_kill())
  • sleep should only happen in test setup if installation actually happens, otherwise we wait 20 seconds for nothing
  • consider specifying message (or message pattern) in expect_warning in order to avoid there being a completely different warning
  • consider mocking as a way to add tests to functions which are difficult to test (such as platform dependent functions)
  • use codemetar as it is recommended by ropensci
  • ask and prompt function argument name is used for the same purposes, consider unifying these
  • is_interactive() function is defined, but not used at all places instead of interactive()
  • fs is already imported -> might consider https://fs.r-lib.org/reference/path_package.html instead of system.file as it cannot fail silently
style

In general the code is very easily readable. However, code style is not fully consistent. Examples: space after if, space before {, indentation of multiple function arguments, using simple or double quotes. Making these consistent can help future potential contributors as it is clear what style they should follow. (I am not sure whether it is possible to configure styler to automatically ensure a style close to your current style.)

  • virtuoso::: in tests is a bit confusing to me and I believe it is not necessary
  • In vos_status() details: `[vos_log()]`` -> [`vos_log()`] for consistency

@edgararuiz-zz
Copy link

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).
For packages co-submitting to JOSS

The package contains a paper.md matching JOSS's requirements with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing:


Review Comments

Test results

4 tests fail. I tested it on my Windows laptop. Something I noticed is that I
have to do vos_install() and then vos_start() every time I tried to use
virtuoso in a new R session. I believe that explains the issues brougth up by
testthat

Loading virtuoso
Testing virtuoso
Running one-time install of Virtuoso for tests...
v | OK F W S | Context
v |  5       | utilities
v |  2       | vos_configure
x |  2 1   2 | vos install errors [0.4 s]
------------------------------------------------------
test-vos_install-errors.R:12: error: We can download the windows installer
HTTP error 404.
1: virtuoso:::download_windows_installer() at C:\Users\edgar\Documents\virtuoso/tests/testthat/test-vos_install-errors.R:12
2: curl::curl_download(download_url, installer) at C:/Users/edgar/Documents/virtuoso/R/vos_install_windows.R:12

test-vos_install-errors.R:32: skip: We get errors running windows installer on non-windows
On windows

test-vos_install-errors.R:42: skip: We get errors running mac installer on non-mac
On windows
------------------------------------------------------
v |  1     1 | vos install
------------------------------------------------------
test-vos_install.R:13: skip: We can download installers
On windows
------------------------------------------------------
v | 11       | Detecting ODBC Drivers
x |  0 1     | test vos_query [33.2 s]
------------------------------------------------------
test-vos_query.R:3: error: (unknown)
No such process, pid 6812, ???
1: vos_start() at C:\Users\edgar\Documents\virtuoso/tests/testthat/test-vos_query.R:3
2: vos_status(p, wait = wait) at C:/Users/edgar/Documents/virtuoso/R/vos_start.R:44
3: p$get_status() at C:/Users/edgar/Documents/virtuoso/R/vos_status.R:36
4: ps_method(ps::ps_status, self)
5: fun(ps::ps_handle(self$get_pid(), self$get_start_time()))
------------------------------------------------------
x |  2 1     | vos_start
------------------------------------------------------
test-vos_start.R:13: error: we can start a vos server and check status
No such process, pid 6812, ???
1: expect_true(p$get_status() %in% c("sleeping", "running")) at C:\Users\edgar\Documents\virtuoso/tests/testthat/test-vos_start.R:13
2: quasi_label(enquo(object), label)
3: eval_bare(get_expr(quo), get_env(quo))
4: p$get_status() %in% c("sleeping", "running")
5: p$get_status()
6: ps_method(ps::ps_status, self)
7: fun(ps::ps_handle(self$get_pid(), self$get_start_time()))
------------------------------------------------------
x |  1 1     | tests that do not need a server connection
------------------------------------------------------
test-without-vos.R:7: failure: vos_process errors when not cached
`vos_process()` did not produce any warnings.
------------------------------------------------------

== Results ===========================================
Duration: 33.8 s

OK:       24
Failed:   4
Warnings: 0
Skipped:  3

Functionality

Some of the funcitonality works, but not all of what is shown in the README. Here is a reprex of what I see:

library(virtuoso)
vos_start()
#> Error in vos_start(): Virtuoso installation not detected.  See vos_install()
vos_install()
vos_start()
#> PROCESS 'virtuoso-t', running, pid 2884.
#> Server is now starting up, this may take a few seconds...
#> latest log entry: 11:06:25 Server exiting
#> Error: No such process, pid 2884, ???
con <- vos_connect()
DBI::dbGetQuery(con, "SPARQL SELECT * WHERE { ?s ?p ?o } LIMIT 4")
#>                                                                              s
#> 1                   http://www.openlinksw.com/virtrdf-data-formats#default-iid
#> 2          http://www.openlinksw.com/virtrdf-data-formats#default-iid-nullable
#> 3          http://www.openlinksw.com/virtrdf-data-formats#default-iid-nonblank
#> 4 http://www.openlinksw.com/virtrdf-data-formats#default-iid-nonblank-nullable
#>                                                 p
#> 1 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#> 2 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#> 3 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#> 4 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#>                                                         o
#> 1 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
#> 2 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
#> 3 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
#> 4 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
example <- system.file("extdata", "person.nq", package = "virtuoso")
vos_import(con, example)
#> Error: <SQL> 'ld_dir('C:/Users/edgar/AppData/Local/Virtuoso/Virtuoso/Cache/c990c6c99cd92278bde129f6fedd5b65', '*', 'rdflib')'
#>   nanodbc/nanodbc.cpp:1587: 42000: [OpenLink][Virtuoso ODBC Driver][Virtuoso Server]FA003: Access to 'C:\Users\edgar\AppData\Local\Virtuoso\Virtuoso\Cache\c990c6c99cd92278bde129f6fedd5b65' is denied due to access control in ini file
vos_query(con, 
          "SELECT ?p ?o 
 WHERE { ?s ?p ?o .
        ?s a <http://schema.org/Person>
       }")
#> [1] p o
#> <0 rows> (or 0-length row.names)

Created on 2019-01-21 by the reprex package (v0.2.1)

@melvidoni
Copy link
Contributor

@cboettig Reviews have been completed. Please, perform the corresponding changes, and comment again answering the reviewers with your updates.

@cboettig
Copy link
Member Author

Thanks both, this is super helpful.

@edgararuiz , I'm traveling at the the moment so won't have access to my Windows machine until next week when I can poke around more. Definitely should not have to re-install each time though! A few questions meanwhile that can help me debug:

  • When you run vos_install(), does this open the standard Windows installation dialog that asks you to install Virtuoso? Can you try again with vos_install(prompt=FALSE)? This will run an non-interactive version of the installer. You should not have to reinstall unless you run vos_uninstall() or manually uninstall Virtuoso.

  • After you run vos_start(), can you let me know what it says on the logfile:

readLines( file.path(virtuoso:::vos_logdir(), "virtuoso.log"))

From the log above, it looks like Virtuoso is installed and starts up but then shuts down. I think this is due to a file permission error somewhere, but I'm not quite sure where.

  • Can you tell me what you get from:
virtuoso:::find_virtuoso_ini()
virtuoso:::vos_db()
virtuoso:::vos_logdir()

Each of those should return file path -- can you determine if your user has write permissions to each of those locations?

Thanks!

@edgararuiz-zz
Copy link

Hi, sorry for the delay in the response, I've been out of town

In a new R session, when I run vos_install() it just runs silently and quickly. And then when I run vos_start() it seems to start ok:

readLines( file.path(virtuoso:::vos_logdir(), "virtuoso.log"))
 [1] ""                                                                                                                                         
 [2] "\t\tThu Jan 31 2019"                                                                                                                        
 [3] "15:45:58 [Using virtuoso.ini in C:\\Users\\edgar\\AppData\\Local\\Virtuoso\\Virtuoso]"                                                    
 [4] "15:45:58 { Loading plugin 1: Type `plain', file `wikiv' in `C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting'"     
 [5] "15:45:58   WikiV version 0.6 from OpenLink Software"                                                                                      
 [6] "15:45:58   Support functions for WikiV collaboration tool"                                                                                
 [7] "15:45:58   SUCCESS plugin 1: loaded from C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting\\wikiv.dll }"            
 [8] "15:45:58 { Loading plugin 2: Type `plain', file `mediawiki' in `C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting'" 
 [9] "15:45:58   MediaWiki version 0.1 from OpenLink Software"                                                                                  
[10] "15:45:58   Support functions for MediaWiki collaboration tool"                                                                            
[11] "15:45:58   SUCCESS plugin 2: loaded from C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting\\mediawiki.dll }"        
[12] "15:45:58 { Loading plugin 3: Type `plain', file `creolewiki' in `C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting'"
[13] "15:45:58   CreoleWiki version 0.1 from OpenLink Software"                                                                                 
[14] "15:45:58   Support functions for CreoleWiki collaboration tool"                                                                           
[15] "15:45:58   SUCCESS plugin 3: loaded from C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting\\creolewiki.dll }"       
[16] "15:45:58 { Loading plugin 4: Type `plain', file `im' in `C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting'"        
[17] "15:45:58   IM version 0.6 from OpenLink Software"                                                                                         
[18] "15:45:58   Support functions for Image Magick 6.9.9"                                                                                      
[19] "15:45:58   SUCCESS plugin 4: loaded from C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting\\im.dll }"               
[20] "15:45:58 { Loading plugin 5: Type `plain', file `wbxml2' in `C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting'"    
[21] "15:45:58   WBXML2 version 0.9 from OpenLink Software"                                                                                     
[22] "15:45:58   Support functions for WBXML2 0.9.2 Library"                                                                                    
[23] "15:45:58   SUCCESS plugin 5: loaded from C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\hosting\\wbxml2.dll }"           
[24] "15:45:58 OpenLink Virtuoso Universal Server"                                                                                              
[25] "15:45:58 Version 07.20.3217-threads for Win64 as of Sep  6 2017"                                                                          
[26] "15:45:58 uses parts of OpenSSL, PCRE, Html Tidy"                                                                                          
[27] "15:45:58 Database version 3126"                                                                                                           
[28] "15:45:58 SQL Optimizer enabled (max 1000 layouts)"                                                                                        
[29] "15:45:59 Compiler unit is timed at 0.000137 msec"                                                                                         
[30] "15:46:01 Roll forward started"                                                                                                            
[31] "15:46:01     3 transactions, 185 bytes replayed (100 %)"                                                                                  
[32] "15:46:01 Roll forward complete"                                                                                                           
[33] "15:46:02 PL LOG: Can't get list of vad packages in C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\vad/"                  
[34] "15:46:02 Checkpoint started"                                                                                                              
[35] "15:46:02 Checkpoint finished, log reused"                                                                                                 
[36] "15:46:02 HTTP/WebDAV server online at 8890"                                                                                               
[37] "15:46:02 Server online at 1111 (pid 8120)"    

If I restart R, and just run vos_start() I get:

Error in vos_start() : 
  Virtuoso installation not detected.  See vos_install()

The log does not have any new entries.

Here are the results of the other commands you requested to run:

> virtuoso:::find_virtuoso_ini()
[1] "C:\\Program Files\\OpenLink Software\\Virtuoso OpenSource 7.20\\database\\virtuoso.ini"
> virtuoso:::vos_db()
[1] "C:\\Users\\edgar\\AppData\\Local\\Virtuoso\\Virtuoso"
> virtuoso:::vos_logdir()
[1] "C:\\Users\\edgar\\AppData\\Local\\Virtuoso\\Virtuoso\\Logs"

@cboettig
Copy link
Member Author

@edgararuiz No worries and thanks! I think I have resolved some of these issues now, though I still have a few edge cases to work out, (including appveyor). Can you test the ropensci-review branch, e.g. with

install_github("cboettig/virtuoso@ropensci-review")

There's still one case I'm working on, if virtuoso server is already running but not controlled by the R session (e.g. because the user doesn't uncheck the box or an R session doesn't shut down properly after starting one), then currently vos_start() will fail to start. I believe I can now solve this, so that R will simply grab the active virtuoso process, but needs a bit more testing.

Thanks for this report. I'll ping soon when I have a few more things ironed out.

@cboettig
Copy link
Member Author

cboettig commented Feb 6, 2019

@edgararuiz Okay, can you install install_github("cboettig/virtuoso@ropensci-review") and give it a second run?

@czeildi Thanks for the stellar review, I think I've addressed each of the issues you've raised now as well. you can see all the changes I've made in response to both of your reviews so far in this open PR, ropensci/virtuoso#25, hopefully makes it somewhat easier to track. For Ildikó 's review, I've summarized the changes point-by-point in the associated issue.

Thanks both for all the super constructive testing. Edgar, hope we've got this working more robustly on Windows now as well, but looking forward to your testing as this kind of thing is particularly tricky to get the cross-platform details working with so much system-specific operations, (even once I've satisfied Appveyor and my local Windows 10 box)

@czeildi
Copy link

czeildi commented Feb 9, 2019

@cboettig I checked your pull request, looks great, now I am happy to recommend approving virtuoso.

Thanks for replying point-by-point and in detail.

I found one small typo: in vos_connect.R Default parameters are approporiate

Regarding the datalake article:

  • the readme works for me fully
  • the problem is not related to that chunk: my virtuoso process is killed after some time seemingly without reason (Running only vos_status() repeatedly and nothing else it reported the process is not running after some time). I think we can let this go at this time.

@edgararuiz-zz
Copy link

@cboettig -

I'm getting the follow when using the new branch:

vos_install()
downloading Virtuoso_OpenSource_Server_7.20.x64.exe ...
 Error in curl::curl_fetch_disk(url, dest) : 
  Timeout was reached: Connection timed out after 10625 milliseconds 

@cboettig
Copy link
Member Author

cboettig commented Feb 17, 2019 via email

@edgararuiz-zz
Copy link

Yes sir, here's a reprex:

devtools::install_github("cboettig/virtuoso@ropensci-review")
#> Skipping install of 'virtuoso' from a github remote, the SHA1 (c0296369) has not changed since last install.
#>   Use `force = TRUE` to force installation
library(virtuoso)
vos_install()
#> downloading Virtuoso_OpenSource_Server_7.20.x64.exe ...
#> Error in curl::curl_fetch_disk(url, dest): Timeout was reached: Connection timed out after 10016 milliseconds

Created on 2019-02-17 by the reprex package (v0.2.1)

@cboettig
Copy link
Member Author

@edgararuiz very weird. Still works for me, and the current branch also has a fallback mechanism it should use if the binary really was not reproducible, so this error looks like only what I can get on master branch. You also restarted R as well?

Can you show me what you see by running virtuoso:::download_windows_installer (no parens, to show function def). The new branch should show like this:

virtuoso:::download_windows_installer
function(version = "7.2.5"){
  exe <- "Virtuoso_OpenSource_Server_7.20.x64.exe"
  download_url <- paste0("https://sourceforge.net/projects/virtuoso/",
                         "files/virtuoso/7.2.5/",
                         "2018_08_28_",
                         "Virtuoso_OpenSource_Server_7.2.x64.exe/download")
  fallback_url <- paste0("https://github.com/cboettig/virtuoso/releases/",
                             "download/v0.1.1/Virtuoso_OpenSource_Server_7.20.x64.exe")
  installer <- normalizePath(file.path(
    tempdir(),
    exe),
    mustWork = FALSE)
  message(paste("downloading", exe,  "..."))
  download_fallback(download_url, installer, fallback_url)
  installer
}

you might also check if you can access that download URL? Is it possible you have some firewall settings that prevent downloads from sourceforge (and github??)

(note that on the other hand, the sourceforge URL in the branch on master has moved, and had not fallback URL, so it should create the exact timeout error you see). Sorry for the trouble!

@edgararuiz-zz
Copy link

Hi @cboettig , it's all good now. For some reason the vos_install() started working this morning. Must have been something on my side, sorry for that.

The short of it, is that I'm good to go with this version, I was able to run some basic stuff, with out problems:

library(virtuoso)
vos_start()
#> PROCESS 'virtuoso-t', running, pid 3412.
#> Server is now starting up, this may take a few seconds...
#> latest log entry: 08:13:16 Server online at 1111 (pid 3412)
con <- vos_connect()
DBI::dbGetQuery(con, "SPARQL SELECT * WHERE { ?s ?p ?o } LIMIT 4")
#>                                                                              s
#> 1                   http://www.openlinksw.com/virtrdf-data-formats#default-iid
#> 2          http://www.openlinksw.com/virtrdf-data-formats#default-iid-nullable
#> 3          http://www.openlinksw.com/virtrdf-data-formats#default-iid-nonblank
#> 4 http://www.openlinksw.com/virtrdf-data-formats#default-iid-nonblank-nullable
#>                                                 p
#> 1 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#> 2 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#> 3 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#> 4 http://www.w3.org/1999/02/22-rdf-syntax-ns#type
#>                                                         o
#> 1 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
#> 2 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
#> 3 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
#> 4 http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat
example <- system.file("extdata", "person.nq", package = "virtuoso")
vos_import(con, example)
vos_query(con, 
          "SELECT ?p ?o 
 WHERE { ?s ?p ?o .
        ?s a <http://schema.org/Person>
       }")
#>                                                 p                        o
#> 1 http://www.w3.org/1999/02/22-rdf-syntax-ns#type http://schema.org/Person
#> 2                          http://schema.org/name                 Jane Doe
#> 3                      http://schema.org/jobTitle                Professor
#> 4                     http://schema.org/telephone           (425) 123-4567
#> 5                           http://schema.org/url   http://www.janedoe.com

Created on 2019-02-19 by the reprex package (v0.2.1)

Having said that there were a couple of things that you may need to be aware of in case you get issues opened from Windows users who are trying it out:

  • Upgrading versions of Virtuoso creates problems when trying to run the new version. It seems that the later version of Virtuoso does not like the transaction file to be tagged with the previous version's number, so the solution is to delete the virtuoso.trx file.

  • If the error comes up that says that Virtuoso cannot open port 1111, go to the Task Manager and kill the Virtuoso Open Source process manually, and try again.

Great job!!

@melvidoni
Copy link
Contributor

Please, @edgararuiz and @czeildi, let me know when you are giving the OK for the onboarding!

@edgararuiz-zz
Copy link

Yes, I am giving the OK for onboarding. Thank you @melvidoni

@czeildi
Copy link

czeildi commented Feb 19, 2019

I am giving the OK too! @melvidoni

@melvidoni
Copy link
Contributor

melvidoni commented Feb 19, 2019

Approved! Thanks @cboettig for submitting and @edgararuiz and @czeildi for your reviews!

To-dos:

  • Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
  • Add the rOpenSci footer to the bottom of your README

" [![ropensci_footer](https://ropensci.org/public_images/ropensci_footer.png)](https://ropensci.org)"

  • Fix any links in badges for CI and coverage to point to the ropensci URL. We no longer transfer Appveyor projects to ropensci Appveyor account so after transfer of your repo to rOpenSci's "ropensci" GitHub organization the badge should be [![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/github/ropensci/pkgname?branch=master&svg=true)](https://ci.appveyor.com/project/individualaccount/pkgname).
  • We're starting to roll out software metadata files to all ropensci packages via the Codemeta initiative, see https://github.com/ropensci/codemetar/#codemetar for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package.

Should you want to awknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field (with their consent). More info on this here.

Welcome aboard! We'd also love a blog post about your package, either a short-form intro to it (https://ropensci.org/tech-notes/) or long-form post with more narrative about its development. (https://ropensci.org/blog/). If you are interested, @stefaniebutland will be in touch about content and timing.

We've started putting together a gitbook with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here.


Notes

Once the above is done, please @cboettig let me know, so we can close the issue.

@stefaniebutland
Copy link
Member

@cboettig You're always welcome to write a post, but I recall you said earlier that you prefer to save your writing energy for papers. 🙂

@cboettig
Copy link
Member Author

Thanks @melvidoni ! I've transferred the repo and updated links. Preparing CRAN submission...

@melvidoni
Copy link
Contributor

Thank you @cboettig! You will be given admin rights shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants