Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow json comments #415

Merged
merged 9 commits into from
Jun 24, 2021
9 changes: 9 additions & 0 deletions samples/conf.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"title": "Script server",
"port": "8000",
// Enable SSL
"ssl": {
"key_path": "./script-server.key",
"cert_path": "./script-server.crt"
}
}
3 changes: 2 additions & 1 deletion samples/configs/write_file.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
// Basic script that writes to a file
"name": "Write to file",
"script_path": "scripts/write_file.sh",
"description": "This script does very simple change, putting \"I'm called\" into user home simple.txt file",
Expand Down Expand Up @@ -40,4 +41,4 @@
"description": "Custom filename"
}
]
}
}
4 changes: 2 additions & 2 deletions src/auth/auth_ldap.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from auth import auth_base
from model import model_helper
from utils import file_utils
from utils import file_utils, custom_json
from utils.string_utils import strip

KNOWN_REJECTIONS = [
Expand Down Expand Up @@ -222,7 +222,7 @@ def _load_groups(self, groups_file):
return {}

content = file_utils.read_file(groups_file)
return json.loads(content)
return custom_json.loads(content)

def _set_user_groups(self, user, groups):
self._user_groups[user] = groups
Expand Down
8 changes: 4 additions & 4 deletions src/config/config_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from model import script_config
from model.model_helper import InvalidFileException
from model.script_config import get_sorted_config
from utils import os_utils, file_utils, process_utils
from utils import os_utils, file_utils, process_utils, custom_json
from utils.file_utils import to_filename
from utils.string_utils import is_blank, strip

Expand Down Expand Up @@ -170,7 +170,7 @@ def list_configs(self, user, mode=None):

def load_script(path, content):
try:
json_object = json.loads(content)
json_object = custom_json.loads(content)
brunomgalmeida marked this conversation as resolved.
Show resolved Hide resolved
short_config = script_config.read_short(path, json_object)

if short_config is None:
Expand Down Expand Up @@ -231,7 +231,7 @@ def _visit_script_configs(self, visitor):
def _find_config(self, name) -> Union[ConfigSearchResult, None]:
def find_and_load(path, content):
try:
json_object = json.loads(content)
json_object = custom_json.loads(content)
short_config = script_config.read_short(path, json_object)

if short_config is None:
Expand All @@ -254,7 +254,7 @@ def find_and_load(path, content):
@staticmethod
def _load_script_config(path, content_or_json_dict, user, parameter_values, skip_invalid_parameters):
if isinstance(content_or_json_dict, str):
json_object = json.loads(content_or_json_dict)
json_object = custom_json.loads(content_or_json_dict)
else:
json_object = content_or_json_dict
config = script_config.ConfigModel(
Expand Down
5 changes: 3 additions & 2 deletions src/migrations/migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from utils import file_utils
from utils.date_utils import sec_to_datetime, to_millis
from utils.string_utils import is_blank
import utils.custom_json as custom_json

__migrations_registry = OrderedDict()

Expand Down Expand Up @@ -207,7 +208,7 @@ def __introduce_access_config(context):
return

content = file_utils.read_file(file_path)
json_object = json.loads(content, object_pairs_hook=OrderedDict)
json_object = custom_json.loads(content, object_pairs_hook=OrderedDict)
brunomgalmeida marked this conversation as resolved.
Show resolved Hide resolved

def move_to_access(field, parent_object):
if 'access' not in json_object:
Expand Down Expand Up @@ -346,7 +347,7 @@ def _load_runner_files(conf_folder):
for conf_file in conf_files:
content = file_utils.read_file(conf_file)
try:
json_object = json.loads(content, object_pairs_hook=OrderedDict)
json_object = custom_json.loads(content, object_pairs_hook=OrderedDict)
result.append((conf_file, json_object, content))
except Exception:
LOGGER.exception('Failed to load file for migration: ' + conf_file)
Expand Down
4 changes: 2 additions & 2 deletions src/model/script_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
read_str_from_config, replace_auth_vars
from model.parameter_config import ParameterModel
from react.properties import ObservableList, ObservableDict, observable_fields, Property
from utils import file_utils
from utils import file_utils, custom_json
from utils.object_utils import merge_dicts

OUTPUT_FORMAT_TERMINAL = 'terminal'
Expand Down Expand Up @@ -245,7 +245,7 @@ def _path_to_json(self, path):
if os.path.exists(path):
try:
file_content = file_utils.read_file(path)
return json.loads(file_content)
return custom_json.loads(file_content)
except:
LOGGER.exception('Failed to load included file ' + path)
return None
Expand Down
5 changes: 3 additions & 2 deletions src/model/server_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from model.model_helper import read_list, read_int_from_config, read_bool_from_config
from model.trusted_ips import TrustedIpValidator
from utils.string_utils import strip
import utils.custom_json as custom_json

LOGGER = logging.getLogger('server_conf')

Expand Down Expand Up @@ -68,7 +69,7 @@ def from_json(conf_path, temp_folder):

config = ServerConfig()

json_object = json.loads(file_content)
json_object = custom_json.loads(file_content)
brunomgalmeida marked this conversation as resolved.
Show resolved Hide resolved

address = "0.0.0.0"
port = 5000
Expand Down Expand Up @@ -173,7 +174,7 @@ def create_authenticator(auth_object, temp_folder):
else:
raise Exception(auth_type + ' auth is not supported')

authenticator.auth_expiration_days = float(auth_object.get('expiration_days', 30))
authenticator.auth_expiration_days = float(auth_object.get('expiration_days', 30))

authenticator.auth_type = auth_type

Expand Down
4 changes: 2 additions & 2 deletions src/scheduling/schedule_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from scheduling import scheduling_job
from scheduling.schedule_config import read_schedule_config, InvalidScheduleException
from scheduling.scheduling_job import SchedulingJob
from utils import file_utils, date_utils
from utils import file_utils, date_utils, custom_json

SCRIPT_NAME_KEY = 'script_name'
USER_KEY = 'user'
Expand All @@ -35,7 +35,7 @@ def restore_jobs(schedules_folder):
for file in files:
try:
content = file_utils.read_file(os.path.join(schedules_folder, file))
job_json = json.loads(content)
job_json = custom_json.loads(content)
ids.append(job_json['id'])

job = scheduling_job.from_dict(job_json)
Expand Down
4 changes: 2 additions & 2 deletions src/tests/config_service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from model.model_helper import InvalidFileException
from tests import test_utils
from tests.test_utils import AnyUserAuthorizer
from utils import file_utils
from utils import file_utils, custom_json
from utils.audit_utils import AUTH_USERNAME
from utils.file_utils import is_executable
from utils.string_utils import is_blank
Expand Down Expand Up @@ -800,7 +800,7 @@ def _validate_config(test_case, expected_filename, expected_body):
all_paths = str(os.listdir(configs_path))
test_case.assertTrue(os.path.exists(path), 'Failed to find path ' + path + '. Existing paths: ' + all_paths)

actual_body = json.loads(file_utils.read_file(path))
actual_body = custom_json.loads(file_utils.read_file(path))
test_case.assertEqual(expected_body, actual_body)


Expand Down
28 changes: 27 additions & 1 deletion src/tests/script_config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from react.properties import ObservableDict, ObservableList
from tests import test_utils
from tests.test_utils import create_script_param_config, create_parameter_model, create_files
from utils import file_utils
from utils import file_utils, custom_json

DEF_AUDIT_NAME = '127.0.0.1'
DEF_USERNAME = 'user1'
Expand Down Expand Up @@ -854,6 +854,32 @@ def test_get_sorted_with_parameters(self):
self.assertEqual(expected, config)


def test_json_comments(self):
config = get_sorted_config(custom_json.loads(
"""{
// Comment 1
"parameters": [
// Comment 2
{"name": "param2", "description": "desc 1"},
{"type": "int", "name": "paramA"},
{"default": "false", "name": "param1", "no_value": true}
],
// Comment 3
"name": "Conf X"
}""")
)

expected = OrderedDict([
('name', 'Conf X'),
('parameters', [
OrderedDict([('name', 'param2'), ('description', 'desc 1')]),
OrderedDict([('name', 'paramA'), ('type', 'int')]),
OrderedDict([('name', 'param1'), ('no_value', True), ('default', 'false')])
]),
])
self.assertEqual(expected, config)


class SchedulableConfigTest(unittest.TestCase):
def test_create_with_schedulable_false(self):
config_model = _create_config_model('some-name', config={
Expand Down
11 changes: 10 additions & 1 deletion src/tests/server_conf_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from model.model_helper import InvalidValueException
from model.server_conf import _prepare_allowed_users
from tests import test_utils
from utils import file_utils
from utils import file_utils, custom_json


class TestPrepareAllowedUsers(unittest.TestCase):
Expand Down Expand Up @@ -233,6 +233,15 @@ def test_enable_script_titles_default(self):
config = _from_json({})
self.assertIs(True, config.enable_script_titles)

def test_comments_json(self):
config = _from_json(
custom_json.loads("""
{
// "title": "my server"
"title": "my server2"
}""")
)
self.assertEqual('my server2', config.title)

class TestAuthConfig(unittest.TestCase):
def test_google_oauth(self):
Expand Down
9 changes: 9 additions & 0 deletions src/utils/custom_json.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import json
import re

def loads (content):
contents=''
for line in content.split('\n'):
if not re.match (r'\s*//.*', line):
contents += line + "\n"
return json.loads(contents)
4 changes: 2 additions & 2 deletions src/web/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from model.server_conf import ServerConfig, XSRF_PROTECTION_TOKEN, XSRF_PROTECTION_DISABLED, XSRF_PROTECTION_HEADER
from scheduling.schedule_service import ScheduleService, UnavailableScriptException, InvalidScheduleException
from utils import file_utils as file_utils
from utils import tornado_utils, os_utils, env_utils
from utils import tornado_utils, os_utils, env_utils, custom_json
from utils.audit_utils import get_audit_name_from_request
from utils.exceptions.missing_arg_exception import MissingArgumentException
from utils.exceptions.not_found_exception import NotFoundException
Expand Down Expand Up @@ -713,7 +713,7 @@ def post(self, user):
for key, value in self.form_reader.files.items():
parameter_values[key] = value.path

schedule_config = json.loads(parameter_values['__schedule_config'])
schedule_config = custom_json.loads(parameter_values['__schedule_config'])
del parameter_values['__schedule_config']

try:
Expand Down