Use jupyter_client's AsyncKernelManager

:+1: Thanks, @davidbrochart.
The failing AppVeyor test is due to a Github webhook that needed to be deleted. I just removed it, so it should drop from the test list.

Not sure about these errors on Windows for python3.5 and 3.6.

Trigger CI.

Now the error is only on Windows/python3.5.

Hmm - Windows 3.5 is sounding more familiar and I definitely recall this google hit: https://bugs.python.org/issue37380.  Still trying to remove the cobwebs.

Thx @davidbrochart
@kevin-bates When the kernel provider will be merged, my understanding is that this async kernel-client implementation will be completely replaced by the kernel-protocol code. Is this correct?

@echarles - yes, that's correct.  We'll want to get that client implementation up on async.  I'm fairly certain @davidbrochart is aware of that: https://github.com/jupyter/jupyter_client/pull/428#issuecomment-594607993

Hmm - Windows 3.5 is sounding more familiar and I definitely recall this google hit: https://bugs.python.org/issue37380. Still trying to remove the cobwebs.

This is not going to be fixed for python3.5 and 3.6 on Windows, and I don't have any workaround.

@echarles - yes, that's correct. We'll want to get that client implementation up on async. I'm fairly certain @davidbrochart is aware of that: jupyter/jupyter_client#428 (comment)

I don't think there is anything to do in jupyter_kernel_mgmt, it already has an async client (it's IOLoopKernelClient).

By golly - I didn't realize that.  I knew it required changes to interact with the async kernel manager, but I never looked closely at the pure 'client' portion.  If you say its good to go, then I'm good.
Looking just now, it doesn't appear to have a mixture of async/await and gen.coroutine/yield and it seems like we should convert the latter at some point.

This is not going to be fixed for python3.5 and 3.6 on Windows, and I don't have any workaround.

So what are our options here?

Require Python 3.6>= on Windows and keep 3.5 support on other platforms.
Or bump jupyter_server to only support Python 3.6 on all platforms.

Is that right?
And if we choose the former, we should probably add a table to the README with "supported" Python versions+platforms.

We even need python>=3.7 for Windows. I would go for option 1.

We even need python>=3.7 for Windows. I would go for option 1.

Why are the 3.6 Windows tests passing, but not the 3.5?

What I'm not understanding is why JKM is working on WIndows 3.5, 3.6?  It's generally the same code since we can't use the async subprocess stuff on Windows there either.

The tests randomly pass on Windows/python3.6, and they seem to always fail with python3.5. This issue says that the fix has been back-ported to python 3.7 only, not beyond.
Since the behavior is random, it might explain why we don't see it in JKM, but I cannot be sure.

Thinking about this PR more, I guess I have to ask why https://github.com/jupyter/notebook/pull/4479 wasn't ported over?  This allows opt-in behavior for those applications/extensions that may require synchronous functionality and I plan on pushing for its merge in Notebook shortly (once a jupyter_client release is cut).

I was not aware https://github.com/jupyter/notebook/pull/4479, I need to look at it.
But I'm wondering why one would need a blocking behavior in a web framework (which is inherently asynchronous).  Any use case?

Good question.  Why it's that way is probably more historical than anything (and before my time), but that's the way it is (for better or worse).   This is the primary reason why we're delaying the adoption of kernel providers.  We need to show that jupyter_server (with "nbclassic") is a viable and compatible replacement to notebook and I think minimizing risk is key for that to succeed.
Given that today's kernel management framework is essentially synchronous, you can have folks that have written their own managers. Most of these custom managers will very likely extend the existing managers and will break when plugged into an async-only framework.  Granted, these custom managers will need to adjust the package name of the superclass (if inherited), but that's all they should have to do to "migrate" to jupyter_server.
When we move to kernel providers, we will need to make it clear that users with their own kernel (and kernelspec) managers must convert them to kernel providers since we will no longer support "class substitution" as a form of customization (wrt kernel discovery and management).  But I don't think we want to force those users to convert their managers twice (once to async, the second to providers).  However, by allowing async to be optional, then those custom managers could choose to stay on the "classic" branch and benefit from async functionality by updating their managers to async after confirming things are behaving as expected via today's "classic" behavior.
I may be blowing this use case out of proportion, but I know the gateway projects (Jupyter Kernel and Enterprise Gateway) use class substitution for their kernel managers.  That's the problem with open source projects, you really don't know who or how many customers you have.  And with a project as popular as Jupyter Notebook, you only find these things out the hard way - when things break.

Thanks for the feedback @kevin-bates. I'm new to the Jupyter ecosystem, and I definitely lack the historical reasons of things being how they are today.
However, on the sync to async switch, my understanding was that it is going to be quite smooth, since we use maybe_future everywhere. Isn't it enough?

I believe the problem is that if a sync custom class method calls what is now an async method you'll encounter the "method was not awaited" hassle until the entire call stack is made async-aware.
To be honest, I haven't tried plugging in a custom class that works in the "classic" sense into a framework that is completely async-aware (because this didn't exist until recently), but I'd be surprised it works.  Wouldn't the sync method require a 'run to completion' wrapper around the async method?

I guess the issue we have in jupyter_server is the opposite (with this PR): we expect an async method, and we might get a blocking one. That's why I have this ensure_async function, which is basically equivalent to the previous maybe_future. These don't hurt, you can put them anywhere you're not sure you will get an async function (at the cost of a tiny performance loss).
But you're right, until we try out we can't be sure it's going to be as smooth as we think. I you think it's better to be more cautious, I can try and port https://github.com/jupyter/notebook/pull/4479 over here in a new PR.

I don't have a huge problem just going async only, but I think it's a little contrary to the project's approach thus far.   I'll defer to others on that.
I think an example might be good here.  Say I have the following custom multi-kernel manager that works in today's (blocking) system...
class MyMultiKernelManager(MappingKernelManager):
    """Custom MultiKernelManager that alters the behavior when interrupts are performed"""
    def interrupt_kernel(self, kernel_id):
        # Do my custom stuff, then defer to the superclass...
        super(MyMultiKernelManager, self).interrupt_kernel(kernel_id)
        # Do more custom stuff...

Now that MappingKernelManager.interrupt_kernel() is async, the call to the superclass is going to fail despite the call to this method being wrapped in ensure_async().  Is that correct?

True. In this direction we need the opposite of ensure_async (what you earlier called a 'run to completion' wrapper). Something like https://github.com/takluyver/jupyter_kernel_mgmt/blob/master/jupyter_kernel_mgmt/client.py#L347-L387

Correct.  I guess my point is that if users need to update their code beyond changing the package name for the class they derive from, then they're faced with making the switch to async anyway.  If there's an opt-in for the async behavior, they'd only have to change the package name of the derived class and can choose to make the switch at their convenience.
Fun fact.  If users happen to also have notebook installed in the same python scope and forget to change their import statement in their custom kernel manager to the jupyter_server package, then I believe they will bypass the entire kernel management framework within jupyter_server and (accidentally) use the one from notebook.  But that's on them. :smile:  The problem is it should all "just work" - probably until notebook is uninstalled.

The opt-in for the async behavior seems like a wiser approach then. Do you think I should do it in a separate PR?

I would hope https://github.com/jupyter/notebook/pull/4479 would be a fairly straightforward port and should probably be a different PR.  We can talk about this at tomorrow's server meeting if you're able to attend.

Sweet! Looks good, @davidbrochart. Thanks!

It still needs some double-checking, and I'm not testing a lot a believe.

Once ready, I'd love to try this branch for jupyterlab.
As the current plan I have in mind is to first merge https://github.com/jupyter/jupyter_server/pull/180 as the work done for jupyterlab depends on that.
I guess the changes of this PR mostly target the Kernel API/services, but I bet it can impact consumers like jupterlab.
Ideally, these changes should be compatible and tested with https://github.com/jupyter/jupyter_server/pull/180

In applying this class hierarchy approach to the Notebook PR, I'm finding the async kernel tests are failing with the following kind of error:
Traceback (most recent call last):
  File "/opt/anaconda3/envs/enterprise-gateway-dev/lib/python3.7/site-packages/tornado/web.py", line 1703, in _execute
    result = await result
  File "/opt/anaconda3/envs/enterprise-gateway-dev/lib/python3.7/site-packages/tornado/gen.py", line 748, in run
    yielded = self.gen.send(value)
  File "/Users/kbates/repos/oss/jupyter/notebook/notebook/services/kernels/handlers.py", line 47, in post
    model = yield maybe_future(km.kernel_model(kernel_id))
  File "/Users/kbates/repos/oss/jupyter/notebook/notebook/services/kernels/kernelmanager.py", line 353, in kernel_model
    "last_activity": isoformat(kernel.last_activity),
AttributeError: 'AsyncIOLoopKernelManager' object has no attribute 'last_activity'

This is because the wrong start_kernel() call is taking place.  Based on the MRO, AsyncMultiKernelManager.start_kernel() is called rather than MappingKernelManager.start_kernel().  As a result, self.start_watching_activity(kernel_id) is not called.  This method is what adds last_activity to the kernel manager instance that is managed by the Mapping/Multi kernel manager.  Then, when the model is accessed from the handler, the async kernel manager instance doesn't have that attribute.
I don't think swapping the two superclasses is what we want either and this will bypass the other async methods in AsyncMultiKernelManager from being called.
We're also going to need to figure out a way to parameterize the kernel tests such that the two forms of mapping kernel managers are tested.  In the Notebook PR, this is trivial since its class-based and the async test can simply start the server with the appropriate manager configured.  Since the serverapp is buried in the pytest fixture stack in the server tests, I'm not sure how to introduce parameters.  I'd rather not force ALL tests to run twice when only the kernel and session tests require that async kernel manager be validated.
I'm hoping @Zsailer can assist us in how to "plumb" the parameters here.

It is difficult to see what's going on since I don't think you have pushed, but have you seen I replaced e.g. super(MappingKernelManager, self) with super() in  jupyter_server/services/kernels/kernelmanager.py? It might be the reason, if you didn't do the same.
For the tests, yes I didn't know how to proceed, so any help is welcome. I'll try again today, so that I can confirm the behavior you're describing above.

This is because the wrong start_kernel() call is taking place. Based on the MRO, AsyncMultiKernelManager.start_kernel() is called rather than MappingKernelManager.start_kernel().

Actually I think this can be solved by swapping the inheritance order, which makes more sense:
class AsyncMappingKernelManager(MappingKernelManager, AsyncMultiKernelManager):

In this last commit MappingKernelManager.shutdown_kernel is not async anymore. I restored the dependency to jupyter_client>=6.1.0, and took Kevin's suggestions into account. I also rewrote run_async, to make it more generic and be able to run an object that is not async. In this case, there is no need to wrap it in a coroutine and run it on an event loop, it can just be returned.
And I added a test_async_manager.py file which is the same as test_manager.py but uses AsyncMappingKernelManager instead of MappingKernelManager. Still, I don't know if I'm testing a lot.

It is difficult to see what's going on since I don't think you have pushed, but have you seen I replaced e.g. super(MappingKernelManager, self) with super() in jupyter_server/services/kernels/kernelmanager.py? It might be the reason if you didn't do the same.

Yes, I saw that the use of super() was empty, then read more about super() in how a blank set of parameters equates to the normal case of placing the current_class, self in the parameter list - and is preferred over the latter (so I'll switch my uses to that moving forward).  My changes included that.  I never pushed because, once I found the reason for the test failures, I don't think its a viable approach.
The issue was pretty easy to see once I figured out what was going on.  The superclass listing of AsyncMultiKernelManager, MappingKernelManager results in a call to the AsyncMultiKernelManager.start_kernel() method.  That's "okay" except for the fact that it results in none of the "enhanced functionality" introduced to the start_kernel() method in MappingKernelManager.  Such functionality includes the addition of activity monitoring - which, when not performed, led to attempts to access an attribute that is not defined.  (Not sure why the decision to use "dynamic" attributes like that was made, but that's water well under the bridge.)
If you swap the superclasses to MappingKernelManager, AsyncMultiKernelManager, you now essentially get synchronous kernel management.  Also not what we want.
Because, by definition, every method is on both MappingKernelManager (and its hierarchy) and AsyncMultiKernelManager (and its hierarchy), only the methods from the first class will be called.
I think we need to focus on parameterizing the fetch fixture such that it starts the server with the default MappingKernelManager and the optional AsyncMappingKernelManager as parameters.  Since we don't want every test to be run test, we'll likely need to have a copy of the fetch fixture that is used by the kernel and session tests.  Once those are done, you should see the async versions of the tests fail (with the current MRO).  I did try changing the MRO on my Notebook PR yesterday, and the tests did pass, but, as mentioned above, that's only because they were invoking the synchronous forms of the methods - which we know lead to successful test results.  I plan on dealing with the parameterized 'fetch' today.

And I added a test_async_manager.py file which is the same as test_manager.py but uses AsyncMappingKernelManager instead of MappingKernelManager. Still, I don't know if I'm testing a lot.

The reason this appears to work is that start_kernel() is defined on DummyAMKM, so its getting called, essentially by-passing the problem.  In the PR, there is no start_kernel() method defined on AsyncMappingKernelManager, leading to the one from AsyncMultiKernelManager getting invoked (and, again, skipping the activity enhancements).
In fact, we shouldn't define functionality on the DummyAMKM class that should be coming from its superclass unless the intent is to override that functionality, but that would be outside the scope of what we're trying to do here.

Yes you're right... I thought that having an empty super() would call the methods of MappingKernelManager or AsyncMappingKernelManager correctly, but it seems to always call the methodes of MappingKernelManager. I will think about a new solution, maybe it's easily fixable.

Sounds good.  We can always fall back on the mixin approach used in 4479 - but I am curious if there's a better way that is also straightforward and not problematic to maintain.  I will begin looking at the pytest fixtures now.

This is not the most beautiful, but it should work in our case. I just introduced a super attribute in MappingKernelManager, and made it point to either MultiKernelManager or AsyncMultiKernelManager.

This works for any methods defined on MappingKernelManager, but for any methods that are purely derived from superclasses, you're going to wind up with sync methods when called on AsyncMappingKernelManager.  Such an example is interrupt_kernel() or signal_kernel().  Also, methods like shutdown_kernel(), who's "synchronicity" differ between MKM and AMKM are not correct.

but I am curious if there's a better way

@kevin-bates...metaclasses? 😄

that is also straightforward and not problematic to maintain

Oh, nevermind... 🤣
Seriously though—this might be an approach.
In short, my thought was that we dynamically construct a MappingKernelManager class that's either async or not. Its inherited classes depend on this async/not-async choice. This would allow us to use the MappingKernelManager prefix in the traits and avoid listing both AsyncMappingKernelManager and MappingKernelManager in ServerApp's classes list.

Ok, how about a totally different approach.
Kevin, in your original PR... rename the mixin class, MappingKernelManagerBase, to MappingKernelManager. Then rename your current MappingKernelManager to SyncMappingKernelManager (or something like that). This would allow MappingKernelManager prefixed traits to still work for both classes...

It doesn't solve the code duplication problem, but it does solve the traitlets/config issue.

Good idea, but it's going to break existing subclasses of MappingKernelManager which I think we want to try to support.  However,  since they'd need to change the package name from which they derived anyway, this might be fine.  I suppose if they didn't update their superclass to SyncMappingKernelManager then they'd find out soon enough.
I think this approach will be problematic in Notebook because existing subclasses don't expect to have to go change their implementation.
I don't really understand the aversion to 4479.  If they have to change their configuration in order to opt-in to use AsyncMKM, then why not change (or add) the applicable traits as well?  We're breaking our picks here.

@davidbrochart - If you add the following override of the argv fixture near the top of services/{kernels,sessions}/test_api.py, it should enable two sets of tests - each using a different MKM.
@pytest.fixture(params=["MappingKernelManager", "AsyncMappingKernelManager"])
def argv(request):
    return ["--ServerApp.kernel_manager_class=jupyter_server.services.kernels.kernelmanager." + request.param]

Thanks Kevin, I will try.

Good idea, but it's going to break existing subclasses of MappingKernelManager which I think we want to try to support. However, since they'd need to change the package name from which they derived anyway, this might be fine. I suppose if they didn't update their superclass to SyncMappingKernelManager then they'd find out soon enough.

Right. Sorry—I didn't think that through enough. This won't work.

I don't really understand the aversion to 4479. If they have to change their configuration in order to opt-in to use AsyncMKM, then why not change (or add) the applicable traits as well? We're breaking our picks here.

The main issue that the help documentation. Both classes have to be listed in the help docs, even though they can't both be used at the same time.
It's a small price, so maybe it's something we have to deal with...

Really not sure what is happening with python3.5.

This works for any methods defined on MappingKernelManager, but for any methods that are purely derived from superclasses, you're going to wind up with sync methods when called on AsyncMappingKernelManager. Such an example is interrupt_kernel() or signal_kernel(). Also, methods like shutdown_kernel(), who's "synchronicity" differ between MKM and AMKM are not correct.

I'm not sure to understand, these methods are called with ensure_async so this should be fine. shutdown_kernel is also taken care of because it is called with run_sync.

Regarding this comment:

This works for any methods defined on MappingKernelManager, but for any methods that are purely derived from superclasses, you're going to wind up with sync methods when called on AsyncMappingKernelManager. Such an example is interrupt_kernel() or signal_kernel(). Also, methods like shutdown_kernel(), who's "synchronicity" differ between MKM and AMKM are not correct.

I had to build a simple python program that mimicked the class structure we're using and I found MRO to be extremely subtle.  Because its breadth-first and because AsyncMultiKernelManager derives from MultiKernelManager, it turns out that methods that only exist on either of the multi-KM classes will be called on their appropriate subclasses.  I figured that interrupt_kernel() would be called within the MappingKernelManager hierarchy (and thus the implementation on MultiKernelManager would be invoked), but it turns out the MRO will go look on AsyncMultiKernelManager when it finds that MappingKernelManager doesn't implement interrupt_kernel() despite MKM deriving from MultiKernelManager.
I also confirmed via a debugger that the appropriate interrupt_kernel() was called depending on which Mapping KM was configured.
I agree about the 3.5 stuff, but I'm wondering if the shutdown_kernel() issue is addressed, they get cleared up as well.

FWIW - the following changes are working in a 3.5 env I just setup (after reproducing GH actions failures).  (Just realized, we should be including the restart=False parameter in the MappingKM signature and superclass call since the MultiKM's define it that way.)
diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py
index 060984bff..1723ea002 100644
--- a/jupyter_server/services/kernels/kernelmanager.py
+++ b/jupyter_server/services/kernels/kernelmanager.py
@@ -300,7 +300,7 @@ class MappingKernelManager(MultiKernelManager):
             type=self._kernels[kernel_id].kernel_name
         ).dec()

-        return run_sync(self.super.shutdown_kernel(self, kernel_id, now=now))
+        return self.super.shutdown_kernel(self, kernel_id, now=now)

     async def restart_kernel(self, kernel_id):
         """Restart a kernel by kernel_id"""
@@ -485,3 +485,21 @@ class AsyncMappingKernelManager(MappingKernelManager, AsyncMultiKernelManager):
         self.super = AsyncMultiKernelManager
         self.super.__init__(self, **kwargs)
         self.last_kernel_activity = utcnow()
+
+    async def shutdown_kernel(self, kernel_id, now=False, restart=False):
+        """Shutdown a kernel by kernel_id"""
+        self._check_kernel_id(kernel_id)
+        kernel = self._kernels[kernel_id]
+        if kernel._activity_stream:
+            kernel._activity_stream.close()
+            kernel._activity_stream = None
+        self.stop_buffering(kernel_id)
+        self._kernel_connections.pop(kernel_id, None)
+
+        # Decrease the metric of number of kernels
+        # running for the relevant kernel type by 1
+        KERNEL_CURRENTLY_RUNNING_TOTAL.labels(
+            type=self._kernels[kernel_id].kernel_name
+        ).dec()
+
+        return await self.super.shutdown_kernel(self, kernel_id, now=now, restart=restart)

Awesome, thanks Kevin for doing all these tests! I'll apply your suggestions later today.

I think we need to decide if we're pinning the floor of jupyter_client to the async changes or not.  Given jupyter_server is (will be) considered the updated thing, I think it's perfectly reasonable to pin JC >= 6.1.0.
If we don't pin, then we need to add a number of protective measures for when folks try to set async kernel management and JC is not adequate.  This includes preventing the server's startup, creating a dummy AsyncMultiKernelManager base to allow resolution of AsyncMappingKernelManager, and updating tests to skip the async KM related versions.  Seems like a hassle for something that may never happen and our tests won't detect it.
In either case, though, because async KM is optional, I think we should also log an informational message when it is configured.  I'm doing this in https://github.com/jupyter/notebook/pull/4479...
        #  Ensure the appropriate jupyter_client is in place.
        if isinstance(self.kernel_manager, AsyncMappingKernelManager):
            if not async_kernel_mgmt_available:
                raise ValueError("You are using `AsyncMappingKernelManager` without an appropriate "
                                 "jupyter_client installed!  Upgrade jupyter_client or change kernel managers.")
            else:
                self.log.info("Asynchronous kernel management has been configured to use '{}'.".
                              format(self.kernel_manager.__class__.__name__))

Of course, you only need the embedded else portion of JC >= 6.1.0.  This is done in init_configurables() after self.kernel_manager is instantiated.

It seems that the error in the CI is because kill is a blocking function, so we should not await here:
https://github.com/jupyter/jupyter_client/blob/master/jupyter_client/manager.py#L647
It is the kill from the subprocess module, right?

I think we need to decide if we're pinning the floor of jupyter_client to the async changes or not. Given jupyter_server is (will be) considered the updated thing, I think it's perfectly reasonable to pin JC >= 6.1.0.

I agree.

Yeah, I was just looking at that - damn.  I agree we should not await self.kernel.kill().  I suspect Windows 3.7 worked because there's a timing window here.

I think it can happen on any OS, it's just a matter of CPU speed.
Restarting CI to check.

Now it works.

The kill method only takes place if the platform doesn't support the KILL signal - which is only Windows.  Also, that code (_kill_kernel()) only gets invoked as a last resort - when the message-based shutdown has not been responded to in a timely manner.  That's where the timing window occurs.  I think we'll need to update JC to not await self.kernel.kill().   That should not prevent this PR from moving forward.

I'll open a PR in jupyter_client.
Is this PR ready to be merged then?

Thanks for opening the JC PR!
Did we want to add any messaging to indicate that the server is using Async KM?  I suppose that message on the console will be short-lived (unless redirected to a log) and tracebacks will likely reflect that async KM is in play, so it's probably not a big deal.
I think I'm ready for this to be merged.  Thank you David!
@Zsailer - are you good with this now?

Thanks Kevin, I just added the messaging you suggested. BTW, I think you should also be marked as an author of this PR.

That's fine - just know that I'm doing the same in NB 4479. :smile:  This has been a good experience and I've learned a lot.  Thank you.

I think the failure for python3.5 could be due to the async features. We had the same issues in nbclient and decided to drop python3.5 (https://github.com/jupyter/nbclient/pull/34#issuecomment-606242122). Asyncio was still a work in progress in python3.5, and the workarounds we use (e.g. the async_generator library) may lead to a different behavior.
What are your opinions about dropping python3.5 in jupyter_server?

:smile: JKM had dropped support for 3.5 initially but added it back in because Voila needed it: https://github.com/jupyter/jupyter_server/pull/142#issuecomment-561596616
The JKM (and Server) async tests were/are passing on 3.5 but it sounds like nbconvert may be pressing harder in certain areas that the tests don't cover perhaps.  One difference between JKM and jupyter_client async support is that JKM uses the async subprocess stuff on all non-Windows platforms (Windows encounters a catch-22 scenario between async subprocess and its event loop).  Perhaps that's a hint.  That said, I'm all for dropping 3.5.
If Voila still needs 3.5, we could disable the optional async kernel manager on 3.5 platforms (i.e., throw an exception and prevent the server's startup).  But once we merge JKM, 3.5 would need to be dropped since JKM is only async.

nbconvert is dropping python3.5 also. For Voila we will need to drop it when we switch to async since Jinja supports async generators only for python3.6+.

Understood.  Since we want to support Voila in 1.0 and we know Voila needs 3.5, I think our only choice is to prevent async kernel managers on 3.5.  When Voila goes async, they'll require 3.6+ and can enable async at that time (in 1.0).  (I'll need to do the same on Notebook 4479.)

So I'm a bit lost. How do we prevent async kernel managers on python3.5? Does it mean we just put this PR on hold?

I'm sorry, it slipped my mind that this PR differs from Notebook 4479 in that NB does not require jupyter_client 6.1.  As a result, it includes this kind of logic: https://github.com/jupyter/notebook/pull/4479/files#diff-bbd307804bb6b8836da9f06b8d8a06e9R1393-R1401
The embedded if statement - once the option has been detected - would check the python version and if 3.5 would raise an applicable message stating that async kernel management is not available on Python 3.5.

Do you think I should this kind of run-time check, including the python version?

I'm not sure what other approaches we'd use and open to suggestions.  We want to support sync kernel managers on 3.5 (for Voila) and async kernel managers on 3.6+.  However that can happen is fine to me.

I just prevent using AsyncMappingKernelManager on python<=3.5 at run-time. The corresponding tests are skipped.
The error with test_create_with_kernel_id[AsyncMappingKernelManager] seems to be related to the last_activity which is slightly different:
E       AssertionError: assert [{'id': '995d...ipynb'}, ...}] == [{'id': '995d...ipynb'}, ...}]
E         At index 0 diff: {'id': '995d233a-b935-4c08-88a0-d8a0d0a6616c', 'path': 'foo/nb1.ipynb', 'name': None, 'type': 'notebook', 'kernel': {'id': '35edcdb4-6f35-4424-a6ad-86d090e2bb0f', 'name': 'python3', 'last_activity': '2020-04-01T16:20:38.702427Z', 'execution_state': 'starting', 'connections': 0}, 'notebook': {'path': 'foo/nb1.ipynb', 'name': None}} != {'id': '995d233a-b935-4c08-88a0-d8a0d0a6616c', 'path': 'foo/nb1.ipynb', 'name': None, 'type': 'notebook', 'kernel': {'id': '35edcdb4-6f35-4424-a6ad-86d090e2bb0f', 'name': 'python3', 'last_activity': '2020-04-01T16:20:38.053...

I believe the test failure is a timing issue where the kernel state and activity timestamp differences are causing issues with the comparison of the kernel or session.  The test is trying to ensure object (kernel,session) information is the same from before and now and the transient portions of those are triggering the failure.
I created these helper methods in the JKM PR to filter out the last_activity/execution_state fields: https://github.com/jupyter/jupyter_server/blob/7f9df58418f13f441af6613bbdee543dae1ceb81/tests/services/sessions/test_api.py#L118-L139

That looks great! Do you mind if I steal your code, or do you want to make a PR on this one?

Not at all - please use it.

Thanks Kevin!
Good to merge now?

I believe this is ready to merge.  I can't recall what option we should use.  Deferring to @Zsailer.

I think the history of this PR is useful to keep around—so I'm going to merge directly (not squash-and-merge). This was a lot of work that's work tracking. Thanks @kevin-bates and @davidbrochart! Great work, here!

I hate to bring this up, but in working on the NB 4479 PR, I'm finding the checks for Python 3.5 should be sys.version_info < (3, 6) rather than sys.version_info <= (3, 5).  I'm running on 3.5.5 and it's not detecting "3.5" correctly.

Good catch Kevin, thanks!
Zach, I think you should squash, we can always refer to this PR even though it's closed (and it has all the useful narrative too).

Thanks again Kevin!
I'm just adding back the removal kernelmanager.AsyncMappingKernelManager in the test to check, I remember that it was needed but I don't know why.

Hmm. That condition is impossible, by definition, so how can it be considered valid for the test?  I think it might be good to understand what's going on.  The 3.5 no-async-kernel-manager test is failing because the trailet for the kernel_manager_class can't be resolved...
E               traitlets.traitlets.TraitError: The 'kernel_manager_class' trait of <jupyter_server.serverapp.ServerApp object at 0x7f997c5b7b70> instance must be a type, but 'jupyter_server.services.kernels.kernelmanager.AsyncMappingKernelManager' could not be imported

and this is impossible in the real world because AsyncMappingKernelManager will always exist.

I need to take a break and look more closely.

Congrats, y'all! Great stuff!


Update README.md

This link is repeated twice

Release notes

In this release, we:

Did this and that

Summary

Breaking Changes

Checklist

[ ] All changes are covered by tests (or not applicable)
[ ] All changes are documented (or not applicable)
[ ] I reviewed the entire PR myself (preferably, on GH UI)
[ ] I described all Breaking Changes (or there's none)

Hey @sashaphmn, thanks for the contribution!
We love to get submissions from the community, but we prefer first-time contributions to be more substantial. If you are excited to make a contribution, feel free to look through our open issues for ideas!
Closing for now.


Improved size for 26 inputs.

https://github.com/bertdobbelaere/SorterHunter/commit/ed236af2177a83aa3a3bd5198e7916e2ad87cd82

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 21.42%. Comparing base (0c7b263) to head (b75cbb8).

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #11   +/-   ##
=======================================
  Coverage   21.42%   21.42%
=======================================
  Files           2        2
  Lines          14       14
=======================================
  Hits            3        3
  Misses         11       11

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


runtime: "panic during panic" on plan9 builders

#!watchflakes
post <- builder ~ `^plan9.*` && `panic during panic`

plan9-386-0intro at e8cda0a6c925668972ada40602ada08468fa90dc

:: Running /tmp/workdir-gnot/go/src/make.rc with args ["/tmp/workdir-gnot/go/src/make.rc"] and env ["home=/usr/glenda" "path=.\x00/bin" "*=" "type=host-plan9-386-0intro" "rcname=gobuildlet" "pid=169" "fn#term%=fn term% {$*}\n" "fn#newworkdir=fn newworkdir {if(test -d $workdir){for(i in `{du -a $workdir|awk '{print $2}'})chmod u+w $i;rm -rf $workdir};mkdir $workdir}\n" "status=" "fn#cd=fn cd {builtin cd $* && awd}\n" "GOROOT_BOOTSTRAP=/tmp/workdir-gnot/go1.4" "workdir=/tmp/workdir-gnot" "objtype=386" "fn#sigexit=" "cflag=" "0=/bin/gobuildlet" "WORKDIR=/tmp/workdir-gnot" "GO_BUILDER_NAME=plan9-386-0intro" "GOBIN=" "GOROOT_BOOTSTRAP=/usr/go-plan9-386-bootstrap"] in dir /tmp/workdir-gnot/go/src

Building Go cmd/dist using /usr/go-plan9-386-bootstrap
Building Go toolchain1 using /usr/go-plan9-386-bootstrap.

Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
# runtime
fatal error: unexpected signal during runtime execution
[signal sys: trap: fault read code=0x0 addr=0x0 pc=0x57001]

runtime stack:
fatal error: unexpected signal during runtime execution
panic during panic
[signal sys: trap: fault read code=0x0 addr=0x0 pc=0x4dc6a]

runtime stack:
fatal error: unexpected signal during runtime execution
stack trace unavailable
go tool dist: FAILED: /tmp/workdir-gnot/go/pkg/tool/plan9_386/go_bootstrap install -gcflags=all= -ldflags=all= -a -i cmd/asm cmd/cgo cmd/compile cmd/link: exit status: 'go_bootstrap 293035: 2'

greplogs --dashboard -md -l -e '(?ms)plan9-.*^panic during panic' --since=2021-01-01
2021-11-19T00:05:59-e8cda0a/plan9-386-0intro
2021-11-11T20:20:50-46b2fc0/plan9-arm
2021-09-07T19:39:04-d92101f/plan9-arm
2021-09-02T16:26:26-08588e6/plan9-arm
2021-08-24T22:10:11-5d863f8/plan9-arm
2021-04-29T13:24:00-858fdbe-eb3fe28/plan9-arm
2021-04-26T18:54:39-0d1280c/plan9-arm
2021-02-23T06:01:29-e521498/plan9-arm
@0intro, @millerresearch, @fhs: is this possibly a duplicate of #35456 and/or #22227?

These all seem to be glitches deep in the internals of the runtime, all different except that fatal error: bad g->status in ready occurs in two of them.
I have no theories.

greplogs -l -e '(?ms)plan9-.*^panic during panic' --since=2022-01-01
2022-11-09T17:33:07-7a92c4f/plan9-arm
2022-09-30T05:48:38-d7dbe01/plan9-arm
2022-08-30T21:30:15-248c34b-bd56cb9/plan9-386-0intro
2022-08-29T20:04:56-dbd0ce8/plan9-386-0intro
2022-08-15T18:40:54-1d66144/plan9-arm
2022-07-08T12:57:49-1dfab61-14abe8a/plan9-386-0intro
2022-03-27T18:38:56-018b78c/plan9-arm
2022-03-24T09:17:34-0ed31eb/plan9-arm
2022-03-21T19:10:35-6be52ab/plan9-386-0intro
2022-02-23T20:51:28-e94f7df/plan9-386-0intro
2022-01-12T01:49:33-d55c255-3d3f5d9/plan9-arm


Set module version to v1

What was changed

Set the mod version to v1
Why?

As we are not currently releasing v2 the mod version conflicts with the github release version, resulting in not being able to import the module in other projects
Checklist

Closes

How was this tested:

unit tests

Any docs updates needed?

Is this some kind of breaking change?

Is this some kind of breaking change?

as it was broken with /v2 as an import, it couldn't be used. This is a breaking change though should fix the importing for the actual use


fix(tooltip): update placement on content change

Since the recompute of position is a heavy calculation and given
that this happens only when tooltip is visible, ie position
is already calculated, we can reuse it to update the position when
widht/height changes.

Closing this PR due to no activity.

Resolved via #3435 .


Question about the RISC-V RV64X extension

Hi dear friends,
It's an honor to deliver the issue here, I'm doing ray tracing programs, which is a method of graphics rendering that simulates the physical behavior of light. I want to know if RISC-V has already supported graphics extension RV32X or RV64X ?  like simple ray tracing program below .
#include <iostream>

int main()
{
    // Image

    const int image_width = 256;
    const int image_height = 256;

    // Render

    std::cout << "P3\n" << image_width << ' ' << image_height << "\n255\n";

    // The pixels are written out in rows with pixels left to right.
    // The rows are written out from top to bottom.
    for (int j = image_height-1; j >= 0; --j) {
        std::cerr << "\rScanlines remaining: " << j << ' ' << std::flush;
        for (int i = 0; i < image_width; ++i) {
            auto r = double(i) / (image_width-1);
            auto g = double(j) / (image_height-1);
            auto b = 0.25;

            int ir = static_cast<int>(255.999 * r);
            int ig = static_cast<int>(255.999 * g);
            int ib = static_cast<int>(255.999 * b);

            std::cout << ir << ' ' << ig << ' ' << ib << '\n';
        }
    }
    std::cerr << "\nDone.\n";
}

thank you
best regards
William

Um ... that's not a raytracer?

Not an issue with vector spec, so closing.


Problems with requiring in Symfony 6.3 project | psr/http-message

Hi there,
I get the following error, while requiring your package via composer in a Symfony 6.3 project:
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires railsware/mailtrap-php * -> satisfiable by railsware/mailtrap-php[1.0.0, ..., 1.7.0].
    - railsware/mailtrap-php[1.0.0, ..., 1.7.0] require psr/http-message ^1.0 -> found psr/http-message[1.0, 1.0.1, 1.1] but the package is fixed to 2.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.
You can also try re-running composer require with an explicit version constraint, e.g. "composer require railsware/mailtrap-php:*" to figure out if any version is installable, or "composer require railsware/mailtrap-php:^2.1" if you know which you need.

It seems to be an issue with a sub dependency of psr/http-message.
Maybe you can check it out 🙌

@the-pulli Thanks for your feedback.
This issue will be fixed after the PR merge -> https://github.com/railsware/mailtrap-php/pull/15

Fix in the new release version - https://github.com/railsware/mailtrap-php/releases/tag/1.7.1


What is best way to compare 2 faces? [Question]

Hello.
I'm very noob in topic about face recognition and only second day playing with it.
I read acticles from author of these libraries 'Vincent Mühler' face-recognition.js and face-api.js tutorials on https://medium.com.
So I tried to compare 2 faces using opencv4nodejs and face-api.js in nodejs back-end.
But using face_api.computeFaceDescriptor(reference_image); is very slow.
All I need is compare only 2 faces. There is no need for training identity in my use case.
Please advice me what is best way to compare 2 faces only in nodejs back-end?
Thank you.

You are probably not using tfjs-node, which speeds things up a lot.

You were right, I wasn't using tfjs-node. Because it wasn't working in my current machine MacOs 10.11 El Capitan. There is some issue with __clock_time in tfjs-node compiled binary. But I tried to install on linux machine and CPU backend it is so much improved. Thank you.
Compute face descriptor performance: 188ms


selenoid-ui not loading running sessions with error

Hi,
I have both selenoid & selenoid-ui running as docker container on RHEL7.6. When I connect to the selenoid-ui remotely, it displays the UI but it is not loading any running session. Following
messages about status are logged in the container-log.
How can I resolve this issue?
$ docker logs -f selenoid-ui
2019/02/26 19:22:13 Listening on :8080
2019/02/26 19:22:41 can't get status (invalid character '<' looking for beginning of value)
2019/02/26 19:22:41 Client added. 1 registered clients
2019/02/26 19:22:43 can't get status (invalid character '<' looking for beginning of value)
2019/02/26 19:22:43 Removed client. 0 registered clients
2019/02/26 19:22:45 Client added. 1 registered clients
2019/02/26 19:22:45 can't get status (invalid character '<' looking for beginning of value)
2019/02/26 19:22:48 can't get status (invalid character '<' looking for beginning of value)
2019/02/26 19:22:48 Removed client. 0 registered clients
Thanks!
Amit

@amitverma12 this error message means Selenoid UI is receiving non-JSON content (e.g. HTML or XML) from Selenoid /status API. Try to do the following request:
$ curl http://selenoid-host.example.com:4444/status

And make sure that is returns JSON.

@vania-pooh - Thanks for your response. I am starting both selenoid & selenoid-ui as docker containers.
if I do curl for status on localhost I get xml with error but if I give the hostname, it returns a json.
I tried starting selenoid-ui with hostname as argument & it started giving
$  ./cm selenoid-ui start --args "--selenoid-uri=http://hostname:4444"
[tvisadm@da3d-gen-tvz-app005 ~]$ docker logs -f selenoid-ui
2019/02/27 16:21:42 Listening on :8080
2019/02/27 16:21:44 Client added. 1 registered clients
2019/02/27 16:21:50 can't get status (context deadline exceeded)
2019/02/27 16:21:55 can't get status (context deadline exceeded)
2019/02/27 16:22:00 can't get status (context deadline exceeded)
Do I need to give the hostname argument for starting selenoid-ui when I am using cm for starting both containers?

@vania-pooh - Thanks for your response. I am starting both selenoid & selenoid-ui as docker containers.
if I do curl for status on localhost I get xml with error but if I give the hostname, it returns a json.
I tried starting selenoid-ui with hostname as argument & it started giving
$ ./cm selenoid-ui start --args "--selenoid-uri=http://hostname:4444"
[user@hostname ~]$ docker logs -f selenoid-ui
2019/02/27 16:21:42 Listening on :8080
2019/02/27 16:21:44 Client added. 1 registered clients
2019/02/27 16:21:50 can't get status (context deadline exceeded)
2019/02/27 16:21:55 can't get status (context deadline exceeded)
2019/02/27 16:22:00 can't get status (context deadline exceeded)
Do I need to give the hostname argument for starting selenoid-ui when I am using cm for starting both containers?

@amitverma12 if both containers are running on the same host then defaults (http://selenoid:4444/) should work because CM automatically links Selenoid UI to Selenoid container and Selenoid container name is selenoid. So basically you need to somehow point Selenoid UI to Selenoid.

@vania-pooh - based on the ps output it seems to be linking correctly
root     13604 13581  1 16:46 ?        00:00:00 /selenoid-ui --selenoid-uri=http://selenoid:4444
But I'm getting the original issue with it
2019/02/27 16:46:37 Listening on :8080
2019/02/27 16:47:47 can't get status (invalid character '<' looking for beginning of value)
2019/02/27 16:47:48 Client added. 1 registered clients
Thanks!
Amit

@amitverma12 couldn't it be because of some proxies configured on your host?

@vania-pooh Yes, I do see a proxy configured on the vm to reach docker-hub. How can I work around it.
Thanks!

@amitverma12 not sure but Selenoid UI don't need any external Internet access.

@vania-pooh Ok, thanks for your help. We can close this issue.


Adding virtual meetings guide to digital accessibility toolkit

(Pick the language of your choice to fill out the following information. / Choisissez la langue de votre choix pour remplir l'information ci-dessous.)
What does this pull request (PR) do? / Que fait cette demande « pull » (PR)?
I added a new section in the Guides index.html, I called it "Virtual Collaboration" and I added there the link for the virtual meetings guide. I added the virtual meeting guide in the Guides folder. There is a French and an English version. This document has been accepted for publication, if you have any questions feel free to ask Martyna Marjanska at<EMAIL_ADDRESS>Additional information (optional) / Information additionnelle (optionel)
General checklist / Liste de contrôle générale
Make your own list for the purpose of your Pull request. /// Faites votre propre liste en fonction des besoins de votre demande « pull ».
I don't understand this part. But I verified the accessibility following WCAG. I also verified the translations. It would not hurt to double check with a screen reader because I was forced to use Narrator for this test.
Related issues / Requêtes associées
No related issues
Screenshots / Captures d'écrans
No screenshots

@kimviens Can we avoid using that many li (bullets) in the document?

Maybe using more p elements.

What do you think @martynka?


How to parse XML with meta, link etc. tags?

I trying to parse XML with cheerio. XML contains  tags. And the following script is return empty string in that case:
$('foo').find('link').text()
How to parse XML with tags ,  and similar?
Thanks in advance.

Thanks for bringing this up. I think it's being caused by a bug in htmlparser2. I've submitted an issue here: FB55/node-htmlparser#16

Thank you. Already use it. :)

But working with  tag remains incorrect.
var cheerio = require("cheerio"),
$ = cheerio.load('', { xmlMode: false });
$('root').append($('').text('foo'));
$('root').append($('').text('some text'));
console.log($.html());
The code above returns the following:
foosome text
instead
foosome text

@SlavaB xmlMode needs to be true.

It doesn't make any difference, true or false.. The result is the same.


rename detection without timeout

It seems the current implementation depends on a timeout. Is it possible to avoid this at least in some cases such as when the native watcher is used?

inotify has IN_MOVED_FROM and IN_MOVED_TO
Node's native fs.watch has an eventType which is either 'change' or 'rename'. It fires for both the old location and new location. Though it seems this is how the underlying inotify API works
Perhaps using inode number together with birthtime? (https://github.com/paulmillr/chokidar/issues/303#issuecomment-290450388)

Turns out that 'rename' fires twice. Once for the old location and once for the new location: https://stackoverflow.com/questions/55296834/how-to-use-nodejs-to-find-out-file-rename-event

I think it's basically impossible. The underlying APIs just don't support it

https://stackoverflow.com/questions/16204646/does-inotify-group-in-moved-from-and-in-moved-to
https://stackoverflow.com/questions/20300955/detecting-foreign-in-moved-from-renames-with-inotify

Actually, I think you can do this using fanotify:

The event FAN_RENAME may contain two information records. One of type FAN_EVENT_INFO_TYPE_OLD_DFID_NAME identifying the old directory entry, and another of type FAN_EVENT_INFO_TYPE_NEW_DFID_NAME identifying the new directory entry.

A couple issues with fanotify:

you'd need a native implementation to use it unless Node.js implements support for it
you can't use it within Docker without granting the CAP_SYS_ADMIN privilege, which is discouraged (https://github.com/fsnotify/fsnotify/issues/114#issuecomment-2123583255)

Yeah I think this is impossible right now given that a goal of the project is to have zero native dependencies like that (unlike stuff like chokidar).


Install includes to include/${project_name} and use more modern CMake

Part of ros2/ros2#1150
This installs includes to include/${PROJECT_NAME} to mitigate include directory search order issues when overriding packages in desktop.
Part of ament/ament_cmake#365
This removes ament_export_libraries and ament_export_include_directories as they're redundant with the exported CMake targets.
Part of ament/ament_cmake#292
This replaces an ament_target_dependencies() call with target_link_libraries().

CI (repos file build: --packages-above-and-dependencies class_loader test: --packages-select class_loader)

Linux
Linux-aarch64
Windows

All of the test failures are due to cpplint include order, otherwise CI LGTM.

cpplint errors are addressed on ros2, so I think we can merge this PR.


[multistage][draft] Fix aggregate type issues

clean up on type issues for agg.
several problem still needs to be fixed:

bool_and/bool_or
boolean support for some aggregates (v1 default cast to 0/1)
tests plans are generated with extra aggs that needs to be modified

Codecov Report

Merging #11068 (7cea611) into master (ed5dea9) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master   #11068     +/-   ##
=========================================
  Coverage    0.11%    0.11%
=========================================
  Files        2200     2146     -54
  Lines      118802   116261   -2541
  Branches    17991    17676    -315
=========================================
  Hits          137      137
+ Misses     118645   116104   -2541
  Partials       20       20

Flag
Coverage Δ

integration1temurin11
?

integration1temurin17
?

integration1temurin20
?

integration2temurin11
?

integration2temurin17
?

integration2temurin20
?

unittests1temurin11
?

unittests1temurin17
?

unittests1temurin20
?

unittests2temurin11
0.11% <0.00%> (+<0.01%)
:arrow_up:

unittests2temurin17
?

unittests2temurin20
?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

...erator/blocks/results/AggregationResultsBlock.java
0.00% <0.00%> (ø)

...aggregation/function/AggregationFunctionUtils.java
0.00% <0.00%> (ø)

...ation/function/BaseBooleanAggregationFunction.java
0.00% <0.00%> (ø)

...inot/query/runtime/operator/AggregateOperator.java
0.00% <0.00%> (ø)

...e/pinot/query/runtime/operator/FilterOperator.java
0.00% <0.00%> (ø)

...pinot/query/runtime/operator/HashJoinOperator.java
0.00% <0.00%> (ø)

...e/operator/LeafStageTransferableBlockOperator.java
0.00% <0.00%> (ø)

...untime/operator/MultistageAggregationExecutor.java
0.00% <0.00%> (ø)

...ry/runtime/operator/MultistageGroupByExecutor.java
0.00% <0.00%> (ø)

...inot/query/runtime/operator/TransformOperator.java
0.00% <0.00%> (ø)

... and 3 more

... and 56 files with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


No Santa hat

Does this issue occur when all extensions are disabled?: Yes

VS Code Version: all

The steps to get the Santa hat back (as mentioned in the "Looking forward…" section of #87440) do not work...

Sorry, this was removed once we enabled the ability for extensions to contribute Product Icon Themes, which can change the gear icon to whatever you want :)
see https://code.visualstudio.com/api/extension-guides/product-icon-theme


Json generated by sprinfox contains equivalent endpoints for different operations that gives error while parsing with swagger-ui

In my spring-mvc project I've two equivalent endpoints with different operations.
Say
http://api.myhost.com/users/{userId} -- GET to fetch user
http://api.myhost.com/users/{userId} -- PUT to update user
Now the json generated by springfox has two separate entries for these endpoints. One for get and one for put.
"paths": { 			"/users/{userId}": { 				"get": { 					"tags": ["my-controller"], 					"summary": "getUser", 					"operationId": "getUserUsingGET", 					"consumes": ["application/json"], 					"produces": ["*/*"], 					"parameters": [{ 						"name": "userId", 						"in": "path", 						"description": "userId", 						"required": true, 						"type": "string" 					}], 					"responses": { 						"200": { 							"description": "OK", 							"schema": { 								"$ref": "#/definitions/ApiResponseDtoÂ«booleanÂ»" 							} 						}, 						"401": { 							"description": "Unauthorized" 						}, 						"403": { 							"description": "Forbidden" 						}, 						"404": { 							"description": "Not Found" 						} 					} 				} 			}, 			"/users/{userId}": { 				"put": { 					"tags": ["my-controller"], 					"summary": "updateUser", 					"operationId": "updateUserUsingPOST_1", 					"consumes": ["application/json"], 					"produces": ["*/*"], 					"parameters": [{ 						"name": "userId", 						"in": "path", 						"description": "userId", 						"required": true, 						"type": "string" 					}], 					"responses": { 						"200": { 							"description": "OK", 							"schema": { 								"$ref": "#/definitions/ApiResponseDtoÂ«booleanÂ»" 							} 						}, 						"201": { 							"description": "Created" 						}, 						"401": { 							"description": "Unauthorized" 						}, 						"403": { 							"description": "Forbidden" 						}, 						"404": { 							"description": "Not Found" 						} 					} 				},
This json gets converted to following YAML code.
paths:   '/users/{userId}':     get:       tags:         - my-controller       summary: getUser       operationId: getUserUsingGET       consumes:         - application/json       produces:         - '*/*'       parameters:         - name: userId           in: path           description: userId           required: true           type: string       responses:         '200':           description: OK           schema:             $ref: '#/definitions/ApiResponseDtoÂ«booleanÂ»'         '401':           description: Unauthorized         '403':           description: Forbidden         '404':           description: Not Found   '/users/{userId}':     put:       tags:         - my-controller       summary: updateUser       operationId: updateUserUsingPOST_1       consumes:         - application/json       produces:         - '*/*'       parameters:         - name: userId           in: path           description: userId           required: true           type: string       responses:         '200':           description: OK           schema:             $ref: '#/definitions/ApiResponseDtoÂ«booleanÂ»'         '201':           description: Created         '401':           description: Unauthorized         '403':           description: Forbidden         '404':           description: Not Found
When I try to parse this payload with swagger-ui, I get the following error
Semantic error at paths./users/{userId} Equivalent paths are not allowed.

Don't really know how you got two endpoints. Thats not possible via springfox. You should see something like this
'/users/{userId}': {
     "get": {...},
     "put": {...}
 }

and not
'/users/{userId}': {
     "get": {...},
},
'/users/{userId}': {
     "put": {...}
 }

Thanks for the reply. Let me give you some more details.
In my java code I've 2 methods in my controller, say, getUserDetails() and updateUserDetails().
They are as follows
`@RequestMapping(value = "/users/{userId}", method = {RequestMethod.PUT})
@ResponseStatus(HttpStatus.OK)
public ApiResponseDto updateUserDetails(@PathVariable String userId, @RequestBody UserDto userDto)
@RequestMapping(value = "users/{userId}", method = RequestMethod.GET)
@ResponseStatus(HttpStatus.OK)
public ApiResponseDto<List> getUserDetails(@PathVariable String userId) `

any clue for this issue?

not had a chance to look at this

Hi.
Can I get some more help here, please.
Thanks.

Haven't had a look at this issue

This should be fixed in 2.7.0


Hooking VirtualProtect causes stack overflow

I'd like to inform you of a bug of mhook 2.4.
The bug causes stack overflow when hooking VirtualProtect and calling the original VirtualProtect from the hooked VirtualProtect, because VirtualProtect called from Mhook_SetHook function calls not the original VirtualProtect but the hooked VirtualProtect, therefore, the hooked VirtualProtect calls the original VirtualProtect recursively until stack overflow is occurred.
Here is simple sample code that causes stack overflow:
BOOL ( WINAPI RealVirtualProtect )( LPVOID lpAddress, SIZE_T dwSize, DWORD flNewProtect, PDWORD lpflOldProtect ) = (BOOL ( WINAPI )( LPVOID, SIZE_T, DWORD, PDWORD )) GetProcAddress( GetModuleHandleA( "kernel32.dll" ), "VirtualProtect" );
BOOL WINAPI HookVirtualProtect( LPVOID lpAddress, SIZE_T dwSize, DWORD flNewProtect, PDWORD lpflOldProtect )
{
return RealVirtualProtect( lpAddress, dwSize, flNewProtect, lpflOldProtect );
}
int main()
{
Mhook_SetHook( (PVOID*) &RealVirtualProtect, HookVirtualProtect );
return 0;
}
My workaround is to call the original VirtualProtect in Mhook_SetHook function.
Thank you in advance.

What if you want to hook NtVirtualProtectMemory? You can't use any alternatives for that :/ Is there a solution for this?

Yes, I know not only VirtualProtect but also VirtualProtectEx and NtProtectVirtualMemory cause stack overflow.
My workaround is to use NtProtectVirtualMemory instead of VirtualProtect in Mhook_SetHook and Mhook_Unhook. And then, if we want to hook NtProtectVirtualMemory, the 3rd and the 4th VirtualProtect in Mhook_SetHook should be replaced with the code of the trampoline as follows:
if ( fnOrigNtProtectVirtualMemory == *ppSystemFunction ) fnNtProtectVirtualMemory = (_NtProtectVirtualMemory) (PVOID) pTrampoline->codeTrampoline;
DWORD NumberOfBytesToProtect = sizeof(MHOOKS_TRAMPOLINE);
PVOID BaseAddress = pTrampoline;
fnNtProtectVirtualMemory( GetCurrentProcess(), &BaseAddress, &NumberOfBytesToProtect, dwOldProtectTrampolineFunction, &dwOldProtectTrampolineFunction);
fnNtProtectVirtualMemory and fnOrigNtProtectVirtualMemory must be brought from ntdlld.dll by GetProcAddress in advance.

Thank you, I will test it :) Could you please have a look at my other issue, LdrLoadDll? Hooking that API causes crash on my application, however I can't see any reason to do that.
In the current case VirtualProtect hooking fails, because the "hooker" uses this API too. But if I'm right, it isn't using any LoadLibrary or similar functions.
Thank you for your answer :)

Can you provide pieces of your code about LdrLoadDll ? I have no sample of it.
And you should get all necessary module library before hook LdrLoadDll .

Argh, sorry, it was my faut. I didn't check my code enough and I made a small mistake.
LdrLoadDll is an undocumented function. if you're interested, you can find infos about it here:
http://undocumented.ntinternals.net/source/usermode/undocumented functions/executable images/ldrloaddll.html
Thanks for your kindness :)

I have tested hooking LdrLoadDll. But my code did not crash.
My sample code is below. (Mhook is the original one: I don't modify anything.)
#include <Windows.h>
#include "mhook-lib/mhook.h"

typedef struct _LSA_UNICODE_STRING {
  USHORT Length;
  USHORT MaximumLength;
  PWSTR  Buffer;
} LSA_UNICODE_STRING, *PLSA_UNICODE_STRING, UNICODE_STRING, *PUNICODE_STRING;

VOID (WINAPI* RtlInitUnicodeString)( _Inout_   PUNICODE_STRING DestinationString,  _In_opt_  PCWSTR SourceString ) = (VOID (WINAPI*)( PUNICODE_STRING, PCWSTR )) GetProcAddress( GetModuleHandleA( "ntdll.dll" ), "RtlInitUnicodeString" );

//=========================================================================
// Get the current (original) address to the functions to be hooked
//
NTSTATUS (NTAPI* LdrLoadDll) ( PWSTR SearchPath, PULONG DllCharacteristics,	PUNICODE_STRING Name, PVOID* BaseAddress ) = (NTSTATUS (NTAPI*)( PWSTR, PULONG, PUNICODE_STRING, PVOID* )) GetProcAddress( GetModuleHandleA( "ntdll.dll" ), "LdrLoadDll" );
NTSTATUS (NTAPI* RealLdrLoadDll) ( PWSTR SearchPath, PULONG DllCharacteristics,	PUNICODE_STRING Name, PVOID* BaseAddress ) = (NTSTATUS (NTAPI*)( PWSTR, PULONG, PUNICODE_STRING, PVOID* )) GetProcAddress( GetModuleHandleA( "ntdll.dll" ), "LdrLoadDll" );
NTSTATUS NTAPI HookLdrLoadDll( PWSTR SearchPath, PULONG DllCharacteristics,	PUNICODE_STRING Name, PVOID* BaseAddress )
{
	OutputDebugString( Name->Buffer );
	return RealLdrLoadDll( SearchPath, DllCharacteristics, Name, BaseAddress );
}

//=========================================================================
// This is where the work gets done.
//
int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nShowCmd )
{
	// Set the hook
	if ( Mhook_SetHook( (PVOID*) &RealLdrLoadDll, HookLdrLoadDll ) ) {
		// Remove the hook
		UNICODE_STRING unicode_string;
		RtlInitUnicodeString( &unicode_string, L"A-dll.dll" );
		PVOID BaseAddress;
		LdrLoadDll( NULL, 0, &unicode_string, &BaseAddress );
		MessageBox( NULL, L"Pause...", L"Notification", MB_OK );

		Mhook_Unhook( (PVOID*) &RealLdrLoadDll );
	}

	return 0;
}

I came here from google. I get stack overflow error when calling original version of function as well. I documented everything on Stack Overflow: http://stackoverflow.com/q/28890309/607407
Is this a bug or is it simply impossible to call original function versions

I managed to hook VirtualProtect by using a flag that signals if the hooking engine is in the middle of the hooking process, exporting it from the lib and checking the flag from within my function hook. This prevents the recursion.


Insufficient TX fee

Description
Currently, when I try to submit a hbar transfer transaction, the JSON RPC relay shows the following error
[2023-12-04 03:19:55.289 +0000] ERROR (relay-eth/1306716 on ip-172-31-39-175): [Request ID: fefa21e3-340b-42c5-98e5-43314bef4e7f] Failed to successfully submit sendRawTransaction for transaction 0x02f8768201281d8459682f0086017988ee7700826050941b95f47e44008eac8592e5fc8ad9d62c98c8a1cf888ac7230489e8000080c080a09ee33dee8a124ab72505d7fe5538a92462268b5e46dadb0e633e9a01a901bb7aa040a6a6c3149f6f3602acc0fdbdb0f91a9cfb4a99f8bc8c4cde9a29b5cbd58ec7
    err: {
      "type": "SDKClientError",
      "message": "transaction<EMAIL_ADDRESS>failed precheck with status INSUFFICIENT_TX_FEE",
      "stack":
          Error: transaction<EMAIL_ADDRESS>failed precheck with status INSUFFICIENT_TX_FEE
              at SDKClient.<anonymous> (/home/ubuntu/hedera/hedera-json-rpc-relay/packages/relay/dist/lib/clients/sdkClient.js:115:40)
              at Generator.throw (<anonymous>)
              at rejected (/home/ubuntu/hedera/hedera-json-rpc-relay/packages/relay/dist/lib/clients/sdkClient.js:25:65)
              at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      "status": {
        "_code": 9
      },
      "validNetworkError": true
    }
[2023-12-04 03:19:55.289 +0000] ERROR (rpc-server/1306716 on ip-172-31-39-175): [Request ID: fefa21e3-340b-42c5-98e5-43314bef4e7f] Error invoking RPC: transaction<EMAIL_ADDRESS>failed precheck with status INSUFFICIENT_TX_FEE

Not sure what's causing it. I also reported another issue about inaccurate gas estimation but I am not sure 100% if it's related.
Steps to reproduce
Connect Metamask to HashIO and try to transfer Hbar to another account
Additional context
No response
Hedera network
testnet
Version
latest
Operating system
None

Hello @pathornteng, I've done a couple tests and was able to successfully send HBAR to accounts using MetaMask and HashIO Relay, both on Mainnet and Testnet, also tried to send to existing accounts, and new accounts (hollow) so it gets auto-created when sending HBAR (Tx Fee is higher when is a hollow account, since you as the sender needs to pay for the costs of creating the account) however, I was able to send HBAR on all scenarios on both testnet and mainnet.
Could you share specific steps to be able to reproduce your issue, or also, share a script you might be using?

Ah, I think it's resolved now. It looks like the network was super overwhelmed and the congestion pricing kicked in, leading this to this error. We can park this issue for now. I will reopen if it circles back.


Running Integration tests against Keycloak on OCP with Cross DC.

Description
Running Integration tests against an Keycloak on OCP with Cross DC.
We understand running all Keycloak tests with a given frequency will help us identify any unknown issues that we might introduce with the changes that are being done to Keycloak and ISPN with the Cross DC effort.
We have an option to run the integration tests that are Arquillian based using a remote mode, this is the Epic to explore the options and run the tests as needed.
Discussion
NA
Issues
No response
Motivation
No response

Is thi a duplicate of this: https://github.com/keycloak/keycloak-benchmark/issues/483


erc20-single-lp-token-balance & crucible-erc20-single-lp-token-balance

Changes proposed in this pull request:

add erc20-single-lp-token-balance strategy
add crucible-erc20-single-lp-token-balance strategy

These strategies are intended to allow for a balanced voting system for users holding a token vs lp token for the same token.
The difference between the two strategies is that the first is LP and token balances direct from wallet, the second is for balances of crucibles owned by the wallet of the user.

@ChaituVR please let me know if you have any requests
Thank you 🙏

Hi, @joehquak would be great if you can make separate PR for each strategy. it would be easy to review/integrate/maintain :)

Thank you @ChaituVR
Closing this PR in favour of 2 new PRs which will be incoming shortly 👍

@ChaituVR When you say 5 requests, if we nest a batch of contract calls within a request, is that 1 request?


If I want to make attention in background.

Hi,
Thank you for this task. If I want to make attention in the background that I need to change rather than the object. For example, in domain A and B, we have horse images, when translating from domain A - B, I want to keep the same horse in domain B but the background will be changed as domain A. How I can do that? Thank you in advance.

I have the same question with you. In other word, how the Attention Network can output a mask (attention map) to keep eye on the foreground object in unsupervised setup? According to the paper, the network architecture of Generators and Attention Networks are almost same except the final activation function. When the final activation function is sigmoid with output channel is 1, output of the network is attention map? I don‘t know how that works.
Moreover, Figure 7 in the paper shows Attention Network can focus on foreground object in early of training. It is amazing! The losses are adversarial loss and cycle-consistency loss during early of training. There are no label information guides the Attention Network to focus on foreground object.
I am looking forward to discussing with you and the author @deep0learning @AlamiMejjati


UnboundLocalError: local variable 'path' referenced before assignment

Hi,
I was using your model trainer, but with considerably less data than from the given example

Trying to train for 3 classes
Approx. 50 sentences in the training set for each class

and what parameters should I provide in order to make a functioning/effective model.
Normally when I run with the given parameters we get an error, and so we were wondering if there was a better way to go about doing that.
Errors include:

for the following paramters
{
"num_epochs": 1,
"batch_size": 20,
"num_filters": 32,
"filter_sizes": "3,4,5",
"embedding_dim": 50,
"l2_reg_lambda": 0.0,
"evaluate_every": 200,
"dropout_keep_prob": 0.5
}
logging.critical('Accuracy on test set is {} based on the best model {}'.format(test_accuracy, path))
UnboundLocalError: local variable 'path' referenced before assignment

try smaller batch_size and evaluate_every


Skip shift positions if from_position is None

Encountered some test data in which active reservations had no from_position and as a result their termination was not possible.

Codecov Report

Merging #328 (9a88f4d) into develop (82be674) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #328   +/-   ##
========================================
  Coverage    93.28%   93.28%
========================================
  Files          129      129
  Lines         4777     4782    +5
  Branches       438      439    +1
========================================
+ Hits          4456     4461    +5
  Misses         233      233
  Partials        88       88

Impacted Files
Coverage Δ

application_form/services/queue.py
96.72% <100.00%> (+0.29%)
:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


Long delay between tapping "unlock" and seeing wait screen with tips

Current Experience:
https://user-images.githubusercontent.com/1348549/215220496-cd242614-4707-4c1c-ae97-144c36d38bc2.mov
Acceptance Criteria

When tapping "unlock" the user should see the "loading" screen as soon as possible
if the PIN is incorrect, the "loading" screen is hidden, the "enter PIN" screen is displayed with some message about the PIN being incorrect

Notes:

The delayed is attached to the wallet key hash validation and unlocking of the wallet by AFJ

I just wanted to take a stab at the User Story and Gherkin Acceptance Criteria for this one.
AS A BC Wallet Credential Holder,
I WANT opening the app to a ready state to be as smooth and quick as possible,
SO THAT I feel confident the app is stable, well designed, and I don't question the apps ability to do the things I really need the app to do for me.
Given the user is <opening BC Wallet>
When they authenticate in the app
Then there is no perceivable delay between authenticating and initializing the app

Examples:
   | opening the app |
   | first opening and onboarding |
   | already onboarded and opening the closed app |
   | already onboarded and opening a backgrounded app |

I don't understand the second point in the Acceptance Criteria.  Sounds like a bug if it doesn't do that by default.

we haven't noticed this behaviour anymore


[spdlog] Update to 1.9.2

Describe the pull request

What does your PR fix?
Updates spdlog to 1.9.2

Which triplets are supported/not supported? Have you updated the CI baseline?
as before, Yes

Does your PR follow the maintainer guide?

Yes

Thanks for the update!


Invert serializer from user default to protocol extension

This was an idea just for discussion and to see any thoughts on this.. moving the logic from the UserDefaults extension to a UserDefaultsSerializable protocol extension. The idea was to let the serializer implement the divergence in handling instead putting the burden on the UserDefaults.
An example of this could be seen with the one-off URL handling where it's in the protocol override instead of in the fetch logic. The trick was to enforce a static protocol conformance on the type and let the UserDefaults be injected into the static function:
public protocol UserDefaultsSerializable {
    associatedtype StoredValue

    var storedValue: StoredValue { get }

    init(storedValue: StoredValue)

    static func fetch(forKey defaultName: String, from userDefaults: UserDefaults) -> Self
    static func fetchOptional(forKey defaultName: String, from userDefaults: UserDefaults) -> Self?
    static func set(_ value: Self, forKey defaultName: String, from userDefaults: UserDefaults)
}

Then the default conformance for all serializers:
extension UserDefaultsSerializable {
    public var storedValue: Self { self }

    public init(storedValue: Self) {
        self = storedValue
    }

    public static func fetch(forKey defaultName: String, from userDefaults: UserDefaults) -> Self {
        fetchOptional(forKey: defaultName, from: userDefaults)!
    }

    public static func fetchOptional(forKey defaultName: String, from userDefaults: UserDefaults) -> Self? {
        guard let object = userDefaults.object(forKey: defaultName) as? StoredValue else { return nil }
        return Self(storedValue: object)
    }

    public static func set(_ value: Self, forKey defaultName: String, from userDefaults: UserDefaults) {
        userDefaults.set(value.storedValue, forKey: defaultName)
    }
}

Then one-off's like URL could be done in the protocol override conformances instead of in the UserDefaults extension:
extension URL: UserDefaultsSerializable {
    public static func fetchOptional(forKey defaultName: String, from userDefaults: UserDefaults) -> Self? {
        userDefaults.url(forKey: defaultName)
    }

    public static func set(_ value: Self, forKey defaultName: String, from userDefaults: UserDefaults) {
        userDefaults.set(value, forKey: defaultName)
    }
}

This swaps the property wrapper logic from a UserDefaults extension to the type's protocol extension:
@propertyWrapper
public struct WrappedDefault<T: UserDefaultsSerializable> {
    private let userDefaults: UserDefaults
    public let key: String

    public var wrappedValue: T {
        get { T.fetch(forKey: key, from: userDefaults) }
        set { T.set(newValue, forKey: key, from: userDefaults) }
    }

    public init(
        forKey key: String,
        defaultValue: T,
        userDefaults: UserDefaults = .standard
    ) {
        self.key = key
        self.userDefaults = userDefaults
        self.userDefaults.register(defaults: [key: defaultValue.storedValue])
    }
}

The tests still pass and added more, but wondering if this is a good direction or any ideas on how to break this.

Thanks for exploring this design @basememara 👍🏼 💯
It's an interesting idea, but I find the inversion a bit awkward (because of the static func). It is slightly more burdensome for conformers of UserDefaultsSerializable too.
I like the nicer approach for handling URL types, but I do not think this change is worth it. If every type required a unique implementation, then I would find this approach more appealing. But the existing one-off hack for URL is ok with me since it is hidden away. 😊
If anything, I would be more inclined to make the UserDefaults extensions public instead of the changes here, but I don't see a strong case (or demand from users) for that right now.

Oh also, I noticed you opened this from a fork.
As a collaborator, you can work directly from a branch in this repo now if you like. 😊 The PR workflow is a bit nicer that way.

Nice thx for giving it some thought. The static didn't sit well with me either, especially on a protocol.
The protocol extension may still work as a default implementation even without this change, so the conformity is optional, but still testing what the implications are of using Self which I believe is determined at runtime so it's ok, but also the implicit implementation maybe confusing or seem magic a bit.
Great feedback thanks. I'll open a separate PR for the typo in the test.


Declarative

@SriSuryaTej Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.


Update outgoing variants for Test Suites and add additional sanity checking

Update the Test Suites, Java and Jacoco Plugins to use the new attributes and categories proposed:

A new category of verification
A new attribute type "org.gradle.verificationtype"
Removal of the Sources interface, DocType constants, Usage.Verification
Update ban list in https://github.com/gradle/gradle/issues/19007 to include anything in the new org.gradle.verificationtype attribute type

These additional checks below might be better broken out into separate issues, depending on the complexity:

Ensure if multiple variants with identical attributes are registered, an error is generated at registration time
Ensure if multiple suites of the same TestType are registered, an error is generated at registration time

This is to be done after #19007.

Attributes spreadsheet: https://docs.google.com/spreadsheets/d/1ilVMAhUQvT_EZNpkPo4fo2kCFWUWoDRdo30UvTk-7So/edit#gid=0

I'm breaking the additional checks out to:
https://github.com/gradle/gradle/issues/19097
https://github.com/gradle/gradle/issues/19097
so they don't hold up merging this.


Test Set

Every sequence of the downloaded test set only has gt in the first picture, but nothing else. How to get the precise and success rate without all gt in the test set?

An evaluation server is being developed for the test set. Once it is completed, the link will be included in this repository.

Thank you！I wonder if you can tell me the approximate online time of the evaluation server.


Still getting in-app news from Franz

Just want to understand our direction

@MichaelBui This is an announcement from Franz, not Ferdi. We have fixed the issue to make sure Ferdi only gets announcements coming from Ferdi with https://github.com/getferdi/ferdi/commit/73ecb428bb699a2dfb27f88e3d9b803c71d881b7. Sorry for the inconvenience.

To further answer your question, Ferdi will forever be free. We have decided to let the community fund the development if they will/can using Open Collective. No freemium model, purely donation based.

Thanks for being free 👍
Anyway, I'm using v5.4.0-beta.4 😅

Thank you for specifying your version. I looked at it a bit more and realized we haven't changed getLatestNews to fetch news only from Ferdi. It was still fetching it based on the server/API you use:
https://github.com/getferdi/ferdi/blob/a0ac009c44b210df4d79ffedbdd892fe7302d22e/src/api/server/ServerApi.js#L461

Fixed in d0afab6 and will ship in the next release.
Thank you for reporting!


Travis CI failing.

Travis CI build fails with:
--- FAIL: TestCC (1.74s)

runner_test.go:44:

runner_test.go:45: running test/0005-for.c.bin failed. exit status 246

runner_test.go:44:

runner_test.go:45: running test/0007-dowhile.c.bin failed. exit status 238

Local machines pass all tests, investigate and fix.

I was wrong, tests fail on my machine, caused by bug in comparisons, fix incoming.

I think the problem is caused by a size mismatch. For the current x86_64 backend, I think it is safe to store all values in full 64 bit precision, and sign extend then on load, or truncate them on store.


Screenshot taken one day later than assigned day in config

Description of Bug
From user post on Nexus, "Screenshots being taken a day after the set "Days" value in the config" reported on 23 May 2020: https://www.nexusmods.com/stardewvalley/mods/4779?tab=bugs&BH=1
I set mine to "Sundays, AnySeason" but for some reason it only takes the screenshot at Monday mornings. I've tried changing it to other days but the same thing happens (it happens the day after the set value).
SMAPI log: https://smapi.io/log/85ee94c4613f4d93acd5fd1e8b5a06fc
Steps to Reproduce
Steps to reproduce the behavior:

Set the day for the screenshot to be taken to any particular day of the week in the config
In-game, see that the screenshot is not taken on the designated day. Then the next in-game day, see that the screenshot is taken.

Expected behavior
Screenshots should be taken on the exact day specified by the user in their config file.

Posting this Issue for documentation purposes, but I believe the code to fix this is already written (just need to test it again and make sure everything looks good).
This issue was caused due to the starting day of the week being incorrect in the code. In Stardew Valley, the first day of the week is a Monday and the last day is Sunday. However, the code represents the first day of the week as Sunday with the last day being Saturday. Again, this should already be fixed if I recall from when I was last able to work on the mod, just need to test it and make any changes if needed.

Possibly related, "The weekday setting doesn't work." reported on Nexus originally on 24 Jul 2022: https://www.nexusmods.com/stardewvalley/mods/4779?tab=bugs&BH=1

This is now completed and has been added as part of the 3.0 release! Closing.


Google OAuth login hangs with unresponsive "Sign in with Google" pop up form

I have a Flutter web app that is using Supabase authentication via this package.
The email/password authentication route is working fine, and I am now trying to add support for Google OAuth authentication. I've setup the client ID's and secret in Supabase, and the redirect API's in GCP.
However, when I run the following code....
Future<bool> loginGoogle() async {
   try {
     final aResponse = await Supabase.instance.client.auth.signInWithOAuth(Provider.google);
     return true;
   } catch (e) {
     return false;
   }
 }

... the 'Sign in with Google' pop up forms appears, but I cannot type anything into the 'Email or phone' text control. In fact all the controls on the Google pop up form are unresponsive.

Any suggestions On what is going wrong?

Hello! Have you ever encountered and successfully resolved this issue? I'm facing a similar problem and would appreciate your insights or guidance.

I did.   The problem is caused by the fact that Google OAuth only works with production code releases.    In my case its a Flutter web app,  where if I run it from the IDE the Google login always fails.  If I build the webapp using 'flutter build web' and then deploy it to my server then the Google login works fin.