Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

settings.json in document directory #712

Closed
mitchellspryn opened this issue Dec 28, 2017 · 4 comments
Closed

settings.json in document directory #712

mitchellspryn opened this issue Dec 28, 2017 · 4 comments

Comments

@mitchellspryn
Copy link
Contributor

Currently, AirSim reads the settings.json from the "my documents" folder on windows, and the user's documents folder in Linux. This creates an issue when attempting to run AirSim in Azure under a service account that does not have access to these folders. For example, when running AirSim with BatchAI, the jobs are run under the _azurebatch service account that does not have write access to anywhere on the disk (so the settings.json cannot be generated automatically during the job, and needs to be manually populated on every machine in the cluster :( ).

I have two proposals:

  1. Allow the user to specify settings.json on the command line
  2. First look in the user's documents folder. Then, if the settings.json is not found, look in the directory of the executable. Then throw error if not found in either of those places.

Of these, I'd prefer #2. If @sytelus approves, I can work on implementation.

@sytelus
Copy link
Contributor

sytelus commented Dec 29, 2017

That's great point. I think we should do both of them. So the settings.json would be used in following order of priority:

  1. Use one from command line if specified
  2. Use one in executable folder if found
  3. Use from document folder

This would enable scenarios where you are spawning same executable but different command lines OR different executables deployed with its own settings so no command line is needed.

Please let me know if you would like to go ahead and put this change in.

@mitchellspryn
Copy link
Contributor Author

Cool. I'll look at implementing this.

@mitchellspryn
Copy link
Contributor Author

The PR is up.

@mitchellspryn
Copy link
Contributor Author

mitchellspryn commented Dec 30, 2017

The PR is merged. Thanks @sytelus! Closing issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants