Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[MXNET-606] Fix R installation in CI #11761

Merged
merged 1 commit into from
Jul 15, 2018
Merged

Conversation

KellenSunderland
Copy link
Contributor

Description

This patch fixes a regression in the R installation in CI.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Relates to: #11601 #11602

Comments

Without manually adding the key we're getting occasional gpg server errors (happens approx 1/10 requests). My understanding is this change was made to align with documentation. Would like to chat about the tradeoffs with @aaronmarkham and @anirudh2290, but it's important that we keep the CI stable for other MXNet developers. Is the main concern with adding the key manually that it doesn't align with documentation? Would you be alright with me updating the doc instructions (I can include updated instructions in this PR).

This patch fixes a regression in the R installation in CI.
@nswamy nswamy merged commit 01cba7b into apache:master Jul 15, 2018
@nswamy
Copy link
Member

nswamy commented Jul 15, 2018

@anirudhacharya

@aaronmarkham
Copy link
Contributor

I recently had to change my docs setup script to use the r.gpg file as well:
#11670 (comment)

I'm curious why it took a few days for this to surface in CI. I was thinking that since it was working there, that I should use how it fetched from the keyserver... and assumed that maybe that keyserver was somehow less flaky than the one for sbt, which was already setup to use a local file.

I don't know the tradeoffs here for local file vs keyserver. At this point, I'm happy that it works with the local file. Is there a security concern?

XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this pull request Aug 29, 2018
This patch fixes a regression in the R installation in CI.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants