Support reactive publishers at the engine level.

This is a PoC that aims to support a more 'direct' line from rx(Class)
to a reactive 'provider'.  Some reactive implementations have features
that can be better leveraged with this.  In our case, we really need to
leverage the end user's project-reactor 'context' when we build the
client.  This use case is exercised in the
ReactorTest#testSubscriberContext test.
Some things like code reuse and test coverage have been postponed until
I get some feedback on the concept.
Apologies for not writing up an issue.  We already had this code written
so I'm hoping it better explains what we need than trying to write it up
in an issue.  Just let me know what you'd like me to do that in that
regard.

@FroMage I think this is something that might be interesting for you to review. Do you have time for a quick check?

Yeah, I think it makes sense to support Publisher for the client. It's not very useful outside of Reactor because Publisher is just bad at dealing with single values, but it doesn't hurt either.

@FroMage @asoldano Please let me know if you are comfortable with me proceeding.  If so, I'll finish the implementation and add tests.

@FroMage @asoldano Please let me know if you are comfortable with me proceeding. If so, I'll finish the implementation and add tests.

Yes, please :-)

@crankydillo , I haven't been able to spend much time here lately; is what you had in mind for this PR completed and ready for review? Thanks

@crankydillo , I haven't been able to spend much time here lately; is what you had in mind for this PR completed and ready for review? Thanks

Not yet.  I hope to have it done by early next week.  Sorry for the delay.

@crankydillo , no hurry at all, I just wanted to understand if you were blocked on something from me/us or not. Cheers

@asoldano The PR is done.  Please start your review when you can.  @anilgursel may be interested as well.

I find no issue with this PR, however it presents a situation in which there are 3 contexts in which
Publisher is used.  One is here.  Another is in resteasy-rxjava2.  A third is coming in  resteasy-client-microprofile.  This presents an issue of properly identifying in which context the return type is being
used.   Work is under way to address this.  There will need to be some minor tweeking of this code
once a solution is implemented to address this.

Hi @crankydillo,
First of all, I want to join the rest of the team in thanking you for this contribution. I'm still trying to absorb it, and I probably don't understand everything.
One general comment. As far as I can tell, most reactive frameworks have two kinds of objects, with a single member (single, mono, uni, ...) and a stream of members (flowable, flux, multi, ...), so it would be interesting to unify all of these. I don't know how feasible that is. Currently, for example, SingleRxInvokerImpl and MonoRxInvokerImpl are built on top of CompletionStageRxInvokerImpl, but there's no unifying RxInvoker that they all subclass. Given that Flux and Flowable (but not Observable) both implement Publisher, maybe something can be done there. Don't know.
And a couple of questions:

The PublisherRxInvokerImpl methods all return a Publisher, but ReactorNettyClientHttpEngine.submitRx() returns a Mono. What is your plan for returning a Flux? A different submitRx() that returns a Flux would clash.

Along similar lines, ReactorTest tests handling a Mono, but I don't see how a Flux would work in the same context. Currently, Flowables, Observables, and Flux's are returned over the SSE infrastructure. and FlowableRxInvokerImpl, ObservableRxInvokerImpl, and FluxRxInvokerImpl understand that. But io.projectreactor.netty:reactor-netty is a whole other world. How would it that work?

Thanks,
Ron

Oh, another thing I was wondering. ReactorNettyClientHttpEngine depends on the existence of reactor.netty.http.client.HttpClient and the reactor-netty library. Is there anything analogous for the other reactive frameworks? I'm guessing, given the similarity of the other frameworks, that reactor-netty could be adapted to work for rxjava2 and mutiny, but I haven't really looked into it.

Hi Ron,
Thanks for the kind words!  I will answer your questions inline; however, you should know that I didn't give this stuff 'great' thought.  Our big need is be able to pass information via Reactor's subscriber context from 'end users' down to the Reactor engine we implement.  That was my main goal, and I only worked a few hours on this:)

Hi @crankydillo,
First of all, I want to join the rest of the team in thanking you for this contribution. I'm still trying to absorb it, and I probably don't understand everything.
One general comment. As far as I can tell, most reactive frameworks have two kinds of objects, with a single member (single, mono, uni, ...) and a stream of members (flowable, flux, multi, ...), so it would be interesting to unify all of these. I don't know how feasible that is. Currently, for example, SingleRxInvokerImpl and MonoRxInvokerImpl are built on top of CompletionStageRxInvokerImpl, but there's no unifying RxInvoker that they all subclass. Given that Flux and Flowable (but not Observable) both implement Publisher, maybe something can be done there. Don't know.

With my change, I'm assuming most reactive RxInvokers would be built using composition of PublisherRxInvokerImpl, not CompletionStageRxInvokerImpl.  I don't know if a base class makes sense.  I probably should have attempted a refactor of the RxJava2 invoker, but I wasn't doing anything like before getting the big picture in.

And a couple of questions:
1. The PublisherRxInvokerImpl methods all return a Publisher, but ReactorNettyClientHttpEngine.submitRx() returns a Mono. What is your plan for returning a Flux? A different submitRx() that returns a Flux would clash.

IMO, if you created an engine that returned a Flux, that would simply be a mistake and should be caught in code review.  I ASSume it would still work; however, it would be inappropriate.  I don't know much about JAXRS+RestEasy+SSE, but, as you mention below, I'm under the impression that is where Flux belongs.

2. Along similar lines, ReactorTest tests handling a Mono, but I don't see how a Flux would work in the same context. Currently, Flowables, Observables, and Flux's are returned over the SSE infrastructure. and FlowableRxInvokerImpl, ObservableRxInvokerImpl, and FluxRxInvokerImpl understand that. But io.projectreactor.netty:reactor-netty is a whole other world. How would it that work?

I'm sorry, but I don't understand what you are asking.  ReactorTest tests the 'make a request and get a (future) response' path, which is accomplished with MonoRxInvoker.  It also tests the 'make a request and get a (future) producer of events' path, which is accomplished with FluxRxInvoker.  Apologies if I'm missing something.  I wish I knew more about SSE stuff, but I haven't really looked into it much.

Oh, another thing I was wondering. ReactorNettyClientHttpEngine depends on the existence of reactor.netty.http.client.HttpClient and the reactor-netty library. Is there anything analogous for the other reactive frameworks? I'm guessing, given the similarity of the other frameworks, that reactor-netty could be adapted to work for rxjava2 and mutiny, but I haven't really looked into it.

I don't know if there is anything analogous for all reactive frameworks; however, HTTP clients is a pretty common use case for them.  Akka-streams has one.  RxJava had RxNetty; however, it's state is uncertain.
AFAIK, it is possible to implement a RxJava2ClientHttpEngine on top of reactor-netty (or akka-streams).  In fact, one of the main benefits of the reactive project is the goal to make such things possible.  Each reactive implementation (akka-streams, rxjava2, reactor) is supposed to support interop.

Hi all!  Any update on this?

I added another use case our clients have, which is to have an efficient, per request timeout.

Hi @crankydillo,
Sorry for the radio silence. I'm going to try to take another look here. I'll have to start over and figure out what I was trying to say in July and August. ;-)
-Ron

Hi @crankydillo,
I should start with my usual apologies for the lateness of my response. To coin a seasonally appropriate metaphor, I feel like I'm trying to wrap my head around a pumpkin where the pumpkin is bigger than my head. ;-)
The last time around I had some half baked questions, and now I have some more. The important difference is that in the meantime I realized that there was a concept here that I didn't even realize that I didn't understand. In particular, I couldn't possibly understand what you're doing without understanding, or even being aware of, the concept of a subscriber context. So my big leap forward is becoming aware of subscriber contexts.
One of the things I was hung up on the last time was how Publisher and Flow fit in. Tell me if I'm wrong, but my impression now is that

you intend to leave FlowInvokerImpl unchanged, and

you're using PublisherRxInvoker in resteasy-client because Publisher is a project independent concept which generalizes Mono, which is specific to the reactor project.

So, part of your changes are in resteasy-client, and the rest are in resteasy-reactor and resteasy-client-reactor-netty. One question I have is: Given that the only implementation of the new concepts in resteasy-client are based on netty, maybe they're too specific to go in resteasy-client.
If I understand correctly, using a subscriber context the way you have done is possible because ReactorNettyClientHttpEngine.submitRx() returns a Mono<T> instead of the Future<T> returned by ApacheHttpAsyncClient4Engine.submit(). I'm wondering if you could have ReactorNettyClientHttpEngine.submit() return a Future<Mono<T>> and dispense with ReactiveClientHttpEngine. Maybe it wouldn't be so pretty, I don't know.
Anyway, I'm still working on my comprehension, but I'm making progress.
-Ron

Hi Ron,
No worries on being late.  Replies inlined.

The last time around I had some half baked questions, and now I have some more. The important difference is that in the meantime I realized that there was a concept here that I didn't even realize that I didn't understand. In particular, I couldn't possibly understand what you're doing without understanding, or even being aware of, the concept of a subscriber context. So my big leap forward is becoming aware of subscriber contexts.
One of the things I was hung up on the last time was how Publisher and Flow fit in. Tell me if I'm wrong, but my impression now is that
1. you intend to leave FlowInvokerImpl unchanged, and

Yes.

2. you're using PublisherRxInvoker in resteasy-client because Publisher is a project independent concept which generalizes Mono, which is specific to the reactor project.

That's correct.

So, part of your changes are in resteasy-client, and the rest are in resteasy-reactor and resteasy-client-reactor-netty. One question I have is: Given that the only implementation of the new concepts in resteasy-client are based on netty, maybe they're too specific to go in resteasy-client.

I think you could make that case; however, I do wonder how much work it will entail.  My initial thought was to do that; however, there are many internal classes that are needed.  Besides, Publisher is in Java 9 and could viewed similar to CompletionStage + benefits.  AFAIK, it's all optional to implementers.  Isn't the only negative the addition of a jar file (until you no longer want to support Java 8)?

If I understand correctly, using a subscriber context the way you have done is possible because ReactorNettyClientHttpEngine.submitRx() returns a Mono instead of the Future returned by ApacheHttpAsyncClient4Engine.submit(). I'm wondering if you could have ReactorNettyClientHttpEngine.submit() return a Future<Mono> and dispense with ReactiveClientHttpEngine. Maybe it wouldn't be so pretty, I don't know.

I will think on this, but my initial impression is it's a bit hacky:)

Anyway, I'm still working on my comprehension, but I'm making progress.

Thanks!

Hi @crankydillo,
There are two related Pull Requests that you might be interested in:

https://github.com/resteasy/Resteasy/pull/2574
https://github.com/resteasy/Resteasy/pull/2580

-Ron

Hi @crankydillo,
Just checking in again.  It would have been nice if Mono, Single, etc., had an interface (other than Publisher) they could all implement, but it's too late for that.  I feel better, though, now that you've confirmed that you're specifically focused on Mono and not Flux, since they live in completely different worlds inside of RESTEasy. That was throwing me off at first.
Right now I'm hung up on the fact that you're introducing a lot of stuff into resteasy-client to be able to implement something in resteasy-reactor. It feels a little like the tail wagging the dog. I would feel better if the new stuff in resteasy-client could also be used in, for example, resteasy-rxjava2. I'm wondering if other reactive frameworks have the same notion of a subscriber context. Do you know about that?
-Ron

Hi @ronsigal ,
I'm on vacation now and can't look at this right now.  I'll be back next week and will add some comments.

re: "on vacation"
Cool.

Hi Ron,
Well, I'm back at work, but now I'm slammed with that:)  As an aside, we are in the process of writing a reactor-netty server adapter, so be on the lookout for that PR soon.

Just checking in again.  It would have been nice if Mono, Single, etc., had an interface (other than Publisher) they could all implement, but it's too late for that. I feel better, though, now that you've confirmed that you're specifically focused on Mono and not Flux, since they live in completely different worlds inside of RESTEasy. That was throwing me off at first.
Right now I'm hung up on the fact that you're introducing a lot of stuff into resteasy-client to be able to implement something in resteasy-reactor. It feels a little like the tail wagging the dog. I would feel better if the new stuff in resteasy-client could also be used in, for example, resteasy-rxjava2. I'm wondering if other reactive frameworks have the same notion of a subscriber context. Do you know about that?

Sorry, but I don't know.  I have worked a small amount with akka-streams and almost none with RxJava.  I believe both might be able to leverage the PublisherRxInvoker for a more 'direct' line.
Once we get the server adapter hammered out, I may have some time to revisit this.  I can try to get the required pieces isolated from rest-client.

@asoldano @ronsigal Now that the server adapter is coming to a conclusion, I'm trying to tackle some older issues we have.  One of those is tied to some unfortunate ThreadLocal coupling we've done.  We hate that coupling, but so far, have not found a way out of it.  Unfortunately, the Mono#fromCompletionStage bridge that this PR would make obsolete causes us issues when we try to use the MonoRxInvoker.  If we can get this (or something like it), we could avoid this bridge and we'd not only get the original benefit I post about in this PR, but also we could stick with our (unfortunate) threadlocals for a little while longer...
So..  I hope you are OK with me picking this back up.  If not, let me know.  I'm almost certain resteasy-rxjava2 could use this.  I will think on @ronsigal 's concern with using Publisher.  All that comes to mind is that we'd have to introduce an interface here..

@asoldano @ronsigal Now that the server adapter is coming to a conclusion, I'm trying to tackle some older issues we have. One of those is tied to some unfortunate ThreadLocal coupling we've done. We hate that coupling, but so far, have not found a way out of it. Unfortunately, the Mono#fromCompletionStage bridge that this PR would make obsolete causes us issues when we try to use the MonoRxInvoker. If we can get this (or something like it), we could avoid this bridge and we'd not only get the original benefit I post about in this PR, but also we could stick with our (unfortunate) threadlocals for a little while longer...
So.. I hope you are OK with me picking this back up. If not, let me know. I'm almost certain resteasy-rxjava2 could use this. I will think on @ronsigal 's concern with using Publisher. All that comes to mind is that we'd have to introduce an interface here..

Hi @crankydillo , I'm definitely OK with you picking this back up. I'd really like to see this reach a state where we all agree and we can merge it :-)
Thanks

Hi @ronsigal ,
A few more thoughts what you were hung up on:)

[snip]

Right now I'm hung up on the fact that you're introducing a lot of stuff into resteasy-client to be able to implement something in resteasy-reactor. It feels a little like the tail wagging the dog. I would feel better if the new stuff in resteasy-client could also be used in, for example, resteasy-rxjava2. I'm wondering if other reactive frameworks have the same notion of a subscriber context. Do you know about that?

Sorry, but I don't know. I have worked a small amount with akka-streams and almost none with RxJava. I believe both might be able to leverage the PublisherRxInvoker for a more 'direct' line.
[snip]

The benefit of what was done here and is very likely going to be true of all the reactive implementations is that you have this 'direct line' from the business logic to the reactive client call.  We currently go through the CompletionStage bridge, which removes a lot of features.  To date, these are the benefits we've noted if we can remove that bridge:

Reactive-Implementation-specific classes (e.g. SubscriberContext can pass from business logic to client).  Original intent of this PR.
Threading 'weirdness' with CompletableFuture can be avoided.  That is discussed here.
And our latest...Business logic based timeouts (e.g. webTarget.request().rx(MonoRxInvoker.class).get(String.class).timeout(Duration.ofMillis(50)) should flow naturally to the inner Publisher.  This allows for the connection to be freed/closed without waiting for the request/response to finish.  The CompletionStage bridge makes that problematic.

It's difficult to see how the above won't be true for most of the reactive implementations.

It's me again:)  I just pushed a refactor to make it clearer that reactive support is optional (dependent on the HTTP engine).  I also refactored RxJava's SingleRxInvokerImpl to leverage this work.  I think one of the earlier comments was wondering if this could be done to support RxJava2 (and other reactive impls).
Really hoping to get some feedback.  We are desperately trying to avoid forking the repo..

Hi @crankydillo,
Long time. ;-)
I'll take a look. Remind me if I don't.
By the way, I've been working on something somewhat related, incorporating https://github.com/eclipse/microprofile-reactive-streams-operators to allow something like
   @GET
   @Path("get/string")
   @Produces(MediaType.TEXT_PLAIN)
   @Stream
   public Publisher<String> get() {
      return ReactiveStreams.of(X_ARRAY).buildRs();
   }

As of now, it exists only on https://github.com/ronsigal/Resteasy/tree/eap7-1661, in the module at https://github.com/ronsigal/Resteasy/tree/eap7-1661/resteasy-reactive-streams-operators.
-Ron

Hi @crankydillo,

[snip]

By the way, I've been working on something somewhat related, incorporating https://github.com/eclipse/microprofile-reactive-streams-operators to allow something like
   @GET
   @Path("get/string")
   @Produces(MediaType.TEXT_PLAIN)
   @Stream
   public Publisher<String> get() {
      return ReactiveStreams.of(X_ARRAY).buildRs();
   }

As of now, it exists only on https://github.com/ronsigal/Resteasy/tree/eap7-1661, in the module at https://github.com/ronsigal/Resteasy/tree/eap7-1661/resteasy-reactive-streams-operators.
-Ron

I'll definitely look at it.  Thanks!
FYI, I'll fix the checkstyle issues shortly.

Hi Ron,

I'll take a look. Remind me if I don't.

Gentle reminder:)

By the way, I've been working on something somewhat related, incorporating https://github.com/eclipse/microprofile-reactive-streams-operators to allow something like
   @GET
   @Path("get/string")
   @Produces(MediaType.TEXT_PLAIN)
   @Stream
   public Publisher<String> get() {
      return ReactiveStreams.of(X_ARRAY).buildRs();
   }

As of now, it exists only on https://github.com/ronsigal/Resteasy/tree/eap7-1661, in the module at https://github.com/ronsigal/Resteasy/tree/eap7-1661/resteasy-reactive-streams-operators.

I did look at this.  My first thought is we have some name collisions (this PR also has a PublisherRxInvoker:).  I do see how you are tying Publisher to jaxrs eventsource.  That is a more natural fit to Publisher, which I think people should not typically associate only 1 value.  I think someone brought that up above.  Anyhow, looks like some nice work.

-Ron

Hi @crankydillo,
re: "Gentle reminder:)"
Well done!! ;-)  I'll look soon.
-Ron

Hi @crankydillo,
Ok, I'm back. I'll have to spend some time drilling down to do a more careful evaluation, but I have some thoughts on an architectural level. Please tell me if I'm making any sense.
I keep going back to my discomfort with the use of Publisher in this context. I would say that your PublisherRxInvokerImpl is much closer to CompletionStageRxInvokerImpl than it is to PublisherRxInvokerImpl in the resteasy-reactive-streams-operators module. In fact, it seems to me (tell me if I'm wrong) that your PublisherRxInvokerImpl and CompletionStageRxInvokerImpl are almost the same when you factor out the underlying network layer, i.e., apache HttpClient vs reactor netty.
In fact, I would go as far as saying that your PublisherRxInvokerImpl could BE CompletionStageRxInvokerImpl, if it returned CompletionStages instead of Monos. For example, I see that you've reworked MonoRxInvokerImpl to extend PublisherRxInvokerImpl so that it turns a (simple) Publisher into a Mono instead of turning a CompletionStage into a Mono like it did before.
If that makes sense, then why not just turn it into something like
public class CompletionStageRxInvokerReactiveImpl implements CompletionStageRxInvoker { ...

Does that make sense? Would it satisfy your requirements? WDYT?
-Ron

Thanks Ron.

I keep going back to my discomfort with the use of Publisher in this context. I would say that your PublisherRxInvokerImpl is much closer to CompletionStageRxInvokerImpl than it is to PublisherRxInvokerImpl in the resteasy-reactive-streams-operators module. In fact, it seems to me (tell me if I'm wrong) that your PublisherRxInvokerImpl and CompletionStageRxInvokerImpl are almost the same when you factor out the underlying network layer, i.e., apache HttpClient vs reactor netty.

They are very similar!  I think your (understandable) hangup is that you associate 'Publisher' with N values.  In this case, N will always be 1.  As such, it is almost identical to CompletionStage other than having other useful properties such as being driven by a subscription and having back-pressure (and the benefits I will link to in the next section:).
Honestly, I don't know why the reactive-streams API doesn't have a first-class 'single-value publisher' concept, but it doesn't seem too.  I'm sure they have a reason.

In fact, I would go as far as saying that your PublisherRxInvokerImpl could BE CompletionStageRxInvokerImpl, if it returned CompletionStages instead of Monos. For example, I see that you've reworked MonoRxInvokerImpl to extend PublisherRxInvokerImpl so that it turns a (simple) Publisher into a Mono instead of turning a CompletionStage into a Mono like it did before.

It could, but then it would not provide the 3 benefits I list here.

re: "would not provide the 3 benefits"
Ok, I thought there might be something I was missing.
Well, what about a MonoRxInvokerNettyImpl? My point is, is there a reason for returning an interface? Could you just return a Mono, or a Single? That would probably involve factoring out what you've added to ClientInvocation and moving it to a separate module, I guess. Or, what about ... creating a RESTEasy specific interface like
public interface ResteasyMono<T> {

    public void subscribe(Subscriber<? super T> s);
}

that can be converted to a Mono? Would that blow benefit 1?

Hi @ronsigal ,
Before I reply to your latest comment, I wonder if it fundamentally gets back to one of your first comments, which had this:

I still can't help being a little uncomfortable using Publisher as a way of abstracting Mono. It would have been nice if Mono, Single, etc., had an interface (other than Publisher) they could all implement, but it's too late for that.

If this is your core issue, I would like to get to the bottom of that.  You mentioned above that Publisher is very much like CompletionStage.  I just have to repeat that it is exactly that, just in the reactive context!  Both represent the chosen interface for future values.  Publisher just has other characteristics.  In fact, Publisher is supposed to be the bridge between other APIs allow for a natural transition.  You can see that in this code where I replaced not only project reactor's integration with CompletionStage, but also rxjava2's (Single.fromPublisher(...)).  This brings yet another benefit for rxjava, which is deferring the HTTP call until subscription.
Anyhow, I just want to reiterate that the authors of reactive streams did not see a need for an interface that represents a single value at least when you are talking about working at the interface level (code).  The engine code here is trying to operate at the interface level.
So if there was an rxjava2 HTTP client engine, that would easily plugin into the MonoRxInvokerImpl as I've defined here, without going through a CompletionStage.  Will you get all the benefits I discuss above?  No, but you should get a few.
Basically, it all plays nice in reactive contexts.

Hey @crankydillo,
Thank you for your patience. It must be frustrating trying to get through to me. ;-)
On the one hand, I can see interpreting Publisher as a general purpose type which can represent 0, 1, ... arbitrary elements. On the other hand, it seems to me that most reactive libraries have a pair of types: Single/Flowable, Mono/Flux, Multi/Uni ... . I do see that reactor has CorePublisher to unify the two. I lean towards associating Publisher with the possibility of multiple items, but maybe I'm flowing against the tide (so to speak). I'll survey some people around here and see what they think.
In the meantime, what do you think of something like
package org.jboss.resteasy.client.jaxrs;

import org.reactivestreams.Subscriber;

public interface Unit<T> {

   void subscribe(Subscriber<? super T> s);
   T get();
   CompletableFuture<U> toFuture();
}

and
package org.jboss.resteasy.client.jaxrs;
...
public interface UnitRxInvoker extends RxInvoker<Unit<?>>
{
   @Override
   Unit<Response> get();
   ...

and
public interface ReactiveClientHttpEngine extends AsyncClientHttpEngine
{
   /**
    * This is the main bridge from RestEasy to a reactive implementation.
    */
   <T, U> Unit<T, U> submitRx(ClientInvocation request,
                                            boolean buffered,
                                            ResultExtractor<T> extractor);

   <T, U> Unit<T, U> fromCompletionStage(CompletionStage<T> cs);

   <T, U> Unit<T, U> just(T t);

   /**
    * How the reactive implementation handles errors.
    */
   <T, U> Unit<T, U> error(Exception e);
}

in resteasy-client-api, and a UnitRxInvokerImpl in resteasy-client. And then
public class UnitMono<U> implements Unit<Mono<U>, U> {

	private Mono<U> mono;

	public UnitMono(Mono<U> mono) {
		this.mono = mono;
	}

	@Override
	public void subscribe(Subscriber<? super U> s) {
		mono.subscribe(s);
	}

	@Override
	public Mono<U> get() {
		return mono;
	}

	@Override
	public CompletableFuture<U> toFuture() {
		return mono.toFuture();
	}
}

in resteasy-client-reactor-netty. Then ReactorNettyClientHttpEngine.submitRx() could return
...
        return new UnitMono(requestTimeout
                .map(duration -> responseMono.timeout(duration))
                .orElse(responseMono)
                .handle((response, sink) -> {
                    try {
                        sink.next(extractor.extractResult(response));
                    } catch (final Exception e) {
                        try {
                            // We release the connection instead of closing it because the WebApplicationException
                            // *may* make use of the response.  However, since we are releasing the
                            // connection here, handlers of the rethrown exception will not be able
                            // to use the stream (we ASSume buffering happened).  An alternative is
                            // to force handlers to be responsible for closing the response; however,
                            // that does open the possibility of leaks..  But then again, so do
                            // several paths you can take with the Client API.
                            response.releaseConnection();
                        } catch (final IOException ie) {
                            log.warn("There was a problem releasing the connection in an error scenario.", ie);
                        }
                        sink.error(e);
                    }
                }));

I haven't worked out every bit of it, but since the original Mono can be extracted from UnitMono, it seems to me that you're not losing anything ...
-Ron

It feels a little weird to be trying so hard to work around the Publisher interface, which at the end of the day, is completely hidden from the end user, but I will give it a go.  However, I can understand the desire for a 'single value' interface in the HTTP call case..
There is some weirdness to bringing Subscription into your proposed Unit interface because subscriptions ultimately get back to the request(Int) problem you are trying to avoid.  I'm als concerned that subscription happens on Unit instance.  Typically, code that gets a Mono from some method call should assume they pass it up the call stack as far as possible before a subscription takes place.  However, in your proposed design, no one should ever directly call subscribe on the Mono returned by get.
Anyhow, I have dedicated the afternoon to make a go of it, so we'll see where we get:)

Hey @crankydillo,
I've been wondering if I'm overreacting to the Publisher issue, so I consulted some people in our "Reactive Valhalla", and I got this response:
Mono is a Publisher because for Reactor everything is a stream, even a single response operation or no
response operation. I consider this as an abuse of the Publisher API. While, yes, technically, you can do
that, but... imagine writing Java code only with java.lang.Object because (almost) everything is an Object?
You can quickly see that it's not great. In addition, when APIs, even when implemented using Reactor,
expose Publisher (such as R2DBC, Mongo client, Neo client), the user needs to re-create a Mono or a
Flux to use it. Except that, how do you know the number of items? There is only one way.... looking at the
code, and it may be not simple.

Anyway, instead of introducing an abstraction, did you consider using CompletionStage? You support them
already, and you can map a Mono to a CompletionStage. The `toFuture` method from Mono does this. Note
that we have developed Reactive Converters [1] which transform Mono into CompletionStage[2].

[1] https://github.com/smallrye/smallrye-reactive-utils/tree/main/reactive-converters
[2] https://github.com/smallrye/smallrye-reactive-utils/blob/main/reactive-converters/converter-reactor/src/main/java/io/smallrye/reactive/converters/reactor/MonoConverter.java

So, I feel like I'm not going too far off the rails here. As for any problems with my off the cuff proposal, there's probably nothing there that I would defend too strenuously. I just wanted to convince myself that something of the sort was possible.
In the end, my goal is to incorporate something that works for you and feels right to me. I'm sure we'll get there. I'd like to include this in release 4.7.0.Final. And, again, thanks for your patience.
-Ron

I was updating my comment when you were writing yours:) Apologies if I came across as harsh.  While I don't fully agree with that quote, I can see both of your points.  I never thought you were off the rails:)
Let me see what I can come up with wrt having a reactive-like single interface that doesn't have the problems you get using CompletionStage.

I hate to sound like a broken record, but I'm still stuck.  It may be because I've gone down a path so far that I can't find my way out. The shortest way to express the situation is we have built a lot around AsyncHttpClientEngine#submitCF, which was actually a contribution by our team.  For example, we have many internal AsyncHttpClientEngine wrappers that take the CompletionStage from the open source implementation and add functionality by making calls to whenComplete/thenApply/etc. on a CompletionStage delegate.  So when I added Publisher support, migrating this code was a simple as changing completionStage.whenApply(..) to Mono.fromPublisher(delegate.submitRx()).map(...).   I'm probably going to have to do quite a bit of work to migrate this to some kind of non-publisher API.  Ultimately to get anything easy to work with, I likely will need your Unit#get, but I don't see how that will work without a bunch of casting.
Unfortunately, this effort has been de-prioritized.  Ultimately, I picked this back up because we feel it closes a gap in some flaky tests we have.  We'd like to close that gap, but we don't (currently) feel it harms our customers.  The other features I added above also are not technically required at this time.
Setting that aside, my gut tells me that the cleanest, quickest way to rectify this may be leveraging the pattern set forth in the MpClient.  So basically, we'd have MonoClient, MonoClientInvocation, etc and ultimately we could end up casting ClientInvocation in the MonoRxInvokerProvider to MonoClientInvocation.  However, this is an awful lot of work that replaces what I've created above which AFAIK will work for any reactive client engine and any reactive RxInvoker..  I may be able to squeeze out a few more hours to PoC that.  Do you see a reason this is a bad path?
Anyhow, not trying to pressure you.  It's not your fault we have other priorities:).   However, I do want to leave you with a few thoughts.  First, I have colleagues that agree that Publisher<Response> is weird-ish:). As I said many times, I always understood that stance.  However, I think something to keep in mind is that I believe Publisher is entirely hidden from the end user.  One of the key parts of the reactive RxInvokers would be to choose the appropriate adaption.   I know examples aren't proof of a good idea, but they are better than nothing.  Here are 3 clients that expose Publisher<Response> Client#execute(Request) as an API:  jetty-reactive-client, rxnetty (ironically, we're trying to replace this with jaxrs2.1+a reactive invoker), and micronaut HTTP client.

Hey @crankydillo,
I'm trying to let all this sink in, and I'm feeling torn in two directions.

I'm starting to realize that the Publisher issue is at least half theological. I mean, I have a certain point of view in which a "degenerate" PublisherRxInvoker that only ever returns Monos is just "weird-ish" (great word), but it's not incorrect either. It seems to me that one way to go would be to keep PublisherRxInvoker and ReactiveClientHttpEngine in resteasy-client and move ReactiveInvocation and PublisherRxInvokerImpl to resteasy-client-reactor-netty. That way the weird-ish part is factored out of resteasy-client.

Well, one problem with that is raised by your update to resteasy-rxjava2, where you have SingleRxInvokerImpl depending on PublisherRxInvokerImpl. Well, maybe having PublisherRxInvokerImpl in resteasy-client isn't unreasonable, since the weird-ish part is factored out in ReactorNettyClientHttpEngine. And, now that I've come that far, I guess the same could be said about ReactiveInvocation. So, I think, once starting down that path, I end up where you are.
Actually, now I'm starting to wonder if ReactorNettyClientHttpEngine could be adapted to return non-unitary Publishers, but that's probably for another time. Well, to project out even further, I'm interested in implementing streaming from client to server, as in gRPC, and maybe, I don't know, ReactorNettyClientHttpEngine could be relevant. If that's something you might be interested in ... .

On the other hand, it still feels, deep down, like a PublisherRxInvoker that can't return "real" Publishers is sort of a bait and switch. Adopting something like Unit would make that go away. Have you made any progress in that direction?

Ok, I feel like <monty-python-intonation>my brain hurts</monty-python-intonation>. On a personal note, I saw your latest remarks last night just before bedtime, and I wished I hadn't. ;-)  But I'm telling you that just to say that I really want to make this work as painlessly as possible, and I guess I'm not doing a great job at it. As always, thanks for your patience.
-Ron

Hey @crankydillo,
I'm trying to let all this sink in, and I'm feeling torn in two directions.
I'm starting to realize that the Publisher issue is at least half theological. I mean, I have a certain point of view in which a "degenerate" PublisherRxInvoker that only ever returns Monos is just "weird-ish" (great word), but it's not incorrect either. It seems to me that one way to go would be to keep PublisherRxInvoker and ReactiveClientHttpEngine in resteasy-client and move ReactiveInvocation and PublisherRxInvokerImpl to resteasy-client-reactor-netty. That way the weird-ish part is factored out of resteasy-client.

If it's any consolation, I think this debate may have happened between the teams that wrote the initial reactive interface (Publisher).  A teammate suggested that some of those people work only with streams, while others have a concept of 'single' things.  I definitely encourage you to read a bit on the reactive stuff.  Publisher is (was?) primarily meant to bridge between the reactive implementations, at least that's what I've been told.  For example, these are perfectly valid:
rxjava2.Single<Integer> int = rxjava2.Single.fromPublisher(reactor.Flux.range(0, Integer.MAX_VALUE))
akka.Flow<Integer> ints = akka.Source.fromPublisher(rxjava2.Single.just(42).toFlowable());

Along those lines, would it help if I made MonoRxInvoker not extend from PublisherRxInvoker?  Given what I said above, does it not feel natural to adapt a Publisher<T> supplied from a PublisherRxInvoker into a Mono<T> inside a MonoRxInvokerImpl?  This is basically what I did in the rxjava2.SingleRxInvoker case..  I didn't do that in the Mono case, because Mono is a Publisher..
This is kind of the key point I've been trying to convey.  Basically, you take Publisher<T>, decide what reactive impl you want to use, and then, if that lib has it, decide to work with 'single' things, which basically boils down to 'requesting 1 item' from the publisher..  So to your friend's point about Object, if I could define my api like this: Object myApi() and then all my clients could do UsefulThing thing = magicBridge(myApi()), then I'm not so sure that would be a bad thing:P
This is also why I think you'll find most HTTP client libs are tightly coupled with a reactive impl (i.e. reactor-netty) will choose to use the latter types in its interface.  They know that anyone who wants to use a different reactive impl can just use an adapter.  If those didn't exist, they'd likely return Publisher<T>..
Again, I can understand the desire to have the return type indicate a 'single value'.  I would be curious to see debate about why that didn't get into the reactive (bridging) api (Publisher).

Well, one problem with that is raised by your update to resteasy-rxjava2, where you have SingleRxInvokerImpl depending on PublisherRxInvokerImpl. Well, maybe having PublisherRxInvokerImpl in resteasy-client isn't unreasonable, since the weird-ish part is factored out in ReactorNettyClientHttpEngine. And, now that I've come that far, I guess the same could be said about ReactiveInvocation. So, I think, once starting down that path, I end up where you are.
[Actually, now I'm starting to wonder if ReactorNettyClientHttpEngine could be adapted to return non-unitary Publishers, but that's probably for another time. Well, to project out even further, I'm interested in implementing streaming from client to server, as in gRPC, and maybe, I don't know, ReactorNettyClientHttpEngine could be relevant. If that's something you might be interested in ... .]

If there was a use case, you could certainly add another method and name it such that it hints that Publisher<T> could deliver more than 1 value.  Presumably, this would be based off the response.  However, that seems much more the realm of an RxInvoker, just like you've done with server side events..

On the other hand, it still feels, deep down, like a PublisherRxInvoker that can't return "real" Publishers is sort of a bait and switch. Adopting something like Unit would make that go away. Have you made any progress in that direction?

I did make progress and opened the draft PR #2803 ; however, it seems complicated and doesn't 'feel right' to me other than this desire to avoid 'a Publisher of 1 thing'.  I am trying to integrate it with our code base, but I still have lots of failures.
Even with that, I still had to expose Publisher<T> from within Unit<T>.  Or at least I think I need to do that:)

Ok, I feel like my brain hurts. On a personal note, I saw your latest remarks last night just before bedtime, and I wished I hadn't. ;-) But I'm telling you that just to say that I really want to make this work as painlessly as possible, and I guess I'm not doing a great job at it. As always, thanks for your patience.

If it's any consolation, I saw your comment at bed time as well.  As to patience, it is me who should be thanking you.  I need this feature from your project.  I don't blame you for pushing for what you feel is right!  If I were you, I would be tempted to say 'go away' (but I hope you don't!).   My main issue is getting time to work on this:)

Hi again, @crankydillo,
I feel like I've just been thinking about all this backwards. I spent some time looking more deeply into my recent parenthetic remark about extending ReactorNettyClientHttpEngine to return "real" Publishers, and now I think that, rather than trying to restrict the interface in resteasy-client to match the implementation in resteasy-client-reactor-netty, the other direction makes more sense. I've written a little test case in which a server method returns a Flux and a client recreates and returns the Flux, and I think this could be an alternative way to implement FluxRxInvokerImpl. Currently, FluxRxInvokerImpl sits on top of our SSE implementation, which happened because the SSE stuff was already there and it works. But the SSE protocol is very general, and there's overhead with field names, comments, etc., so I have a suspicion that a more direct implementation would be more efficient.
In effect, I think we could consider what you've already done as a first step towards a more general treatment. In particular, we could create a JIRA describing a future extension, and we could merge your current work as the first step.
For the first time since we started our long discussion, I feel comfortable. I'm sorry it took me so long to get here, and I hope I haven't sent you too far into the weeds, but it feels good to see the train pull into the station. [Extra points for mixed metaphors?]

What do you think?

Do you have any interest in working on Phase II? Of course, I understand if you have other priorities.

-Ron

@ronsigal (hopefully) polite bump:)

Hey @crankydillo,
I think I see the mountain top. Or at least a mesa.
I appreciate the introduction of UniPublisherRxInvoker and UniPublisherRxInvokerImpl, but the fact is that, as of my last comment, I had come around to thinking that PublisherRxInvoker and PublisherRxImvokerImpl weren't so bad. That is, they promise the return of a Publisher, and that's what they do. It's ReactorNettyClientHttpEngine that "falls short", so to speak. So my goal would be to expand the functionality of ReactorNettyClientHttpEngine at some point. Could you change the names back? [Don't hate me!!]
There are a few things we need to do to wrap this up.

We need a JIRA to cover this pull request..
We need a JIRA to cover future work on ReactorNettyClientHttpEngine, extending it to return a "real" Publisher.
Maybe there's room for expanding Section 51.3.8. "Reactor Netty Client Engine" of the RESTEasy User Guide. WDYT?

Do you have time for 1 and 3? If not, I'll take a stab at them, but I think you could probably do a better job. I can take care of 2.
Also, there's a simple conflict in ReactorNettyClientHttpEngine.
Thanks,
Ron

awesome!

@ronsigal I will take care of 1, 3, and solve the merge conflict within the next few hours.  Thanks!

Woo hoo!! Thanks, @crankydillo. ;-)

@ronsigal I believe I did everything.  I hope the windows build failure is unrelated.  Let me know if there's anything else you need me to do.

Oh, by the way, the test failure is probably nothing. I'll re-start the tests just to be obsessive.

Hey @crankydillo,

Do you think you could add something to Section 51.3.8. "Reactor Netty Client Engine" of the RESTEasy User Guide explaining the benefits of the new feature. I just think that you would do a better job than I would. If the docbook details are a barrier, you could just do it in plain text and I'll translate it.

Ugh, I forgot about that.  Will do!

Hey @crankydillo,

a feature that you must support.

Hmmmm. That's interesting. Ever since RESTEasy 4.0.0.Final, we've distinguished between public and non-public modules. E.g., resteasy-client-api and resteasy-core-spi are public, and resteasy-client and resteasy-core aren't. The idea is that resteasy-client and resteasy-core hold implementation code, which is subject to change.
I guess my first question is: In what sense do you consider this new feature to be experimental? Given that this is something you intend to use, I was under the impression that it's something a little more than experimental ... . Is it something that you can see going away at some point?
Beyond that, my reaction is manifold:

I'm thinking that PublisherRxInvoker really ought to be in resteasy-client-api. I should have thought of that before.
PublisherRxInvokerImpl is in resteasy-client, so that's safe.
The use of PublisherRxInvokerImpl shows up in MonoRxInvokerImpl. I guess this is somewhat fuzzy, and maybe we (RESTEasy) should think some more about segregating MonoRxInvoker and MonoRxInvokerImpl. But the behavior behind the scenes is in PublisherRxInvokerImpl, so it's an implementation detail.

Hmmm. Well, I was thinking that the advantages of the new implementation that you've discussed would be worth making public. But then, as you say, we are, in effect, making the behavior of the implementation public.
I guess it comes down to how stable the new behavior is. So that takes me back to my first question: "In what sense do you consider this new feature to be experimental?"
-Ron

Hey @crankydillo,

a feature that you must support.

Hmmmm. That's interesting. Ever since RESTEasy 4.0.0.Final, we've distinguished between public and non-public modules. E.g., resteasy-client-api and resteasy-core-spi are public, and resteasy-client and resteasy-core aren't. The idea is that resteasy-client and resteasy-core hold implementation code, which is subject to change.
I guess my first question is: In what sense do you consider this new feature to be experimental? Given that this is something you intend to use, I was under the impression that it's something a little more than experimental ... . Is it something that you can see going away at some point?
Beyond that, my reaction is manifold:
1. I'm thinking that PublisherRxInvoker really ought to be in resteasy-client-api. I should have thought of that before.

2. PublisherRxInvokerImpl is in resteasy-client, so that's safe.

3. The use of PublisherRxInvokerImpl shows up in MonoRxInvokerImpl. I guess this is somewhat fuzzy, and maybe we (RESTEasy) should think some more about segregating MonoRxInvoker and MonoRxInvokerImpl. But the behavior behind the scenes is in PublisherRxInvokerImpl, so it's an implementation detail.

Hmmm. Well, I was thinking that the advantages of the new implementation that you've discussed would be worth making public. But then, as you say, we are, in effect, making the behavior of the implementation public.
I guess it comes down to how stable the new behavior is. So that takes me back to my first question: "In what sense do you consider this new feature to be experimental?"
-Ron

RE this being experimental.  I'm not entirely sure what to say.  We have high hopes to use the features this will add.  So I guess we'll know once this is in.  We do have some proof that we need these features.  My initial reactor to your request for documented features is that I hate to do that without tests; however, I forgot that I already had a test that shows how reactor's context can be passed down into an engine.  I have augmented one of the tests to show that a business logic timeout will cause an eager disconnect on the client connection.  So..I've covered 2 of the 3 benefits I described.  I'll add those to the document.
Do you want me to move PublisherRxInvoker?

