Drop "empty callsets array = variants only" semantics for SearchVariants

The issue is being forced by the proposal to switch to proto3; proto3 does not disambiguate between the cases of null and [].
This has the side-effect that it is no longer possible to retrieve only the variants (sans calls). My preference is to eventually add support back in via cross-cutting response field masking for the API, e.g. via the proto3 FieldMask specification.
Alternatives considered:

Add another boolean field, e.g. mask_calls which could be passed to get the same behavior. Again, I think it makes more sense to avoid one offs and just have cross-cutting API support for field masking, but it could be acceptable in the interim, if the community feels strongly that we should not lose this functionality for an unknown duration.
Drop 'return all variant calls' semantics. I feel this would be surprising default behavior, since if I compose a minimal query, I expect to get almost everything back. In medium scale analysis even, e.g. thousands of callsets, having to pass all of these callset IDs will be cumbersome.

-1.
Why do we need to have this short-cut at all? From having used this API quite a lot, I strongly prefer the sane semantics of having [] correspond to no calls, ["csId1"] correspond to the callset with ID "csId1", etc. It is a semantic mess to have the empty list correspond to all calls, and also leads to ambiguity --- what order should the calls be returned in?

Thanks Jerome. I fully expected the -1 since this proposal is counter to what you suggested on previous threads. I'd like to crystallize the arguments and get more input.

also leads to ambiguity --- what order should the calls be returned in?

Actually, unless I missed something the documentation doesn't specify the behavior you describe even if you've provided call set IDs. The only statement I can find about relative call ordering is this caveat for servers which don't populate callsetId (which I hadn't actually read before). We could make the documentation stricter on this point, but does it actually matter anyways to a client? Are there other ambiguities this would introduce?

a semantic mess

Could you elaborate? In my mind, I would explain this endpoint as follows: you're searching for variant calls within a VariantSet. callSetIds can also be provided to further filter the results. My proposed semantics for [] I think follow naturally.

Generally, +1, on doing away with this magic behavior, -1 on
this approach.
This functionality was added to address a problem UCSC's had.
The API was simply too slow for an brca exchange browser due the
the volume of calls returned.  The brca exchange current goes
directly to VCFs because of this.
Another approach is to change the data structure.  having the
calls contain in variants seems less that an ideal
representation.
Mark
CH Albach<EMAIL_ADDRESS>writes:

The issue is being forced by the proposal to switch to proto3; proto3 does not
disambiguate between the cases of null and [].
This has the side-effect that it is no longer possible retrieve only the
variants (sans calls). My preference is to eventually add support back in via
cross-cutting response field masking for the API, e.g. via the proto3 FieldMask
specification.
Alternatives considered:
• Add another boolean field, e.g. mask_calls which could be passed to get the
same behavior. Again, I think it makes more sense to avoid one offs and
just have cross-cutting API support for field masking, but it could be
acceptable in the interim, if the community feels strongly that we should
not lose this for an unknown duration.
• Drop 'return all variant calls' semantics. I feel this would be surprising
default behavior, since if I compose a minimal query, I expect to get
almost everything back. In medium scale analysis even, e.g. thousands of
callsets, having to pass all of these callset IDs will be cumbersome.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
You can view, comment on, or merge this pull request online at:
https://github.com/ga4gh/schemas/pull/488
Commit Summary
• Drop "empty callsets array = variants only" semantics for SearchVariants.
File Changes
• M src/main/resources/avro/variantmethods.avdl (3)
Patch Links:
• https://github.com/ga4gh/schemas/pull/488.patch
• https://github.com/ga4gh/schemas/pull/488.diff
—
Reply to this email directly or view it on GitHub.*

@diekhans I think you are mainly arguing that we retain the ability to retrieve only the variants. Does this mean that you would be satisfied with both alternatives (1) and (2) that I listed? I'm going to consider fundamental changes to the Variant record as out of scope for this PR, though I agree this is the likely root cause of the API's awkwardness in this case.

@diekhans

having the calls contain in variants seems less that an ideal representation

+n on this. When starting to look at structural changes, there are "no" recurring variants; searches are addressed at some feature (breakpoint, DUP | DEL ...) occurring in your query interval. So searches are against calls, or in VCF terms against variants-with-dermined-call-positivity.
Therefore, the best scenario for query targets would be to have per sample "variant calls".
Sorry for not being up to speed with everything going on in this area, but I recently realised that the current variants.avdl certainly isn't optimal even for e.g. simple copy number changes.

I agree with Jerome, that we should have a flag to return all
calls and an empty array or null return no calls.  This seems
like cleaner semantics.  Although, we also need to look at
consistency across the API (don't know the queries well
enough to comment).  Is it a filter or request?
The field mask is interesting, however would want to understand
what this means for the overall semantics of the API.  Does this
cause cases were objects with omitted fields are passed to code
that misinterprets the omitted fields and no data?  How does
impact caching of objects?
Agreed that simplifying the object structure is a separate task,
but something that needs looked at.
CH Albach<EMAIL_ADDRESS>writes:

@diekhans I think you are mainly arguing that we retain the ability to retrieve
only the variants. Does this mean that you would be satisfied with both
alternatives (1) and (2) that I listed? I'm going to consider fundamental
changes to the Variant record as out of scope for this PR, though I agree this
is the likely root cause of the API's awkwardness in this case.
—
Reply to this email directly or view it on GitHub.*

Although, we also need to look at
consistency across the API (don't know the queries well
enough to comment).  Is it a filter or request?

I did a quick scan and I don't see anything equivalent. Again, this is probably a special case due to the nested structure of Variant/Call.

The field mask is interesting, however would want to understand
what this means for the overall semantics of the API.  Does this
cause cases were objects with omitted fields are passed to code
that misinterprets the omitted fields and no data?  How does
impact caching of objects?

I think there is minimal impact to the API semantics, if supported it would be an optional parameter to use on any request - but by default the behavior would be the same as today. The concerns you raised are the concerns of a client who has decided to utilize/support the field masking feature. But yes, for clients using field masking, they should take extra care to ensure that:

they are using the feature correctly
they are not passing the full record to another library which expects additional fields to be populated
if they are also caching records, the field mask is accounted for in the caching semantics

Thanks for confining my suspicion that there isn't other cases
like this in the API.
It would be best if field masking needs to be handled
separately.  I would suggest going ahead with a short term flag
in the request that says all variants.  Feel free to start a
ticket to discuss masking, but it needs some careful
consideration.
I will save concerns and approaches to addressing them for
for the field mask PR.
CH Albach<EMAIL_ADDRESS>writes:

Although, we also need to look at
consistency across the API (don't know the queries well
enough to comment). Is it a filter or request?

I did a quick scan and I don't see anything equivalent. Again, this is probably
a special case due to the nested structure of Variant/Call.
The field mask is interesting, however would want to understand
what this means for the overall semantics of the API. Does this
cause cases were objects with omitted fields are passed to code
that misinterprets the omitted fields and no data? How does
impact caching of objects?

I think there is minimal impact to the API semantics, if supported it would be
an optional parameter to use on any request - but by default the behavior would
be the same as today. The concerns you raised are the concerns of a client who
has decided to utilize/support the field masking feature. But yes, for clients
using field masking, they should take extra care to ensure that:
• they are using the feature correctly
• they are not passing the full record to another library which expects
additional fields to be populated
• if they are also caching records, the field mask is accounted for in the
caching semantics
—
Reply to this email directly or view it on GitHub.*

BTW, this needs to be implemented in the reference server and compliance suite before merging.

I'll say it again --- why do we need a mechanism to return all calls? The client can make a SearchCallSets request to find the IDs, and can then make a SearchVariants with these callSetIds as a parameter. There is no need for a second mechanism to do this.  We do need a mechanism to get variants with no calls, and there is already a sane, semantically clean way to do this in the API. Therefore, Occam's Razor says we drop the null==everything semantics, and move on.
I usually agree with pretty much everything you say @calbach, so I'm genuinely confused about where you're coming from here. To answer your questions:

Actually, unless I missed something the documentation doesn't specify the behavior you describe even if you've provided call set IDs. The only statement I can find about relative call ordering is this caveat for servers which don't populate callsetId (which I hadn't actually read before). We could make the documentation stricter on this point, but does it actually matter anyways to a client? Are there other ambiguities this would introduce?

Well, the documentation is wrong on this point then. The callSetIds argument is a list. The returned calls field is a list. Surely the expected behaviour is that there is a one-to-one mapping between the callSetIds and the calls lists: they should have the same length, and they should have the same order. Given list of callSetIds,  I can't imagine any server doing anything other than returning the calls list in the same order: it just doesn't make sense to do anything else.
This matters to clients. There are efficiency gains to be had from knowing the order in which the calls are returned. If I'm generating some statistics for the calls I'm interested in,  I can allocate some arrays and know that the index corresponding to callSetId x will always be the same. Otherwise, I have to maintain a dictionary, indexed by callSetId, and I have to look this up every time. I can't mask out ID field from the Call object, because it's always necessary.
Unless we require that calls are returned in the order of the specified list of callSetIds, there is a lot of ambiguity and unnecessary hassle for the client. Is the ordering of calls stable within a server implementation, or can it vary from page-to-page, variant-to-variant? This would be a source of nasty and annoying client bugs, as well as portability issues (clients will write code that depends on the ordering of a given server).
This is part of what I mean by a semantic mess. If we depart from the obvious bijection between the input and output lists, there are all sorts of twisty loopholes that open up. We gain nothing from opening up these loopholes.
By far the worst semantically is equating the empty list with the list of all calls. This makes no sense, and is mathematically horrible. The empty list is a perfectly well defined object, with perfectly well defined semantics, which also happen to correspond exactly with something that we need. We need to be able to return variants with no calls. We have no need for a mechanism to return all calls in some arbitrary order.
You make the point that you would expect all the calls to be returned by default. I would expect exactly the opposite. Unless I specifically ask for the calls, I don't want them. I don't want a flood of thousands (or hundreds of thousands) of calls coming through, eating bandwidth and slowing parsing down to a crawl. The variant is a summary of the information that we know, and it's a reasonable thing to just return this summary unless more specific information about particular calls is requested.

excellent points @jeromekelleher
One concern about this and the field mask, is the semantic
meaning of the GA4GH objects. Are the objects shopping bags or
are they part of a data model?  This is something I have never
understood about GA4GH.
I am worried about the shopping bag approach without any kind of
indication what as put in the bag.  Does an Variant object with
no calls indicate there were no calls in the VCF or that they
were not requested?  I have never programmed a data model with
this kind of ambiguity.  It seems error prone.
Jerome Kelleher<EMAIL_ADDRESS>writes:

I'll say it again --- why do we need a mechanism to return all calls? The
client can make a SearchCallSets request to find the IDs, and can then make a
SearchVariants with these callSetIds as a parameter. There is no need for a
second mechanism to do this. We do need a mechanism to get variants with no
calls, and there is already a sane, semantically clean way to do this in the
API. Therefore, Occam's Razor says we drop the null==everything semantics, and
move on.
I usually agree with pretty much everything you say @calbach, so I'm genuinely
confused about where you're coming from here. To answer your questions:
Actually, unless I missed something the documentation doesn't specify the
behavior you describe even if you've provided call set IDs. The only
statement I can find about relative call ordering is this caveat for
servers which don't populate callsetId (which I hadn't actually read
before). We could make the documentation stricter on this point, but does
it actually matter anyways to a client? Are there other ambiguities this
would introduce?

Well, the documentation is wrong on this point then. The callSetIds argument is
a list. The returned calls field is a list. Surely the expected behaviour is
that there is a one-to-one mapping between the callSetIds and the calls lists:
they should have the same length, and they should have the same order. Given
list of callSetIds, I can't imagine any server doing anything other than
returning the calls list in the same order: it just doesn't make sense to do
anything else.
This matters to clients. There are efficiency gains to be had from knowing the
order in which the calls are returned. If I'm generating some statistics for
the calls I'm interested in, I can allocate some arrays and know that the index
corresponding to callSetId x will always be the same. Otherwise, I have to
maintain a dictionary, indexed by callSetId, and I have to look this up every
time. I can't mask out ID field from the Call object, because it's always
necessary.
Unless we require that calls are returned in the order of the specified list of
callSetIds, there is a lot of ambiguity and unnecessary hassle for the client.
Is the ordering of calls stable within a server implementation, or can it vary
from page-to-page, variant-to-variant? This would be a source of nasty and
annoying client bugs, as well as portability issues (clients will write code
that depends on the ordering of a given server).
This is part of what I mean by a semantic mess. If we depart from the obvious
bijection between the input and output lists, there are all sorts of twisty
loopholes that open up. We gain nothing from opening up these loopholes.
By far the worst semantically is equating the empty list with the list of all
calls. This makes no sense, and is mathematically horrible. The empty list is a
perfectly well defined object, with perfectly well defined semantics, which
also happen to correspond exactly with something that we need. We need to be
able to return variants with no calls. We have no need for a mechanism to
return all calls in some arbitrary order.
You make the point that you would expect all the calls to be returned by
default. I would expect exactly the opposite. Unless I specifically ask for the
calls, I don't want them. I don't want a flood of thousands (or hundreds of
thousands) of calls coming through, eating bandwidth and slowing parsing down
to a crawl. The variant is a summary of the information that we know, and it's
a reasonable thing to just return this summary unless more specific information
about particular calls is requested.
—
Reply to this email directly or view it on GitHub.*

On Wed, Nov 11, 2015 at 4:46 AM, Jerome Kelleher<EMAIL_ADDRESS>wrote:

I'll say it again --- why do we need a mechanism to return all calls? The
client can make a SearchCallSets request to find the IDs, and can then
make a SearchVariants with these callSetIds as a parameter. There is no
need for a second mechanism to do this. We do need a mechanism to get
variants with no calls, and there is already a sane, semantically clean way
to do this in the API. Therefore, Occam's Razor says we drop the
null==everything semantics, and move on.

Strongly +1 this comment. Get rid of the all calls option. Passing in the
list of CallSets to the method is no more expensive than the return value
when all callSets are specified.

This seems important. When you request info from a record, there must be a
distinction between "that field was null" and "you didn't ask for the value
of that field". This seems to be tied up in scoping the query and the
problem of how to return only part of a coherent data structure. I guess
this means a distinction between two types of missing values: "this part is
missing in the actual data structure" and "this part is missing because you
didn't ask for it so we didn't want to waste bandwidth sending it to you".
On Wed, Nov 11, 2015 at 10:14 AM, Benedict Paten<EMAIL_ADDRESS>wrote:

On Wed, Nov 11, 2015 at 4:46 AM, Jerome Kelleher<EMAIL_ADDRESS>
wrote:

I'll say it again --- why do we need a mechanism to return all calls? The
client can make a SearchCallSets request to find the IDs, and can then
make a SearchVariants with these callSetIds as a parameter. There is no
need for a second mechanism to do this. We do need a mechanism to get
variants with no calls, and there is already a sane, semantically clean
way
to do this in the API. Therefore, Occam's Razor says we drop the
null==everything semantics, and move on.

Strongly +1 this comment. Get rid of the all calls option. Passing in the
list of CallSets to the method is no more expensive than the return value
when all callSets are specified.
—
Reply to this email directly or view it on GitHub
https://github.com/ga4gh/schemas/pull/488#issuecomment-155865394.

OK, I've gone a bit deeper on this issue and I've discussed with my team. @jeromekelleher given your premises I think I agree with your conclusion. However I think there may be some more fundamental disagreement about those premises, and possibly there is another dimension to SearchVariants which hasn't been discussed yet - I'd like to resolve these points before continuing.

A Variant need not have a Call for all CallSets in the VariantSet.

Note that these semantics are looser than those of a server where 1 VariantSet == 1 VCF file, since the VCF spec requires 1 Call / sample for all variants (I'm including 'no call' here, which is still explicitly modeled by a Call resource). The current Variants API is ambiguous on this point, but my assertion is supported by the variants diagram. These semantics allow for a more compact representation of a variant set, but I also think that the requirement of a full JOIN of Variant/CallSet within a VariantSet may be a technically unreasonable invariant for a server to enforce (especially if a CallSet is associated with more than one VariantSet as is allowed today - though I don't agree with that and maybe that's slated for removal).
What are the semantics of having no corresponding Call resource for a CallSet on a given Variant? We can be explicit in documenting this, but I think it naturally follows the same semantics as not representing a Variant in your VariantSet at all.
We should clarify the spec on this point and improve the corresponding documentation one way or the other.

If you accept my assertion in (1), we now need to clarify whether SearchVariants() is performing an OUTER or INNER JOIN of Variants and CallSets.

More concretely, if I request {callSetIds: ["callsetA"]}, do I get back:
a. All Variants, even those with 0 matching calls (OUTER JOIN).
b. Only Variants that have at least one matching call (INNER JOIN).
If it's (a), then the semantics you propose follow naturally. If it's (b), then I find my proposal to be more natural. Google's API currently implements (b) - I feel that (b) is the desired behavior in this case, as it allows for a terse response which includes only the relevant Variants. If I'm searching by CallSet, I care about the Calls - the Variant is just side-data used to interpret these Calls.

When you request info from a record, there must be a
distinction between "that field was null" and "you didn't ask for the value
of that field".

Yes, this is a better articulation of my concerns around returning 0 calls by default. This is also generally a problem with the SearchVariants endpoint whenever you are filtering by callSetId. The root of the issue is that a Variant is not really a standalone resource as the API currently defines it. Rather there are Variants and Calls which have been fused into a single representation for the sake of convenience. I think we will have to live with this caveat unless we are willing to change that representation.

This is a great analysis @calbach, and has brought up a number of things I hadn't realised. My position would be, make the semantics OUTER JOIN, return a NO CALL for every missing call that is requested, and keep the request semantics I've argued for above. We absolutely need to clarify the points you've raised though, and I think we need some wider input here.
The `convenience' of merging variants and calls is looking more questionable all the time...

Who else should we be getting input from on this? @lh3 maybe?
Perhaps @sarahhunt can comment on what EBI does here with their implementation of the variants API (though I'm guessing this is also closely tied to VCF)?
The 'NO CALL' is not a bad solution (though not my first preference). We would have to make this clear in the documentation since I think this generally indicates a lack of sufficient quality data to make a call - compared to the semantics of not including a variant at all, which generally means that you just didn't see any non-reference calls in your study (unless this is a gVCF). Returning a reference call for all missing calls is probably overstepping and could easily lead to errors.

IMHO, containing the Call objects in Variant is not
the best representation due to it's unbounded size.
A query to return the calls as a separate step would be give the
ability to filter what could be a very long list and result.
CH Albach<EMAIL_ADDRESS>writes:

Who else should we be getting input from on this? @lh3 maybe?
Perhaps @sarahhunt can comment on what EBI does here with their implementation
of the variants API (though I'm guessing this is also closely tied to VCF)?
The 'NO CALL' is not a bad solution (though not my first preference). We would
have to make this clear in the documentation since I think this generally
indicates a lack of sufficient quality data to make a call - compared to the
semantics of not including a variant at all, which generally means that you
just didn't see any non-reference calls in your study (unless this is a gVCF).
Returning a reference call for all missing calls is probably overstepping and
could easily lead to errors.
—
Reply to this email directly or view it on GitHub.*

@calbach - you are correct, our implementation is VCF based and we return 'no call's for missing data. If the call was 'attempted' but there was not enough information to make it, it makes sense to return this information.
My preference would be to move calls out of Variant and add a calls search with a default 'return all in set' mode and optional filtering on callset.

I've now hit this problem with the compliance suite: VariantsGetByIT.checkVariantsGetResultsMatchSearchResults is failing because of this problem.
The protobuf variant_service.proto includes the following under SearchVariantsRequest:
// Only return variant calls which belong to call sets with these IDs.
// If an empty array, returns variants without any call objects.
// If null, returns all variant calls.
repeated string call_set_ids = 2;

AFAIUI, that comment is impossible. The default value for a repeated field is the empty list. There is no distinction between empty list and null possible. If you wish to permit such a distinction (I've read through the above discussion briefly but I'm not sure there was a conclusion), one way of expressing this in protobuf would be:
message CallSetIds {
  repeated string call_set_ids = 1;
}
oneof call_set {
  bool all = 2;
  CallSetIds ids = 3;
}

That bool in there is completely pointless, but sadly protobuf doesn't support a void type that I can find (capnproto does, for example).
Fixing up the server is possible: ga4gh/datamodel/variants.py getVariants just needs the first branch conditional changing to if not callSetIds: but that breaks the server's own tests as they assume there is a distinction between None and the empty list.

I've reverted this PR to what Jerome originally suggests on the thread, i.e. [] == variants only. No one else on the thread seems particularly keen on the 'return all variant calls' use case, and this oneof proposal would unfortunately cause JSON wire format incompatibility:
{ids: {callSetIds: ["1", "2"]}} vs {callSetIds: ["1", "2"]}
I think lets minimize the diff right now to unblock the proto work, and we can re-evaluate adding the 'get all calls' functionality at a later juncture.
This PR also does not touch the issue of INNER vs OUTER JOINs or call set ordering. The semantics here will remain undefined for the time being, as it sounds like general opinion is that we should fundamentally change the API rather than update the semantics of what we have today.

I've also updated the protobuf branch to reflect this.

Great, thanks @calbach!
+1

The score for this PR is currently at 0. @diekhans if you flip your -1 -> +1, we should have consensus to merge.

I'm developing a tangentially related API (with proto3), and have been working heavily with searchVariants() for quite some time now. I ran into the similar null, [], [id1, id2] issue for a different set of objects and was hoping to follow the pattern you were using and ran into this thread... With that disclaimer, I can provide a little bit of feedback here given my experience with the SearchVariantsRequest (apologies for duplicating conversation, but additional input shouldn't hurt):
My primary use case for searchVariants is {callSetIds: null }. Under the filter possibilities you propose, I will have to pass a list with all callSetIds to get the equivalent behavior, assuming I want to access the actual call information across a set of samples (which I frequently do over 8000 samples). Note that our group has assumed the callSetId filter is performing an INNER join, not an outer join. Also, CallSet ordering in the calls object has never been a concern.
As you have already noted a bit above, the issue with CallSetIds could be solved by:

proceed with what you have and make the client responsible for constructing the complete list of callsets.

(again, as you mention could eventually be replaced by field masks - that looks like the right solution assuming the group decides to stick with these schemas)

message CallSetIds {
  bool return_calls = 1;
  repeated string callset_ids  = 2;
}

message SearchVariantsRequest {
  ...
  CallSetIds callset_ids = n;
 ...
}

Note that the return_calls could also just be an optional bool in the SearchVariantsRequest object you have now, as well.
with option 2 you happen to get an additional bonus behavior, which is not seen in the API today (which could be useful to some).  Only variants belonging to a subset of CallSets, without call info: (return_calls = false, callset_ids not empty)

with option 1, you proceed with the current api implementation and are always returning call info given a callset filter.
Perhaps the question is: would the community find behavior 4 useful?

Thanks @jacmarjorie for the feedback. I think in general the usefulness of (4) will depend on the semantics of 'call absence' in the variant set; it translates into a light-weight call to get all variants which are 'relevant' for a given subpopulation. I think it's probably not a huge win in terms of functionality, but in my opinion it's a logical mode to support.
Currently I think there's not much agreement on the primary use cases, perhaps you can add some more insight here from your experience? I'll list out my primary use cases in order of importance as reference from my experience serving the API and interacting with customers:

Get variants+calls for all call sets.

Get variants-only (for either all call sets, or a limited number).
Get variants+calls excluding some callsets, (e.g. this CallSet may have issues with QC).

So can you technically achieve my primary use case using my current PR? Sure, e.g. by including 8K call set IDs in the request. Is that made easy for the client? Not really. Does that matter? It depends on whether people agree that this is a primary use case.
My latest approach with this PR has been to remove the more controversial aspects of dealing with INNER vs OUTER JOIN semantics by dropping the "short-hand" notation for getting all variant calls. I still believe that the API needs an opinion on this, and I think it should have the short-hand, but I may try to tackle this in a follow-up PR if this is needed to unblock proto work. Of course, I'll also be happy if consensus changes quickly and I can change this PR to something similar to what you describe.

How many samples do the Google customer VCFs tend to have?  The
problem with the `all calls' default behavior was that it
generated slow performance because some VCFs had thousands of
samples.
CH Albach<EMAIL_ADDRESS>writes:

Thanks @jacmarjorie for the feedback. I think in general the usefulness of (4)
will depend on the semantics of 'call absence' in the variant set; it
translates into a light-weight call to get all variants which are 'relevant'
for a given subpopulation. I think it's probably not a huge win in terms of
functionality, but in my opinion it's a logical mode to support.
Currently I think there's not much agreement on the primary use cases, perhaps
you can add some more insight here from your experience? I'll list out my
primary use cases in order of importance as reference from my experience
serving the API and interacting with customers:

Get variants+calls for all call sets.

Get variants-only (for either all call sets, or a limited number).
Get variants+calls excluding some callsets, (e.g. this CallSet may have
issues with QC).

So can you technically achieve my primary use case using my current PR? Sure,
e.g. by including 8K call set IDs in the request. Is that made easy for the
client? Not really. Does that matter? It depends on whether people agree that
this is a primary use case.
My latest approach with this PR has been to remove the more controversial
aspects of dealing with INNER vs OUTER JOIN semantics by dropping the
"short-hand" notation for getting all variant calls. I still believe that the
API needs an opinion on this, and I think it should have the short-hand, but I
may try to tackle this in a follow-up PR if this is needed to unblock proto
work. Of course, I'll also be happy if consensus changes quickly and I can
change this PR to something similar to what you describe.
—
Reply to this email directly or view it on GitHub.*

An option that explicitly specifies the behavior rather than
implicitly specifying it in an option would be both clearer and
eliminate these trade offs were overriding the implicit behavior
is bad for someone.
I would rather have much clearer semantics than avoid having an
extra option.
CH Albach<EMAIL_ADDRESS>writes:

Thanks @jacmarjorie for the feedback. I think in general the usefulness of (4)
will depend on the semantics of 'call absence' in the variant set; it
translates into a light-weight call to get all variants which are 'relevant'
for a given subpopulation. I think it's probably not a huge win in terms of
functionality, but in my opinion it's a logical mode to support.
Currently I think there's not much agreement on the primary use cases, perhaps
you can add some more insight here from your experience? I'll list out my
primary use cases in order of importance as reference from my experience
serving the API and interacting with customers:

Get variants+calls for all call sets.

Get variants-only (for either all call sets, or a limited number).
Get variants+calls excluding some callsets, (e.g. this CallSet may have
issues with QC).

So can you technically achieve my primary use case using my current PR? Sure,
e.g. by including 8K call set IDs in the request. Is that made easy for the
client? Not really. Does that matter? It depends on whether people agree that
this is a primary use case.
My latest approach with this PR has been to remove the more controversial
aspects of dealing with INNER vs OUTER JOIN semantics by dropping the
"short-hand" notation for getting all variant calls. I still believe that the
API needs an opinion on this, and I think it should have the short-hand, but I
may try to tackle this in a follow-up PR if this is needed to unblock proto
work. Of course, I'll also be happy if consensus changes quickly and I can
change this PR to something similar to what you describe.
—
Reply to this email directly or view it on GitHub.*

After trying to catch up with the thread, I believe the scope needs to be reduced in order for @calbach to proceed with the protobuf definitions. So can an executive decision be made about how to support returning calls (or not returning calls) and move all other things (ordering ad inner/outer joins) to a separate issue?
I believe that @diekhans is suggesting that the presence of a bool flag in the SearchVariantsRequestObject would provide a functionality that would explicitly say return calls or not return calls. This seems to be in line with what @haussler is saying in making a distinction between a null value and not asking for a value of a certain field. I believe this approach would allow @calbach to continue and would address all possible use cases associated with variants/call objects being returned. This would leave inner and outer join up to the implementation for now until the group wants to make a more firm decision on if they want to support inner, outer, or both.
Here is some discussion on the topic, but want to stress the importance of the question @calbach wants answered in ordered to merge this PR.
TL;DR.
Summary of possible use cases (all of which are valid and probably should be supported by the API) ignoring ordering, that is:
1. all variants, no call info
2. all variants, all call info
a. call info only contains calls from callsets with that variant (INNER JOIN)
b. call info contains all callsets in the variant set, absense of the reported variant on a callset gives [0,0] genotype? or the call list is empty? (OUTER JOIN)
3. only variants belonging to a subset of callsets, with call info

only variants belonging to a subset of callsets, without call info

(The bold are the use cases I frequently encounter, however I have performed 2b. client side).
Note the documentation appears to argue for an outer join (at least when call set id is not present), which would be consistent with a multi-sample VCF:
  /**  The name of the call set this variant call belongs to.  If this field is not present, the ordering of the call sets from a  `SearchCallSetsRequest` over this `VariantSet` is guaranteed to match  the ordering of the calls on this `Variant`.  The number of results will also be the same.  */

With all that - I don't think the argument is whether one of these use cases is more valid or not because a case could be argued for each one, and removing one of the above would limit the API or require additional processing on client side (as I have done with 2b before).
If you think about how one is pulling down variants right now through the API, the standard procedure is /datasets/search, /variantssets/search, /variants/search, it would add yet an additional call (to essentially retrieve similar data to separately request the calls - and even then you would still want the variant information associated with these calls). That makes it hard to argue for an additional /calls/search function which would essentially duplicate /variants/search with call information.
And finally, I'm happy to open these up as an issue (if there isn't already one created).

