XNAT target ping problem

[aarontovars]

Describe the bug After the merge from #54 , some small commits were done. One of them (commit 2f6f1de) changed the code to test if ping is OK. Before this change, if connection to XNAT was successful, ping was OK but after this commit, even though ping is OK, test is not passed and ping appears as None.

I just checked in a jupyter notebook your version and my current version to understand what may be happening since on my side ping is not working.

From your version, I got:

c:\Users\FAaro\Documents\GitHub\AH-1\src\dicom-orchestrator\tests\test_pyxnat.ipynb Cell 6 line 8
      5 ping_ok = False
      7 if target.host:
----> 8     ping_result, *_ = await async_run(f"ping -w 1 -c 1 {target.host}")
      9     if ping_result == 0:
     10         ping_ok = True

c:\Users\FAaro\Documents\GitHub\AH-1\src\dicom-orchestrator\tests\test_pyxnat.ipynb Cell 6 line 4
      2 async def async_run(cmd, **params) -> Tuple[Optional[int], bytes, bytes]:
      3     """Executes the given command in a way compatible with ayncio."""
----> 4     proc = await asyncio.create_subprocess_shell(
      5         cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, **params
      6     )
      8     stdout, stderr = await proc.communicate()
      9     return proc.returncode, stdout, stderr

File C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\asyncio\subprocess.py:205, in create_subprocess_shell(cmd, stdin, stdout, stderr, limit, **kwds)
    202 loop = events.get_running_loop()
    203 protocol_factory = lambda: SubprocessStreamProtocol(limit=limit,
    204                                                     loop=loop)
--> 205 transport, protocol = await loop.subprocess_shell(
    206     protocol_factory,
    207     cmd, stdin=stdin, stdout=stdout,
    208     stderr=stderr, **kwds)
    209 return Process(transport, protocol, loop)

File C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\asyncio\base_events.py:1648, in BaseEventLoop.subprocess_shell(self, protocol_factory, cmd, stdin, stdout, stderr, universal_newlines, shell, bufsize, encoding, errors, text, **kwargs)
   1646     debug_log = 'run shell command %r' % cmd
   1647     self._log_subprocess(debug_log, stdin, stdout, stderr)
-> 1648 transport = await self._make_subprocess_transport(
   1649     protocol, cmd, True, stdin, stdout, stderr, bufsize, **kwargs)
   1650 if self._debug and debug_log is not None:
   1651     logger.info('%s: %r', debug_log, transport)

File C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\asyncio\base_events.py:498, in BaseEventLoop._make_subprocess_transport(self, protocol, args, shell, stdin, stdout, stderr, bufsize, extra, **kwargs)
    494 async def _make_subprocess_transport(self, protocol, args, shell,
    495                                      stdin, stdout, stderr, bufsize,
    496                                      extra=None, **kwargs):
    497     """Create subprocess transport."""
--> 498     raise NotImplementedError

However, in my implementation I indeed have a way to check correct ping by checking the status of the response connecting to the XNAT target host.

This is what I got:

{'ping': True, 'loggedin': True, 'err': ''}

by running my test_connection function:

async def test_connection(self, target: XnatTarget) -> str:
        url = f"{target.host}/data/auth"

        async with aiohttp.ClientSession() as session:
            ping_ok = False
            try:            
                async with session.get(url, auth=aiohttp.BasicAuth(target.user, target.password)) as resp:
                    ping_ok = resp.status != 404
                    response_ok = resp.status == 200
                    text = await resp.text() if not response_ok else ""
                    return dict(ping=ping_ok, loggedin=response_ok, err=text)

            except Exception as e:
                return dict(ping=ping_ok, loggedin=False, err=str(e))

[aarontovars]

Any possible suggestion @joshy @tblock79 @RoyWiggins ?

Thanks in advance for your help! :)

[RoyWiggins]

I'm not sure. The error you're getting in Jupyter is coming from deep inside asyncio, quite likely that's caused by your runtime environment (eg running in Jupyter or Windows):

https://stackoverflow.com/questions/44633458/why-am-i-getting-notimplementederror-with-async-and-await-on-windows

It's probably not related to the issue you're having.

One thing is that your "ping" definition is simply different- we are doing a literal "ping " to see if the host is reachable and online, regardless of the status of the service itself. This is mostly to make check you've typed in the hostname of a reachable server, and to distinguish between "this server doesn't exist or isn't reachable" and "the service you're trying to reach is misconfigured or disabled." Your code is doing a full HTTP request instead.

It may be that you want to have three results from a test: ping, http status not a 404, and http status is actually 200. This makes sense since you want to know 1) whether the xnat service is up and 2) whether your credentials are correct.

You can return all three statuses as a dictionary and they will be passed to this template to be displayed:

https://github.com/mercure-imaging/mercure/blob/master/webinterface/templates/targets/xnat-test.html

I'm not sure why it's not displaying properly. If "ping -w 1 -c 1 " at the command line works then it should display correctly.

On Wed, Nov 8, 2023, 3:35 AM Aarón Tovar Sáez @.***> wrote:

Any possible suggestion @joshy https://github.com/joshy @tblock79 https://github.com/tblock79 @RoyWiggins https://github.com/RoyWiggins ?

Thanks in advance for your help! :)

— Reply to this email directly, view it on GitHub https://github.com/mercure-imaging/mercure/issues/60#issuecomment-1801321279, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABILWYV4U3XMVMBEFBYYX63YDM75XAVCNFSM6AAAAAA6XKILJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBRGMZDCMRXHE . You are receiving this because you were mentioned.Message ID: @.***>

[aarontovars]

Thank you so much for your reply.

I now see what is going on in here. Correct ping is done if the input entered in the Host/IP field is an IP or domain without http:// otr https://. However, to check log in correctly and to later send to XNAT, Host/IP field is expected to be in the form http://[IP or domain] or https://[IP or domain].

[aarontovars]

Hi @joshy @tblock79 @RoyWiggins,

I have proposed a solution to this on #62

[tblock79]

Merged your PR into the main branch. Many thanks for your work on this!