@ronsigal Please see my last 2 commits.

Hey @crankydillo,
Ok, I think you've added something nice to RESTEasy, something that is consistent with the general movement towards reactive programming, so I think we can bet that it's more than experimental. And I think it's worth letting people know about it. So, +1 for documentation.
Given that the definition of PublisherRxInvoker is determined by the definition of RxInvoker, I think it's safe to move it to resteasy-client-api.
Cool.

I've extended the User Guide discussion a bit by borrowing some language from RESTEASY-2943. Let me know if any of it should be changed or could be imporved.

@ronsigal I moved PublisherRxInvoker.

Well, I think we're there. I feel like there should be some kind of button pushing or ribbon cutting ceremony. Possibly on CNN.
@crankydillo, it's been a pleasure working with you. Like Ernie Banks, Chicago Cubs infielder said, "Let's play two." ;-)

Well, I think we're there. I feel like there should be some kind of button pushing or ribbon cutting ceremony. Possibly on CNN.
@crankydillo, it's been a pleasure working with you. Like Ernie Banks, Chicago Cubs infielder said, "Let's play two." ;-)

Likewise.  Really appreciate you taking the time!


Generates 2 moderate severity vulnerabilities

Please update "markdown-it". See https://github.com/advisories/GHSA-6vfc-qv3f-vr6c

Any updates on this?

PR #204 has been open for a while. If like myself you would like a temporary workaround for the audit issues you can use force-resolutions to force the fixed version of markdown-it. Just beware there may be compatibility issues.


Give nice error message if nipaprc isn't readable by the user

Should help with https://github.com/SpriteLink/NIPAP/issues/1116
Example error
(nipap)➜  nipap-cli git:(barisa/cli) ✗ ./nipap Failed to read file, make sure file /Users/barisa/.nipaprc is readable by current user

I don't immediately see what errors this will catch that aren't already caught by the code at https://github.com/bbaja42/NIPAP/blob/711f9e728f398e15c4842bd856536fd529ff258f/nipap-cli/nipap#L58-L69
Can you elaborate a little bit on when this code will help?
Do you need with with part?
I.e., isn't this
    try:
        with(open(userrcfile)):
            pass

better written as
    try:
        open(userrcfile)

Well, with is required so that file handle is closed after opening it.
But since this is a CLI script, no harm is done even if the file handle is
left open; and as result the code is smaller.
If you'd like, I can remove it.
On 30 March 2017 at 21:58, Kristian Larsson<EMAIL_ADDRESS>wrote:

I don't immediately see what errors this will catch that aren't already
caught by the code at https://github.com/bbaja42/NIPAP/blob/
711f9e728f398e15c4842bd856536fd529ff258f/nipap-cli/nipap#L58-L69
Can you elaborate a little bit on when this code will help?
Do you need with with part?
I.e., isn't this
try:
    with(open(userrcfile)):
        pass

better written as
try:
    open(userrcfile)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/SpriteLink/NIPAP/pull/1143#issuecomment-290542999,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAufRLFGIS-PQsN_IR1C3HF7QTqD4cWFks5rrBdzgaJpZM4Mu5Yz
.

Bah, I just saw your comment about #1116 😜

Hmm, I would assume that there is no open file handle because you don't catch the return value of open() so python gc will immediately see it's not used and close it again but this is an assumption, perhaps an incorrect one, from my side.

I think you are right about GC destroying the variable, but the file handle needs manual closing afaik.
http://preshing.com/20110920/the-python-with-statement-by-example/

Yeah, you're right. File descriptor is still there. I checked in /proc/X/fd for python3 REPL open()

Anyway, this looks good so I'm merging. Thanks! 😄

Wohoo :)
On 30 Mar 2017 10:26 p.m., "Kristian Larsson"<EMAIL_ADDRESS>wrote:

Merged #1143 https://github.com/SpriteLink/NIPAP/pull/1143.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/SpriteLink/NIPAP/pull/1143#event-1023122287, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAufRLLKTnjZtX7QwfdC2WIQBrfoOolEks5rrB4NgaJpZM4Mu5Yz
.


cannot remove database because of sqllab query constraint

Make sure these boxes are checked before submitting your issue - thank you!

[x] I have checked the superset logs for python stacktraces and included it here as text if any
[ ] I have reproduced the issue with at least the latest released version of superset
[x] I have checked the issue tracker for the same issue and I haven't found one similar

Superset version
0.14.1
Expected results
Can remove database even if it has been previously queried in sqllab
Actual results
Delete record integrity error: (sqlite3.IntegrityError) NOT NULL constraint failed: query.database_id [SQL: 'UPDATE "query" SET database_id=?, changed_on=? WHERE "query".id = ?'] [parameters: ((None, '2016-12-15 16:08:45.164903', 5),)
Steps to reproduce
With caravel intalled on a sqllite db:

add a database
query it with sqllab
remove the database

Something like the following untested hunk should do the trick:
diff --git a/superset/models.py b/superset/models.py
index 10928ff..dde86bf 100644
--- a/superset/models.py
+++ b/superset/models.py
@@ -2520,7 +2520,7 @@ class Query(Model):
         DateTime, default=datetime.utcnow, onupdate=datetime.utcnow, nullable=True)

     database = relationship(
-        'Database', foreign_keys=[database_id], backref='queries')
+        'Database', foreign_keys=[database_id], backref=backref('queries', cascade='all, delete-orphan'))
     user = relationship(
         'User',
         backref=backref('queries', cascade='all, delete-orphan'),


[Contextual Cursor] Add new icon in the tooltip like altar icon and other things

This issue is for the external plugin: {Contextual Cursor}
Hi Hydrox6 I'm loving the plugin, it's a good replacement for the normal runelite tooltip. But please can you add more icons on its tooltip? like when you're hovering your mouse on a fairy ring there will be its own icon showing up or on different altars like the dark altar, its own icon will show up. there are a lot of objects around runescape and I hope there will an icon available for each of them too. Idk if this is possible but I hope it is.

Adding an icon for even a large amount of unique things is honestly not a good idea; everything would become cluttered, especially when not even everything has an icon at all. That's not even mentioning that there's not a way to add an item to a specific thing right now (something I've wanted to do for quite a while, but I've got too big of a backlog :d )


Prepare repo for hashing/content-addressing/deser

Rename current Lurk.Expr to Lurk.Syn or Lurk.AST or something (or at least comment for clarification)
Implement the following ScalarExpr definition in Lean

pub enum ScalarExpr<F: LurkField> {
    Nil,
    Cons(ScalarPtr<F>, ScalarPtr<F>),
    Comm(F, ScalarPtr<F>),
    Sym(ScalarPtr<F>),
    Fun(ScalarPtr<F>, ScalarPtr<F>, ScalarPtr<F>),
    Num(F),
    Str(ScalarPtr<F>, ScalarPtr<F>),
    Thunk(ScalarThunk<F>),
    Char(F),
}

Implement the node-wise hashing of ScalarExpr (can use https://github.com/lurk-lang/lurk-rs/blob/3297384697a494ebb55dc32738c679ee07fd385c/src/store.rs#L2365 as reference)
Implement conversion functions from Lurk.AST to Lurk.ScalarExpr

For 3, there isn't really anywhere other than the implementation to look at unfortunately, but its not so complicated. All we really are doing is to hash a node with k scalars is using a poseidon hash of k arity. And k is always a number between 0 and 6. So, like, for Cons(ScalarPtr<F>, ScalarPtr<F>), each ScalarPtr has 2 Fs, so we use an arity-4 poseidon hash.

Closing in favor of more atomic issues


Stereo rendering on GearVR

Just raising this for discussion purposes and recording info.
For GearVR our interface takes over output to screen from Godot. This means our interface is always used on the primary viewport and we simply provide our render buffers to GearVR.
More info to follow...

I'm closing this, recent changes to Godot master allow us to do what Oculus recommends.


[Table Viz] Table columns not match with group_by control

@michellethomas

Codecov Report

Merging #5329 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #5329   +/-   ##
=======================================
  Coverage   61.33%   61.33%
=======================================
  Files         369      369
  Lines       23484    23484
  Branches     2713     2713
=======================================
  Hits        14405    14405
  Misses       9067     9067
  Partials       12       12

Impacted Files
Coverage Δ

superset/assets/src/visualizations/table.js
9.73% <0%> (ø)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72d815c...eb0e6e8. Read the comment docs.

Sorry when we were talking about this yesterday I didn't realize that sortBy shouldn't be a list. I think we should figure out why sortBy is a list in this situation.


[CLI] change cluster creation cost savings mode default

What does this PR do?
This PR changes the default for cluster creation:
instead of having customers opt-into cost savings mode, we'll ask them to opt-out of cost savings mode.
Based on internal discussions - see LAI2-10134
Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

[ ] Was this discussed/approved via a GitHub issue? (not for typos and docs)
[ ] Did you read the contributor guideline, Pull Request section?
[ ] Did you make sure your PR does only one thing, instead of bundling different changes together?
[ ] Did you make sure to update the documentation with your changes? (if necessary)
[x] Did you write any new necessary tests? (not for typos and docs)
[ ] Did you verify new and existing tests pass locally with your changes?
[ ] Did you list all the breaking changes introduced by this pull request?
[ ] Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

[ ] Is this pull request ready for review? (if not, please submit in draft mode)
[ ] Check that all items from Before submitting are resolved
[ ] Make sure the title is self-explanatory and the description concisely explains the PR
[ ] Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?
Make sure you had fun coding 🙃

Do we have the "cluster" feature documented already?

Wouldn't it make sense to turn it around and have an enable-performance flag instead?

Wouldn't it make sense to turn it around and have an enable-performance flag instead?

I agree, avoiding the negation makes the flag more readable. let me change that.

Do we have the "cluster" feature documented already?

the documentation is in a pending PR, I'll leave a comment to adjust it according to this change.


Improvements for training and predictions

Add option to train on full data without a holdout for validation and testing
Scale back predictions if scaler was applied
Record predictions in a CSV file with extrapolated dates
Add .gitignore file to exclude data and logs folders

Thanks a lot!!!!! I will check it and merge your pull request in these days.

Sorry for the mistake in the name of the branch, it should be "LTSF" not "LSTM" :D


script ulozto_downloader should not have same name as the directory ulozto_downloader

One is a hyphen and one is an underscore. I just didn't notice.


Receive work but Send does not work in iOS

https://blockchain.info/tx/3b0487ec13a1155f43d30de4abfa8e4c7abef0a6a2a9946819343cfd475e25c2
I sent few amount bitcoin from Blue wallet to my wallet.
And Transaction confirmed successfully but it never received.
The receiver address is the correct.
And coin was sent from blue wallet.

Looks like the problem on the side of second wallet? Tx visible on blockchain info and I assume all details are correct
On May 8, 2018, 13:44 +0100, StanislavSegreev<EMAIL_ADDRESS>wrote:

https://blockchain.info/tx/3b0487ec13a1155f43d30de4abfa8e4c7abef0a6a2a9946819343cfd475e25c2
I sent few amount bitcoin from Blue wallet to my wallet.
And Transaction confirmed successfully but it never received.
The receiver address is the correct.
And coin was sent from blue wallet.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Maybe it will correct. Thanks.

Hi, May I ask something?
At Request page, the public address is shown by Editable TextInput.
Why do you use TextInput? Why is it Editable?
Thanks.

I think its so the text inside input can be selected and copied. If the input is not editable it cant be selected.
On May 12, 2018, 02:34 +0100, StanislavSegreev<EMAIL_ADDRESS>wrote:

Hi, May I ask something?
At Request page, the public address is shown by Editable TextInput.
Why do you use TextInput? Why is it Editable?
Thanks.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

That's great idea. Now I am also developing bitcoin wallet, of course based on blue wallet.
Could we discuss more details to improve Blue Wallet?
Let me know if you agree.
My email<EMAIL_ADDRESS>Thanks.

Perfect. I just checked iOS app on App Store.
Great Success!!!
I wanna to detach wallet from Expo. isn't there any other problem?
Because I have to use React native link. but it's not possible with Expokit so I decided to detach.
Okay?

Should work fine, but I haven't tried that
On Sat, May 12, 2018 at 3:05 PM, StanislavSegreev<EMAIL_ADDRESS>wrote:

Perfect. I just checked iOS app on App Store.
Great Success!!!
I wanna to detach wallet from Expo. isn't there any other problem?
Because I have to use React native link. but it's not possible with
Expokit so I decided to detach.
Okay?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/Overtorment/BlueWallet/issues/8#issuecomment-388557565,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB0x-SWBs_6vX9Q3VYLROfW-AQ3g1Fk0ks5txuw_gaJpZM4T2k1c
.

Can I send bitcoin without fee in this app?

Yes theoretically if you put 0 in “fee” input, but these days nodes reject such transactions with error “min relay fee not met”
On May 14, 2018, 16:02 +0100, StanislavSegreev<EMAIL_ADDRESS>wrote:

Can I send bitcoin without fee in this app?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

Now I want to add new coin wallet to your project.
This coin was published 4 months ago and there won't any api libraries like bitcoin js.
It is developed by Qt and only qt desktop wallet is available now.
It will be great if you guide the general steps to develop this kind mobile wallet.
Thanks.

This might help, I wrote it couple of years ago https://habrahabr.ru/post/266779/
Your coin might have essentially the same cryptography, so maybe bitcoinjs will work (as it works for dogecoin/litecoin/dash/bcash), if not you need to find something close and rewrite it till it works.
And of course you'll need API to fetch/push data to your coin's network


Data in table is not updated when dataObject is set to empty array

This used to work on v1.8.2, but since updating to v1.9.0, the data in table will not update to reflect the changes set in dataObject.
Example:  Initially array is set to 500 objects. Then with external triggered, we change the array to emtyo array.  The data table still showing the 500 objects.

@kwge - thanks for reporting. I've got a fix, just testing it now. Sorry for the regression, it was some over-zealous error checking.
Should be a fix available today.

Fixed in 5cebd00


uan-config-import failing with OOMKill on NERSC systems for uan 2.5.10 and 2.5.11

Need to change memory request/limits up to 512Mi.
2023-04-08 11:04:13,903 - cf-gitea-import - INFO - Attempting to fetch gitea repository: http://gitea-vcs/api/v1/repos/cray/uan-config-management
2023-04-08 11:04:13,999 - cf-gitea-import - INFO - Cloning repository: uan-config-management
Traceback (most recent call last):
  File "/opt/csm/cf-gitea-import/./import.py", line 402, in <module>
    git_repo = clone_repo(
  File "/opt/csm/cf-gitea-import/./import.py", line 119, in clone_repo
    return Repo.clone_from(clone_url, workdir)
  File "/usr/lib/python3.9/site-packages/git/repo/base.py", line 1148, in clone_from
    return cls._clone(git, url, to_path, GitCmdObjectDB, progress, multi_options, **kwargs)
  File "/usr/lib/python3.9/site-packages/git/repo/base.py", line 1086, in _clone
    finalize_process(proc, stderr=stderr)
  File "/usr/lib/python3.9/site-packages/git/util.py", line 386, in finalize_process
    proc.wait(**kwargs)
  File "/usr/lib/python3.9/site-packages/git/cmd.py", line 502, in wait
    raise GitCommandError(remove_password_if_present(self.args), status, errstr)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git clone -v<EMAIL_ADDRESS>/tmp/uan
  stderr: 'Cloning into '/tmp/uan'...
POST git-upload-pack (175 bytes)
POST git-upload-pack (gzip 1517 to 789 bytes)
fatal: fetch-pack: invalid index-pack output
'

I suspect the real bug is that it's trying to pull down the gitea repo into a tmpfs.  We have a lot of branches and content in our various repos -- it might be best to clone only the master branch during this clone (e.g., git clone --depth 1 <url> -b master) to ensure site-added content doesn't generate this problem.

Reference: CASMCMS-8531 - Planned for CSM 1.4.1.

Pulled into CSM 1.4.0 here https://github.com/Cray-HPE/csm/pull/2119


docs: Improvements to README.md and CONTRIBUTING.md

This PR

Simplifies README.MD to focus on the most common uses of the repo
Expands CONTRIBUTING.MD to include contribution use cases like updating file names, and more information on tests.

@maximearmstrong It would be helpful to get your perspective on how to write CONTRIBUTING.md so it's easy for a contributor to find the tests and run them on their code changes.

@maximearmstrong I've incorporated your feedback - this should be ready for review again.


chore: enable arm builds, use self-hosted runner

:tada: This PR is included in version 1.0.0-beta.13 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:

:tada: This PR is included in version 1.0.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


[SPARK-35100][ML][FOLLOWUP] AFT cleanup

What changes were proposed in this pull request?
remove unaccessible code path
Why are the changes needed?
for succinctness
Does this PR introduce any user-facing change?
No
How was this patch tested?
existing testsuite

Test build #139279 has started for PR 32765 at commit 6b3364e.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43803/

Test build #139279 has finished for PR 32765 at commit 6b3364e.

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

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

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43803/

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

friendly ping @srowen

Merged to master! Thanks @srowen


[FEATURE] Add HTTP transport to the UI socket server

Duplicate issue

[X] I've searched for any related issues and avoided creating a duplicate issue.

Description
The UI socket server should also handle HTTP protocol via express.
The implementation can use the object factory design pattern to create a stable, type safe API to deal with the two application level protocols: WS and HTTP.
The protocol choice shall be a configuration tunable.
Attachments
No response

Commit https://github.com/sap/e-mobility-charging-stations-simulator/commit/fe94fce08a020e0c35e354c2282ec0587a605f4e implement the object factory with server API abstraction.


feat: support use storybook to dev rslib module federation format assets and also support hmr

Summary
Support use storybook to dev rslib module federation format assets and also support hmr
Many capabilities are integrated here.
https://github.com/rspack-contrib/storybook-rsbuild/pull/165
https://github.com/module-federation/core/pull/3131
Related Links

[ ] Tests updated (or not required).
[x] Documentation updated (or not required).

Many capabilities are integrated here.
https://github.com/rspack-contrib/storybook-rsbuild/pull/165
https://github.com/module-federation/core/pull/3131

I'll release a new version of storybook-addon-rsllib.

The lockfile changed too much, you could try to reset lockfile to main HEAD first then re-run pnpm i to update it.

The lockfile changed too much, you could try to reset lockfile to main branch first then re-run pnpm i to update it.

it seems normal change, you can also have a try, i get the same result

The initial investigation appears to be caused by peer dep doppelganger.

@Timeless0911 PTAL https://github.com/web-infra-dev/rslib/pull/349/commits/0aa8efc94790a258b5357ab844a837827ba68cf4. I disabled auto-install-peers.

@Timeless0911 PTAL 0aa8efc. I disabled auto-install-peers.

LGTM, it's more strict now. And is this PR going to merge right now?

I will add --lib args afterwards.


Create Event Kicks Out of Feature

Create an Event. When I enter in a POC Badge the App kicks out back to the Home page. I don't even have to leave the POC Badge input box before it kicks out.

That issue was for when I was logged in. It apparently works when a CIO creates an Event. Should we delete this issue?


nixos/update-users-groups: fix uid/gid instability

Description of changes
nixos/update-users-groups: don't mark previously used ids as used during uid/gid iteration
this line breaks uid/gid allocation and causes uid/gid instability when /var/lib/(uid|gid)-map exist but /etc/(passwd|group) don't exist.

Things done

Built on platform(s)

[X] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[X] sandbox = true

[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[X] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:
https://discourse.nixos.org/t/prs-ready-for-review/3032/3424

With this change we would lose track of previously used IDs though, won't we? If so we could be reassigning those previously used IDs to new users, which has implications for access management and when reviving users.
I also don't see the exact problem, why is this causing instability?
Could you explain a bit more what the issue is here and why your change will not cause issues with reassigning previously used IDs to new users?


Feedback: Week 6 OkHttp

Please provide your feedback by posting a comment against this issue.

Hi, have you considered Vert.x - very stable and well maintained (last commit 9 hrs ago) set of libraries for writing performant and distributed rest APIs and more. It is a Java library but may have  been just as easy to use as okhttp - http://vertx.io

@abacaj I have not looked into vertx.io in-depth yet. I am aware of vertx.io from last few years(when I was working with Red Hat)  but I don't think it has gone mainstream or people use it a lot.  Do you use it?

I don't use it professionally since we actually use full .net stack, but I did use it for a personal project and found it very intuitive. Their documentation is excellent. It's essentially broken into many libraries, but vertx-web is comparable to asp.net and has support for everything you can think of, the best part you choose what features you need.
Take the following code snippet as an example of how expressive it is:

  HttpServerResponse response = routingContext.response();
  response.putHeader("content-type", "application/json");
  response.write(someJSON).end();

});```

@abacaj yes it certainly looks clean.  I will see if I can fit vertx into my series and probably write about it. Thanks.

That's great, look forward to reading more from your series. Great work so far.


chore(ci): update github action versions

📝 Description
This pull request updates the github action versions to support Node 20, since the Node.js 16 actions are deprecated.
⛳ Current behavior (updates)
Deprecated actions are used.
🚀 New behavior
New actions should work perfectly fine, no warning message gets displayed.
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/

Summary by CodeRabbit

Documentation

Updated contribution guidelines to require Node.js 20.x and pnpm 9.1.0 or higher.

Bug Fixes

Corrected a typo in the bug report template for browser options.

Chores

Updated GitHub Actions and workflows to use the latest versions of actions for dependency installation and repository checkout.

New Features

Updated the required Node.js and PNPM versions in package.json to support newer technologies.

I've updated pnpm and Node.js to their latest versions. If there are any incompatibilities, I can revert those changes and only upgrade the gh-action versions here.


Soul Machine painted stairs and slabs look like regular machines

Issue Description:
Cobblestone blocks get painted with soul machines correctly, but stairs and slabs get the regular machine texture.
What happens:
I put a Soul Machine blank machine and a cobblestone stair or slab in the painting machine. It gives me a stair or slab that says its painted with the soul machine, but it's the wrong color.
What you expected to happen:
I expected to get stairs that were the same color as the soul machine block.
Steps to reproduce:

Put cobblestone stair and soul machine decorative block in painting machine
Look at the stair it gives you.
Visually compare the stair with the soul machine decorative block.

Affected Versions (Do not use "latest"):

EnderIO: <IP_ADDRESS>_beta
EnderCore: <IP_ADDRESS>-beta
Minecraft: 1.10.2
Forge: <IP_ADDRESS>5

Oh, I see what's happening. For those blocks "all" textures are set to the same texture, but apparently the "particle" texture is not one of "all"...


Intrivo onboarding + HL7 sender improvements

This PR onboards Intrivo as an HL7 sender.
Two improvements are included in this PR:

A pair of new HL7 schemas for training and production mode so Intrivo and future HL7 senders can be onboarded in training or production mode with settings changes only, no additional child schema required for each sender.
Switch the valueset for Ethnicity to the sender-automation set so we can support either the CDC or HL7 values from senders.

Test Steps:

Process one or more of the Intrivo sample messages in keybase: prime_dev_ops/sender_info/Intrivo/ReportStreamSampleHL7. You will need to add a patient state in PID-11.4, as we don't currently have the ability to accept a message without the patient_state populated.

Changes

Adds sender settings for Intrivo - organizations.yml, staging yamlito, prod yamlito
Adds hl7-ingest-covid-19-training.schema and hl7-ingest-covid-19-prod.schema, described above
Modify hl7-ingest-covid-19.schema to use sender-automation valueset for ethnicity

Checklist
Testing

[x] Tested locally?
[x] Ran ./prime test or ./gradlew testSmoke against local Docker ReportStream container?
[ ] (For Changes to /frontend-react/...) Ran npm run lint:write?
[ ] Added tests?

Linked Issues

Fixes #5419

Pull reviewers stats
Stats for the last 30 days:

User
Total reviews
Time to review
Total comments

jeremy-page
27
36m
3

carlosfelix2
14
14h 15m
45

kevinhaube
11
1h 21m
8

MauriceReeves-usds
9
1h 50m
0

clediggins-usds
9
2h 13m
12

TomNUSDS
6
25m
20

brick-green
6
36m
0

bgantick
5
1h 22m
2

oslynn
5
13h 24m
2

jorg3lopez
5
2h 30m
0

sethdarragile6
4
1h 47m
3

mreifman
3
32m
1

ahay-agile6
3
3m
1

doug-s-nava
2
2h 32m
10

JosiahSiegel
1
3m
0

RickHawesUSDS
1
6m
2

jimduff-usds
1
12h 26m
0

Processed the first HL7 sample test. A few things I noticed:

I had to change the processing mode code from T to P
In the FHS-6 the File Receiving Facility has a date/time input, is this a bug in our code?
FHS-4 and FHS-5 have AIMS.INTEGRATION.STG and AIMS.PLATFORM as their input, not sure if this should be like this

Same thing with BHS-6 Batch Receiving Facility has a data/time input instead of a name, might be a bug
Do BHS-4 and BHS-5 have correct inputs?

MSH-5 and MSH-6 I'm not sure if these fields have the proper input

If the focus is not the input that they sent, then their test file was successfully ingested by our api. Great job, Joel.

Update:
After reviewing the payload, I found no error and the data looked good. I emailed intrivo with the next steps to follow. Intrivo will prepare files to send to a receiver of their choice. This is the final step before going live to production.

@jorg3lopez @BerniXiongA6
Karim from Intrivo sent a message on 8/22 asking for an update on the test HL7 messages for VT.