Instead of a boolean flag, I would suggest a enum to define the
operation. There are current four proposed use cases.  Even with
two cases, a enum is clearer and allows for expansion.
The array of ids then become an argument to the operation.
Jaclyn Smith<EMAIL_ADDRESS>writes:

After trying to catch up with the thread, I believe the scope needs to be
reduced in order for @calbach to proceed with the protobuf definitions. So can
an executive decision be made about how to support returning calls (or not
returning calls) and move all other things (ordering ad inner/outer joins) to a
separate issue?
I believe that @diekhans is suggesting that the presence of a bool flag in the
SearchVariantsRequestObject would provide a functionality that would explicitly
say return calls or not return calls. This seems to be in line with what
@haussler is saying in making a distinction between a null value and not asking
for a value of a certain field. I believe this approach would allow @calbach to
continue and would address all possible use cases associated with variants/call
objects being returned. This would leave inner and outer join up to the
implementation for now until the group wants to make a more firm decision on if
they want to support inner, outer, or both.
Here is some discussion on the topic, but want to stress the importance of the
question @calbach wants answered in ordered to merge this PR.
TL;DR.
Summary of possible use cases (all of which are valid and probably should be
supported by the API) ignoring ordering, that is:

all variants, no call info

all variants, all call info
a. call info only contains calls from callsets with that variant (INNER JOIN)
b. call info contains all callsets in the variant set, absense of the reported
variant on a callset gives [0,0] genotype? or the call list is empty? (OUTER
JOIN)

only variants belonging to a subset of callsets, with call info

only variants belonging to a subset of callsets, without call info

(The bold are the use cases I frequently encounter, however I have performed
2b. client side).
Note the documentation appears to argue for an outer join (at least when call
set id is not present), which would be consistent with a multi-sample VCF:
/**  The name of the call set this variant call belongs to.  If this field is not present, the ordering of the call sets from a  SearchCallSetsRequest over this VariantSet is guaranteed to match  the ordering of the calls on this Variant.  The number of results will also be the same.  */
With all that - I don't think the argument is whether one of these use cases is
more valid or not because a case could be argued for each one, and removing one
of the above would limit the API or require additional processing on client
side (as I have done with 2b before).
If you think about how one is pulling down variants right now through the API,
the standard procedure is /datasets/search, /variantssets/search, /variants/
search, it would add yet an additional call (to essentially retrieve similar
data to separately request the calls - and even then you would still want the
variant information associated with these calls). That makes it hard to argue
for an additional /calls/search function which would essentially duplicate /
variants/search with call information.
And finally, I'm happy to open these up as an issue (if there isn't already one
created).
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub*

@diekhans - thousands of CallSets per VariantSet seems to be typical from the public and/or widely available datasets I'm familiar with. Not sure if the question was informational, or a possible argument against this feature, if it's the latter: at the end of the day, if the client actually wants all calls, lack of API support is just putting a roadblock in their way and not avoiding the performance issue.
Note: The pagination semantics at least allow you some flow control on the server; in the most extreme case you could limit yourself to returning 1 variant (w/ all calls) per page to negate performance issues. Google's implementation will do this response sizing based on the number of calls returned.
@jacmarjorie thanks for adding those, this is very useful to the discussion. I think the INNER vs OUTER JOIN issue should perhaps be split off from this PR; feel free to create an issue if you'd like to. Note: if it's an OUTER join, the interaction between a possible retrieveCalls and callSetIds field is dubious - but perhaps this can be overlooked for the time being.
RE: boolean field
There's also a small question of whether it should be retrieveCalls or maskCalls, i.e. what is the default behavior. My preference is for the latter, following my personal rule of 'the most obvious query should yield the most obvious result' - in other words don't drop fields which are documented in the response value unless that behavior is explicitly requested. Based on previous comments, I would guess others may disagree with this as the default behavior.
Sounds like there is some vague support for adding this boolean flag. I'd like to hear @jeromekelleher's thoughts as well.

@calbach Current behavior is callSetIds = null as default. This would be equivalent to retrieveCalls, no? And would be consistent to not dropping fields that are documented in the response.

@calbach the question was just informational.  I was curious
about the data that google genomics users create, if there are
any patterns there that could be informative to the right
default.
The flow control wasn't the issue with performance.  It was the
cost of converting VCF to JSON and then read large data that
isn't needed.
Personally, I think variants containing calls vs a query to
return them isn't the best representation.  It's an
indeterminate sized object requiring these kind of queries.
CH Albach<EMAIL_ADDRESS>writes:

@diekhans - thousands of CallSets per VariantSet seems to be typical from the
public and/or widely available datasets I'm familiar with. Not sure if the
question was informational, or a possible argument against this feature, if
it's the latter: at the end of the day, if the client actually wants all calls,
lack of API support is just putting a roadblock in their way and not avoiding
the performance issue.
Note: The pagination semantics at least allow you some flow control on the
server; in the most extreme case you could limit yourself to returning 1
variant (w/ all calls) per page to negate performance issues. Google's
implementation will do this response sizing based on the number of calls
returned.
@jacmarjorie thanks for adding those, this is very useful to the discussion. I
think the INNER vs OUTER JOIN issue should perhaps be split off from this PR;
feel free to create an issue if you'd like to. Note: if it's an OUTER join, the
interaction between a possible retrieveCalls and callSetIds field is dubious -
but perhaps this can be overlooked for the time being.
RE: boolean field
There's also a small question of whether it should be retrieveCalls or
maskCalls, i.e. what is the default behavior. My preference is for the latter,
following my personal rule of 'the most obvious query should yield the most
obvious result' - in other words don't drop fields which are documented in the
response value unless that behavior is explicitly requested. Based on previous
comments, I would guess others may disagree with this as the default behavior.
Sounds like there is some vague support for adding this boolean flag. I'd like
to hear @jeromekelleher's thoughts as well.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub*

@jacmarjorie yes, the current default behavior is to return all variant calls which is equivalent to setting retrieveCalls to true. In proto3, the default value for a boolean field is false if left uninitialized, which is why I was suggesting the inverse, i.e. maskCalls defaults to false so all calls are returned by default. The client would need to explicitly set this to true to get variants-only.
RE: enum
I don't think there is a need to support both INNER and OUTER JOINS, I think we need to decide which one is correct (I feel strongly that INNER is correct) and make this clear in the documentation. Note that this distinction should be irrelevant to a VCF-backed implemented, as all variants are guaranteed to have a Call entry for all CallSets.

RE: default behavior
I don't know what the right default behavior is, however I think
we should talk to people who do variant analysis as part of
their day to day work, like @lh3, and not optimize for either
the google data center or the reference server.
RE: enum
I don't know if we need outer join or not, however enum makes it cleaner
to extend for the future.
Either enum or boolean (with the possibility of adding more booleans)
is better than implicit behavior based on what is specified.  Although
all of this makes SQL look eloquent.
Summary:
Lets talk to people who actually use VCF.
CH Albach<EMAIL_ADDRESS>writes:

@jacmarjorie yes, the current default behavior is to return all variant calls
which is equivalent to setting retrieveCalls to true. In proto3, the default
value for a boolean field is false if left uninitialized, which is why I was
suggesting the inverse, i.e. maskCalls defaults to false so all calls are
returned by default. The client would need to explicitly set this to true to
get variants-only.
RE: enum
I don't think there is a need to support both INNER and OUTER JOINS, I think we
need to decide which one is correct (I feel strongly that INNER is correct) and
make this clear in the documentation. Note that this distinction should be
irrelevant to a VCF-backed implemented, as all variants are guaranteed to have
a Call entry for all CallSets.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub*

I'm afraid I still don't understand why we need the 'return all calls' special case. The INNER JOIN vs OUTER JOIN semantics is an orthogonal question, since we must define the JOIN semantics for any subset of all callsets in any case (won't there be variants that don't have calls even if we have only one callSetId?). Having the possibility of different semantics when we get all callsets is yet another argument for not having what is (in my opinion and experience) an ugly and problematic special case.

@calbach Given the switch to protobuf can we close?


Support for Amazon-ES.

Description
Hi Mate! Great project!! Can I suggest the inclusion of Amazon-ES (Spain) to the store lists? I believe they are the same URLs as in the DE version but changing the sufix. I can help with any translation if you want!
Thanks in advance!!

That would be great! Don't mind having more stores. Give a try and create a PR!


[Bug]:  TypeError: Cannot read properties of undefined (reading 'url')

