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

shiny java package example #6

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

marekhorst
Copy link
Member

Forgot to create PR for shiny package example.

You should find shiny package installation details in readme file.

marekhorst and others added 14 commits February 5, 2016 18:35
Issue #2

Before introducing the changes, the `make check` command failed
due to an error and warnings. Introduced changes:

- `@export` was moved because in the original position (at the bottom of
  the comment block) it was invisible to roxygen for some reason.
- The comments inside body of the functions starting with `#'` were
  interpreted by roxygen. Changed that to `##`.
Issue #2

If the new RoxygenNote field is not present, the DESCRIPTION file gets
regenerated when documentation is generated (`make docs`). This removes
all the comments in the file.
Issue #2

`make clean build` command didn't work with the previous for of the
Makefile.
Closes #4
This should prevent problems with old jars left in this directory after
the version of the dependency in a subproject is changed.
…topts support to handle named input arguments
removing leading '.' causing troubles
changing R package installation command
improving scripts by handling missing parameters properly
adding eclipse .project resource to ingnored files
`./scripts/build/artifactory.sh`

in root project directory. It creates both package and `latest.txt` files to be uploaded to artifactory.
`latest.txt` file contains name of the current module and its main purpose is to serve as pointer to the latest module version in artifactory.
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't you write "package" instead of "module"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@@ -0,0 +1,81 @@
#!/bin/bash
# To be executed from the root project directory.
# Obtains R package from remote artifactory and executes local.sh script with package downloaded to temporary directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove all references to Artifactory in this code (comments, variable names etc.) since the code is more generic - it just retrieves files accessible through HTTP protocol. It doesn't matter if it's Artifactory or some other HTTP-supporting service. I would use a description like "repository" or "artifact repo" or something like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I went with "repository".

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@mkobos
Copy link
Contributor

mkobos commented Feb 18, 2016

I analyzed the dependencies of the build and deploy scripts and here are some general reflections.

The deployment-related scripts should be separate from R package's code (and probably placed in a separate source code project) because they don't need much knowledge about the package itself, details follow below.

  1. Scripts to upload the built package to the artifacts repository. These scripts should be only interested in taking a certain file and putting it in artifacts repository (from the scripts' POV, it doesn't matter what the file contains). Note that taking into consideration the way we currently use Artifactory and limitations that it forces upon us (e.g., we're not able to list files in given directory), it would be easier to simply treat a certain shared NFS directory as the repository. Functionalities of the scripts follow.
    1. Uploading the package. In the current approach, this would mean taking a .tar.gz package, generating latest.txt file for it and uploading it to Artifact. Note that the functionality uploading the package to Artifactory is not needed by Jenkins machine because it has it implemented already.
    2. Downloading the package. This is something that deployment/artifactory.sh script does if you skip one of the last lines that calls deployment/local.sh script.
    3. Listing available packages. This is not implemented. It's a nice-to-have feature but not really essential as for now. Apparently this functionality is not easy to implement when you're using Artifactory as the artifacts repository. It would be easy to do if you used a shared NFS directory.
  2. Scripts for deploying package. These scripts are only interested in taking a local R package file with shiny app inside and deploying it in a Shiny server. The package itself can be any R package as long as it follows a convention that things to be copied to Shiny server's directory are inside of shiny directory of the package. Functionalities of the scripts follow.
    1. Deploy package accessible locally in local Shiny server. This is something that deployment/local.sh script does currently if you skip the part where latest.txt file is handled (because this functionality shouldn't know anything about that file because it's related to accessing artifacts repository and not deploying the application).
    2. Deploy package accessible locally in remote Shiny server. I'm not sure if this functionality is currently needed.
  3. Utility scripts that link scripts listed above to make developer's life easier. Functionalities of the scripts follow.
    1. Deploy package from artifacts repository in local Shiny server. This downloads the package first and then deploys it in local Shiny server.
    2. Deploy package from artifacts repository in remote Shiny server. This is not implemented now. I'm not sure if it's really needed. With this functionality in place it would be probably easier for the developer to deploy the application since he wouldn't have to log into the machine with the Shiny server to deploy the application.

Apart from the deployment-related scripts mentioned above, we have a functionality which is a part of package's build process of generating a file with a git hash. The file should be accessible from within the code of the application.

@mkobos
Copy link
Contributor

mkobos commented Feb 18, 2016

I placed some high-level remarks in #6 (comment).

@mkobos mkobos assigned marekhorst and unassigned mkobos Feb 18, 2016
@marekhorst
Copy link
Member Author

I've just pushed commits with all the fixes mentioned in comments.

As for the general reflections: we should discuss it at the next meeting.

@marekhorst marekhorst assigned mkobos and unassigned marekhorst Mar 4, 2016
@mkobos
Copy link
Contributor

mkobos commented Mar 4, 2016

I've just pushed commits with all the fixes mentioned in comments.

As for the general reflections: we should discuss it at the next meeting.

As we talked in-person, the task of preparing the deployment code can wait because it became a rather low-priority one.

@mkobos mkobos assigned marekhorst and unassigned mkobos Mar 4, 2016
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