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

Add support for InterSystems IRIS as an additional database platform #377

Merged
merged 15 commits into from
Sep 30, 2024

Conversation

bdeboe
Copy link
Contributor

@bdeboe bdeboe commented Sep 16, 2024

Hi,

this PR introduces support for the InterSystems IRIS Data Platform to SqlRender. It includes all the required replacement rules to make sure OHDSI tool-generated statements run fine on InterSystems IRIS. In addition, appropriate unit tests and changes to the ref/doc pages are included in this PR.

This PR for SqlRender is the first step of adding InterSystems IRIS support to a broader set of OHDSI tools, as the dialect needs to become part of a SqlRender release in order to be able to submit corresponding PRs for the DatabaseConnector, CommonDataModel, ArachneCommons, WebAPI and Atlas repositories that we have lined up and ready to submit. As such, it would be great if this could make it into the upcoming 1.18.1 release, and we are ready to make any further changes to the PR as required to facilitate approval.

Context: Dr. Qi Li has recently reached out to @schuemie to discuss our work at InterSystems and the OHDSI toolset support effort, which we've done with support of the Oddyseus Data Services team and @alex-odysseus in particular.

@schuemie
Copy link
Member

I think this post was pretty clear: we don't have the resources to support more database platforms.

The HADES workgroup is working on a manifesto for defining the minimum requirements under which we'd consider adding a new platform. The current thinking is:

  1. Must have a test server available to OHDSI developers (e.g. for unit testing)
  2. PRs for SqlRender and DatabaseConnector
  3. Using forks of SqlRender and DatabaseConnector, demonstrate that all HADES packages run without issue

@bdeboe
Copy link
Contributor Author

bdeboe commented Sep 24, 2024

Hi @schuemie ,

thanks for your response. Indeed the post you referenced was clear, and as such we've invested in anticipating these totally reasonable requirements.

This is our PR for SqlRender, as we agreed with @alex-odysseus it would be the logical one to submit first given project interdependencies. We can surely trigger the one from our DatabaseConnector fork right away as you prefer, but rather have this discussion in one place. With the Odysseus Data Services team, we validated the key HADES packages over the past few months and would be happy to share their report, and look into any remaining ones you consider essential.

As for the test server, we've been working with an externally accessible test server we can keep up and maintain for a while to come. As an alternative, we'd like to discuss an approach using docker images of our free tier that can be spun up using GitHub Actions, which we've used for a few other open source / community ventures.

Please let us know if you'd like to set up a conversation or if there's any additional information or PRs we can provide right away.

Thanks,
benjamin

@schuemie schuemie changed the base branch from main to develop September 30, 2024 06:11
@schuemie schuemie merged commit e5463e2 into OHDSI:develop Sep 30, 2024
1 of 4 checks passed
schuemie added a commit that referenced this pull request Sep 30, 2024
@schuemie
Copy link
Member

I've accepted the PR, so you can proceed with showing that everything works. If the other requirements aren't met (test server, showing all HADES packages run on IRIS) support will be removed.

Your PR broke several unit tests that I had to fix. This is not OK. In the future, run and pass all unit tests before submitting a PR.

Also, please target the develop branch.

@schuemie
Copy link
Member

Yes, please e-mail them to me

@schuemie
Copy link
Member

schuemie commented Oct 9, 2024

@bdeboe v1.19.0 has just been released, which includes this PR

@bdeboe
Copy link
Contributor Author

bdeboe commented Oct 9, 2024

That is great news. Thanks @schuemie !
Our IT group is configuring the test server required for the DatabaseConnector submission this week, so I hope to send in the next set of PRs shortly.

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

Successfully merging this pull request may close these issues.

2 participants