What happened?
When I try to upload an image I get this error msg
- error TypeError: Cannot read properties of undefined (reading 'url')     at eval (webpack-internal:///(rsc)/./node_modules/uploadthing/dist/chunk-LQBAB5WK.mjs:257:36)     at Object.POST (webpack-internal:///(rsc)/./node_modules/uploadthing/dist/chunk-66XXRAVC.mjs:285:32)
What did you expect to happen?
should upload without any issue
Version
  "uploadthing": "^5.6.1"
  "@uploadthing/react": "^5.6.1",

Reproduction Steps
the files like this
app/api/uploadthing/core.ts
import { createUploadthing, type FileRouter } from 'uploadthing/next';

const f = createUploadthing();

// FileRouter for your app, can contain multiple FileRoutes
export const ourFileRouter = {
	// Define as many FileRoutes as you like, each with a unique routeSlug
	imageUploader: f({ image: { maxFileSize: '4MB' } }).onUploadComplete(
		async ({ metadata, file }) => {
			// This code RUNS ON YOUR SERVER after upload
			console.log('Upload complete for userId:');

			console.log('file url', file.url);
		}
	),
} satisfies FileRouter;

export type OurFileRouter = typeof ourFileRouter;

app/api/uploadthing/route.ts
import { createNextRouteHandler } from 'uploadthing/next';

import { ourFileRouter } from './core';

// Export routes for Next App Router
export const { GET, POST } = createNextRouteHandler({
	router: ourFileRouter,

});

utils/uploadthing.ts
import { generateComponents } from '@uploadthing/react';

import type { OurFileRouter } from '@/app/api/uploadthing/core';

export const { UploadButton, UploadDropzone, Uploader } =
	generateComponents<OurFileRouter>();

and the component like this:
'use client'
import {  UploadDropzone } from '@/utils/uploadthing';

import<EMAIL_ADDRESS>
const Uploader = () => {
	return (
    <UploadDropzone
        endpoint="imageUploader"
        onClientUploadComplete={(res) => {
          // Do something with the response
          console.log("Files: ", res);
          alert("Upload Completed");
        }}
        onUploadError={(error: Error) => {
          // Do something with the error.
          alert(`ERROR! ${error.message}`);
        }}
      />
	);
};

export default Uploader;

PS: I tried to downgrade to [5.3.0] and it's working fine
Code of Conduct

[X] I agree to follow this project's Code of Conduct

Works for me, can you provide a reproduction repo please?

I have the same problem

Works for me, can you provide a reproduction repo please?

just follow the docs

I have the same problem

try to downgrade to [5.3.0]

Works for me, can you provide a reproduction repo please?

just follow the docs

I did.

I have the same problem

try to downgrade to [5.3.0]

That breaks some types.

Working minimal repo here: https://github.com/juliusmarminge/ut-repro
Without a consistent reproduction repo, unfortunately we will not be able to help you any further. Feel free to reopen with a link to a repo where you can reproduce this error.

Not minimal, but anyway bugged: https://github.com/St1ggy/discord-clone/tree/uploadthing-bug

git clone https://github.com/St1ggy/discord-clone/tree/uploadthing-bug
git checkout uploadthing-bug
Create a .env and put in your secrets
bun i
bun dev
Upload file. Meet the bug.

Works fine on my machine


Aborted processes in integrated terminal cannot return to shell

VSCode Version: 1.45.1
OS Version: macOS Catalina 10.15.4

Steps to Reproduce:

create and compile a c program (or anything that can receive SIGABRT)

void abort();
int main(void)
{
    abort();
}

run the program.
after abort() is called, the control never goes back to the shell.

Extras:
I have already tested with zsh and bash, none of them works, so it shouldn't be the shell's fault.

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

Oops, my fault. Fixed in the insiders


ci: added miri job

Closes #1088

Codecov Report

Merging #1103 (210950e) into main (e0dbcae) will decrease coverage by 0.03%.
The diff coverage is n/a.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
@@            Coverage Diff             @@
##             main    #1103      +/-   ##
==========================================
- Coverage   74.63%   74.61%   -0.03%
==========================================
  Files         321      321
  Lines       34954    34954
==========================================
- Hits        26088    26081       -7
- Misses       8866     8873       +7

Flag
Coverage Δ

unit-tests
74.61% <ø> (-0.03%)
:arrow_down:

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

Impacted Files
Coverage Δ

crates/stages/src/stages/bodies.rs
91.57% <0.00%> (-1.56%)
:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Yep, there's problems with #[test_fuzz] and #[tokio::test] as specified by tokio's miri job. I've been debugging this for a while and the solution doesn't seem so trivial, I'm not sure how tokio solved this, they just seem to accepted the fact that some tests may fail by using --no-fail-fast.

I believe the tokio crate has some comments regarding miri and tokio::test tokio::main in its codebase

I did some investigation with the help of @fkrause98 and we don't see a way out from this right now:

The general problem is that miri doesn't support many functions and syscalls which are implemented in libraries like tokio and test-fuzz. In particular macros like #[test_fuzz] and #[tokio::test] derive unsupported functionality by miri. This means that any test executed with these will fail.
We think tokio's way of dealing with this is by only testing lib tests by using the --lib flag, not the bin crate one's which is where they implement the most unsupported functionality. There's only one test in their lib which would fail and they make miri ignore it by the use of #[cfg_attr(miri, ignore)].  They also use the --no-fail-fast flag I think because of convenience, so only one failed miri test doesn't stop the whole job. I don't think it's related to the problem.

We could theoretically implement #[cfg_attr(miri, ignore)] in every test that fails because of unsupported, but:

They're too many. Every async tokio test will need to be ignored, also every fuzz test, and I found some of these which are generated by other functions, something that adds complexity.
Everytime a new test is implemented, we need to be careful to not use unsupported functionality or to implement the ignore macro. This creates burden for people who didn't jump into this rabbit hole yet.

Possible ways of action:

Using the -Zmiri-panic-on-unsupported flag and find a way to filter out every test which fails because of unsupported functionality (this is relevant)
Could it be possible to make miri ignore every test and enable it only on those we think could have UB?

New ideas and insight about this are welcome, we may be missing something

I think in extension to above, we should probably not fail CI if this job fails seeing as miri is unstable/experimental? @mattsse

@gakonst using the -Zmiri-panic-on-unsupported flag every test that fails because of a panic should be about unsupported functionality, and every other which fails with an error should be because of UB. Also we can filter just by the unsupported operation string.

I arrived to this solution by creating a custom github action, it works by:

Outputting the tests results in json, parsing and formatting these in a readable way with jq.
Tests status can be ok, failed or unsupported. Unsupported tests' errors are supressed by the use of -Zmiri-panic-on-unsupported, although some tests seem to bypass this and throw errors on stdout anyway instead of just panicking.
Because of the unsupported tests, the job would always fail, so I piped the output to a script that exits with status 101 only if a regex matches that a test has status failed (because of UB). Else it exits with 0.
Proptests seem to hang with miri and get sigkilled

This is all really crumbly, the action facilitates things a bit but miri is slow and unstable (oficially experimental), I would only recommend to run this job maybe in a set apart branch that's up to date with main. Maybe a CI job with miri should be scraped entirely and just run it locally with a limited amount of tests each time.

oof it looks like it doesn't like FFI either

We can integrate this with nextest which is pretty nice: https://nexte.st/book/miri.html

I discarded nextest because it doesn't yet support tests run in json output

@xqft what's the plan here? I think if we don't have a "clean" way of integrating with success, let's close and revisit later.

agreed - thank you for taking the time in digging into this.


Add an ECR repository for the SageMaker training image

SageMaker requires its training images to be in ECR.  Docker Hub won't work.

Plan
Trello card

Whoops,       "ecr:BatchDeleteImage", shouldn't be in the "read" permissions...


[SPARK-27995][PYTHON] Note the difference between str of Python 2 and 3 at Arrow optimized

What changes were proposed in this pull request?
When Arrow optimization is enabled in Python 2.7,
import pandas
pdf = pandas.DataFrame(["test1", "test2"])
df = spark.createDataFrame(pdf)
df.show()

I got the following output:
+----------------+
|               0|
+----------------+
|[74 65 73 74 31]|
|[74 65 73 74 32]|
+----------------+

This looks because Python's str and byte are same. it does look right:
>>> str == bytes
True
>>> isinstance("a", bytes)
True

To cut it short:

Python 2 treats str as bytes.
PySpark added some special codes and hacks to recognizes str as string types.
PyArrow / Pandas followed Python 2 difference

To fix, we have two options:

Fix it to match the behaviour to PySpark's
Note the differences

but Python 2 is deprecated anyway. I think it's better to just note it and for go option 2.
How was this patch tested?
Manually tested.
Doc was checked too:

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/11615/
Test PASSed.

Test build #106370 has started for PR 24838 at commit 9ec1350.

cc @BryanCutler

Test build #106370 has finished for PR 24838 at commit 9ec1350.

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

Merged build finished. Test PASSed.

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

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/11619/
Test PASSed.

Test build #106374 has started for PR 24838 at commit 554d508.

Test build #106374 has finished for PR 24838 at commit 554d508.

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

Merged build finished. Test PASSed.

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

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/11630/
Test PASSed.

Test build #106385 has started for PR 24838 at commit 0457d26.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/11631/
Test PASSed.

I refined the words more.

Merged to master.
The diff was only words in comments. I checked the doc build and linter locally for doubly sure.
Thanks for review and approvals @dongjoon-hyun and @viirya

Test build #106386 has started for PR 24838 at commit d03fe51.

Test build #106385 has finished for PR 24838 at commit 0457d26.

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

Merged build finished. Test PASSed.

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

Test build #106386 has finished for PR 24838 at commit d03fe51.

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

Merged build finished. Test PASSed.

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

+1 on the note. Also, specifying the schema as string type should work too.

Oh, right - we noticed this and fixed in that way in Pandas UDF right :-).


Error  from version 1.5.2

I can't open the mod from version 1.5.2, it gives an error identical when you open any other mod without putting it in the mod folder

I understood why, i download the Java 19, and it works, but only 1.17 i was need 1.19

if i use in 1.18 will it work? i think this update only for caves from -0


Setup release action

Is this PR ready to be reviewed ? @iocanel

Any update here ? @iocanel

Figuring this should be closed out in favor of #63 .. if no objections?


gsmartcontrol and ario collide

Issue description
I have gsmartcontrol installed
$ which gsmartcontrol
/home/m/.nix-profile/bin/gsmartcontrol

and wanted to install ario:
$ nix-env -iA nixos.ario
installing 'ario-1.5.1'
building '/nix/store/rp77jbk9b8r597477gl6wdn0fcxy82k1-user-environment.drv'...
error: Packages '/nix/store/w1ns1nmfnydymswfqznqkckhgla4hfq0-gsmartcontrol-0.8.7/share/icons/hicolor/icon-theme.cache' and '/nix/store/ia576bhbypi1vhjgikyxyqx0g0vqy8d8-ario-1.5.1/share/icons/hicolor/icon-theme.cache' have the same priority '5'use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' to change the priority of one of the conflicting packages ('0' being the highest priority)
builder for '/nix/store/rp77jbk9b8r597477gl6wdn0fcxy82k1-user-environment.drv' failed with exit code 1
error: build of '/nix/store/rp77jbk9b8r597477gl6wdn0fcxy82k1-user-environment.drv' failed

Steps to reproduce
nix-env -iA nixos.gsmartcontrol
nix-env -iA nixos.ario

Technical details

system: "x86_64-linux"
host os: Linux 4.14.24, NixOS, 18.09pre133621.56fb68dcef4 (Jellyfish)
multi-user?: yes
sandbox: yes
version: nix-env (Nix) 2.0
channels(root): "nixos-18.09pre133621.56fb68dcef4"
channels(m): ""
nixpkgs: /nix/var/nix/profiles/per-user/root/channels/nixos/nixpkgs

This happens when build scripts run gtk-update-icon-cache automatically. We never want this to happen in Nixpkgs because the cache will break immutability. Probably need a setup-hook to remove this automatically (it should be run automatically by system-path.nix for you).

hicolor-icon-theme does this.

I fixed this for gsmartcontrol in https://github.com/NixOS/nixpkgs/commit/9acdfbfd7280f7ae45411e2f3c005269dc053e78 but there are many packages that still contain the file.


[Event Hubs] Processor Release Prep

Summary
The focus of these changes is to prepare the Event Hubs Processor package for an out-of-band bugfix release.

API change check
API changes are not detected in this pull request.


Consider 3xx responses as successfull and cache them

Cache headers are applied only to successful responses. In the case, this means HTTP responses codes >= 200 and < 300. I think 3xx responses should also be considered successful responses, and therefore included in the default behavior. I would still not cache 1xx responses and client (4xx) and error (5xx) responses.
For example, CloudFront caches 3xx responses: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/http-3xx-status-codes.html

Hey @ragazzojp,
0.3.0 now contains the ResponseIsSuccessfulOrRedirect evaluator that can be used with only_if=ResponseIsSuccessfulOrRedirect. I hope it works for you.
BTW: be sure to check the changelog. The release has other improvements and does have breaking changes.


Perfomance Metrics?

We've been having some consistent problems with ElasticSearch and are nearly convinced to move away from it in an effort to simplify things since we're already using PGSQL.
Does anyone have any performance metrics that are current to April 2015 for pg_search results vs the other usual options?
I've seen some interesting discussion here:

(one benchmark is quite dated; 2009) http://stackoverflow.com/questions/10875674/any-reason-not-use-postgresqls-built-in-full-text-search-on-heroku
https://github.com/codeforamerica/ohana-api/issues/139

These links are interesting and reveal some truth but anyway you should  do all the benchmarks yourself against your data and with your specific queries to this data because performance is highly dependent on data characteristics. Benchmarking random reads / random writes on random data won't reveal a whole picture.

Yes, when using something like ElasticSearch you have a completely different architecture with a large number of variables, not limited to:

how the data is distributed
the nature and complexity of the queries
hosting PostgreSQL and ElasticSearch on the same server or on a different server as the application
network latency
CPU / memory power on each machine
indexes
versions of PostgreSQL and ElasticSearch

So it's outside of the scope of this project to provide any sort of performance metrics. I'd welcome anyone to blog about any performance testing they've done for their individual use cases. It would be useful to add a wiki page outlining things like this.


chore(plugin-shiki): tweaks

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

32 of 44 (72.73%)  changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.05%) to 56.545%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

plugins/markdown/plugin-shiki/src/node/markdown/applyHighlighter/createMarkdownFilePathGetter.ts
1
7
14.29%

plugins/markdown/plugin-shiki/src/node/markdown/applyHighlighter/getLanguage.ts
5
11
45.45%

Totals

Change from base Build<PHONE_NUMBER>:
0.05%

Covered Lines:
1010

Relevant Lines:
1600

💛  - Coveralls


Learn Relational Databases courses don't open

Describe the Issue
The start button opens an "about:blank" page in my browser rather than navigating me to GitPod unlike the NPM backend challenges.
Affected Page
https://www.freecodecamp.org/learn/relational-database/learn-relational-databases-by-building-a-mario-database/build-a-mario-database
Your code
N/A

Expected behavior
GitPod should have opened up and created a new repository for me to start CodeRoad in.
Screenshots
No response
System

Device: Desktop
OS: Windows 10
Browser: Firefox
Version: 124.0

Additional context
No response

My browser was blocking it. Never mind.

What was the issue @a2937? Was there a setting or something you needed to change?

There was a setting in Firefox I had to change. I unfortunately forgot what I had setting I had to change. It might have had something to do with my protection settings.

It looks like the issue is resolved so I am going to go ahead and close this 👍


Automation Script to connect remote server and install devtron after creation of cluster from Devtron's app

Here I am doing POC for resolving Automation problem where I have to create a KOPs cluster after connecting to remote bastion from Devtron's custom App >>>Deploy Devtron in same cluster>>>Run Automation in same cluster>>> Delete Cluster
This will help us to run our automation to run dynamically in any newly created cluster.

Already invested 3 days in this sprint ,as of now I am blocked on this so closing it and create a new task for the same in next sprint for the specific work which is remaining.


Fixed bug in TextInputs when scrolling

Checklist
(Check off all the items before submitting)

[x] Build process is done without errors. All tests pass in the /lib directory.
[x] Self-reviewed the code before submitting.
[x] Meets accessibility standards.
[x] Added/updated documentation to /website as needed.
[x] Added/updated tests as needed.

Description
Fixes bug related to #1800 where, on scroll, text inputs where displaying NaN.
The solution has been adding the pre-condition of the field being a number field before applying the handleWheel callback.

We can solve the useEffect dependencies issue splitting the onWheel functionality:

Use the onWheel event of the Input component to do the increment/decrement functionality (remember to remove the event.preventDefault, is a passive event):

    const handleNumberInputWheel = (event: React.WheelEvent<HTMLInputElement>) => {
      if (document.activeElement === inputRef.current)
        event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value);
    };

onWheel={numberInputContext?.typeNumber === "number" ? handleNumberInputWheel : undefined}

