Skip to content

PYTHON-1878 Add mongodb+srv URIs to Atlas Connectivity tests #538

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

Merged
merged 6 commits into from
Jan 6, 2021
Merged

PYTHON-1878 Add mongodb+srv URIs to Atlas Connectivity tests #538

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3e986fd
PYTHON-1878 Add mongodb+srv URIs to Atlas Connectivity tests
ShaneHarvey Jan 4, 2021
d7092c5
Enable trace with silent:false for easier debugging
ShaneHarvey Jan 5, 2021
3391025
PYTHON-1878 fix env variable naiming bug
ShaneHarvey Jan 5, 2021
08b5053
PYTHON-1878 Fix Jython issue
ShaneHarvey Jan 5, 2021
da3f217
PYTHON-1878 Skip Jython SRV tests
ShaneHarvey Jan 5, 2021
658eb17
PYTHON-1878 Ensure that we don't accidentally duplicate the test URIs…
ShaneHarvey Jan 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions .evergreen/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,29 @@ functions:
silent: true
working_dir: "src"
script: |
# DO NOT ECHO WITH XTRACE (which PREPARE_SHELL does)
PYTHON_BINARY=${PYTHON_BINARY} ATLAS_REPL='${atlas_repl}' ATLAS_SHRD='${atlas_shrd}' ATLAS_FREE='${atlas_free}' ATLAS_TLS11='${atlas_tls11}' ATLAS_TLS12='${atlas_tls12}' sh ${PROJECT_DIRECTORY}/.evergreen/run-atlas-tests.sh
cat <<EOT > prepare_atlas_connectivity.sh
export ATLAS_FREE='${atlas_free}'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can further optimize some of these invocations by switching to using subprocess.exec instead of shell.exec and then setting either theinclude_expansions_in_env option or the add_expansions_to_env option to automatically set the appropriate environment variables. I suppose we don't need to do it right now, but being able to eventually eliminate the use of $PREPARE_SHELL would be a huge improvement IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, I am just suggesting this as a future optimization/refactor. Not saying that we need to do it in this PR. Would love to hear your thoughts on it @ShaneHarvey

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool idea! Can you open a ticket with this comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://jira.mongodb.org/browse/PYTHON-2481

export ATLAS_REPL='${atlas_repl}'
export ATLAS_SHRD='${atlas_shrd}'
export ATLAS_TLS11='${atlas_tls11}'
export ATLAS_TLS12='${atlas_tls12}'
export ATLAS_SRV_FREE='${atlas_srv_free}'
export ATLAS_SRV_REPL='${atlas_srv_repl}'
export ATLAS_SRV_SHRD='${atlas_srv_shrd}'
export ATLAS_SRV_TLS11='${atlas_srv_tls11}'
export ATLAS_SRV_TLS12='${atlas_srv_tls12}'
EOT
- command: shell.exec
type: test
params:
working_dir: "src"
script: |
# Disable xtrace (just in case it was accidentally set).
set +x
. ./prepare_atlas_connectivity.sh
rm -f ./prepare_atlas_connectivity.sh

PYTHON_BINARY=${PYTHON_BINARY} bash ${PROJECT_DIRECTORY}/.evergreen/run-atlas-tests.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a more user friendly way to manage secrets. We load the env variables in secret (with silent:true) and then we're free to use xtrace and view test output as usual. Now we can actually see why a task failed :)

Copy link
Contributor

Choose a reason for hiding this comment

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

<3 it!


"add aws auth variables to file":
- command: shell.exec
Expand Down
41 changes: 26 additions & 15 deletions .evergreen/run-atlas-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#!/bin/bash

# Don't trace to avoid secrets showing up in the logs
# Exit on error and enable trace.
set -o errexit
set -o xtrace
Copy link
Member Author

Choose a reason for hiding this comment

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

We can do this safely. We just need to remember not to print or use the secret variables in the shell script.


export JAVA_HOME=/opt/java/jdk8

Expand All @@ -15,27 +16,37 @@ if [ -z "$PYTHON_BINARY" ]; then
fi

IMPL=$(${PYTHON_BINARY} -c "import platform, sys; sys.stdout.write(platform.python_implementation())")
if [ $IMPL = "Jython" -o $IMPL = "PyPy" ]; then
echo "Using Jython or PyPy"

if [ $IMPL = "Jython" ]; then
# The venv created by createvirtualenv is incompatible with Jython
$PYTHON_BINARY -m virtualenv --never-download --no-wheel atlastest
. atlastest/bin/activate
trap "deactivate; rm -rf atlastest" EXIT HUP
pip install certifi
PYTHON=python
else
IS_PRE_279=$(${PYTHON_BINARY} -c "import sys; sys.stdout.write('1' if sys.version_info < (2, 7, 9) else '0')")
# All other pythons work with createvirtualenv.
. .evergreen/utils.sh
createvirtualenv $PYTHON_BINARY atlastest
fi
trap "deactivate; rm -rf atlastest" EXIT HUP

