Fixed #35028 -- Disabled server-side bindings for named cursors on psycopg>=3.

While we provide a cursor_factory based on the value of the server_side_bindings option to psycopg.Connection it is ignored by the cursor method when name is specified for QuerySet.iterator() usage and it causes the usage of psycopg.ServerCursor which performs server-side bindings.
Since the ORM doesn't generates SQL that is suitable for server-side bindings when dealing with parametrized expressions a specialized cursor must be used to allow server-side cursors to be used with client-side bindings.
Thanks Richard Ebeling for the report.

@apollo13, @francoisfreitag, @dvarrazzo I'd appreciate eyes on this one.
The ServerCursor definition was weird to me as I found the postgres.Cursor, postgres.ClientCursor, and postgres.ServerCursor quite confusing.

class
bindings
server cursors

Cursor
server
no

ClientCursor
client
no

ServerCursor
server
yes

What confused me is that the client prefix of ClientCursor means client-side bindings while in the case of ServerCursor it means server cursor. In other words, the prefixes cannot be considered as variant of the same dimension, one is about bindings the other is about the nature of the cursor itself.

Well, at least we named our cursor with server bindings ServerBindingCursor and not ServerCursor :D I somewhat agree that a flag would be nice(r) since it limits the amount of classes that you have in the end. Not sure if it is possible to change in psycopg and deprecate the confusing naming accordingly :)

Well, at least we named our cursor with server bindings ServerBindingCursor and not ServerCursor :D I somewhat agree that a flag would be nice(r) since it limits the amount of classes that you have in the end. Not sure if it is possible to change in psycopg and deprecate the confusing naming accordingly :)

right not sure either, I'm happy to help if you believe it's the case @dvarrazzo.

If we calculate all possible outcome (client/server-side binding, sync/async, client/server-side cursors) one would end up with eight classes?

Exactly, it could be cut down to 4 if it allowed a parameter to be specified and we could avoid extra changes to create_cursor if it was possible to specify a server_cursor_factory as well during connect.

Hello! I started writing something and it turned out to be more rambling than I though. So here's a brain dump for you :yum:
Psycopg 3.0 started with Cursor (client-side, server-binding) and ServerCursor (server-side, server-binding). In 3.1 we introduced the ClientCursor (client-side, client-binding) and, because the class name is its interface (the user must either instantiate it directly or use the class name as cursor_factory) we didn't want to call it with a punitive decoration of prefix such as ClientBindingClientSideCursor.
As a bounus (probably of no interest for Django), psycopg 3.2 will introduce the RawCursor: client-side, server-binding (hence a variation of Cursor) but with "raw" postgres parameters ($1, $2, ...). Of course all of the above has an Async version.
The inconsistency between ClientCursor meaning client-binding and ServerCursor meaning server-side wasn't lost on me, but I couldn't find better names, ones not eliciting a WTF from the final users. However it's only inconsistent if you want to assume that Client and Server in these names mean the opposite role. If you take them as independent flags (server- or non-server-side, client- or non-client-binding), stating the difference from the psycopg 3 default (which is client-side-stored, server-side-binding) there is method in the madness. This is also consistent in the way we use Async, as a present or missing prefix, and we don't prefix with Sync every async counterpart.
We are not planning to have a server-side, client-binding cursor (whose name ServerClientCursor would actually be ridiculous) because client-binding is mostly useful for DDL, whose statement don't support server-side binding; however these statements don't return tuples, so no cursor is open on the server side. It is occurring now to me that there could be some utility for a RawServerCursor, although not in Django.
I am not really looking into changing the naming in psycopg, but, if I should, I would better define these present/missing prefixes, and rename ServerCursor -> ServerSideCursor and ClientCursor -> ClientBindingCursor, leaving Cursor as it is; (the strategy would allow an upgrade path using aliases). What is definitely not planned is to unify these classes and use constructor flags instead, because the implementation would be heavily penalised and static typing would become murkier (server-binding cursors would expose a mogrify() method but it couldn't do anything but raising TypeError, for instance). Of course nothing stops you from adding your own factory functions in Django, taking parameters such as server_side, client_binding, raw_placeholder... and dispatching to the right psycopg class.
If, for Django, the standard is client-binding cursors, I assume you can have your own set of  flag prefixes, with more explicit names if you like, so that you would have:

Django class
Storage
Binding
wrapping Psycopg class

Cursor
Client
Client
ClientCursor

ServerSideCursor
Server
Client
ServerCursor

ServerBindingCursor
Client
Server
Cursor

However, I wonder (but please excuse me if I am making the wrong assumption, I am not overly familiar with the latest implementation and usage of Django, and my intention is not to be inflammatory):

does it really make sense for Django to user server-side cursors altogether? Django's query lifetime is tied to the one of a web page so cursors have a short lifetime anyway. Why do you use, or give the possibility to the users, to create server-side cursors?

does it really make sense for Django to use server-binding cursors? I mean, for sure it makes sense for performance reasons (prepared statements, memory footprint) but, if certain queries break because the ORM doesn't generate suitable SQL, I think the choice is either to fix the ORM or to use client-binding cursors everywhere, not to delegate to the user the responsibility to choose the cursor, given the fact that Django's PoV is to abstract the user from the database as much as feasible.

About the last point, I'll give you another historical reference. I started writing the Psycopg 3 Django backend when psycopg 3.0 was in its development phase: writing the backend was actually very important to figure out many corner cases of the behaviour and the interface, having scuffles with variadic functions, etc. At the time, the utility of ClientCursor wasn't obvious, because I had little experience about many of the server differences and the changes required by using server-binding cursors. It's only past 3.0 release that emerged that using psycopg.sql in every place would have been pretty much a deal-breaker for many projects where query performance doesn't matter and the queries are a mix of DDL, DML, queries (for instance in Ansible).
With the current experience, if I could git rebase the history of Psycopg and Django and how they intertwined, and considering Django interaction with the database (my understanding of it), I would organise things so that:

Django DDL (schema creation, migrations) always use client-binding cursors;
Django DML/Query (ORM queries) always use server-binding cursors (assuming GROUP BY queries are fixed);
no use of server-side cursors;
no choice given to the user within the Django interface; use psycopg raw object for everything else.

Hello! I started writing something and it turned out to be more rambling than I though. So here's a brain dump for you 😋

Haha, thank you -- that is a lot to digest.

As a bounus (probably of no interest for Django), psycopg 3.2 will introduce the RawCursor: client-side, server-binding (hence a variation of Cursor) but with "raw" postgres parameters ($1, $2, ...). Of course all of the above has an Async version.

Yeah, we won't support those :)

If you take them as independent flags (server- or non-server-side, client- or non-client-binding), stating the difference from the psycopg 3 default (which is client-side-stored, server-side-binding) there is method in the madness.

Shrug, while it makes sense, it is not immediately obvious (especially since the prefix is solely Server/Client and not something like ServerSide/ClientBinding). I understand that naming is hard and I can't come up with anything better.

We are not planning to have a server-side, client-binding cursor (whose name ServerClientCursor would actually be ridiculous) because client-binding is mostly useful for DDL, whose statement don't support server-side binding; however these statements don't return tuples, so no cursor is open on the server side. It is occurring now to me that there could be some utility for a RawServerCursor, although not in Django.

I guess it depends. We had to switch the default in Django to client-binding because as of now we cannot express everything otherwise, but we still need to keep support for server-side.

I am not really looking into changing the naming in psycopg, but, if I should, I would better define these present/missing prefixes, and rename ServerCursor -> ServerSideCursor and ClientCursor -> ClientBindingCursor, leaving Cursor as it is; (the strategy would allow an upgrade path using aliases).

No plans to tell you what to do, and I fully understand your rationale. All I can say is that the existing naming is kinda confusing :)

What is definitely not planned is to unify these classes and use constructor flags instead, because the implementation would be heavily penalised and static typing would become murkier (server-binding cursors would expose a mogrify() method but it couldn't do anything but raising TypeError, for instance).

Makes sense, thanks.

If, for Django, the standard is client-binding cursors, I assume you can have your own set of flag prefixes, with more explicit names if you like, so that you would have:
Django class 	Storage 	Binding 	wrapping Psycopg class
Cursor 	Client 	Client 	ClientCursor
ServerSideCursor 	Server 	Client 	ServerCursor
ServerBindingCursor 	Client 	Server 	Cursor

Yes, if I am not miss-reading it, this is what this PR does already.

However, I wonder (but please excuse me if I am making the wrong assumption, I am not overly familiar with the latest implementation and usage of Django, and my intention is not to be inflammatory):

No worries, we appreciate any outside view.

does it really make sense for Django to user server-side cursors altogether? Django's query lifetime is tied to the one of a web page so cursors have a short lifetime anyway. Why do you use, or give the possibility to the users, to create server-side cursors?

We basically only use it for .iterator() (https://docs.djangoproject.com/en/5.0/ref/models/querysets/#iterator) to avoid loading the whole result in memory. This was introduced as a performance optimization (it exists for years). Whether this feature is heavily used, I cannot tell.

does it really make sense for Django to use server-binding cursors? I mean, for sure it makes sense for performance reasons (prepared statements, memory footprint) but, if certain queries break because the ORM doesn't generate suitable SQL, I think the choice is either to fix the ORM or to use client-binding cursors everywhere, not to delegate to the user the responsibility to choose the cursor, given the fact that Django's PoV is to abstract the user from the database as much as feasible.

Don't mean to be inflammatory, but I have to pass that one back to you. When I took up your PR for psycopg3 in Django the only option in psycopg was server-binding cursors. Part of my motivation for supporting psycopg3 was to give the psycopg project a rather large exposure where people would start using psycopg3 heavily. So we did our best to support server-side bindings and were rather relieved (when we figured out we couldn't possibly handle all edge cases in our ORM easily) when you (re)introduced (compared to v2) client-side bindings. The user cannot really choose the cursor in Django though. It is a setting and you either use server-side or client-side bindings. I guess if client-side bindings had existed by the time we worked on finalizing the PR we wouldn't ever had exposed server-side bindings (at least not in the initial revision). Since we already had adjusted many things to support server-side bindings it seemed to be a waste to throw that away again, and as you said there should be some performance gains (and maybe more efficient wire repr?) -- so the work is maybe not in vain. But we didn't really benchmark that.

About the last point, I'll give you another historical reference. I started writing the Psycopg 3 Django backend when psycopg 3.0 was in its development phase: writing the backend was actually very important to figure out many corner cases of the behaviour and the interface, having scuffles with variadic functions, etc. At the time, the utility of ClientCursor wasn't obvious, because I had little experience about many of the server differences and the changes required by using server-binding cursors. It's only past 3.0 release that emerged that using psycopg.sql in every place would have been pretty much a deal-breaker for many projects where query performance doesn't matter and the queries are a mix of DDL, DML, queries (for instance in Ansible).

ACK, I figured since you worked on the Django backend that you determined that server-side bindings where still the way to go, which is why I didn't ask for client-side bindings early on. When I was at the point to ask about them, the ansible folks already had you in favor of them.

if I could git rebase the history of Psycopg and Django and how they intertwined

Hehe, I fear that won't be possible. And to be honest I am not that unhappy with the outcome. We do default to client-binding cursors by default with the option to completely switch Django over to server-binding cursors. I think parts (schema stuff etc) use the SQL classes to basically emulate a client-binding cursor, so in essence what you are recommending. We cannot switch to server-binding cursors by default yet (some ORM queries are still problematic), but at least we do have the option for users to try them on their project while giving them a simple setting to switch back to the working client-bindings (which will become even more important once we drop psycopg2 support which is the "other way" to get back the old behavior).
Either way, thank you for your hard work on psycopg and your help here -- much appreciated.

If you take them as independent flags (server- or non-server-side, client- or non-client-binding), stating the difference from the psycopg 3 default (which is client-side-stored, server-side-binding) there is method in the madness.

Shrug, while it makes sense, it is not immediately obvious (especially since the prefix is solely Server/Client and not something like ServerSide/ClientBinding). I understand that naming is hard and I can't come up with anything better.

Thank you for the conversation all the same. I have improved the description of our different cursors in the cursors doc page.
I also said:

We are not planning to have a server-side, client-binding cursor (whose name ServerClientCursor would actually be ridiculous) because client-binding is mostly useful for DDL, whose statement don't support server-side binding; however these statements don't return tuples, so no cursor is open on the server side. It is occurring now to me that there could be some utility for a RawServerCursor, although not in Django.

However later, in the summary table, I wrote:

If, for Django, the standard is client-binding cursors, I assume you can have your own set of flag prefixes, with more explicit names if you like, so that you would have:

Django class
Storage
Binding
wrapping Psycopg class

Cursor
Client
Client
ClientCursor

ServerSideCursor
Server
Client
ServerCursor

ServerBindingCursor
Client
Server
Cursor

...this is wrong, because Django's ServerSideCursor wants to be client-binding but psycopg's ServerCursor is server-binding, so it cannot be a simple wrapper as the others. It's not difficult to implement one, for instance using a ClientCursor.mogrify() return... but is it worth? Because:

does it really make sense for Django to user server-side cursors altogether? Django's query lifetime is tied to the one of a web page so cursors have a short lifetime anyway. Why do you use, or give the possibility to the users, to create server-side cursors?

We basically only use it for .iterator() (https://docs.djangoproject.com/en/5.0/ref/models/querysets/#iterator) to avoid loading the whole result in memory. This was introduced as a performance optimization (it exists for years). Whether this feature is heavily used, I cannot tell.

Uhm... this feature really makes little sense to me. I don't see any memory to be saved by using a result that must be all loaded in memory anyway to render a web page, and up to 2k records are fetched anyway in the first page. But, because DISABLE_SERVER_SIDE_CURSORS defaults to false, people iterating on an aggregation recordset are exposed to bug 35208.
So, I agree with @charettes in this comment: I would set DISABLE_SERVER_SIDE_CURSORS to True, or to "not server_side_binding" to avoid an "accidental server-side binding".

We do default to client-binding cursors by default with the option to completely switch Django over to server-binding cursors. I think parts (schema stuff etc) use the SQL classes to basically emulate a client-binding cursor, so in essence what you are recommending. We cannot switch to server-binding cursors by default yet (some ORM queries are still problematic), but at least we do have the option for users to try them on their project while giving them a simple setting to switch back to the working client-bindings (which will become even more important once we drop psycopg2 support which is the "other way" to get back the old behavior).

Thank you for clarifying your perspective here, and yes, it makes sense to have introduced most of the infrastructure to support server-side binding, which at the moment remains an opt-in feature, until the ORM provides complete support for all the queries generated (and I wonder if I couldn't give you a hand in this project).

@dvarrazzo thank you for chiming in that's really insightful.

Uhm... this feature really makes little sense to me. I don't see any memory to be saved by using a result that must be all loaded in memory anyway to render a web page, and up to 2k records are fetched anyway in the first page.

While the ORM is used in the context of web page rendering it's also used by long running management commands and background tasks that might benefit from this feature. It was considered valuable enough to be enabled by default almost 7 years ago now https://github.com/django/django/pull/7587 so the question it less about should we and more how can we in my opinion as there might be applications in the wild that rely on it.
With that in mind...

It's not difficult to implement one, for instance using a ClientCursor.mogrify() return...

Could you confirm that this approach seem sane to you? Have we missed anything obvious?

Thank you for clarifying your perspective here, and yes, it makes sense to have introduced most of the infrastructure to support server-side binding, which at the moment remains an opt-in feature, until the ORM provides complete support for all the queries generated (and I wonder if I couldn't give you a hand in this project).

I think your help would be very welcome but I'm not sure we can realistically achieve this work without considering breaking the expressions api (documented api to define objects that generate (sql: str, params: tuple)) without significant investment. While the grouping and ordering part of the SQL compiler has ways to treat references to SELECT members differently from knowing my way in this area it's no going to straightforward to make changes particularly because the generated SQL needs to be not only valid for Postgres but for SQLite, MySQL, and most notably Oracle as well. Even if the focus is entirely on getting the ORM to generate server-bindable SQL for Postgres only we still need to make sure we don't break anything for all the other backends. Things like ordering modifier (nulls last emulation), transforms by parametric order by, etc comes to mind.
In the current state of things psycopg>=3 risks to not be usable at all on Django 4.2 applications that requires the use of server-side cursors to be used as unless we make an exception we missed the backport window.

I have improved the description of our different cursors in the cursors doc page.

Thanks, that looks like a great improvement to have!

We basically only use it for .iterator() (https://docs.djangoproject.com/en/5.0/ref/models/querysets/#iterator) to avoid loading the whole result in memory. This was introduced as a performance optimization (it exists for years). Whether this feature is heavily used, I cannot tell.

Uhm... this feature really makes little sense to me. I don't see any memory to be saved by using a result that must be all loaded in memory anyway to render a web page, and up to 2k records are fetched anyway in the first page. But, because DISABLE_SERVER_SIDE_CURSORS defaults to false, people iterating on an aggregation recordset are exposed to bug 35208.

That is not true (not for Django at least), there is no need to load everything into memory to render a webpage. Assuming the WSGI server is spec conforming we can send chunked responses in a streaming fashion where no more than a single chunk is kept in memory in Django or the WSGI server (well maybe more than a chunk depending on how some buffers are used, but you get the idea). Granted this requires you to be careful and usually is only done for single views where you'd require the memory savings, but it is doable. Also as Simon pointed out, the ORM is not solely used to serve webpages.

We do default to client-binding cursors by default with the option to completely switch Django over to server-binding cursors. I think parts (schema stuff etc) use the SQL classes to basically emulate a client-binding cursor, so in essence what you are recommending. We cannot switch to server-binding cursors by default yet (some ORM queries are still problematic), but at least we do have the option for users to try them on their project while giving them a simple setting to switch back to the working client-bindings (which will become even more important once we drop psycopg2 support which is the "other way" to get back the old behavior).

Thank you for clarifying your perspective here, and yes, it makes sense to have introduced most of the infrastructure to support server-side binding, which at the moment remains an opt-in feature, until the ORM provides complete support for all the queries generated (and I wonder if I couldn't give you a hand in this project).

I'd like to point out that you are and have been super helpful already. As long as it is not annoying you, we'd like to continue to ping you here and there on our issues (and in exchange help fixing issues where we can in psycopg -- though it usually just works and you have been quick to merge my PRs anyways). Full support of server-side bindings will be a bit harder though as Simon already pointed out. I feel like we are slowly reaching the boundaries of what we can do within our boundaries of multi database support and backwards compat.

Thanks y'all :+1: We seem to have a solution, but no one is quite happy with how it looks. Am I right? If yes, shouldn't we wait and remove it from the review queue? or maybe we want to move it forward as a temporary fix :thinking: Sorry, for being torn :upside_down_face:

Oh, I am happy with the solution as it is. The rest was just us babbling about a perfect world.

Thanks y'all 👍 We seem to have a solution, but no one is quite happy with how it looks. Am I right?

Hello!
No, I don't think so, I think this is an ok solution for a thorny problem with delicate backwards compatible implication but in marginal scenarios... I think it's good to go.
Thank you very much :slightly_smiling_face:

@charettes Thanks for the patch :+1:
@apollo13 @dvarrazzo Thanks for reviews :star2:


How to train LoFTR on custom dataset?

I know there's doc regarding this question, but in all fairness it looks a lot harder to setup and to get all of the metadata necessary, is there a step-by-step guide?
Something similar to YOLO, so that this neural network can achieve its' full potential. If someone is willing to leave a valuable comment, it would mean a world to me.

I have a similar problem, do you have a solution?Is there any low-cost way to build loftr receivable training data！


C++ EH interop broken with libc++abi

I'm still in the process of debugging problems on OpenBSD and I managed to track down the initial failure to the fact that on OpenBSD, we are using libc++ as the C++ standard library. Under libc++, std::type_info does not have virtual members such as ::__do_upcast() or, more importantly, ::__do_catch() (those seem to be a GNU thing, and not part of the C++ standard, afaict). This was tripping up the typeinfo_test.cc check and causing it to not know what to link as the C++ runtime (which meant that we subsequently were missing symbols when linking executables). I have a branch that tweaks CMake to detect that and work around it, but obviously all the C++ EH interop tests are still failing. Example:
(gdb) run
Starting program: /home/vagrant/build/libobjc2/Test/CXXExceptions
Error while reading shared library symbols:
Dwarf Error: wrong version in compilation unit header (is 4, should be 2) [in module /usr/libexec/ld.so]

Program received signal SIGBUS, Bus error.
0x000008fd3df381a4 in __llvm_retpoline_r11 () from /usr/lib/libc++abi.so.1.0
(gdb) bt
#0  0x000008fd3df381a4 in __llvm_retpoline_r11 () from /usr/lib/libc++abi.so.1.0
#1  0xdfdfdfdfdfdfdfdf in ?? ()
#2  0x000008fd3df3e8b2 in unwind_phase2_forced (uc=Variable "uc" is not available.
) at /usr/src/lib/libcxxabi/../libunwind/src/UnwindLevel1.c:277
#3  0x000008fd3df3e7e7 in _Unwind_Resume (exception_object=0x8fcf3e4c660) at /usr/src/lib/libcxxabi/../libunwind/src/UnwindLevel1.c:386
#4  0x000008fa6f96546a in finally () from /home/vagrant/build/libobjc2/Test/CXXExceptions
#5  0x000008fa6f9654c6 in main () from /home/vagrant/build/libobjc2/Test/CXXExceptions
(gdb) frame 3
#3  0x000008fd3df3e7e7 in _Unwind_Resume (exception_object=0x8fcf3e4c660) at /usr/src/lib/libcxxabi/../libunwind/src/UnwindLevel1.c:386
386     /usr/src/lib/libcxxabi/../libunwind/src/UnwindLevel1.c: No such file or directory.
        in /usr/src/lib/libcxxabi/../libunwind/src/UnwindLevel1.c
Current language:  auto; currently minimal
(gdb) p *exception_object
$1 = {exception_class =<PHONE_NUMBER>2891098079, exception_cleanup = 0xdfdfdfdfdfdfdfdf, private_1 =<PHONE_NUMBER>2891098079, private_2 =<PHONE_NUMBER>2891098079}

Yes, this is a known issue.  libsupc++ and libcxxrt are the only supported C++ runtimes.  I suggest encouraging OpenBSD to switch to libcxxrt, libc++abi is not compatible.

I've looked into this a bit, and it turns out that the libc++ ABI is not that hard to support. The differences I discovered are as follows:

It uses CLNGC++ as the exception class instead of GNUCC++
Deciding whether a handler catches an exception is achieved via a can_catch() virtual method instead of __do_catch() (we can use one to implement the other).
Instead of deriving directly from std::type_info, libc++abi intersperses a __cxxabiv1:__shim_type_info class (this is the one defining can_catch()) for all its type info implementations.

I was able to hack together a working implementation, with the main issue being the fact that __shim_type_info does not inherit the constructor from  std::type_info. (In fact none of the derived type_info classes in libc++abi have any constructors – apparently they all rely on the data structures being filled by the compiler, as we do with gnustep::libobjc::__objc_class_type_info. As a result, the compiler is very unhappy when trying to setup the gnustep::libobjc::__objc_id_type_info (we can neither call a constructor from the base class nor initialize the __type_name member).
For now, I've worked around that issue by keeping std::type_info as the nominal base class for our type_info implementations and fudging with the vtable layout until it matched the one for __shim_type_info (😢) .
So I'm now already so far into ‘this can't be so hard’-territory that I'm willing to commit to working out the rest of this and making sure that it continues to work (it should be reasonably easy to extend the build matrix for Azure Pipelines to include a build the links against libc++). What do you think?

Sounds good.  I looked at this a couple of months ago and came to a similar conclusion: it's possible, but I probably can't be bothered to work on it.  Happy to merge something that works.
Note that it's libc++abi, not libc++ that is the issue: libc++ with libcxxrt works fine, so you'd need to install libc++ built with libc++abi.  I think the LLVM repo is that we use for CI has libc++ built in that configuration, so you can probably enable it with the matrix thing.

Note that it's libc++abi, not libc++ that is the issue

Yes of course, I've changed the description to reflect that.

We’d also be very interested in this. After much back and forth I now came to the conclusion that there’s no good way to use libcxxrt on Android, as the shared C++ runtime library comes precompiled using libc++abi.
I’ve given the "libc++-hack" branch a try and with it throwing/catching an exception in an ObjC++ file works great! I’m happy to do additional testing or anything else to help finalize this.

@ngrewe would you be able to rebase or merge master into the libc++-hack branch? I just noticed it’s missing some commits, including fixes for weak ref handling that we’re running into with the branch.
Other than that the libc++abi integration seems to be working well for us.


Using request body (json payload) for DELETE http requests

DELETE /api/stacks/{ID} uses request body payload.
It is a little bit unusual to utilize a request body for DELETE http requests. Many HTTP client libraries don't support it, so it forces a developer to construct requests from low level. Another thing, I guess that some popular web servers if use them as proxy, cut body for DELETE requests as they do for GET by default, so it requires additional configuration for them.

Adding reference: https://github.com/zalando/lizzy/blob/release2.0/lizzy/swagger/lizzy.yaml#L182-L186
Good point. We will have to review that.

From rfc2616:

The presence of a message-body in a request is signaled by the
inclusion of a Content-Length or Transfer-Encoding header field in
the request's message-headers. A message-body MUST NOT be included in
a request if the specification of the request method (section 5.1.1)
does not allow sending an entity-body in requests. A server SHOULD
read and forward a message-body on any request; if the request method
does not include defined semantics for an entity-body, then the
message-body SHOULD be ignored when handling the request.

So unless the body is explicitly forbidden (and it isn't for DELETE), then it's allowed. If an HTTP client library or proxy don't support the body in DELETE requests then it's a bug on their side.

From the HTTP wiki page:
In June 2014, the WG released an updated six-part specification obsoleting RFC 2616:
...

RFC 7231, HTTP/1.1: Semantics and Content

(HTTP/1.1): Semantics and Content, https://tools.ietf.org/html/rfc7231

A payload within a DELETE request message has no defined semantics;
sending a payload body on a DELETE request might cause some existing
implementations to reject the request

But, it is kind of your API design decision, and you of course can use payload for DELETE requests. I just pointed out that it is unusual.

Hey @jmcs @rafaelcaricio, anything more to do here? Otherwise, let's close? cc @kaygorodov

I just want to point out that HTTP DELETE without a payload is an incomplete design.  Every multi-user web service needs a concept of Optimistic Concurrency if the users share resources.  The only way to prevent one user from overwriting the changes of another user is with Optimistic Concurrency.  The only way to get Optimistic Concurrency is by passing a row version or time stamp in the DELETE request.
Further, atomic deletes are a requirement for most professional systems.  If a POST can atomically add some set of resources (let's say stocks in a request to BUY), then you MUST support the ability to atomically delete that same resource (you can't possibly delete some of the stocks in a BUY and leave open the possibility that they aren't all deleted).
The REST API spec is incomplete/brain damaged without the explicit ability to provide a payload in a DELETE call.

The only way to get Optimistic Concurrency is by passing a row version or time stamp in the DELETE request.

That sounds like metadata which would be more appropriately indicated using request headers rather than a payload. Consider the parallel case of PUT, where the payload should indicate the desired state of the resource, not pre-conditions for the action being taken.

That sounds like metadata which would be more appropriately indicated using request headers rather than a payload. Consider the parallel case of PUT, where the payload should indicate the desired state of the resource, not pre-conditions for the action being taken.
It's not metadata.  In a larger sense, the revision is part of the identity of the resource.

I would put them in the URL parameters.
Time stamp
DELETE /resource/1234?timestamp="2019-11-13T22:12:19.435Z"
or row version
DELETE /resource/1234?row_version=2

We all know how to use the Query parameter.  My understanding is that this topic has to do with the proper interpretation of REST and, to my point, is REST a complete specification for realistic, LOB APIs.

@DRAirey1

I just want to point out that HTTP DELETE without a payload is an incomplete design
...
Further, atomic deletes are a requirement for most professional systems.  If a POST can atomically add some set of resources (let's say stocks in a request to BUY), then you MUST support the ability to atomically delete that same resource (you can't possibly delete some of the stocks in a BUY and leave open the possibility that they aren't all deleted).

You've got a completely messed up view of what HTTP is and what it should be used for. As Jim Webber pointed correclty out HTTP is a protocol whose application domain is the transfer of documents over the Web. It is NOT your application protocol! Some side effects of the document management causes some business activities to occur, but at it's core HTTP still just sends a document per request around and in that regard HTTP's operation are well-fitting. You place a document somewhere, you get a unique identifier in form of a resource locator (URL/URI) back and can then perform further document management operations on that resource, such as removing it.
If you need to remove something within that document you either can send a new version of that document to the server and ask it to replace the document with the new one (-> PUT), send some instructions the server should perform to modify the resource to a desired state (-> PATCH) or send a document the server knows how to act upon accordingly (-> POST). DELETE is for sure the wrong operation as this basically asks the server to remove the link between the URI and the resource. It doesn't even guarantee that the actual document is removed from its storage location to start with, just that a further GET operation triggered on the same URI doesn't return the previously stored document.
The POST operation is defined to process the payload according to the server's own semantics. At no point does the HTTP spec state that it has to process the request atomically! PATCH does, but not POST. In addition to that, HTTP works best on single resources, not on batches! You might create multiple resources at once but how do you inform a client about that? Send multiple Location headers within the response? Use one Location header with multiple URIs in it? Both of these attempts are also not standard conform and thus lack the support for interoperability.

Every multi-user web service needs a concept of Optimistic Concurrency if the users share resources

HTTP has something similar to optimistic locking present. Please read through RFC 7232
So, please,  before stating that HTTP is poorly defined, read through the spec, grasp what its purpose is and then reevaluate and formulate your statement as currently you seem to lack proper knowledge of the protocol you complain about.

My understanding is that this topic has to do with the proper interpretation of REST and, ..., is REST a complete specification for realistic, LOB APIs without the ability to delete the same resources that have a row version?

REST isn't a specification, REST is  a thesis of an architectural style which provides some long-term benefits over RPC-based solutions such as the freedom to evolve the service over time without having to fear breaking clients due to changes done to the server side. The ultimate goal in a REST architecture is that a service is able to serve a plethora of clients, especially those not under your direct control, and a single client is able to operate with plenty of different APIs without having to constantly being modified. This is basically only possible if the whole architecture, including APIs, intermediaries and clients, is carefully designed and implemented. It is to easy to introduce coupling and therefore a lot of discipline is required before the benefits a REST architecture advocates are visible.
All REST therefore does is define a set of constraints that if followed constantly will path the way to a system that is highly decoupled, failure tollerant and evolvable. The details of the actual communication are still kept at the transport level, i.e. HTTP in almost all cases. And as mentioned before, HTTP basically ships just documents around but does not trigger or invoke business processes directly. We put some business level semantics onto some of the document management processes and trigger certain business level rules and/or actions based on such events. Hence, neither REST nor HTTP can and should be blamed here.


Reject unrecognized options:

Rererencing https://github.com/tc39/proposal-realms/issues/160 from @erights:
Some of the options that we will want the spec to specify (indeed, most of these are already in the draft spec), the shim cannot implement. This is an implementation limit of the shim as compared with the spec it emulates. When such an implementation limit is hit, rather than deviate silently from specified behavior, the shim should raise an implementation-limit error to say that such a limit was hit.

Fixed


Add way to remove arbitrary element

Yeah, it's O(n) and will need to be protected with mutexes, but still useful when it's a rare case...

For now I guess you can loop through pushing and popping until you get it


Issue submitting guidelines

Thanks @RobertoNovelo but we already have a https://github.com/LightTable/LightTable/blob/master/CONTRIBUTING.md guide. If you're interested in helping, check out https://github.com/LightTable/LightTable/blob/master/CONTRIBUTING.md#code-contributions


Add Genshin Impact notice

The way to run Genshin is not only extremely gray-zone, it violates the ToS. As playing on Linux could very well get the developer to hard-ban a high-investment account, users should be warned in order for them to make that choice with the right information.

The note is in the wrong format, but I'll add something like this later and probably also mark the game as broken

All right then. Should I keep this open or create an Issue instead?

I think I'll just reopen #461 ^^

Then I'll close this. Thanks!


[SPARK-18718][TESTS] Skip some test failures due to path length limitation and fix tests to pass on Windows

What changes were proposed in this pull request?
There are some tests failed on Windows due to the wrong format of path and the limitation of path length as below:
This PR proposes both to fix the failed tests by fixing the path for the tests below:

InsertSuite
Exception encountered when attempting to run a suite with class name: org.apache.spark.sql.sources.InsertSuite *** ABORTED *** (12 seconds, 547 milliseconds)
    org.apache.spark.sql.AnalysisException: Path does not exist: file:/C:projectsspark	arget	mpspark-177945ef-9128-42b4-8c07-de31f78bbbd6;
    at org.apache.spark.sql.execution.datasources.DataSource$$anonfun$14.apply(DataSource.scala:382)
    at org.apache.spark.sql.execution.datasources.DataSource$$anonfun$14.apply(DataSource.scala:370)
    at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:241)

PathOptionSuite
- path option also exist for write path *** FAILED *** (1 second, 93 milliseconds)
  "C:[projectsspark	arget	mp]spark-5ab34a58-df8d-..." did not equal "C:[\projects\spark\target\tmp\]spark-5ab34a58-df8d-..." (PathOptionSuite.scala:93)
  org.scalatest.exceptions.TestFailedException:
      at org.scalatest.Assertions$class.newAssertionFailedException(Assertions.scala:500)
      at org.scalatest.FunSuite.newAssertionFailedException(FunSuite.scala:1555)
  ...

UDFSuite
- SPARK-8005 input_file_name *** FAILED *** (2 seconds, 234 milliseconds)
  "file:///C:/projects/spark/target/tmp/spark-e4e5720a-2006-48f9-8b11-797bf59794bf/part-00001-26fb05e4-603d-471d-ae9d-b9549e0c7765.snappy.parquet" did not contain "C:\projects\spark\target\tmp\spark-e4e5720a-2006-48f9-8b11-797bf59794bf" (UDFSuite.scala:67)
  org.scalatest.exceptions.TestFailedException:
    at org.scalatest.Assertions$class.newAssertionFailedException(Assertions.scala:500)
    at org.scalatest.FunSuite.newAssertionFailedException(FunSuite.scala:1555)
  ...

and to skip the tests belows which are being failed on Windows due to path length limitation.

SparkLauncherSuite
Test org.apache.spark.launcher.SparkLauncherSuite.testChildProcLauncher failed: java.lang.AssertionError: expected:<0> but was:<1>, took 0.062 sec
  at org.apache.spark.launcher.SparkLauncherSuite.testChildProcLauncher(SparkLauncherSuite.java:177)
    ...

The stderr from the process is The filename or extension is too long which is equivalent to the one below.

BroadcastJoinSuite
04:09:40.882 ERROR org.apache.spark.deploy.worker.ExecutorRunner: Error running executor
java.io.IOException: Cannot run program "C:\Progra~1\Java\jdk1.8.0\bin\java" (in directory "C:\projects\spark\work\app-20161205040542-0000\51658"): CreateProcess error=206, The filename or extension is too long
    at java.lang.ProcessBuilder.start(ProcessBuilder.java:1048)
    at org.apache.spark.deploy.worker.ExecutorRunner.org$apache$spark$deploy$worker$ExecutorRunner$$fetchAndRunExecutor(ExecutorRunner.scala:167)
    at org.apache.spark.deploy.worker.ExecutorRunner$$anon$1.run(ExecutorRunner.scala:73)
Caused by: java.io.IOException: CreateProcess error=206, The filename or extension is too long
    at java.lang.ProcessImpl.create(Native Method)
    at java.lang.ProcessImpl.<init>(ProcessImpl.java:386)
    at java.lang.ProcessImpl.start(ProcessImpl.java:137)
    at java.lang.ProcessBuilder.start(ProcessBuilder.java:1029)
    ... 2 more
04:09:40.929 ERROR org.apache.spark.deploy.worker.ExecutorRunner: Error running executor

  (appearently infinite same error messages)

...

How was this patch tested?
Manually tested via AppVeyor.
Before
InsertSuite: https://ci.appveyor.com/project/spark-test/spark/build/148-InsertSuite-pr
PathOptionSuite: https://ci.appveyor.com/project/spark-test/spark/build/139-PathOptionSuite-pr
UDFSuite: https://ci.appveyor.com/project/spark-test/spark/build/143-UDFSuite-pr
SparkLauncherSuite: https://ci.appveyor.com/project/spark-test/spark/build/141-SparkLauncherSuite-pr
BroadcastJoinSuite: https://ci.appveyor.com/project/spark-test/spark/build/145-BroadcastJoinSuite-pr
After
PathOptionSuite: https://ci.appveyor.com/project/spark-test/spark/build/140-PathOptionSuite-pr
SparkLauncherSuite: https://ci.appveyor.com/project/spark-test/spark/build/142-SparkLauncherSuite-pr
UDFSuite: https://ci.appveyor.com/project/spark-test/spark/build/144-UDFSuite-pr
InsertSuite: https://ci.appveyor.com/project/spark-test/spark/build/147-InsertSuite-pr
BroadcastJoinSuite: https://ci.appveyor.com/project/spark-test/spark/build/149-BroadcastJoinSuite-pr

Test build #69672 has started for PR 16147 at commit b0074b5.

Build started: [TESTS] org.apache.spark.sql.sources.InsertSuite
Build started: [TESTS] org.apache.spark.sql.sources.PathOptionSuite
Build started: [TESTS] org.apache.spark.sql.UDFSuite
Build started: [TESTS] org.apache.spark.launcher.SparkLauncherSuite
Build started: [TESTS] org.apache.spark.sql.execution.joins.BroadcastJoinSuite
Diff: https://github.com/apache/spark/compare/master...spark-test:17D17BC5-186C-4952-9467-7F75C9C2ABCC

Test build #69672 has finished for PR 16147 at commit b0074b5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/69672/
Test PASSed.

Merged build finished. Test PASSed.

It sounds like some of the failures aren't just due to tests, but due to some functionality that doesn't work on Windows? or do I misundesrtand it?
It still may be the right thing to do to disable these tests so that the rest of the tests are a little more meaningful for Windows, but if that's right, then, this is disabling tests that point out things that don't work, which is a bit funny.

It seems some tests are being failed in some cases when running executor as process (e.g. local-cluster). In this case, it fails because simply the classpath is too long (https://gist.github.com/HyukjinKwon/5bc81061c250d4af5a180869b59d42ea) and I thought the workaround is to shorten this by, for example, installing right under C:\ and etc on Windows.
If we build and run tests on Windows, I guess we would easilly meet the test failures and I persuaded myself this might be the limitation of OS and therefore the tests should be disabled.
I am happy to figure out to shortening the classpath via using, for example, wildcard or related path and etc if you think this is something to be fixed.

Hi @srowen I just manually downloaded spark 2.1.0 rc1 and then tested as below:
I tested both
.\bin\spark-shell.cmd --master local-cluster[2,1,1024]

spark.stop()
val spark = SparkSession.builder().master("local-cluster[2,1,1024]").appName("testing").getOrCreate()

with the codes below:
val df1 = spark.createDataFrame(Seq((1, "4"), (2, "2"))).toDF("key", "value")
val df2 = spark.createDataFrame(Seq((1, "1"), (2, "2"))).toDF("key", "value")
df1.join(df2, df1("key") === df2("key") && df1("value") > df2("value"), "inner").show()

+---+-----+---+-----+
|key|value|key|value|
+---+-----+---+-----+
|  1|    4|  1|    1|
+---+-----+---+-----+

In this case, the executor was executed with the command below:
"C:\Program Files\Java\jdk 1.8.0_91\bin\java" -cp C:\spark\bin\..\conf\;C:\spark\bin\..\jars\*
-Xmx1024M -Dspark.driver.port=49235 org.apache.spark.executor.CoarseGrainedExecutorBackend
--driver-url spark://CoarseGrainedScheduler@<IP_ADDRESS>:49235 --executor-id 0 --hostname <IP_ADDRESS>
--cores 1 --app-id app-20161205174932-0000 --worker-url spark://Worker@<IP_ADDRESS>:49258

which is pretty short. So, I think this might be okay.

Thank you so much @srowen .

Just FYI, I ran some more tests for each package for myself and grepped local-cluster before submitting this PR and It seems there are not many same instances. If I face the same problems frequently a lot, I would definitely try to work around within the test codes in the future.

(note to myself) I tracked down where the paths are added. It seems mostly they are added in StandaloneSchedulerBackend.scala#L86, AbstractCommandBuilder.java#L145-L189 and SparkBuild.scala#L826-L827 via AbstractCommandBuilder.java#L202

Merged to master


Frontend: Allow custom resource actions/columns in generic ResourceTable component

Environment

Python version: 3.8.10
MrMap version: b23b7e2d74eb6f9690d6a09e33455f428b2e44e1

User Story
As a developer, I want to have a configuration possibility in the ResourceTable component, so that I can add resource-specific actions/columns.

Acceptance criteria

[x] Given I am a developer and I am applying the ResourceTable component, when I configure additional actions, I want that these are shown in the actions column.
[x] Given I am a developer and I am applying the ResourceTable component, when I configure additional columns, I want that these are displayed in the table.

A solution has been merged into react-prototyping and now into develop:
  ...
  return <RepoTable
            repo={repo}
            columns={columns}
            actionRef={actionRef as any}
            onAddRecord='/registry/services/add'
          />;

The actions are controlled via the action column in the columns property: If omitted, default actions (New + Edit) are provided automatically based on the 'onAddRecordandonEdit` properties.
If provided, the explicit configuration of the actions applies:
export const WmsTable = (): JSX.Element => {
  const actionRef = useRef<RepoActionType>();
  const columns: RepoTableColumnType[] = [{
    dataIndex: 'id',
    title: 'ID'
  }, {
    dataIndex: 'title',
    title: 'Titel'
  }, ... {
    key: 'actions',
    title: 'Aktionen',
    valueType: 'option',
    render: (text: any, record:any) => {
      return (
        <>
            <Button
              danger
              size='small'
              onClick={ () => {
                actionRef.current?.deleteRecord(record);
              }}
            >
              Löschen
            </Button>
        </>
      );
    }
  }];

  return <RepoTable
            repo={repo}
            columns={columns}
            actionRef={actionRef as any}
            onAddRecord='/registry/services/add'
          />;
};


Error

Hi!
Could you please give me some assistance to understand what's going wrong on my config?

Please include the """minimum""" (A small one, consisting of really minimal elements needed to reproduce this bug) configuration file that can be reproduced independently and the steps required to reproduce it. I am not a psychic.

Sorry.
I added the image with my config settings, as you can see on my first message.
What I know is when I turn the dictionary off, the bug is gone.
In the last version of the plugin, it's never happened. So, I don't know enough about your modifications on the current version of the plugin to figure it out what to do or where is the cause of it.
Again, I'm sorry to not provide you a great info about. Despite that, could you please guide me to find something that is value to help you fix this?

Hi, had the same error as you. It's probably the updated cmp-dictionary settings (document -> command -> wn). My Arch linux didn't have the "wn" (wordnet) command. I Installed the AUR wordnet-common and wordnet-cli packages.
That fixed it, gl!

Thank you, @stamina!
I use the Xerolinux, it's an Arch base linux as well.
I installed wordnet-common and wordnet-cli and the Error was gone!
Thanks a lot!
Have a nice day you all! 👍🏾

Thank you, @stamina!
I use the Xerolinux, it's an Arch base linux as well.
I installed wordnet-common and wordnet-cli and the Error was gone!
Thanks a lot!
Have a nice day you all! 👍🏾


Add App.PageForm interface

Describe the problem
Firsr,  thank you so much for adding the form property on the page store! It's a huge feature for me.
I was wondering if the property could be better typed than any. I understand that we can't use the auto-generated type ActionData for this, but could users have the freedom to type it as they see fit, the same as with App.PageData? Is this something that can be done?
Describe the proposed solution
Add the App.PageForm interface.
Alternatives considered
No response
Importance
nice to have
Additional Information
No response

You can import the generated types in any file:
src/routes/+page.server.ts
import type { Actions } from './$types';

export const actions: Actions = {
	default: async () => {
		return { test: 1, value: '' };
	}
};

src/lib/MyForm.svelte

I also lean towards keeping it as any, for the reasons outlined above. Even if you decide on a common denominator, it's not checked that what you type actually is correct (on contrast to App.PageData).

Oh I ser! I hadn't realized how it differed from App.PageData. Still, the proposed solution is a bit cumbersome, especially if you have deeply nested routes with complex names, which isn't uncommon in Sveltekit. I wonder if this could be made easier?

Not sure if this works correct but the idea is using a global namespace you can extend with the actions you want to use elsewhere
// src/routes/some/[deep]/route/+page.server.ts
import type { ActionData, Actions } from './$types'

declare global {
  namespace Forms {
    MySigninForm: ActionData
  }
}

export actions: Actions = {
 ...
}

// src/lib/MySigninForm.svelte

$: form = $page.form as Forms.MySigninForm

It would be possible to extend that namespace in the generated types so you only have to do:
$: Form = $page.form as Forms["some/[deep]/route"]

@david-plugge Hey the global Forms namespace is actually a really nice solution! Thanks a lot for that.
@dummdidumm I didn't mean for this to turn into a support discussion, so feel free to close it if you don't think this will be added to the framework.

Closing as implementing this similarly to App.PageData would be too limiting, since all your (probably very diverse) forms will have to have a common denominator, and if we allow defining an interface without checking it is adhered to it's not of much use.


Support module mapping files per system

I don't think that this is very relevant now that modules can be set with the -S option from the command line.


Cross platform builds

Getting arch from pkgs.stdenv.hostPlatform.uname.processor makes it hard to do cross platform builds. A quick fix would be to take it as input in the derivations (like pkgs currently) and use the above as the default value. That way, consumers can pass in different archs and pkgs and cross build.

Hi @roshanr95,
you are right about that. The reasoning why I did choose to not include this for cross-platform builds is that I believe the native build and the cross-build are not going to be identical. I have not checked that, but I believe that should be the case given different tool chains used to produce each of them.
If that assumption is correct, then arguing about builds being the same, produced from the same source/definition becomes more of a head ache.
I have an item to explore if this is something that can be solved easily, but I did not have time to get around to this.

Makes sense. The way I usually do my cross builds is using Qemu and --system on Nix, especially if reproducibility is important.

Cross-compilation support has been implemented as part of https://github.com/aws/aws-nitro-enclaves-sdk-bootstrap/pull/39.
It can be used by instantiating nixpkgs as appropriate for the desired host platform, and passing it as the pkgs parameter accepted by the top-level derivation, like so:
$ uname --machine
x86_64
$ nix-build -A all --arg pkgs '(import ./nixpkgs.nix { }).pkgsCross.aarch64-multiplatform'
/nix/store/gpxqr7gbm3029dwvjc1075692nbzvn1d-enclaves-blobs-aarch64
$ tree result
result
└── aarch64
    ├── Image
    ├── Image.config
    ├── init
    ├── linuxkit
    └── nsm.ko

1 directory, 5 files
$ file result/aarch64/init
result/aarch64/init: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, stripped

Note that currently, the produced binaries are still different between native and cross-compiled builds, exactly as @foersleo suspected previously.

As I mentioned above, I build cross-platform using qemu, which usually gives the same binaries as native builds (albeit veeery slow). It's quite simple to set up (on linux anyway) via https://github.com/tonistiigi/binfmt. In Nix, you have to set the system correctly (e.g. using --system with a flake) and pass in the right nixpkgs (e.g. using nixpkgs.${system}.pkgs with a flake). Can confirm that it gives the same binaries for the kernel builds as well and furthermore, it matches the artifacts from before this PR as well.
Just leaving this here in case people are looking for identical cross platform builds and don't mind the performance hit.


Choose output directory

Hello :)
First thanks for making this plugin. I just discovering it after having used datapage_gen and would like to check if it's better, but there are a few things which prevent me from switching to your plugin, so I'm opening this issue (and another one)
I'd like to be able to specify the output directory, because my site has 2 languages and I need to generate files for both english and french layouts, and out put in both the "en" and "fr" directories of my website structure.
As I understand it from the README the output is obviously the same if I use the same data file.
Thanks in advance.

I had a similar case with a site in both Arabic and English. I just made them separate collections, see:
https://github.com/mnyrop/sudanphoto/blob/ecd72e6a81fcf6879fa8e8090bf3419d381d587a/_config.yml#L27
In essence, naming the collections archive/en and archive/ar generate them to the directories en and ar within archive respectively. You would just have one data file (csv, json, or yaml) for each collection (e.g., archive-en.csv and archive-ar.csv:
_data/
|__ archive-en.csv
|__ archive-ar.csv
_archive/
|__en/
|____ page1.md
|____ page2.md
|__ar/
|____ page1.md
|____ page2.md

OK, sounds fair. Thanks for the reply :)


[GDNative] const functions cannot be registered

Godot 3.0 pre-alpha master
Windows 10 64 bits MSVC
When making a GDNative library using the C++ kit, I figured out C++ functions declared as const produce compilation errors when used in method bindings. As a result, none can be declared as such (mostly getters, calculations and things that should be guaranteed to be read-only). This is supported when making a module, I expected it to work in GDNative too.

Ups, my bad. Report here, has nothing to do with GDNative, only with the bindings. https://github.com/GodotNativeTools/cpp_bindings


Sandbox Dataset Choices for Admins

Definition: "admin", a user with the is_staff model attribute marked as True.
On the query section on the visualization page, admins should have access to datasets contained in SandboxDB.

Admins can now access sandbox datasets. Datasets and versions that are from SandboxDB are prefixed with sDB: right now, but that can easily be changed in settings.py at theSANDBOX_PREFIX variable.
Anything prefixed with SANDBOX_PREFIX is routed to SandboxDB instead.


[Scope Change] - codeunit 7204 "CDS Setup Defaults"

Base application version 21.2.49946.51093 (DK Localization)
For codeunit 7204 "CDS Setup Defaults" the reset functions for the dataverse intgration are tagget with [Scope('OnPrem')]:
[Scope('OnPrem')]
procedure ResetSalesPeopleSystemUserMapping

[Scope('OnPrem')]
procedure ResetCustomerAccountMapping

[Scope('OnPrem')]
procedure ResetVendorAccountMapping

[Scope('OnPrem')]
procedure ResetContactContactMappin

[Scope('OnPrem')]
procedure ResetCurrencyTransactionCurrencyMapping

[Scope('OnPrem')]
procedure ResetPaymentTermsMapping

[Scope('OnPrem')]
procedure ResetShipmentMethodMapping

[Scope('OnPrem')]
procedure ResetShippingAgentMapping

This makes it hard to extend the standard fields mapping as described here because the ResetCustomerAccountMapping procedure does not run..
Please remove the onPrem scope from the Reset* procedures - and also from the sales integration..
Thank you - and merry christmas!

Request rejected. Subscribe to the events that are dedicated for overriding the defaults:
"CDS Setup Defaults" codeunit
// after someone chooses 'Use Default Synchronization Setup' on Dataverse Connection Setup.
// here is where you can code your defaults
[IntegrationEvent(false, false)]
local procedure OnAfterResetConfiguration(CDSConnectionSetup: Record "CDS Connection Setup")
begin
end;

// more granular event for Customer-Account mapping
[IntegrationEvent(false, false)]
local procedure OnAfterResetCustomerAccountMapping(IntegrationTableMappingName: Code[20])
begin
end;

// more granular event for Vendor-Account mapping
[IntegrationEvent(false, false)]
local procedure OnAfterResetVendorAccountMapping(IntegrationTableMappingName: Code[20])
begin
end;

// more granular event for Contact mapping
[IntegrationEvent(false, false)]
local procedure OnAfterResetContactContactMapping(IntegrationTableMappingName: Code[20])
begin
end;

"CRM Setup Defaults" codeunit
// after someone chooses 'Use Default Synchronization Setup' on Microsoft Dynamics 365 Connection Setup.
// here is where you can code your defaults
[IntegrationEvent(false, false)]
local procedure OnAfterResetConfiguration(CRMConnectionSetup: Record "CRM Connection Setup")
begin
end;
// more granular events for each mapping

[IntegrationEvent(false, false)]
local procedure OnResetItemProductMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetOpportunityMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetPriceListHeaderPricelevelMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetPriceListLineProductPricelevelMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetResourceProductMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetSalesInvoiceHeaderInvoiceMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetSalesInvoiceLineInvoiceMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetSalesOrderMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetBidirectionalSalesOrderMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

[IntegrationEvent(false, false)]
local procedure OnResetBidirectionalSalesOrderLineMappingOnAfterInsertFieldsMapping(IntegrationTableMappingName: Code[20])
begin
end;

Hi
Thank you for the reply - I am indeed trying to subscribe to the events but they are not raised since the procedure that should raise them are marked as [Scope('OnPrem')]..
Example: Consider the publisher OnAfterResetCustomerAccountMapping - it is only raised inside the ResetCustomerAccountMapping and therefore never raised in a BC online environment..

There are three ways to make sure this method will be called:
First way (if setup is not enabled before)
Open Dataverse Connection Setup
Enable the setup
Second way (this is if the setup is already enabled)
Open Dataverse Connection Setup
Choose action 'Use Default Synchronization Setup'
Third Way (this is if the setup is already enabled)
Open Integration Table Mappings
Select CUSTOMER Mapping
Choose 'Use Default Synchronization Setup'

You're right - somehow my subscriber was not called (probably delay in deploy..) but now it's called as expected.
I can work from here - ticket can be closed.
Thank you


Is this ever going to be updated?

Is this repository ever going to be updated and/or worked on or has it been abandoned?

Possibly. I've been in talks with people who want to help make this production quality.

Interesting.  So does that mean that it will no longer be open source or what not?

It would stay open source.

Oh, I'm not inherently opposed to open source for-profit stuff, it just affects to the extent to which I contribute and/or use something.  I use lots of stuff that was created by for-profit companies.  My only recommendation though it to do thorough testing and don't challenge other technologies without providing accurate apples-to-apples comparisons.  Just went through an ordeal with a guy at Huggingface at this link  However, I just checked the repository again and it looks like he might have opened up some re-testing so...we'll see.  I haven't read it yet.
Just my 2 cents.  :-)  I look forward to seeing what you produce...if I knew anything about C++ I might even consider contributing, but I'll definitely be following!

@skeskinen There are talks in llama.cpp to support BERT, although I don't know the current status.
https://github.com/users/ggerganov/projects/7?pane=issue&itemId=37112645

Hey @skeskinen and guys, I've made another fork to support multilingual tokenizer, real batch inference and the bge series model. Check this out if you're interested. embeddings.cpp


Fatal error when searching in not existing folder

This error comes when i'm trying to find file in non existing folder, or if dropbox won't be able find at least one file/folder
[04-Mar-2017 01:12:15 UTC] PHP Fatal error:  Uncaught exception 'Kunnu\Dropbox\Exceptions\DropboxClientException' with message '{"error_summary": "path/not_found/.", "error": {".tag": "path", "path": {".tag": "not_found"}}}' in /path_to_folder/dropbox/kunalvarma05/dropbox-php-sdk/src/Dropbox/Http/Clients/DropboxGuzzleHttpClient.php:67
Stack trace:
/path_to_folder/dropbox/kunalvarma05/dropbox-php-sdk/src/Dropbox/DropboxClient.php(155): Kunnu\Dropbox\Http\Clients\DropboxGuzzleHttpClient->send('https://api.dro...', 'POST', '{"path":"\/2017...', Array)
/path_to_folder/dropbox/kunalvarma05/dropbox-php-sdk/src/Dropbox/Dropbox.php(250): Kunnu\Dropbox\DropboxClient->sendRequest(Object(Kunnu\Dropbox\DropboxRequest))
/path_to_folder/dropbox/kunalvarma05/dropbox-php-sdk/src/Dropbox/Dropbox.php(264): Kunnu\Dropbox\Dropbox->sendRequest('POST', '/files/list_fol...', 'api', Array, NULL)
/ho in /path_to_folder/dropbox/kunalvarma05/dropbox-php-sdk/src/Dropbox/Http/Clients/DropboxGuzzleHttpClient.php on line 67

This issue only appears if folder is not exist in DropBox which is sending as first parameter
$dropbox->search('/not_exist_folder/', 'file_name')
request as below return fatal error

@tilekus That's an expected Exception. If you try searching inside a non-existing folder, Dropbox's API will respond with an error.
More Info => https://www.dropbox.com/developers/documentation/http/documentation#SearchError

Firstly, many thanks for pulling this great SDK together, I was starting to panic when the native Dropbox API was deprecated... I'm seeing the same exception using listFolder when the folder doesn't exist:
$listFolderContents = $dropbox->listFolder($path);
Is this also by design as it didn't throw an exception in the native V1 API? I'm struggling to find an elegant way around it....

@markfb Dropbox's API only returns errors (in responses). See => https://www.dropbox.com/developers/documentation/http/documentation#error-handling
Here's how to handle the exceptions thrown by the SDK:
try {
    $listFolderContents = $dropbox->listFolder($path);
} catch (DropboxClientException $e) {
    echo $e->getMessage();
}

Cheers!

I've now found a way around this. Thanks!

@markfb What was your solution?

Unfortunately it isn't particularly elegant. I'm storing a document count for each folder in my database and checking that it's 1 or more before using listFolder...


bug : checkout deals

after clicking the checkout deals it does not disappear and the interface is not good for exclusive -deals -offers card.
I will do this please assign me

@Risheendra183 Thanks for creating the issue in BuddyTrail!!! The issue has been assigned to you.
Before you start working on your PR, please make sure to:

⭐ Star the repository if you haven't already.
Pull the latest changes to avoid any merge conflicts.
Attach before & after screenshots in your PR for clarity.
Include the issue number in your PR description for better tracking.

Contributors will start at Level 1, and their level will increase based on the quality and quantity of their contributions. The more effort and value you bring, the faster you'll progress. Put forth your best effort to unlock new levels and opportunities!
I'm excited to see your contribution as part of GSSOC EXTENDED 2024! 🎉
Don't forget to follow me on GitHub for more updates.
Happy open-source contributing! 🚀


Bluetooth: Host: Don't do tx_notify in conn_recv

TLDR: The removed code could not guarantee its purpose and was prone to deadlocks.
Digging in git history:
The code, introduced in https://github.com/zephyrproject-rtos/zephyr/pull/21056, was there to make sure the Host has invoked all pending HCI Number of Packets Completed callbacks before accepting new ACL data.
This was done to ensure the higher layers are informed of a successful TX by the Controller before any ACL data that is the result of that TX arrives at the application. Concretely, this was used by ATT to detect pipelined requests from the peer.
ATT pipelining detection was added in https://github.com/zephyrproject-rtos/zephyr/commit/4a57bf6e6cd1e4b1fad87ff2611ff604800ec7b6.
However this relies on the Controller delivering the Number of Packets Completed event promptly, which it is not required to do by spec. So, while this happens to work with some Controllers, there is no guarantee.
When we found some Controller implementations that did not deliver the Number of Packets Completed event promptly, we removed the ATT pipelining detection in https://github.com/zephyrproject-rtos/zephyr/pull/66012. But we forgot to remove the call to tx_notify in conn_recv, which suffers from the same lack of guarantee.
It used to be that BT RX thread would perform the work of tx_notify itself when it needed it to be done. This was changed in https://github.com/zephyrproject-rtos/zephyr/pull/68394. After this, the code forms a dependency from the BT RX thread to the system work queue. Dependencies like this are a liability and should not exist unless necessary. If the system work queue is busy with long-running work, this delays the processing of the ACL data. And if the system work queue is blocked, e.g. by a call into Bluetooth API, this can cause a deadlock.
There is probably more to add to this, but I think this change is sufficiently motivated already. Removing this code should be fine because of the lack of guarantee that it does anything useful.

Looks good to me. Any idea why the Babblesim check is failing?

Looks good to me. Any idea why the Babblesim check is failing?

tests/bsim/bluetooth/audio/test_scripts/mcs_mcc.sh is failing. I had a look and did not understand it. I'll dig a bit deeper. @Thalley, do you have any ideas?

Looks good to me. Any idea why the Babblesim check is failing?

tests/bsim/bluetooth/audio/test_scripts/mcs_mcc.sh is failing. I had a look and did not understand it. I'll dig a bit deeper. @Thalley, do you have any ideas?

Running the test locally shows that the the l2cap_sent callback in OTS is not called.
The logs from the BSIM from this PR:
d_00: @00:00:06.531220  Reading Icon Object succeeded
d_00: @00:00:06.531220  [00:00:06.531,219] <dbg> bt_mcc: bt_mcc_read_icon_url: conn is NULL
d_01: @00:00:06.548827  [00:00:06.548,797] <dbg> bt_ots: l2cap_disconnected: Channel 0x84c34fc disconnected
d_01: @00:00:06.549259  [00:00:06.549,255] <dbg> bt_mcs: read_icon_url: Icon URL read, offset: 0, len:127, URL: http://server.some.where/path/icon.png
d_00: @00:00:06.549518  [00:00:06.549,499] <dbg> bt_ots: l2cap_disconnected: Channel 0x84c7f68 disconnected

When running locally without this change:
d_00: @00:00:06.531220  Reading Icon Object succeeded
d_00: @00:00:06.531220  [00:00:06.531,219] <dbg> bt_mcc: bt_mcc_read_icon_url: conn is NULL
d_01: @00:00:06.548827  [00:00:06.548,797] <dbg> bt_ots: l2cap_sent: Outgoing data channel 0x82543fc transmitted
d_01: @00:00:06.548827  [00:00:06.548,797] <dbg> bt_ots: l2cap_sent: Scheduled TX on L2CAP CoC is complete
d_01: @00:00:06.548827  [00:00:06.548,797] <dbg> bt_ots: oacp_read_proc_cb: OACP Read Op over L2CAP is completed
d_01: @00:00:06.548827  [00:00:06.548,797] <dbg> bt_mpl: on_object_send: Object Id 0x000000000100, offset 127, length 0
d_01: @00:00:06.548827  [00:00:06.548,797] <dbg> bt_mpl: on_object_send: Read complete
d_01: @00:00:06.548827  [00:00:06.548,797] <dbg> bt_ots: l2cap_disconnected: Channel 0x82543fc disconnected
d_01: @00:00:06.549259  [00:00:06.549,255] <dbg> bt_mcs: read_icon_url: Icon URL read, offset: 0, len:127, URL: http://server.some.where/path/icon.png
d_00: @00:00:06.549518  [00:00:06.549,499] <dbg> bt_ots: l2cap_disconnected: Channel 0x8256e88 disconnected

If the sent callback not called, then oacp_read_proc_cb is not called and the state of the object is not set to BT_GATT_OTS_OBJECT_IDLE_STATE.

We should probably add a test to prove that we have fixed a problem

I think audio is doing something similar in nature to ATT pipelining detection, which as described in the PR description, we had to remove.
The audio code is expecting the bt_l2cap_chan_ops.sent event before the bt_l2cap_chan_ops.disconnected event. But this is not guaranteed.
The disconnected event is caused by the RX of an L2CAP Disconnection Request control packet in this case.
Now look at the documentation of bt_l2cap_chan_ops.sent:
https://github.com/zephyrproject-rtos/zephyr/blob/200de7c00a0a1beb905ec8565a24fce1d498e13b/include/zephyr/bluetooth/l2cap.h#L367-L376
Aside: I think the above documentation is not technically correct. Generically, this event is that the controller produced a Number of Packets Completed event, giving us a buffer credit for a specific connection. For each credit we receive, we invoke one callback from the FIFO of sent packets. It does not mean anything beyond that in general. For example, the controller could give a credit for a connection without having sent anything on that connection yet, maybe because some other process freed some memory. Any meaning assigned to credit events beyond the availability of a buffer is controller specific. We should document this.
But ignoring the aside above, the documentation states that "When the controller does so is implementation dependent". So, expecting this callback before some RX is not guaranteed to work with a generic Controller.
Also, the assertion could be triggered on purpose by the remote, so it's a DOS vector.
I'm still thinking about this problem, but inputs and critique is welcome.

Alternatively we define the order of events so that all outstanding RX and TX are always finalized before the disconnected callback (perhaps for both L2CAP, ISO and ACL), and make sure that all Zephyr-defined implementations follow this order.

You mean make our dialect of HCI with these requirements? That sounds good from a Host perspective. But will this be accepted by the community? I suspect there will be a lot of friction.

If we do not want to define a specific order of events

If it was clear what the order of events should be, then we can define that, and we'll make the Host so it conserves this order. But it is out of our hands if the Controller reorders these events. And so long as the Controller is allowed to reorder the events, I question the point of the Host trying to conserve it.
If we can define requirements for an ordering with respect to the HCI messages, like the ACL data and Num Complete events, I would really like that. I think we should, because will make our APIs more understandable. It would really help us if application and Host developers negotiated some contracts.
@Thalley, would something the following be sensible?

bt_l2cap_chan_ops.disconnected comes after bt_l2cap_chan_ops.sent had been invoked for each successful return from bt_l2cap_chan_send().
bt_l2cap_chan_ops.sent means that the "corresponding" Controller buffer is "destroyed". That can be by Num Complete, Disconnect Complete, HW error or HCI reset. It does not mean the message has been sent anywhere.

This does mean bt_l2cap_chan_ops.disconnected could be delayed by the Controller withholding Num Complete, even if the Host has already received an L2CAP disconnect request.

@alwa-nordic I'm not an expert in L2CAP CoC, so I need some things clarified first.

Is it valid to terminate a CoC while there is an outstanding read or write?
If it is valid, does the host wait for the credits from remote or controller, or does it reset the credits for the next connection?

Ideally we would treat CoC similar to ACL and ISO where once they have been terminated, we don't expect any future events for the connections and can reset them once the disconnect event has been received, which allows us to clear the pending TX/RX and return callbacks. This is also how we treat the static L2CAP channels IIRC.

Is it valid to terminate a CoC while there is an outstanding read or write?

Let's reach for a stateless description without the "when". Since ACL packets (in the same direction) are completely ordered on a connection, we could say:
A L2CAP_DISCONNECTION_REQ or L2CAP_DISCONNECTION_RSP in a ACL stream invalidates the channel's handle at that point in that stream. Not a point in time, but a point in the stream of packets. This happens independently in the RX and TX directions. The handle is still valid in packets that are ordered before, but it's not possible to refer to that channel in later packets, so in effect, no data can be sent after and no signaling packets can refer to it either.
We can legally send an L2CAP disconnect at any time, and it is, in a clear sense, ordered after any data we already sent.
Higher layer protocols may have some concept of "reads" or "writes", and impose further restrictions. But at L2CAP level, there are no data transactions.
It may also interest you that the Core spec is says the following on dropping data on L2CAP disconnection:

Once an L2CAP_DISCONNECTION_REQ packet is issued, all incoming data in transit on this L2CAP channel shall be discarded and any new additional outgoing data shall be discarded. Once an L2CAP_DISCONNECTION_REQ packet for a channel has been received, all data queued to be sent out on that channel shall be discarded.

Core 3.A.4.6 v6.0
Assuming "issued" refers to sequencing the TX, it allows the Host to start dropping incoming packets immediately when it wants to close the channel. This is good for error recovery. OTOH, the as-if rule allows the Host to choose to receive and give any remaining RX packets to the application, so long as it can pretend it happened before the L2CAP_DISCONNECTION_REQ in the eyes of the remote. It's not possible to send more credits though, since the handle is invalidated in the TX stream.
Likewise for the reception, it gives a lot of freedom to the implementation on how much to drop. But again, since remote can never refer to this channel again, once credits run out, the rest of the data must be dropped. Oh, and it would be against the as-if rule to send something on the channel as an effect of receiving the disconnect, like "thanks for the disconnect".

If it is valid, does the host wait for the credits from remote or controller, or does it reset the credits for the next connection?

L2CAP credits scoped to a channel and lost on L2CAP disconnect. We can think of it as the channel and credits still existing in the stateless description, but provably unreachable and irrelevant.

Ideally we would treat CoC similar to ACL and ISO where once they have been terminated, we don't expect any future events for the connections and can reset them once the disconnect event has been received, which allows us to clear the pending TX/RX and return callbacks. This is also how we treat the static L2CAP channels IIRC.

I think I agree. It seems natural that disconnection is the last event. OTOH, if there is a further "destroyed" event, then that can be though of as the true disconnect. Like TCP has shutdown() vs close(). I am interested in this discussion. It looks like our L2CAP API does have bt_l2cap_chan.destroy, just not documented. I'm not even sure if it's public API.

all data queued to be sent out on that channel shall be discarded.

The BSIM test is failing to how this part changed. Previously when the data was discarded, it triggered the sent callback, which is arguably semantically incorrect but is nice for flow control in the upper layers.

It looks like our L2CAP API does have bt_l2cap_chan.destroy, just not documented. I'm not even sure it's public API.

I don't think that's for upper layers to use, given the lack of documentation. We do however have the
	/* @brief Channel released callback
	 *
	 * If this callback is set it is called when the stack has release all
	 * references to the channel object.
	 */
	void (*released)(struct bt_l2cap_chan *chan);

Callback, which may be similar, but not too sure.
I think what OTS wants/expects is that when a CoC is no longer connected, it does this check to ensure that we are in a similar state as we were before the CoC was connected. Maybe it should be using the released, assuming that released is call at some point after the disconnection. Not sure how/when it is called.

	/* @brief Channel released callback
	 *
	 * If this callback is set it is called when the stack has release all
	 * references to the channel object.
	 */
	void (*released)(struct bt_l2cap_chan *chan);

Thanks! I read that documentation as saying the invocation of released is the last time the Host will reference chan. Once released is entered, the application can free chan or reuse it for the next L2CAP connection. The application cannot free chan before. It implies disconnected has already been invoked, or that it never will be. And it can only come after all sent callbacks.
If the disconnected or sent callbacks come after released, it would be a clear API violation.

I think what OTS wants/expects is that when a CoC is no longer connected, it does this check to ensure that we are in a similar state as we were before the CoC was connected. Maybe it should be using the released, assuming that released is call at some point after the disconnection. Not sure how/when it is called.

Yes, to reach the same state as before bt_l2cap_chan_connect or bt_l2cap_server.accept we must wait for released. In between those points, the Host requires chan to be alive.

all data queued to be sent out on that channel shall be discarded.

The BSIM test is failing to how this part changed. Previously when the data was discarded, it triggered the sent callback, which is arguably semantically incorrect but is nice for flow control in the upper layers.

Revised a bit:

bt_l2cap_chan_ops.sent must be invoked exactly once for each successful return from bt_l2cap_chan_send.
bt_l2cap_chan_ops.sent must not come after bt_l2cap_chan_ops.disconnected.
bt_l2cap_chan_ops.sent means either that the "corresponding" Controller buffer is "destroyed". That can be by Num Complete, Disconnect Complete, HW error or HCI reset. Or it means the channel is disconnected. It does not mean the message has been sent anywhere. It does not mean the Controller has freed a buffer, unless the channel state is connected.

The meaning of bt_l2cap_chan_ops.sent got a bit watered down. It this ok?

The meaning of bt_l2cap_chan_ops.sent got a bit watered down. It this ok?

I think what you are describing is the existing behavior before this PR, right?
One issue that the sent callback has when compared to e.g. bt_gatt_write_func_t, is that it does not contain a status or err argument. It is not possible for the application to determine if the send was finished successfully, or aborted (which I guess is pretty similar to num_complete_pkts).
The sent callback is defined as
	/** @brief Channel sent callback
	 *
	 *  This callback will be called once the controller marks the SDU
	 *  as completed. When the controller does so is implementation
	 *  dependent. It could be after the SDU is enqueued for transmission,
	 *  or after it is sent on air.
	 *
	 *  @param chan The channel which has sent data.
	 */
	void (*sent)(struct bt_l2cap_chan *chan);

So the current documentation does not define the behavior for what happens at disconnect, and we want to define that we can do it whatever way we think is best.

bt_l2cap_chan_ops.sent must be invoked exactly once for each successful return from bt_l2cap_chan_send.

Agree, that would be ideal, but does not meet the current documentation, as it may neither be sent nor enqueue for transmission in the controller, right? Unless I'm forgetting how the host works, there's a chance that it is enqueue in the host, and not yet sent to the controller.

bt_l2cap_chan_ops.sent must come before bt_l2cap_chan_ops.disconnected.

This would also be ideal, and make the L2CAP API more similar to ISO and ATT/GATT.

The meaning of bt_l2cap_chan_ops.sent got a bit watered down. It this ok?

I think what you are describing is the existing behavior before this PR, right?

Maybe. I was thinking we should first decide on a behavior we want, that makes sense. Once we have what we want we can look at how it's different from what we have.

One issue that the sent callback has when compared to e.g. bt_gatt_write_func_t, is that it does not contain a status or err argument. It is not possible for the application to determine if the send was finished successfully, or aborted (which I guess is pretty similar to num_complete_pkts).

I agree this is not optimal. Actually, bt_gatt_write_func_t is a bad example. It also has no way to know that the ATT write did not happen. err is the ATT response code. But what if there is no response? Seems we have a tendency to forget error handling in Zephyr. :P

The sent callback is defined as
	/** @brief Channel sent callback
	 *
	 *  This callback will be called once the controller marks the SDU
	 *  as completed. When the controller does so is implementation
	 *  dependent. It could be after the SDU is enqueued for transmission,
	 *  or after it is sent on air.
	 *
	 *  @param chan The channel which has sent data.
	 */
	void (*sent)(struct bt_l2cap_chan *chan);

So the current documentation does not define the behavior for what happens at disconnect, and we want to define that we can do it whatever way we think is best.

bt_l2cap_chan_ops.sent must be invoked exactly once for each successful return from bt_l2cap_chan_send.

Agree, that would be ideal, but does not meet the current documentation, as it may neither be sent nor enqueue for transmission in the controller, right? Unless I'm forgetting how the host works, there's a chance that it is enqueue in the host, and not yet sent to the controller.

Correct. We would have to change the definition of sent.

bt_l2cap_chan_ops.sent must come before bt_l2cap_chan_ops.disconnected.

This would also be ideal, and make the L2CAP API more similar to ISO and ATT/GATT.

I can make a PR to ensure this. It's going to require some work to do it cleanly though.
Apart from that, there are some other details to decide on:

When does bt_l2cap_le_chan.state change on during disconnect? It has states BT_L2CAP_DISCONNECTING and BT_L2CAP_DISCONNECTED. Should we say it enters BT_L2CAP_DISCONNECTING at the point Host before the Host starts discarding data, and BT_L2CAP_DISCONNECTED when we are ready to invoke bt_l2cap_chan_ops.disconnected?
bt_l2cap_chan_ops.status has a BT_L2CAP_STATUS_SHUTDOWN bit. Changes to this field are notified by bt_l2cap_chan_ops.status. Is this relevant to disconnect, and if so, when is it set?

Previously when the data was discarded, it triggered the sent callback, which is arguably semantically incorrect but is nice for flow control in the upper layers.

Does OTS use sent for flow control? I think sent is not good for flow control. We should use credits instead.

Previously when the data was discarded, it triggered the sent callback, which is arguably semantically incorrect but is nice for flow control in the upper layers.

Does OTS use sent for flow control? I think sent is not good for flow control. We should use credits instead.

I'm not familiar with the inner workings for L2CAP, but since there isn't a credit callback when sending, isn't sent the callback that indicates a returned credit?
Would be great to get input from @jhedberg in this PR as well. I'm too unfamiliar with L2CAP to properly determine how it should be used and how it is implemented at this point.
If @ekohandel is still around, they can perhaps also share their initial thoughts behind OTS's use of L2CAP

I'm not familiar with the inner workings for L2CAP, but since there isn't a credit callback when sending, isn't sent the callback that indicates a returned credit?

The sent callbacks does not say anything about availability of credits. The current number of credits is stored in bt_l2cap_le_chan.tx.credits. I think we should either add a credit callback (or we could bake it into bt_l2cap_chan_ops.status).
BTW, I think it's best not to think of credits as "returned". They are completely one-time use. They are given by the data receiver and used up by the data sender.

If @ekohandel is still around, they can perhaps also share their initial thoughts behind OTS's use of L2CAP

Sorry for the very late reply on this. It has been a while and I had to look through the code again to remind myself of how OTS uses L2CAP.
In the read procedure case the OTS layer will ask the application layer for the data but it may not get all data that it is asking for so it will send as much as it gets from the application. It uses the sent callback to either ask application for more data or determine that all data was sent and the procedure is done. I guess this could be considered flow control but it is flow control at application layer and not at the L2CAP layer.
Looking at this more closely there seems to be a flaw in the oacp_read_proc_cb function where there is no handling for the L2CAP connection close. The ocap_write_proc_cb function sets the bt_gatt_ots_l2cap.closed callback and uses that to reset the cur_obj->state.type to BT_GATT_OTS_OBJECT_IDLE_STATE. Adding that to oacp_read_proc_cb may address the immediate failure.
This assumes that the closed callback is always called before disconnected is called. Is that a safe assumption?
As an aside, I think there might be yet another flaw in OTS. The current implementation assumes that an object read or write is either fully completed or the connection is disconnected during that read or write procedure to reset the object state to BT_GATT_OTS_OBJECT_IDLE_STATE. The following scenario could pose an interesting behavior:

Client starts an OTS write procedure on an object but doesn't send all expected data through the L2CAP connection so object stays locked
Client switches to a different object
Client switches back to the original object and wants to start another write but the object is still locked and a new write cannot be started.

I would need to spend some time writing a test to reproduce this scenario as it would be hard to do that in normal conditions.

I want to us to improve our L2CAP interface. I'm looking for ways to put it on a better foundation. RFC.

It uses the sent callback to either ask application for more data or determine that all data was sent and the procedure is done.

The sent callback is poorly defined without relying on Controller specific behavior. I think we should avoid sent until someone gives a satisfactory definition that we document, and then use sent only on the basis of that definition.
I think it would be better if OTS asks for more data as a reaction to receiving credits or when buffers are free, as that is that that determines how much data can be sent.
To avoid sent, could we instead consider the procedure done at a different point? Maybe there is something in the OTS protocol that tells us when the procedure is complete?
If the use of sent is meant as flow control, then I think a service alone like OTS is not in a good position to understand the needs of the whole system and manage its own flow control.
The Host limits the use of Controller per ACL connection, so the application does not need to worry about that.
Within a single connection the we can consider adding QoS features for L2CAP TX too, if it's has use cases. But remote can also manage fairness between L2CAP channels using credits and higher layer flow control.


Update make commands to set no-TTY flag

Closes #375
Code Changes

[ ] Set --no-TTY for targets called within github actions

Steps to Confirm

[ ] Confirm this PR includes test output

Pre-Merge Checklist

[ ] All CI Pipelines Succeeded
[ ] Documentation Updated

Description Of Changes
Docker Compose version 2.2.2 used to guess whether to set the no-TTY flag when running the docker compose run command. As of 2.2.3 the flag is never set for you.
See the related PR - docker-compose-9035
To understand the flag better, see the following:
https://stackoverflow.com/questions/30137135/confused-about-docker-t-option-to-allocate-a-pseudo-tty
What I think was happening before was that when github actions ran our docker compose run command it would set the no-TTY flag for us, and when running locally it would not.
I can't say i 100% understand how this flag works or why we don't see output without it but setting it should be the equivalent of our previous behavior.

Related issue:
https://github.com/actions/virtual-environments/issues/5022

Triggered a test failure through this PR and verified we see output now!
https://github.com/ethyca/fides/runs/5298397726

The only weird thing about this change is that when you run make pytest-unit it now uses the ---no-TTY flag locally when it didnt use to.
This doesn't change the functionality but the interaction with the terminal isn't the same. The new output is no longer color coded and formatting might be a little different.
with -T:

without -T:

This doesn't concern me too much though because we don't really use these make targets as part of our dev process. We could write some logic so emulate the old behavior but I don't think it is worth it.

Nice work finding this - do you have a link to a GH actions change or announcement that confirms they disable TTY mode in runners now?
We should preserve TTY mode in local terminals. Can we include this flag only in the pr_checks.yml file?

@PSalant726 I made the changes you suggested. We not only set CI_ARGS when the CI environment variable is set. This is based on this - https://docs.github.com/en/actions/learn-github-actions/environment-variables


Paste from clipboard into QR scan screen

Is your feature request related to a problem? Please describe.
Sometimes it is awkward to get a transaction from another application into LW in order to use LW as a signing wallet.
This new feature would make LW more flexible and easier to interoperate in a larger ecosystem.
Describe the solution you'd like
Add a "paste" icon to the QR scan screen, similar to Anchor:

Depending on the contents of the clipboard:

If the clipboard contains a QR image, process it as if captured by camera
If the clipboard contains ESR text, process it

This is a basic functionality. The paste processor could become pretty "smart" as far as recognizing other requested actions from the clipboard (e.g. an invite code).

Still a "nice to have" IMHO but not particularly high priority.
I looked at this again today. The standard Flutter Clipboard service cannot paste images (text/plain only), but this package will: https://pub.dev/packages/super_clipboard . Meanwhile mobile_scanner  has an analyzeImage interface which will run the barcode processing on an image from a file. So we could copy the clipboard image into a local temporary file, then call the barcode analyzer.
FWIW mobile_scanner keeps making breaking changes but we should probably look at keeping up with the upgrades.


macOS: Use autorelease pools

Autorelease pools ensure that autoreleased objects are freed regularly. These changes also reduce the number of allocated objects and memory usage considerably. This is typical of any custom event loop in macOS (and iOS), such as SDLs pump events:
https://github.com/libsdl-org/SDL/blob/68a4bb01e0d549bfa1f2418dd9463f62980eb210/src/video/cocoa/SDL_cocoaevents.m#L564-L569

CC @RevoluPowered It would be good to see if this helps with the issue you guys are facing

I'm pretty surprised we did not have a dedicated pool set up in the event-loop. That's probably my fault, and I forgot to re-add it after removing old NSAutoreleasePool syntax (still used in 3.x) when we were switching to ARC.

It might be worth checking if an iOS version of this would impact https://github.com/godotengine/godot/issues/76425.

A dedicated pool for event loop makes sense. Not sure if it will have any impact for Main:: calls, these should not do much Obj-C related stuff, but I guess won't hurt.

I wouldn't have thought so either, but I checked the number of objects in the autorelease pool before and after those setup calls, and it was not insignificant. If there is no pool, they never get released, so it does help.


Joinsign

Everything works but joinsign not, the shield is not updated, this is an empty sign :(
Pls fix!

Plugin is not completed.


Upgrade Keycloak to 20.0.1

This upgrades the docker image and the client libraries.
I've added in the verbose flag to the startup script because this will
give us helpful exceptions the next time something strange goes wrong
with an upgrade.

Keycloak 20.00 had the same problem as 20.01, there was an account-console client that it expected to be there and wasn't so there was a null pointer. I've manually added in the console and updated the realm-json and it seems happy now.