Remove the useEffect from the TextInput and move it to the NumberInput. This makes sense since this functionality should only be available in that component:

    const numberInputRef = React.useRef<HTMLInputElement>(null);

    useEffect(() => {
      const input = numberInputRef.current?.getElementsByTagName("input")[0] as HTMLInputElement;
      const preventDefault = (event: WheelEvent) => {
        event.preventDefault();
      };

      input?.addEventListener("wheel", preventDefault, { passive: false });
      return () => {
        input?.removeEventListener("wheel", preventDefault);
      };
    }, []);

    return (
      <NumberInputContext.Provider value={{ typeNumber: "number", minNumber: min, maxNumber: max, stepNumber: step }}>
        <NumberInputContainer ref={numberInputRef}>

What do you think?

This looks like a much more logical approach, encapsulating the NumberInput functionality encapsulated within the appropiate component and reducing all the unnecessary dependencies inside the TextInput component.
Thanks for your time 😄


Introduce a patient context picker

When the end user has access to multiple patient records, but the user/application would like to scope the session to a single patient record, we need some way to allow the user to select a patient from the list that they have access to.

Resolved with https://github.com/Alvearie/keycloak-extensions-for-fhir/pull/11
I still have some work to do to finish the test automation.  Currently, we spin up a keycloak container and a mock fhir server, but we havn't yet worked out how to invoke the custom browser flow.


Return empty list and 200OK when no impacted clusters found for given recommendation

Description

With this change, the recommendation provided to the rule/{rule_selector}/clusters_detail endpoint is first checked for existence, and in case it is not in the existing external rules, a 404 is returned.
If the rule exists and the DB returns an ItemNotFound error, the proxy will return a JSON with an empty data field and a 200 OK status instead of proxying the 404 error to the client.

Type of change
Please delete options that are not relevant.

Breaking change (fix or feature that would cause existing functionality to not work as expected)

Testing steps
Updated UTs
Checklist

[x] make before_commit passes
[ ] updated documentation wherever necessary
[x] added or modified tests if necessary
[ ] updated schemas and validators in insights-data-schemas in case of input/output change

Codecov Report

Merging #709 (78163c1) into master (c184425) will increase coverage by 0.16%.
The diff coverage is 55.26%.

@@            Coverage Diff             @@
##           master     #709      +/-   ##
==========================================
+ Coverage   52.53%   52.70%   +0.16%
==========================================
  Files          20       20
  Lines        2541     2554      +13
==========================================
+ Hits         1335     1346      +11
- Misses       1050     1051       +1
- Partials      156      157       +1

Impacted Files
Coverage Δ

server/handlers_v2.go
55.55% <55.26%> (+1.25%)
: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 c184425...78163c1. Read the comment docs.


ENYO-887 If a control is out of viewport, we will not spot it.

Issue
In fitstUse app, user can focus some controls which are out of viewport.
Cause

FirstUse app does not have guide code like spotlightDisable = true when its custom panels changes index.
Spotlight does not concern control's absoluteBounds when it spot.

Fix
Add a condition to check its absoluteBounds
Enyo-DCO-1.1-Signed-off-by: David Um<EMAIL_ADDRESS>

@DavidUM, the current behavior is by design. In certain cases, we rely upon being able to spot something that is currently positioned offscreen. For example, using your branch:
In moon.Scroller2dSample, use 5-way controls to move down and to the right, then try to move back up and to the left, and you'll see that you can't.
In moon.DataListSample, collapse the top drawer, scroll down to about item 30, then try navigating back up to the top of the list using 5-way up. You'll get stuck at Item 21.
The right fix for this issue is for the app to use the APIs that we have provided to dynamically disable spotlight for controls that they have intentionally positioned offscreen.

@DavidUM  - the jira attached to this is closed. Can this PR be closed also? Thanks.


Why nm(m-1)/2 not nmlog(m) for clustering

I saw that you implemented VPtree and probably tried it. why did you decide to go with canopy clustering after all? vptree scales better

Can you describe where in the tree search that the scaling is defined as "better"?  Would like to understand more.  My understanding is that you still have to do k distance calculations and each element has log n levels.

The time cost to search a Vantage-Point tree to find a single nearest neighbor is O(log n). There are log n levels, each involving k distance calculations, where k is the number of vantage points (elements) at that position in the tree. - https://en.wikipedia.org/wiki/Vantage-point_tree

The context for this obscure question is apparently this commented out line: https://github.com/OpenRefine/OpenRefine/blob/ddd9bf9aa80a43c68bf6d8fb4dc9d0b8bb29dcd5/main/src/com/google/refine/clustering/knn/kNNClusterer.java#L236-L238
from the original implementation over a decade ago. It certainly would be easy enough to offer the user a choice if we could characterize their performance profiles in a way that would make sense to a user (and VP Trees outperform the Pass-Join algorithm).

@kirilligum @tfmorris I reached out to Stefano about this and he replied back with this when I asked him about VPTree and if he had issues with it at the time :

I have zero recollection of why... O() notation is tricky because it hides constants so it's possible that even if one algo is better from a computational complexity perspective, it's actually slower in practice because of implementation or size of the data used.
Honestly, the only way to know is to try the two and measure the difference.
Sorry if that's not really that helpful :)


script run on proxmox 7

Hi,
after trying the scipt on a fresh proxmox install, the first dkms install fails. I followed the guide found in https://wvthoog.nl/proxmox-7-vgpu/. In the attached log seems something tottaly breaks when the 5.12 patch is applied. Any chance for an insight what I might doing wrong?
driver version : NVIDIA-Linux-x86_64-460.32.04-vgpu-kvm
gpu : NVIDIA Corporation GM206GL [Quadro M2000] (yea not like the mostly tested 1050,1650,1600,2060, but theoritaclly, the card should be in the supported ranges)
make.log

/var/lib/dkms/nvidia/460.32.04/build/nvidia-vgpu-vfio/nvidia-vgpu-vfio.c:162:25: error: initialization of 'int (*)(struct mdev_device *)' from incompatible pointer type 'int (*)(struct kobject *, struct mdev_device *)' [-Werror=incompatible-pointer-types]
  162 |     .create           = nv_vgpu_vfio_create,
      |                         ^~~~~~~~~~~~~~~~~~~
/var/lib/dkms/nvidia/460.32.04/build/nvidia-vgpu-vfio/nvidia-vgpu-vfio.c:162:25: note: (near initialization for 'vgpu_fops.create')

This error is caused by this change to the linux kernel: https://github.com/torvalds/linux/commit/c2ef2f50ad0ccf5460bf4824bc6669240b6c7936 so you need a kernel version prior to that or you will need more patches.

Additionally, with a fully updated proxmox installation as of Nov 28th, 2021, the kernel is updated to 5.13.10-1-pve.  Following instructions in this repo and here fail, and I believe an update to the code will be required to get it working on the latest installation of proxmox 7.1

I did manage to succeed on the first install of the drivers due to -  as DualCoder pointed - the latest kernel changes break the install (the 7.0 proxmox is 5.11, and 7.1 is 5.13). On the 7.0, in order to avoid the upgrade of the pve-headder, you just have to skip the installation of any header upgrade when updating or skip any update calls.
Despite that though, for my try, I have not managed to make the magical phrase "vgpu unlock patch applied" be shown in dmesg in either gpu I tried: a K620, a K2200 and a m2000. The first 2 are maxwell1.0, the second is maxwell 2.0. One is not a high end, but the other 2 were "ok" cards at their timelines.
I haven't managed to find any time to test it on a 6.5 proxmox installation on the m2000 - i did try with the k620,k2200 and again no magic phrase. But it would be problematic as the iso is upgraded to the latest headers (5.10 I believe) which still needs to have patches applied. Downgrading an iso's header look wrong to me. And the other option would be to try a centos-like installation and see if it could work there.

It looks like if you use the newest version of the NVIDIA drivers from the trial website, 470.82, it works on Proxmox 7.1.
One small thing that took me too long to realize is that the dmesg output for vgpu unlock is turned off by default, so you'll never see the "vGPU unlock patch applied" message, even if it's working.  That kind of threw me for a loop for a while.
(you can turn it on and rebuild your patched drivers here: https://github.com/DualCoder/vgpu_unlock/blob/f432ffc8b7ed245df8858e9b38000d3b8f0352f4/vgpu_unlock_hooks.c#L567)

you can also run mdevctl types - if you have output there it should be working and if you do not have output something is not right/correctly done

It looks like if you use the newest version of the NVIDIA drivers from the trial website, 470.82, it works on Proxmox 7.1.
One small thing that took me too long to realize is that the dmesg output for vgpu unlock is turned off by default, so you'll never see the "vGPU unlock patch applied" message, even if it's working. That kind of threw me for a loop for a while.
(you can turn it on and rebuild your patched drivers here):
https://github.com/DualCoder/vgpu_unlock/blob/f432ffc8b7ed245df8858e9b38000d3b8f0352f4/vgpu_unlock_hooks.c#L567

The same 5.12 patch required for the 470.82 to work under Proxmox 7.1?

Dont think that this is working woth 470 I wpuöd use the same version as the linked one - know that the newer driver like 470 have additional checks in it - one check the vram of the spoofed gpu and the real amount from the gpu and if not matching error out.
Also 5.12 kernels can be patched
Vgpu was broken in 5.13 and again in 5.14, these kernels are not working yet (I heard a patch is incomimg next week for 5.14/5.15)

Any news on this? Patch available?
Thank you.

https://transfer.sh/AyU4xv/fourteen.patch
Works on some newer 5.14.x and 5.15+ - some earlier 5.14 are not working woth that.

https://transfer.sh/AyU4xv/fourteen.patch
Works on some newer 5.14.x and 5.15+ - some earlier 5.14 are not working with that.

file not found.
And is it safe to update to 5.15 under proxmox?

Saw that :/ - otherwise use this patch - https://github.com/VGPU-Community-Drivers/Merged-Rust-Drivers/tree/main/NVIDIA-Linux-x86_64-460.73.01-grid-vgpu-kvm/kernel/patches - it should be usable with just the kvm driver too and this patches - some work needs to be done bc if you are not using merged, it will try to patch to much / not existing things - also users in the discord have it running ;) so I would say its save

@VGPU-Community-Drivers can you please give a link to the Discord-Channel? Thx

In the google doc https://docs.google.com/document/d/1pzrWJ9h-zANCtyqRgS7Vzla0Y8Ea2-5z2HEi4X75d2Q/edit?usp=sharing first pagejoin our discord :)

Due to my server setting, I do not want to use the merge driver.  And I tried the kvm driver KVM-460.73.02, the fourteen.patch did not complete.  And it seems to apply to only merge driver?

Is there a working version for kernel 5.15+ yet ?

Yes. You probably need to check the discord.
actionhenkt @.***> 於 2022年1月6日 週四 上午12:36 寫道：

Is there a working version for kernel 5.15+ yet ?
—
Reply to this email directly, view it on GitHub
https://github.com/DualCoder/vgpu_unlock/issues/84#issuecomment-1005884674,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAEVQOXVEQMRXZ67T5SHD5TUURXRXANCNFSM5INAN3VQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you commented.Message ID:
@.***>

in general 460 driver now works with 5.15.
470 still pending.

Patch 5.16 is out and is running - just some imports fixed to get 5.16 working -> a patch for 5.13 is also comming for the sad ubuntu users but not yet implemented on git drivers

do you mean patch for 460 or 470 driver?

Forgot the 460 part

i always dont understand how these patches work.. do I have to apply all for 5.16?
or just apply sixteen for 5.16?

The simplest way is to add them on dkms and let it do it for you, as you can see above link if you go one folder up to the dkms conf.
But yes for 5.16 you need the patches for 5.12+5.14 too
It only applies patches that are req for the kernel you have and are needed, also it does not make sense to have a complete merged 5.16 patch with all prior patch merged to one file. Bc of the duplication/maintainability of the patches/applying them

problem is there is no fifteen patch around.  And prormox 7.1 can opt into 5.15 only.
Anyway, I can confirm the merge driver works in 5.15 kernel under promox with 460 driver.

5.15 is enabled with fourteen patch - there are no changes between 5.14.x to 5.15 that would have req a patch

I've seen the vGPU unlock patch applied, but my 1070ti still shows up as a 1070ti, not p4


Refactor MSRIOGroup for easier extensibility

We would like a way to have an MSRIOGroup pull MSR descriptions from multiple files.

Architectural registers are shared between multiple platforms.
Some data might change depending on SKU.

Consensus seems to be that moving the data from the msr_*cpp files into JSON format would be a good implementation for this.

https://github.com/geopm/geopm/commit/99a75f04feebf8ac271c6d452457d40ebfd38199
https://github.com/geopm/geopm/commit/ba2a071814871c78b67eebdf563ba307ce735d7c
https://github.com/geopm/geopm/commit/1b203661cce6b7881f697b1ada4ec3d62624afc9


add connection string to azure blobstorage output

We can create a connection to the Azure Storage Account through a connection string.
The Azure API in this output is in preview and doesn't support connection string directly yet, so we need the extract the Storage Account Name and Access Key from the connection string.
Using a connect string allows us to use the output on Azure Blob Storage emulators, like https://github.com/Azure/Azurite.

The https://github.com/Azure/Azurite is nice indeed, I'll try to have a look at the integration tests over the weekend.

Hi! I replaced azure-storage-blob-go library by azure-SDK-for-go, the same library already used by table_storage output, since azure-storage-blob-go is in preview (for a long time) and doesn't support connection string, which makes us to parse the connection string in some hacky ways.
Also added additional parameter do define the container's access level: private, blob or container.
Changed blob_type values to lowercase, It's a breaking change, but since this is a beta output, and I believe most of the people don't set this parameter anyway (defaults to block).

I'll try to add the integration tests, I already tested it with Azure and Azurite and seems to work fine.
Not sure how to run individual integration tests, though

Looking good @mfamador, I don't think this needs to be blocked on integration tests, they're more of a nice-to-have, are these PR otherwise ready to merge?

Yeah, I think they're good to go, I already tested in Azure and Azurite. But I'll try to add the integration tests when I have the time, but probably it'd help to have the corresponding inputs as well, right?


I7eterw patch 1

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


Incompatibility with some Elden Ring Loader mods

Thanks for your work! I have encountered an incompatibility between the ModEngine<IP_ADDRESS>-proton-v3 and any one of these mods:

Skip the Intro
Adjust the FOV
Unlock the Framerate

When any one of these mods is present, I can go in-game just fine, but my other mod, Diablo Style Loot, doesn't load. I followed your installation instructions exactly.

I solved it by editing my mod_loader_config.ini to look like this:
[modloader]
load_delay = 5000
show_terminal = 0

[loadorder]
lua.dll = 1
modengine2.dll = 2
UnlockTheFps.dll = 3
AdjustTheFov.dll = 4
SkipTheIntro.dll = 5


Fix performance issues 3 - Add cache mechanism for speedup world coordinate use

Description

This PR is for cache some of the expensive calculation connected with world coordinates
Type of change

[ ] Bug-fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

References

How has this been tested?

[ ] example: the test suite for my feature covers cases x, y, and z
[ ] example: all tests pass with my change

Final checklist:

[ ] My PR is the minimum possible work for the desired functionality
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[ ] I have added tests that prove my fix is effective or that my feature works

@sofroniewn As there is a job which starts removing part of properties from layers to the global state line in # #1919. Did this PR is important, or it will be suppressed by moving also extend outside layers?

@sofroniewn As there is a job which starts removing part of properties from layers to the global state line in # #1919. Did this PR is important, or it will be suppressed by moving also extend outside layers?

I think this PR might still be very important. I'm hoping some of the other refactoring continues to improve things, but we might need this too. I'll try to get to review soon, possibly after some of these other in progress refactors are finished.

Code for generating performance plot.
import napari
import sys

import numpy as np

data = np.random.random((10, 100, 100))

viewer = napari.Viewer()
for iteration in range(100):
    print(iteration)
    viewer.add_image(
        np.copy(data)
    )
viewer.close()
sys.exit()

And I'm not sure if cleaning should be connected to all events. Maybe there should be a shorter list of only these ones which is connected with changing fo extent world?

I do not know How the MacOS test fail could be connected to this PR, but it is the only MacOS test enabled for PR.

I do not know How the MacOS test fail could be connected to this PR, but it is the only MacOS test enabled for PR.

I've seen similar failures on recent PRs, so my guess is it's unrelated to these changes.

I'd really prefer to avoid the internal event connection. I don't think it is needed and it won't result in the best performance. See my comment inline for an alternative proposal.

Agree, the biggest issue we've seen with self connections is that some external listener will get the first event before the second one has triggered, resulting in a "partially updated" state.

Agree, the biggest issue we've seen with self connections is that some external listener will get the first event before the second one has triggered, resulting in a "partially updated" state.

Yes. This workaround is a small thing that allows getting speedup without totally code refactor. To get this behavior someone needs to connect to the first position, when _clean_cache is connected in the constructor.
#2702is still not introduced in the codebase. Many extensive calculations were performed on property access (please remember that even hasattr will trigger such calculation). In the napari data model, there are mixed data properties (data itself, scale, transform, properties, etc) and data visualization properties (visibility, opacity, colormap), which make it harder to cache @property ... calls.  But such refactor is a few weeks of full-time work (especially when would like to keep backward compatibility with already created plugins.

Superseded by #3606


Remove users/teams/namespaces/registries

Is it planned to build routes for deleting users/teams/namespaces/registries?
To delete the registry would be interesting because currently only one is supported, and you can't change it.

Yes it is. Right now distribution (the registry v2) doesn't have delete capabilities. We are going to add the possibility to remove users, teams, images,... as soon as this is implemented inside of distribution.

Sorry if I'm wrong, but I thought the registry would only have to support the deletion of images. The authentication is handled completely by the index?

You are right, registry is only aware of images.
We will have to take care of user removal, but we cannot do that right now because we cannot propagate the removal event from portus to the registry. When removing an user portus will also remove all the images inside of his private namespace for example.


[FEATURE] Talos support for v2 data engine

Is your improvement request related to a feature? Please describe (👍 if you like this request)

Talos support was recently added in #3161, but doesn't include the v2 data engine due to its preview status.
Describe the solution you'd like

Track the implementation of v2 data engine support on Talos. It might be too early to start planning, but these are some known options:

[ ] Move dependencies to pods eg #6798
[ ] Remove unsupported host dependencies eg bash
[ ] Add host dependencies via Talos extensions. I'm happy to help implement this

Additional context

I've tested v1.6.0-rc2 via https://pl4nty.github.io/longhorn-charts and succeeded until the following error when attaching a PVC.

2024-01-26T03:16:46.412777662Z [longhorn-instance-manager] time="2024-01-26T03:16:46Z" level=error
msg="Failed to create engine pvc-c864947e-5fda-4d26-9f56-d949c9beeb0d-e-0"
func="spdk.(*Engine).Create.func2" file="engine.go:114" engineName=pvc-c864947e-5fda-4d26-9f56-d949c9beeb0d-e-0

error="failed to start NVMe initiator pvc-c864947e-5fda-4d26-9f56-d949c9beeb0d:
failed to get file lock for initiator pvc-c864947e-5fda-4d26-9f56-d949c9beeb0d:
Failed to lock, response:
nsenter: failed to execute bash, expected NSFileLock LOCKED"

frontend=spdk-tcp-blockdev ip=<IP_ADDRESS> port=20006 replicaAddressMap="map[pvc-c864947e-5fda-4d26-9f56-d949c9beeb0d-r-ee59f025:<IP_ADDRESS>:20001]" volumeName=pvc-c864947e-5fda-4d26-9f56-d949c9beeb0d

This appears to be caused by a dependancy on bash via nsfilelock:
https://github.com/longhorn/nsfilelock/blob/fa7c83ad0003bbfffc0014f79ec09efef7dcc869/nsfilelock.go#L64-L70
https://github.com/longhorn/go-spdk-helper/blob/4f19a2cd0e085141083d3014d753b67e5c64f39b/pkg/nvme/initiator.go#L126-L129

ref: https://github.com/siderolabs/pkgs/pull/1015

Thanks, I'm looking forward to testing this! Are there any plans to run verification against Talos v1.8.0 (latest as of Sep 24th)? Nothing worries me from the changelog, but it'd be nice to be sure


Better Docker for Mac support

Docker for Mac is not necessarily working like Docker Machine. Currently, you get a sort of an error:
$ docker-clean -r
No active host found
Host does not exist: ""
Host does not exist: ""
Running docker-machine env ...
Error: No machine name(s) specified and no "default" machine exists
New IP Address for  :

Thanks for pointing this out.  I just started noticing this on Docker for Mac on my machine as well.  I will take a look here

For several weeks, I have no problem running docker-clean with Docker for Mac.


Project generated by template does not compile

The problem

How to reproduce

Steps to reproduce:

npx @react-native-community/cli init --template @reason-react-native/template Moo
cd "/Users/xxx/dev/Moo" && npx react-native run-ios

Expected behavior
A running app
Actual behavior
  "___darwin_check_fd_set_overflow", referenced from:
      _RAND_poll in libcrypto.a(rand_unix.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

warning: The iOS Simulator deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 6.0, but the range of supported deployment target versions is 8.0 to 13.2.99. (in target 'OpenSSL-Universal' from project 'Pods')
warning: The iOS Simulator deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 5.0, but the range of supported deployment target versions is 8.0 to 13.2.99. (in target 'CocoaAsyncSocket' from project 'Pods')
warning: no rule to process file '/Users/xxx/dev/Moo/ios/Pods/Flipper-RSocket/rsocket/benchmarks/CMakeLists.txt' of type 'text' for architecture 'x86_64' (in target 'Flipper-RSocket' from project 'Pods')
warning: no rule to process file '/Users/xxx/dev/Moo/ios/Pods/Flipper-RSocket/rsocket/benchmarks/README.md' of type 'net.daringfireball.markdown' for architecture 'x86_64' (in target 'Flipper-RSocket' from project 'Pods')
warning: no rule to process file '/Users/xxx/dev/Moo/ios/Pods/Flipper-RSocket/rsocket/README.md' of type 'net.daringfireball.markdown' for architecture 'x86_64' (in target 'Flipper-RSocket' from project 'Pods')```

** BUILD FAILED **

The following build commands failed:
        Ld /Users/xxx/Library/Developer/Xcode/DerivedData/Moo-cytfnsrzeitdpzcscdoggzleroeu/Build/Products/Debug-iphonesimulator/Moo.app/Moo normal x86_64
(1 failure)

## Environment

<!--
Please include all necessary versions & relevant informations
-->

## Additional context

I've used this template 50+ times before without issue over the past 6 months, wondering if it's a recent change that's caused a problem, or is something about my environment. My existing apps all run as expected.

Hum I don't see how the template can be responsible for this.
It seems to be more a iOS dependency issue according to the message. Try investigating on the lib mentioned in the error message itself.

Hum I don't see how the template can be responsible for this.
It seems to be more a iOS dependency issue according to the message. Try investigating on the lib mentioned in the error message itself.

Finally figured this out, so recording the solution here in case others have the same problem...
Essentially you now must have Xcode 12.0 or higher installed to build with this template. Something to do with it now being React Native 0.63.
For more information


Added simulated provider for debugging without hardware attached

Couple of comments from my part, but looks good to go. Brian will sign off. (nice work btw :-))