Skip to content

Conversation

@mkundu1
Copy link
Contributor

@mkundu1 mkundu1 commented Mar 21, 2022

Demonstrate usage of pytest with a custom docker image that runs a test grpc server. This PR is for figuring out what is needed to run pytest with a docker image. For the Fluent image, following additional items will be needed:

  1. Login id and PAT for ghcr.io.
  2. Fluent license server specification (not sure how this will work in github machines)

Update: Everything is in place to connect to Fluent docker container within github CI.

@mkundu1 mkundu1 changed the title Demonstrate pytest with docker Demonstrate pytest with docker - not for merge Mar 21, 2022
@mkundu1 mkundu1 requested a review from seanpearsonuk March 21, 2022 01:24
@mkundu1 mkundu1 requested a review from dnwillia-work March 21, 2022 01:34
@akaszynski
Copy link
Contributor

Fluent license server specification (not sure how this will work in github machines)

At the moment, we use a private license server implemented in:
https://github.com/pyansys/pymapdl/blob/4822d49d94b8e849a57c25c3f74193fa8032c61a/.ci/start_mapdl.sh#L4

as specified by the GitHub secret:
https://github.com/pyansys/pymapdl/blob/4822d49d94b8e849a57c25c3f74193fa8032c61a/.github/workflows/ci.yml#L127

A few notes about secrets and how we secure the license server IP.

  • Secrets are only available within this repository. This means forks from this repository do not have those secrets, which in term means that only those with write permissions to this repository have the ability to access those secrets.
  • This protects secrets from misuse and abuse as secrets within GitHub can be easily echoed using one of many "hacks" like character shifting, partial echoing, base64 encoding to disk, etc. GitHub CI only protects against direct echoing to logs to avoid log scraping.

@mkundu1 mkundu1 changed the title Demonstrate pytest with docker - not for merge Pytest with Fluent docker - not ready for merge Mar 23, 2022
@mkundu1 mkundu1 force-pushed the testing/docker branch 2 times, most recently from ceadcf0 to 2c900d0 Compare March 23, 2022 08:44
@mkundu1
Copy link
Contributor Author

mkundu1 commented Mar 23, 2022

At the moment, we use a private license server implemented in: https://github.com/pyansys/pymapdl/blob/4822d49d94b8e849a57c25c3f74193fa8032c61a/.ci/start_mapdl.sh#L4

Hi @akaszynski, sorry about my ignorance, what is a private license server and how to set up one? I assume it will be specified by an ip address which is visible by github runners. Is this something we should request IT to set up or is something available at pyansys level?

Thanks for the notes about github secrets.

@mkundu1
Copy link
Contributor Author

mkundu1 commented Mar 23, 2022

Demonstrate usage of pytest with a custom docker image that runs a test grpc server. This PR is for figuring out what is needed to run pytest with a docker image. For the Fluent image, following additional items will be needed:

  1. Login id and PAT for ghcr.io.
  2. Fluent license server specification (not sure how this will work in github machines)

In the latest revision, I'm pulling the Fluent docker image using my github id and PAT. Only the license server configuration is remaining.

@akaszynski
Copy link
Contributor

@mkundu1, licensing server IP sent via private communication.

@mkundu1 mkundu1 changed the title Pytest with Fluent docker - not ready for merge Run pytest with Fluent docker image Mar 23, 2022
@mkundu1 mkundu1 marked this pull request as ready for review March 23, 2022 12:46
@mkundu1
Copy link
Contributor Author

mkundu1 commented Mar 23, 2022

Added the private license server sent by Alex in github secrets. The unittest that connects with Fluent docker image runs fine now in github CI.

@mkundu1 mkundu1 requested a review from akaszynski March 23, 2022 12:52
@akaszynski
Copy link
Contributor

I think we're going to have to rethink the way we do tests, especially starting the container outside of pytest.

While I'm not opposed with doing it this way, this approach assumes that you have docker and the fluent image locally. I think it's preferable to start the fluent service outside of the unit testing suite and then attempting to connect from within the pytest framework. That way the service can be started with either docker or can be started from a local connection by launching fluent locally if applicable.

If you intend to keep this unit test, allow it to be skipped if not running on CI, especially as users will want to be able to run local unit testing without pulling docker (if available). Also, instead of waiting a fixed amount of time, rather keep trying to form the connection persistently. This should be handled in the from ansys.fluent.core.session.Session class, or if you prefer, in a loop like:

timeout = time.time() + 60  # 60 second timeout
while time.time() < timeout:
    # attempt server connection
    if session.check_health() == "SERVING":
         break
session.check_health() == "SERVING"

@mkundu1
Copy link
Contributor Author

mkundu1 commented Mar 27, 2022

I think we're going to have to rethink the way we do tests, especially starting the container outside of pytest.

While I'm not opposed with doing it this way, this approach assumes that you have docker and the fluent image locally. I think it's preferable to start the fluent service outside of the unit testing suite and then attempting to connect from within the pytest framework. That way the service can be started with either docker or can be started from a local connection by launching fluent locally if applicable.

If you intend to keep this unit test, allow it to be skipped if not running on CI, especially as users will want to be able to run local unit testing without pulling docker (if available). Also, instead of waiting a fixed amount of time, rather keep trying to form the connection persistently. This should be handled in the from ansys.fluent.core.session.Session class, or if you prefer, in a loop like:

timeout = time.time() + 60  # 60 second timeout
while time.time() < timeout:
    # attempt server connection
    if session.check_health() == "SERVING":
         break
session.check_health() == "SERVING"

Thanks, I'll think about these in a subsequent PR. Right now, we are trying to get the doc CI passing (in PR #222) which will connect with a Fluent's docker container.

@mkundu1
Copy link
Contributor Author

mkundu1 commented Mar 27, 2022

Closing this as we'll first pull the docker image during the doc CI run (PR #222)

@mkundu1 mkundu1 closed this Mar 27, 2022
@mkundu1 mkundu1 deleted the testing/docker branch June 30, 2022 04:26
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.

3 participants