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

[enhancement]: Remove jsonConfig schema validation and move it to adapter-check #2421

Open
1 task done
klein0r opened this issue Apr 2, 2024 · 2 comments
Open
1 task done

Comments

@klein0r
Copy link
Contributor

klein0r commented Apr 2, 2024

No existing issues.

  • There is no existing issue for my request.

Related problems

The schema validation creates a lot of issues and warnings for end users. Should be removed from the admin adapter.

async validateJsonConfig(adapterName) {
let schema;
try {
const schemaRes = await axios.get(this.JSON_CONFIG_SCHEMA_URL);
schema = schemaRes.data;
} catch (e) {
this.adapter.log.debug(`Could not get jsonConfig schema: ${e.message}`);
return;
}
const res = await this.adapter.getForeignObjectAsync(`system.adapter.${adapterName}`);
if (res?.common.adminUI?.config === 'json') {
try {
const ajv = new Ajv({ allErrors: false, strict: false });
const adapterPath = path.dirname(require.resolve(`iobroker.${adapterName}/package.json`));
const jsonConfPath = path.join(adapterPath, 'admin', 'jsonConfig.json');
const json5ConfPath = path.join(adapterPath, 'admin', 'jsonConfig.json5');
let jsonConf;
if (fs.existsSync(jsonConfPath)) {
jsonConf = fs.readFileSync(jsonConfPath, {
encoding: 'utf-8',
});
} else {
jsonConf = fs.readFileSync(json5ConfPath, {
encoding: 'utf-8',
});
}
const validate = ajv.compile(schema);
const valid = validate(JSON5.parse(jsonConf));
if (!valid) {
this.adapter.log.warn(
`${adapterName} has an invalid jsonConfig: ${JSON.stringify(validate.errors)}`
);
}
} catch (e) {
this.adapter.log.debug(`Error validating schema of ${adapterName}: ${e.message}`);
}
}
}

Description

Should be checked by https://adapter-check.iobroker.in

Additional context

No response

@mcm1957
Copy link
Contributor

mcm1957 commented Apr 2, 2024

I disagree

Most developers do not regularly check their adapters before the release a new version, Due to many false positive situation this could not be listed as error either So this check will likely be ignored (like several others :-( )

I will consider adding the check to the repochecker but it conflicts with current implementation as a useful output cannot be concentrated into one line ouf output. And the checker is designed to return an array of messagelines wihich in turn are formatted by the cli, by website (iobroker.dev) or scripts at repository checker. So not sure if an usefull implmentation could be done in a simple way.

Currently this solution is working fine, so I do not see a big benefit to revert it. OIn addition if a user claims that something is not workiung, the jsonConfig error would be visible in log togetehr with the problem.

But of course - we can discuss everything...

@klein0r
Copy link
Contributor Author

klein0r commented Apr 2, 2024

Most developers do not regularly check their adapters before the release a new version

That is another problem - but (in my opinion) not a valid reason to face end users with cryptic warnings in their logs. If we raise a warning for everything which is not developed 100% correctly, we spam the logs with unnecessary information (like missing translations).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants