Skip to content

Fix #5248 - Typo in Schematron extension: .sch, not .sct! #5333

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.

Sign up for GitHub

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

Jump to bottom
Merged
jmarrec merged 2 commits into from
Dec 23, 2024
Merged

Fix #5248 - Typo in Schematron extension: .sch, not .sct! #5333

jmarrec merged 2 commits into from
Dec 23, 2024

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Dec 19, 2024

Pull request overview

Pull Request Author

  • All new and existing tests passes

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • Code Style, strip trailing whitespace, etc.
  • All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
  • Labeling is ok
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified

@jmarrec jmarrec added component - Utilities Other Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Dec 19, 2024
@jmarrec jmarrec requested a review from shorowit December 19, 2024 15:15
@jmarrec jmarrec self-assigned this Dec 19, 2024
@shorowit shorowit mentioned this pull request Dec 19, 2024
Merged
7 tasks
@shorowit
Copy link
Contributor

shorowit commented Dec 19, 2024

@jmarrec I'm testing it out and overall it's working well except for one thing. One of our OS-HPXML tests is to verify that our schematron file itself is valid relative to its schema. This test now fails with:

/srv/jenkins/openstudio/docker-volumes/ubuntu-2404/Openstudio/src/utilities/xml/XMLValidator.cpp@271 : XML path extension '.sch' not supported.

Can we at least allow .sch file extensions for XSD schema validation too? Or maybe we shouldn't bother to check for the file extension at all since there can be other file extensions for XML files.

@jmarrec
Copy link
Collaborator Author

jmarrec commented Dec 19, 2024 •
edited
Loading

@shorowit your error says you tried to call XMLValidator::validate("something.sch") but the link you shared is .xsd? which?

Also, couldn't you do that validation with whatever ruby or python external lib is doing?

OpenStudio/src/utilities/xml/XMLValidator.cpp

Lines 251 to 272 in 2d4cbea

bool XMLValidator::validate(const openstudio::path& xmlPath) {
reset();
if (!openstudio::filesystem::exists(xmlPath)) {
std::string logMessage = fmt::format("XML File '{}' does not exist.", toString(xmlPath));
m_logMessages.emplace_back(Fatal, "openstudio.XMLValidator", logMessage);
LOG_AND_THROW(logMessage);
} else if (!openstudio::filesystem::is_regular_file(xmlPath)) {
std::string logMessage = fmt::format("XML File '{}' cannot be opened.", toString(xmlPath));
m_logMessages.emplace_back(Fatal, "openstudio.XMLValidator", logMessage);
LOG_AND_THROW(logMessage);
}
if (xmlPath.extension() == ".xml") {
auto t_xmlPath = openstudio::filesystem::system_complete(xmlPath);
m_xmlPath = t_xmlPath;
} else {
std::string logMessage = fmt::format("XML path extension '{}' not supported.", toString(xmlPath.extension()));
m_logMessages.emplace_back(Fatal, "openstudio.XMLValidator", logMessage);
LOG_AND_THROW(logMessage);
}

Maybe we should log warn when the extension of the file to validate is not XML and let it go to hell if it's not actually an xml file...

@shorowit
Copy link
Contributor

shorowit commented Dec 19, 2024

@shorowit your error says you tried to call XMLValidator::validate("something.sch") but the link you shared is .xsd? which?

We are trying to validating something.sch (the schematron XML) against its XSD schema. Sorry, I could have linked to the schematron file too, it's here.

Maybe we should log warn when the extension of the file to validate is not XML and let it go to hell if it's not actually an xml file...

That'd be fine by me.

@ci-commercialbuildings
Copy link
Collaborator

ci-commercialbuildings commented Dec 20, 2024 •
edited
Loading

CI Results for 940a26a:

@shorowit
Copy link
Contributor

shorowit commented Dec 20, 2024

@jmarrec Happy to test the latest commit if an Ubuntu build gets posted. Just seeing a single build for Windows right now.

@jmarrec
Copy link
Collaborator Author

jmarrec commented Dec 23, 2024

@wenyikuang FYI, I just rebooted the unix (2 ubuntus and the macos) with a temp cbci_jenkins_lib branch that will wipe the build dirs. Hopefully that'll solve a couple of weird build failures we're seeing on all PRs right now.

@jmarrec
Copy link
Collaborator Author

jmarrec commented Dec 23, 2024

the locateOrCreateBinaryComment is malfunctioning for Ubuntu 24.04 at least. But anyways, wiping the build dir helped and I added the 24.04 deb / tar.gz links above @shorowit so you can test.

@jmarrec jmarrec changed the title Fix #5248 - Typo in Schematorn extension: .sch, not .sct! Fix #5248 - Typo in Schematron extension: .sch, not .sct! Dec 23, 2024
shorowit
shorowit approved these changes Dec 23, 2024
View reviewed changes
Copy link
Contributor

@shorowit shorowit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that it works in OS-HPXML, thanks!

@jmarrec jmarrec merged commit 0314c3e into develop Dec 23, 2024
4 of 6 checks passed
@jmarrec jmarrec deleted the 5248-SchematronExtension branch December 23, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shorowit shorowit shorowit approved these changes

Assignees

@jmarrec jmarrec

Labels

component - Utilities Other Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow .sch extensions for schematron documents

4 participants

@jmarrec @shorowit @ci-commercialbuildings