DSSClient spontaneaously reverts to prod, even when configured with different host

[hannes-ucsc]

The prescribed method to switch the DSSClient to a different host is

client = hca.dss.DSSClient()
client.host = "https://dss.example.com/v1"

There are two problems with this approach:

For one the swagger def is downloaded (or loaded from disk) in the constructor, defaulting to the HCA prod instance. There is no guarantee that the dss.example.com endpoint has a compatible API.

More puzzling is that the client seems to sporadically revert to using the prod instance, even when configured to a different host as outlined above. This must be a race condition. I do have multiple threads sharing a client.

If I instantiate the client in the following way, the problem goes away (the client does not revert to prod):

 config = HCAConfig("hca")
 config['DSSClient'].swagger_url = 'https://dss.example.com/v1/swagger.json'
 self.dss_client = DSSClient(config=config)

This is all I got, I don't have time to dig deeper right now.

[hannes-ucsc]

I also want to mention that I have a group of threads sharing a single DSSClient instance. There are multiple such groups, each group has their own instance. Multiple thread groups are created over the life-time of the process. I am not sure if the reversion happens within one DSSClient instance or at the creation of a new instance.

[hannes-ucsc]

My gut feeling is that this is related to the shared state in the DSSClient class attributes, either the urlib session or the swagger spec.

[calvinnhieu]

Hey @hannes-ucsc,

I'd need to know more details about where you're setting client.host and when you might be creating new instances of DSSClient to be certain, but it seems like what you think is happening here is correct.

For one, the prescribed method for updating the host is susceptible to this issue (and should be updated/clarified in the docs) because it does not interact with the client's config object (as you've discovered), which is loaded from and cached at ~/.config/hca/config.json using tweak. There are two ways to update the host and have it persist:

1. The method you described (programmatically)
2. Configure the swagger_url field in ~/.config/hca/config.json

What I suspect is happening in your case is that a DSSClient instance for which you set client.host = <host> will behave as you expect, however, any instances for which you do not specify this, will read the swagger_url from dcp-cli/hca/default_config.json or ~/.config/hca/config.json which most likely point to prod. In either case, since the client's config object wasn't modified, at some point, tweak will write the instance's config object to the cache at ~/.config/hca/config.json, most likely pointing to prod.

The second code block you provided modifies the client's config object for which tweak will write to the cached config, resulting in all future DSSClient instances reading the updated swagger_url. Let me know if this clears things up and satisfies your use cases!

(Note: reversioning should not be occurring over the lifetime of a single DSSClient instance as client.host is set on initialization, unless we are building URLs with something other than client.host/updating client.host.)

[Bento007]

It may be related this line of code where the class is directly modified with the methods picked up from the swagger.

dcp-cli/hca/util/__init__.py

Line 436 in 56e2f09

setattr(self.__class__, method_name, types.MethodType(client_method, SwaggerClient))

[hannes-ucsc]

Tests that reproduce the issues, confirming my suspicion.

from concurrent.futures import ThreadPoolExecutor
import importlib
import logging
import os
import tempfile
import unittest
from unittest import mock


def setUpModule():
    logging.basicConfig(level=logging.INFO,
                        format="%(asctime)s %(levelname)s %(name)s %(threadName)s: %(message)s", )
    logging.getLogger('humancellatlas').setLevel(logging.DEBUG)


class MyTest(unittest.TestCase):

    def test_swagger_spec_from_correct_host(self):
        with tempfile.TemporaryDirectory() as home:
            with mock.patch.dict(os.environ, HOME=home):
                from hca.dss import DSSClient
                integration = DSSClient()
                integration.host = f'https://dss.integration.data.humancellatlas.org/v1'
                # FIXME: assertion fails
                self.assertEqual('dss.integration.data.humancellatlas.org', integration._swagger_spec['host'])

    def test_client_race(self):
        num_repeats = 10
        num_threads = 2
        for repeat in range(num_repeats):
            with self.subTest(repeat=repeat):
                with tempfile.TemporaryDirectory() as home:
                    with mock.patch.dict(os.environ, HOME=home):
                        import hca.dss

                        def f(_):
                            integration = hca.dss.DSSClient()
                            integration.host = f'https://dss.integration.data.humancellatlas.org/v1'
                            integration.get_bundle(replica='aws', uuid='1e276fdd-d885-4a18-b5b8-df33f1347c1a')

                        with ThreadPoolExecutor(num_threads) as tpe:
                            # FIXME: fails randomly with 404 from DSS because endpoint reverts to prod
                            self.assertTrue(all(x is None for x in tpe.map(f, range(num_threads))))

                        importlib.reload(hca.dss)

    def test_client_race_with_workaround(self):
        num_repeats = 10
        num_threads = 2
        for repeat in range(num_repeats):
            with self.subTest(repeat=repeat):
                with tempfile.TemporaryDirectory() as home:
                    with mock.patch.dict(os.environ, HOME=home):
                        import hca.dss
                        from hca import HCAConfig

                        def f(_):
                            host = 'dss.integration.data.humancellatlas.org'
                            # FIXME: fixme this is convoluted and shouldn't be required of a user
                            hca_config = HCAConfig()
                            hca_config['DSSClient'].swagger_url = f'https://{host}/v1/swagger.json'
                            integration = hca.dss.DSSClient(config=hca_config)
                            integration.get_bundle(replica='aws', uuid='1e276fdd-d885-4a18-b5b8-df33f1347c1a')

                        with ThreadPoolExecutor(num_threads) as tpe:
                            self.assertTrue(all(x is None for x in tpe.map(f, range(num_threads))))

                        importlib.reload(hca.dss)

    def test_swagger_spec_from_correct_host_with_workaround(self):
        for reset in False, True:
            with self.subTest(reset=reset):
                with tempfile.TemporaryDirectory() as home:
                    with mock.patch.dict(os.environ, HOME=home):
                        from hca.dss import DSSClient
                        from hca import HCAConfig
                        prod = DSSClient()
                        self.assertEqual('dss.data.humancellatlas.org', prod._swagger_spec['host'])
                        if reset:
                            DSSClient._swagger_spec = None
                with tempfile.TemporaryDirectory() as home:
                    with mock.patch.dict(os.environ, HOME=home):
                        hca_config = HCAConfig()
                        host = 'dss.integration.data.humancellatlas.org'
                        hca_config['DSSClient'].swagger_url = f'https://{host}/v1/swagger.json'
                        integration = DSSClient(config=hca_config)
                        # FIXME: assertion fails if `reset` is False.
                        # Resetting the cached spec shouldn't be necessary and is not thread-safe
                        self.assertEqual(host, integration._swagger_spec['host'])