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

Fix for flaky keyservers #11601 #11602

Merged
merged 1 commit into from
Jul 11, 2018
Merged

Fix for flaky keyservers #11601 #11602

merged 1 commit into from
Jul 11, 2018

Conversation

larroy
Copy link
Contributor

@larroy larroy commented Jul 7, 2018 •
edited
Loading

Description

See title. Fixes #11601

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

marcoabreu
marcoabreu reviewed Jul 7, 2018
View reviewed changes
Copy link
Contributor

@marcoabreu marcoabreu left a comment

Choose a reason for hiding this comment

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

Would it make sense to chain them with ||?

@larroy larroy changed the title Fix for flaky keyserver keyserver.ubuntu.com #11601 Fix for flaky keyservers #11601 Jul 7, 2018
@larroy larroy force-pushed the gpg_keys branch 2 times, most recently from 68a8473 to a05a9d0 Compare July 7, 2018 16:16
KellenSunderland
KellenSunderland suggested changes Jul 8, 2018
View reviewed changes
ci/docker/install/export_gpg_keys.sh
# specific language governing permissions and limitations
# under the License.

gpg --keyserver keyserver.ubuntu.com --recv 2EE0EA64E40A89B84B2DF73499E82A75642AC823
Copy link
Contributor

Choose a reason for hiding this comment

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

This command will fail occasionally. I don't think this addresses the problem (unless I'm missing something).

Copy link
Contributor Author

@larroy larroy Jul 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This is executed only manually when needed, they keys are now committed to the repo. See the other files. It will not fail anymore unless the keys change. The keys are now in the filesystem

Copy link
Contributor

Choose a reason for hiding this comment

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

Totally missed that this was checked in! Sorry about that. Seems fine in that case.

KellenSunderland
KellenSunderland reviewed Jul 9, 2018
View reviewed changes
ci/docker/install/ubuntu_r.sh
#gpg --keyserver keyserver.ubuntu.com --recv-key $key || \
# gpg --keyserver keyserver.pgp.com --recv-keys $key || \
# gpg --keyserver ha.pool.sks-keyservers.net --recv-keys $key ;
#gpg -a --export $key | apt-key add -
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 just remove this and add a comment describing what r.gpg is and how to get it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point, I think it's fine to leave that for documentation purposes. We can add a comment, but I think is overkill and expensive to go through another round of PR validation just for that. I can add it on a later change.

KellenSunderland
KellenSunderland reviewed Jul 9, 2018
View reviewed changes
ci/docker/install/ubuntu_scala.sh
@@ -29,7 +30,10 @@ apt-get install -y openjdk-8-jdk
apt-get install -y openjdk-8-jre

echo "deb https://dl.bintray.com/sbt/debian /" | tee -a /etc/apt/sources.list.d/sbt.list
apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 2EE0EA64E40A89B84B2DF73499E82A75642AC823
# ubuntu keyserver is very flaky
#apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 2EE0EA64E40A89B84B2DF73499E82A75642AC823
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@KellenSunderland
Copy link
Contributor

Thanks for addressing this Pedro. This will improve reliability quite a bit.

@larroy
Copy link
Contributor Author

larroy commented Jul 10, 2018

@szha

@larroy
Copy link
Contributor Author

larroy commented Jul 11, 2018

@marcoabreu please merge

@marcoabreu marcoabreu merged commit 4ae0cd9 into apache:master Jul 11, 2018
@larroy
Copy link
Contributor Author

larroy commented Jul 11, 2018

Thank you Marco. We missed you.

@KellenSunderland KellenSunderland mentioned this pull request Jul 15, 2018
Merged
5 tasks
XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this pull request Aug 29, 2018
@larroy @marcoabreu
Fix for flaky keyserver keyserver.ubuntu.com. Fix apache#11601 (apach…
dd7c673 
…e#11602)
@larroy larroy deleted the gpg_keys branch November 15, 2018 18:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marcoabreu marcoabreu marcoabreu left review comments

@KellenSunderland KellenSunderland KellenSunderland approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flaky keyserver keyserver.ubuntu.com
3 participants
@larroy @KellenSunderland @marcoabreu