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 SAP Hana Module #3017

Closed

Conversation

Thomas-Grimmeisen
Copy link

Hello,

I created a module, which is able to correctly use the SAP HANA Express Edition Docker Image (https://hub.docker.com/_/sap-hana-express-edition/plans/f2dc436a-d851-4c22-a2ba-9de07db7a9ac?tab=instructions).

According to the checklist all requirements are fullfilled.

Default docker image
• [y] Should be a Docker Hub official image, or published by a reputable source (ideally the company or organisation that officially supports the technology)
• [n] Should have a verifiable open source Dockerfile and a way to view the history of changes
• [y] MUST show general good practices regarding container image tagging - e.g. we do not use latest tags, and we do not use tags that may be mutated in the future
• [y] MUST be legal for Testcontainers developers and Testcontainers users to pull and use. Mechanisms exist to allow EULA acceptance to be signalled, but images that can be used without a licence are greatly preferred.
Module dependencies
• [y] The module should use as few dependencies as possible,
• [y] Regarding libraries, either:
• they should be compileOnly if they are likely to already be on the classpath for users' tests (e.g. client libraries or drivers)
• they can be implementation (and thus transitive dependencies) if they are very unlikely to conflict with users' dependencies.
• [y] If client libraries are used to test or use the module, these MUST be legal for Testcontainers developers and Testcontainers users to download and use.
API (e.g. MyModuleContainer class)
• [y] Favour doing the right thing, and least surprising thing, by default
• [y] Ensure that method and parameter names are easy to understand. Many users will ignore documentation, so IDE-based substitutes (autocompletion and Javadocs) should be intuitive.
• [y] The module's public API should only handle standard JDK data types and MUST not expose data types that come from compileOnly dependencies. This is to reduce the risk of compatibility problems with future versions of third party libraries.
Documentation
• [y] Every module MUST have a dedicated documentation page containing:
• [y] A high level overview
• [y] A usage example
• [y] If appropriate, basic API documentation or further usage guidelines
• [y] Dependency information
• [y] Acknowledgements, if appropriate
• [y] Consider that many users will not read the documentation pages - even if the first person to add it to a project does, people reading/updating the code in the future may not. Try and avoid the need for critical knowledge that is only present in documentation.

@Thomas-Grimmeisen
Copy link
Author

Hello folks,
@bsideup @kiview @rnorth
I can see that some checks still did not run. Do I have to take any action to execute them?

@alex-alvarezg
Copy link

Any news when this will be reviewed?

@rnorth
Copy link
Member

rnorth commented Sep 24, 2020

@alex-alvarezg @Thomas-Grimmeisen I'm afraid we can't commit to a timeframe for reviewing - as you can see, we have quite a backlog or PRs, and work on this in our personal time. Sorry!

@jakobbraun jakobbraun mentioned this pull request Dec 2, 2020
@bsideup
Copy link
Member

bsideup commented Dec 2, 2020

@Thomas-Grimmeisen could you please clarify this:

SAP HANA Express Edition does only work on Linux environments. It does not support Windows or Mac OS

Does it mean that SAP HANA does not work on Windows/Mac even if started as a (linux) container with Docker?

@bsideup
Copy link
Member

bsideup commented Dec 2, 2020

Also, I see that you represent SAP. Given the licensing restrictions and the need to use a private (Docker Store) image, would it make sense to host this module in your org instead?

The only difference is that instead of using org.testcontainers:hana, the users will be adding a dependency on com.sap.db:testcontainers-hana, but it would allow you to control the CI infrastructure for it, as well as the licensing questions.

@alex-alvarezg
Copy link

@Thomas-Grimmeisen could you please clarify this:

SAP HANA Express Edition does only work on Linux environments. It does not support Windows or Mac OS

Does it mean that SAP HANA does not work on Windows/Mac even if started as a (linux) container with Docker?

I'm running it on Mac Dockerized without issues.

@Thomas-Grimmeisen
Copy link
Author

@bsideup I do not represent SAP for this topic. I just decided to contribute the module here since it could be useful for people using HANA Systems. Regarding the licensing: I basically copied the mechanism used for accepting the License of mssqlserver which is part of this repository. I don't see any difficulties about that.

@Thomas-Grimmeisen could you please clarify this:

SAP HANA Express Edition does only work on Linux environments. It does not support Windows or Mac OS

Does it mean that SAP HANA does not work on Windows/Mac even if started as a (linux) container with Docker?

Yes it is not supported to run HANA Express from Windows/Mac according to the documentation of the image (see overview section https://hub.docker.com/_/sap-hana-express-edition/plans/f2dc436a-d851-4c22-a2ba-9de07db7a9ac?tab=instructions ). On my Windows machine it does not work because of incompatibilities of mounted volumes.

@stale
Copy link

stale bot commented Jun 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

@eddumelendez
Copy link
Member

Thanks for the PR @Thomas-Grimmeisen ! But as mentioned by @bsideup, given the licensing and usage of Docker Store. We have decided to do not merge it.

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.

None yet

5 participants