if [ $IMPL = "Jython" -o $IMPL = "PyPy" ]; then
echo "Using Jython or PyPy"
python -m pip install certifi
else
IS_PRE_279=$(python -c "import sys; sys.stdout.write('1' if sys.version_info < (2, 7, 9) else '0')")
if [ $IS_PRE_279 = "1" ]; then
echo "Using a Pre-2.7.9 CPython"
$PYTHON_BINARY -m virtualenv --never-download --no-wheel atlastest
. atlastest/bin/activate
trap "deactivate; rm -rf atlastest" EXIT HUP
pip install pyopenssl>=17.2.0 service_identity>18.1.0
PYTHON=python
python -m pip install pyopenssl>=17.2.0 service_identity>18.1.0
else
echo "Using CPython 2.7.9+"
PYTHON=$PYTHON_BINARY
fi
fi

echo "Running tests"
$PYTHON test/atlas/test_connection.py
echo "Running tests without dnspython"
python test/atlas/test_connection.py

# dnspython is incompatible with Jython so don't test that combination.
Copy link
Member Author

Choose a reason for hiding this comment

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

See my comment in the Jira ticket: https://jira.mongodb.org/browse/PYTHON-1878?focusedCommentId=3548124&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-3548124

if [ $IMPL != "Jython" ]; then
python -m pip install dnspython
echo "Running tests with dnspython"
PYMONGO_MUST_HAVE_DNS="1" python test/atlas/test_connection.py
fi
73 changes: 53 additions & 20 deletions test/atlas/test_connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,30 @@
import pymongo
from pymongo.ssl_support import HAS_SNI

try:
import dns
HAS_DNS = True
except ImportError:
HAS_DNS = False

_REPL = os.environ.get("ATLAS_REPL")
_SHRD = os.environ.get("ATLAS_SHRD")
_FREE = os.environ.get("ATLAS_FREE")
_TLS11 = os.environ.get("ATLAS_TLS11")
_TLS12 = os.environ.get("ATLAS_TLS12")

ATLAS_REPL = os.environ.get("ATLAS_REPL")
ATLAS_SHRD = os.environ.get("ATLAS_SHRD")
ATLAS_FREE = os.environ.get("ATLAS_FREE")
ATLAS_TLS11 = os.environ.get("ATLAS_TLS11")
ATLAS_TLS12 = os.environ.get("ATLAS_TLS12")
ATLAS_SRV_REPL = os.environ.get("ATLAS_SRV_REPL")
ATLAS_SRV_SHRD = os.environ.get("ATLAS_SRV_SHRD")
ATLAS_SRV_FREE = os.environ.get("ATLAS_SRV_FREE")
ATLAS_SRV_TLS11 = os.environ.get("ATLAS_SRV_TLS11")
ATLAS_SRV_TLS12 = os.environ.get("ATLAS_SRV_TLS12")

PYMONGO_MUST_HAVE_DNS = os.environ.get("PYMONGO_MUST_HAVE_DNS")


def _connect(uri):
if not uri:
raise Exception("Must set env variable to test.")
client = pymongo.MongoClient(uri)
# No TLS error
client.admin.command('ismaster')
Expand All @@ -40,29 +55,47 @@ def _connect(uri):


class TestAtlasConnect(unittest.TestCase):

@classmethod
def setUpClass(cls):
if not all([_REPL, _SHRD, _FREE]):
raise Exception(
"Must set ATLAS_REPL/SHRD/FREE env variables to test.")
@unittest.skipUnless(HAS_SNI, 'Free tier requires SNI support')
def test_free_tier(self):
_connect(ATLAS_FREE)

def test_replica_set(self):
_connect(_REPL)
_connect(ATLAS_REPL)

def test_sharded_cluster(self):
_connect(_SHRD)

def test_free_tier(self):
if not HAS_SNI:
raise unittest.SkipTest("Free tier requires SNI support.")
_connect(_FREE)
_connect(ATLAS_SHRD)

def test_tls_11(self):
_connect(_TLS11)
_connect(ATLAS_TLS11)

def test_tls_12(self):
_connect(_TLS12)
_connect(ATLAS_TLS12)

@unittest.skipUnless(HAS_DNS, 'SRV requires dnspython')
@unittest.skipUnless(HAS_SNI, 'Free tier requires SNI support')
def test_srv_free_tier(self):
_connect(ATLAS_SRV_FREE)

@unittest.skipUnless(HAS_DNS, 'SRV requires dnspython')
def test_srv_replica_set(self):
_connect(ATLAS_SRV_REPL)

@unittest.skipUnless(HAS_DNS, 'SRV requires dnspython')
def test_srv_sharded_cluster(self):
_connect(ATLAS_SRV_SHRD)

@unittest.skipUnless(HAS_DNS, 'SRV requires dnspython')
def test_srv_tls_11(self):
_connect(ATLAS_SRV_TLS11)

@unittest.skipUnless(HAS_DNS, 'SRV requires dnspython')
def test_srv_tls_12(self):
_connect(ATLAS_SRV_TLS12)

@unittest.skipUnless(PYMONGO_MUST_HAVE_DNS, 'dnspython is optional')
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Can you add a comment/docstring detailing what this test achieves? Specifically, we don't end up silently skipping all tests if dnspython is not installed (instead we get a failure due to this test).

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed this up a bit. PTAL at the new approach:

@unittest.skipUnless(HAS_DNS or MUST_TEST_SRV, 'SRV requires dnspython')

def test_dnspython_was_installed(self):
self.assertTrue(HAS_DNS, 'PYMONGO_MUST_HAVE_DNS is set but dns could '
'not be imported')


if __name__ == '__main__':
Expand Down