Modified mssql_conf.py for SQL on Linux HA Ansible playbook#370
Merged
uc-msft merged 1 commit intomicrosoft:masterfrom Feb 23, 2018
clayton006:master
Merged
Modified mssql_conf.py for SQL on Linux HA Ansible playbook#370uc-msft merged 1 commit intomicrosoft:masterfrom clayton006:master
uc-msft merged 1 commit intomicrosoft:masterfrom
clayton006:master
Conversation
arsing
reviewed
Feb 22, 2018
| msconfigfile.close() | ||
| if msconfigfileresult is None: | ||
| setup_env = os.environ.copy() | ||
| subprocess.check_call( |
Member
There was a problem hiding this comment.
Make this common with the case below.
need_to_set_up = True
if os.path.isfile(...):
if re.search(...) is not None:
need_to_set_up = False
if need_to_set_up:
subprocess.check_call(...)
arsing
reviewed
Feb 22, 2018
| if setup_sa_password is not None: | ||
| if os.path.isfile('/var/opt/mssql/mssql.conf'): | ||
| changed = False | ||
| msconfigfile = open('/var/opt/mssql/mssql.conf', 'r') |
Member
There was a problem hiding this comment.
Use with instead of manual close()
arsing
reviewed
Feb 22, 2018
| if setup_sa_password is not None: | ||
| if os.path.isfile('/var/opt/mssql/mssql.conf'): | ||
| changed = False | ||
| msconfigfile = open('/var/opt/mssql/mssql.conf', 'r') |
arsing
reviewed
Feb 22, 2018
| if os.path.isfile('/var/opt/mssql/mssql.conf'): | ||
| changed = False | ||
| msconfigfile = open('/var/opt/mssql/mssql.conf', 'r') | ||
| msconfigfiledata = msconfigfile.read() |
Contributor
Author
|
I'll review your feedback, make changes and re test. |
arsing
reviewed
Feb 22, 2018
|
|
||
| changed = True | ||
|
|
||
arsing
reviewed
Feb 22, 2018
| if os.path.isfile('/var/opt/mssql/mssql.conf'): | ||
| changed = False | ||
| else: | ||
| with open ("/var/opt/mssql/mssql.conf", "r") as mssql_conf_file: |
Member
|
LGTM with the two whitespace issues fixed. |
…s file is now created by default in SQL Server 2017 CU4 on Linux. Before CU4 this file was created after "/opt/mssql/bin/mssql-conf setup" was executed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I modified the mssql_conf.py script by altering the detection logic for the /var/opt/mssql/mssql.conf file as this file is now created by default in SQL Server 2017 CU4 on Linux. Before CU4 this file was created after "/opt/mssql/bin/mssql-conf setup" was executed. The changes employ a regex to scan for the word "accepteula" as this would signify that the "/opt/mssql/bin/mssql-conf setup" has been executed successfully at one point.
I have tested these changes and the playbook now executes successfully.