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

Fix Device Simulation setup script #425

Merged
merged 11 commits into from
Nov 15, 2018
Merged

Conversation

dluc
Copy link
Member

@dluc dluc commented Nov 14, 2018

Description and Motivation

  1. fix scripts to work with other shells other than Bash
  2. improve some common errors detection
  3. sync scripts in the 2 solutions, with and without hub

Change type

  • Bug fix
  • New feature
  • Enhancement
  • Breaking change (breaks backward compatibility)

Checklist:

  • All tests passed
  • The code follows the code style and conventions of this project
  • The change requires a change to the documentation
  • I have updated the documentation accordingly

This change is Reviewable

@dluc dluc requested a review from a team November 14, 2018 00:53
@dluc
Copy link
Member Author

dluc commented Nov 15, 2018

Scripts tested and working fine. The only issue I noticed is a missing "testing" tag in the docker-compose when deploying the branch with CLI, unrelated to this PR

@dluc dluc changed the title Fix Device Simulation setup script DON'T MERGE - Fix Device Simulation setup script Nov 15, 2018
Copy link
Contributor

@ppathan ppathan left a comment

Choose a reason for hiding this comment

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

Should these improvements be applied to RM as well? I'm talking in addition to setupwrapper changes

Copy link
Contributor

@hathind-ms hathind-ms left a comment

Choose a reason for hiding this comment

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

Please see comment on te

@hathind-ms hathind-ms changed the title DON'T MERGE - Fix Device Simulation setup script Fix Device Simulation setup script Nov 15, 2018
@hathind-ms hathind-ms merged commit 0a87ed0 into master Nov 15, 2018
@hathind-ms hathind-ms deleted the 2018-11-13-fixDSscripts branch November 15, 2018 21:12
@dluc
Copy link
Member Author

dluc commented Nov 15, 2018

@ppathan yes, these 2 PRs contain improvements/fixes that we should apply to other solutions:

  1. Copy the new setup-wrapper.sh and change it to point to the relevant GitHub URL, note how it doesn't use set -ex to avoid leaking secrets
  2. In the ARM template invoke setup-wrapper.sh (instead of setup.sh) and use bash instead of sh
  3. From setup.sh take the retry logic used to install Docker and other minor improvements about error detection

Note that the setup now creates a log file under /app

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.

3 participants