generated from finos-labs/project-blueprint
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Log warning when validating a spec against the wrong core schema versions #800
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import { checkCoreSchemaVersion } from './bundled-schemas'; | ||
|
||
jest.mock('./helper.js', () => { | ||
return { | ||
initLogger: () => { | ||
return { | ||
info: jest.fn(), | ||
debug: jest.fn(), | ||
error: jest.fn(), | ||
warn: jest.fn() | ||
}; | ||
} | ||
}; | ||
}); | ||
|
||
describe('checkCoreSchemaVersion', () => { | ||
it('Return true for non-core schema URLs', () => { | ||
expect(checkCoreSchemaVersion('https://not-calm.org/spec.json', '2024-10', false)) | ||
.toBeTruthy(); | ||
}); | ||
|
||
it('Return true for core schema URLs matching loaded version', () => { | ||
expect(checkCoreSchemaVersion('https://calm.finos.org/calm/draft/2024-10/meta/core.json', '2024-10', false)) | ||
.toBeTruthy(); | ||
}); | ||
|
||
it('Return false for core schema URLs that dont match loaded version', () => { | ||
expect(checkCoreSchemaVersion('https://calm.finos.org/calm/draft/2020-01/meta/core.json', '2024-10', false)) | ||
.toBeFalsy(); | ||
}); | ||
}); |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import { readdirSync } from 'fs'; | ||
import { CALM_META_SCHEMA_DIRECTORY } from '../consts'; | ||
import { initLogger } from './helper'; | ||
|
||
const calmSchemaRegex = new RegExp('https://.*/draft/([0-9-]+)/meta/.*'); | ||
|
||
export function getBundledSchemaVersion(debug: boolean): string { | ||
const logger = initLogger(debug); | ||
try { | ||
const files = readdirSync(CALM_META_SCHEMA_DIRECTORY); | ||
if (files.length > 0) { | ||
return files[0]; | ||
} | ||
} | ||
catch (_) { | ||
logger.warn('Error looking up bundled CALM schemas. Disabling checks for incorrect core schema versions.'); | ||
return null; | ||
} | ||
logger.warn('Did not find bundled CALM schemas. Disabling checks for incorrect core schema versions.'); | ||
return null; | ||
} | ||
|
||
/** | ||
* Check whether a Schema URL is a core CALM schema, and if it is, log a warning if it doesn't match the bundled version. | ||
* @param uri The schema URI to check | ||
* @param bundledVersion The CALM schema version that was loaded | ||
* @param debug Whether to log debug info | ||
* @returns true if the schema version is valid, false otherwise | ||
*/ | ||
export function checkCoreSchemaVersion(uri: string, bundledVersion: string, debug: boolean) { | ||
const logger = initLogger(debug); | ||
|
||
if (bundledVersion === null) { | ||
return true; | ||
} | ||
|
||
const matches = calmSchemaRegex.exec(uri); | ||
|
||
if (!matches || matches.length < 2) { | ||
// not a CALM core schema | ||
return true; | ||
} | ||
|
||
const requestedCoreSchemaVersion = matches[1]; | ||
if (requestedCoreSchemaVersion === bundledVersion) { | ||
return true; | ||
} | ||
|
||
const warningMessage = `WARNING: attempting to load a core CALM schema with a version (${requestedCoreSchemaVersion}) that was not bundled with the CALM CLI. ` + | ||
'This may produce unexpected errors. ' + | ||
`The bundled version is ${bundledVersion}. `; | ||
logger.warn(warningMessage); | ||
return false; | ||
} |
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would this work if organisations want to extend the core calm schema but add their own spectral rules for their extensions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you mean if there is effectively a second set of core schemas - the extensions offered by an org - and I guess they would need versioning too?
I guess what I have here is a pretty narrow definition of a 'core schema'.
The reason we embarked on this was that Jim was doing a validate and getting a ton of weird errors because he was using something still pointing at the first release.
What would you say to instead having it just log a warning if you do validation against a spec not found/registered in the schema directory, instead of all this weirdness with 'core schemas'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would make a lot more sense to me. One scenario I was also concerned about (although more a future roadmap end state ) is if I am referencing someone else's CALM spec for node information do I care if its not on the same version of core version anyways. I'm not sure we would want to mandate it, but also not sure what the implications would be on our current tooling. It's kind of why on this comment I was indicating that maybe the tooling goes via a defined model but then that also implies every piece of tooling is extendable or its extendable by folks supplying the necessary "my calm schema -> tooling model" interpreters as part of the cli setup.