xen: modular expression

Motivation for this change
See commit messages.
Things done

[X] Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)

[X] NixOS

[X] Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
[X] Fits CONTRIBUTING.md.
[X] Works for me.

Things not done

[ ] Tested usability of all qemu binary files
[ ] Tested all variants to boot

This is pretty much a complete rewrite of the xen's expression.
Test on some your machines before merging!

Rebased on master and added a patch for nixos.
[X] Tested that all predefined xen expressions boot

Superficially, this looks good. I don't know Xen, nor really understand how its pieces fit together. I just happen to have applied a lot of XSA patches to it. One concern I have is Xen 4.5 seems to be at  the end of its lifecycle. Not yet, but coming up, with 4.6 and 4.7 already out. Can this PR also update to 4.7?

Not in this PR, please no.
The result would be that this won't ever get applied until 4.7 works as well as 4.5 and I'll have to backport new patches to nixpkgs xen to my branch for another year.
The core of this patch got born quite awhile ago when I tried to update to newly released 4.6 and before old versions were scrubbed from nixpkgs and I backported bleeding from all the orifices four or five times since then.
Let's just merge this so that if anyone gets serious about getting 4.7 working they'll do it on a less copy-pastey and more modular foundation. I can't make 4.6 work well enough yet.
One piece at a time, please.

 Builds and boots fine on my machines.

Overall, as the defacto maintainer of Xen (despite not being a user)
I'm not really excited about having 3 variations of Xen packaged.
Remaining as close as possible to upstream makes my life easier.

Debian, the rest of nixpkgs and I disagree. You don't compile a separate
ffmpeg for every program that uses it, hence you shouldn't compile two
(!) separate qemus for xen.
I think its wise to reuse as much nixpkgs as possible. qemu's
expression from nixpkgs has ~6 "*Support" switches that are directly
applicable here. Should I duplicate them in xen expression too? That's
nuts.
The problem with xen is that its tarballs are too much
"enterprise-level software". Fetching of sources from the net in make
is nuts, building multiple internal submodules that are lager than xen
itself is nuts too. Yes, its easier to distribute stuff this way in
distribution-agnostic way, but there's no reason to comply with this
enterprise-level craziness with nix. Nix can build stuff by itself.
Ideally, we want to drop qemu building from xen expression
altogether, override qemu's src with corresponding revision from
xen's fork and provide that with vanilla xen. Not sure how this is
gonna work with qemu-traditional, probably needs some code stealing
from Debian and/or more options to qemu expression. This patch set
moves xen in that direction without, AFAICS, loosing anything.

Typical VM usage involves lots of wasted space as each VM has
plenty of its own overhead.

Typical - maybe. But you can use xen as a kind of micro-kernel to run
bare-metal programs in OCaml and Haskell, and those don't waste anything
and run on credit-card-sized computers.

Typical users will be receiving a packaged Xen from the upstream
cache, eliminating build time.
Users that don't use the upstream cache are going to be spending
plenty of time compiling, that a reduction in Xen isn't going to be
a sizable improvement.

qemu built with all the "*Support" options set is 271M. I think that
is considerable. Arguably, you don't need that for typical xen usage.
Typical x86-only qemu is 26M. Multiply that by two. You can get very
usable suckless.org Linux systems fit in 50M.

Having three will be an increase in resource utilization for
Hydra, and more personally impactful, a much greater increase in
time required to test when I apply patches to Xen.

That's backwards logic: "now we have more test cases, so we have more
opportunities to fail them, so lets remove the tests".

Indeed, the -slim variation even calls out that its parts may get
out of sync. I don't know what that means or how to fix it, but it
doesn't seem like a good scenario.

ffmpeg gets out of sync with its referers too. What do we do in that
case? We make another expression and hold it till its needed or update
outdated packages. I see no problem here.

This makes me wonder why someone would want to make these changes in
the first place, and wonder if the additional code complexity is worth
the configurability.

Complexity where? All options influence is this:
configureFlags = []
++ optional (!withInternalQemu) "--with-system-qemu" # use qemu from PATH
++ optional (withInternalTraditionalQemu) "--enable-qemu-traditional"
++ optional (!withInternalTraditionalQemu) "--disable-qemu-traditional"
++ optional (withSeabios) "--with-system-seabios=${seabios}"
++ optional (!withInternalSeabios && !withSeabios) "--disable-seabios"

++ optional (withOVMF) "--with-system-ovmf=${OVMF}"
++ optional (withInternalOVMF) "--enable-ovmf";

and xenfiles.
Everything else is a refactoring to allow simpler per-version
expressions. 90% of 4.5.nix is references to sources, not build
scripts. I'd say its less complex now than it was before.
(Also note that OVMF was not supported before, I can't build vanilla
Xen's OVMF. And the external one builds.)

If there are more compelling and specific reasons to have the
configurability, I could be convinced that it is worth it.

See above.

If there are more compelling and specific reasons to have variations
provided by Nixpkgs, I could be convinced it is worth that, too.

See above.

I wouldn't mind these changes so much, but current Xen maintenance
typically falls to me. Having additional configurability makes it
harder to know if my changes are breaking something, and named

See "backwards logic" above.

variations adds an implicit signal that this configuration is a
supported.

That's correct. But the thing is that these configurations are actually
more useful than the vanilla one, unless maybe if you just trying xen
out for the first time or something.
But, again, if you don't use it yourself then what's the difference? I
assume you simply compiled it before. Now you can compile three
versions, or, if you're lazy, just continue to compile the vanilla
version, I'll take care of the rest. (Actually, the most likely case is
that if vanilla compiles, the rest will compile too, since they compile
less).
Also, if you look at my contribution history to nixpkgs, you'll notice
that changes to xen were among the first things I ever contributed
here af32fd6ce3ce2ae138c2fc2bc5c6067216f8b828
I'd say I have enough history with xen expression to know what power
users need.

Sorry, I misread the description and thought you implied you don't need the light version unless you know why you need it, whereas it is clear now you meant you don't need qemu-traditional unless you know why you need it.

Correct. Feel free to reword that to be clearer.

It sets up its own bridges and firewall config, runs its own dnsmasq
and stuff. I kinda planned to fix that later too (currently I just
comment out that part in my branch and setup everything myself
per-machine, because who needs options, right?).
In short, for me your comment reads like: "more (not useful for me)
options (I don't use xen, btw) introduce new test cases, meh, don't want
to run those, let's not".
I send the-rays-of-cosmic-love your way for the time you put into
reviewing this PR, but I disagree.
My proposal: read the old expression, read the new expression, ignore
new configurability, feel the difference (I think the level of space
required to correctly indent the expression usually correlates with its
craziness, this patch considerably reduces that metric), then think
about poor power users that want to run MirageOS on their credit-cards
with Xen and want all those options, reconsider.

Jan Malakhovski<EMAIL_ADDRESS>writes:

Debian, the rest of nixpkgs and I disagree. You don't compile a separate
ffmpeg for every program that uses it, hence you shouldn't compile two
(!) separate qemus for xen.
I think its wise to reuse as much nixpkgs as possible. qemu's
expression from nixpkgs has ~6 "*Support" switches that are directly
applicable here. Should I duplicate them in xen expression too? That's
nuts.

This seems like sound reasoning.

The problem with xen is that its tarballs are too much
"enterprise-level software". Fetching of sources from the net in make
is nuts, building multiple internal submodules that are lager than xen
itself is nuts too. Yes, its easier to distribute stuff this way in
distribution-agnostic way, but there's no reason to comply with this
enterprise-level craziness with nix. Nix can build stuff by itself.

This must already be handled by our current expression, since make can't
access the network anyway.

Having three will be an increase in resource utilization for
Hydra, and more personally impactful, a much greater increase in
time required to test when I apply patches to Xen.

That's backwards logic: "now we have more test cases, so we have more
opportunities to fail them, so lets remove the tests".

No, I disagree. This is adding many more features without test cases.
Many more ways it could build, but produce invalid results.

This makes me wonder why someone would want to make these changes in
the first place, and wonder if the additional code complexity is worth
the configurability.

Complexity where? All options influence is this:

well, that, but I was also surprised to see the over-all nix expression
gain 200 lines of code over the old version. Not that this is
necessarily bad.

Everything else is a refactoring to allow simpler per-version
expressions. 90% of 4.5.nix is references to sources, not build
scripts. I'd say its less complex now than it was before.
(Also note that OVMF was not supported before, I can't build vanilla
Xen's OVMF. And the external one builds.)

Nice.

I wouldn't mind these changes so much, but current Xen maintenance
typically falls to me. Having additional configurability makes it
harder to know if my changes are breaking something, and named

See "backwards logic" above.

I disagree it is backwards logic.

variations adds an implicit signal that this configuration is a
supported.

That's correct. But the thing is that these configurations are actually
more useful than the vanilla one, unless maybe if you just trying xen
out for the first time or something.
But, again, if you don't use it yourself then what's the difference? I
assume you simply compiled it before. Now you can compile three
versions, or, if you're lazy, just continue to compile the vanilla
version, I'll take care of the rest. (Actually, the most likely case is
that if vanilla compiles, the rest will compile too, since they compile
less).

I am thrilled to have another Xen maintainer around who I can ping when
issues come up :)

Also, if you look at my contribution history to nixpkgs, you'll notice
that changes to xen were among the first things I ever contributed
here af32fd6ce3ce2ae138c2fc2bc5c6067216f8b828

I did notice that, but chose to leave out any arguments around your
history of contributions as I took your frequency of contributions as a
negative signal on how likely it is I could ping you and get prompt
assistance.

Sorry, I misread the description and thought you implied you don't
need the light version unless you know why you need it, whereas it is
clear now you meant you don't need qemu-traditional unless you know
why you need it.

Correct. Feel free to reword that to be clearer.

In this case, I would prefer you make it more clear, but please see
the end of my message.

It sets up its own bridges and firewall config, runs its own dnsmasq
and stuff. I kinda planned to fix that later too (currently I just
comment out that part in my branch and setup everything myself
per-machine, because who needs options, right?).

Yeah... I know :/ have read it, it is definitely an interesting module.
To be clear, I'm not anti-option. I'm
anti-options-for-the-sake-of-options.

In short, for me your comment reads like: "more (not useful for me)
options (I don't use xen, btw) introduce new test cases, meh, don't want
to run those, let's not".

They are not test cases, they are just packages to compile. This is an
important distinction. Compilation does not guarantee it'll work. Having
more cases may compile but be broken gives a very bad user experience
when a user hits them, and makes it much more complicated to handle when
security patches don't cleanly apply for whatever reason.

I send the-rays-of-cosmic-love your way for the time you put into
reviewing this PR, but I disagree.

:) That is okay, we're all pushing for a better NixOS.

My proposal: read the old expression, read the new expression, ignore
new configurability, feel the difference (I think the level of space
required to correctly indent the expression usually correlates with its
craziness, this patch considerably reduces that metric), then think
about poor power users that want to run MirageOS on their credit-cards
with Xen and want all those options, reconsider.

Yeah, I have read the old expression. It is crazy. I do appreciate the
clean-up you have here, it is why I've tried so hard to explain my
position. My goal with this review was:
Don't push back so hard that it is a "no", but do push back hard enough
that it requires real justification on your part.
I think you've made a good case and justification for the work you've
done. In the veign of ffmpeg, have to ask: why offer -vanilla and
-slim at all? Why not just make our default and only expression the
-light version?
When Xen's XSAs include patches for qemuu and qemut, how should those be
handled? Are those patches fully included in the version of qemu we
already have, and Xen is distributing them only because they have their
own (old?) copy?
Thank you for your thorough reply,
Graham

The problem with xen is that its tarballs are too much
"enterprise-level software". Fetching of sources from the net in make
is nuts, building multiple internal submodules that are lager than xen
itself is nuts too. Yes, its easier to distribute stuff this way in
distribution-agnostic way, but there's no reason to comply with this
enterprise-level craziness with nix. Nix can build stuff by itself.

This must already be handled by our current expression, since make can't
access the network anyway.

Yep, I'm being philosophical there.

That's backwards logic: "now we have more test cases, so we have more
opportunities to fail them, so lets remove the tests".

No, I disagree. This is adding many more features without test cases.
Many more ways it could build, but produce invalid results.

I agree that having nixos tests for booting xen would be nice, but I'm
not sure if this is possible, I never tried to boot xen on top of
kvm, and I don't think it will work.

This makes me wonder why someone would want to make these changes in
the first place, and wonder if the additional code complexity is worth
the configurability.

Complexity where? All options influence is this:

well, that, but I was also surprised to see the over-all nix expression
gain 200 lines of code over the old version. Not that this is
necessarily bad.

Most of that is .patch files for things I couldn't fix with sed,
packages.nix and doc strings.

But, again, if you don't use it yourself then what's the difference? I
assume you simply compiled it before. Now you can compile three
versions, or, if you're lazy, just continue to compile the vanilla
version, I'll take care of the rest. (Actually, the most likely case is
that if vanilla compiles, the rest will compile too, since they compile
less).

I am thrilled to have another Xen maintainer around who I can ping when
issues come up :)

Also, if you look at my contribution history to nixpkgs, you'll notice
that changes to xen were among the first things I ever contributed
here af32fd6ce3ce2ae138c2fc2bc5c6067216f8b828

I did notice that, but chose to leave out any arguments around your
history of contributions as I took your frequency of contributions as a
negative signal on how likely it is I could ping you and get prompt
assistance.

Firstly, I had a fair share of rather meh experiences in pushing changes
to nixpkgs when people rejected them for years and then reimplemented
them themselves (with toposorting of fileSystems people picked up my
rejected branch #11484, which was nice, but most of the time things
simply get reimplemented, e.g. I'm netbooting NixOS for >3 years now,
and my original rejected proposals got reimplemented in netboot
installer ~30% ATM), most of the time reimplementations are rather 1:1
with my old changes. This is rather demotivating.
Some IMO killer features (like defaults in nixpkgs.config for nixos)
were rejected with such fervor that I started my own private fork with
~200 patches ATM which I rebase quite a lot. Since a lot of stuff in
those ~200 patches inter-depends it becomes less and less trivial to
separate pieces that can be merged into vanilla nixpkgs. So most of the
time I do nothing unless stuff gets really annoying to merge (this PR is
an example of that).
Also, most of the time pushing something considerable takes 1x effort in
coding and 5x effort in trying to convince people that they want it.
Which is totally fine, I'm sure I'm like that too from the outside. But
this is exhausting and I'd rather do something else.
Secondly, I hate contexts switches, prefer the "deep work" approach and
so I frequently seclude myself from everything. If you really need my
input, like something is seriously on fire - write me a couple lines
directly, not through github. Direct e-mails have their own filter rules
and I don't ignore those tags for more that a week, usually.

They are not test cases, they are just packages to compile. This is an

In ideal world it does.

important distinction. Compilation does not guarantee it'll work. Having
more cases may compile but be broken gives a very bad user experience

But there's no way to know if something doesn't work without actually
trying. I doubt more than 10 people use nixpkgs' xen ATM. There's no
way we can test everything with such a user base. And without those
options userbase for this expression is gonna be less, e.g. I'm not
using vanilla xen, so at least one user less.

when a user hits them, and makes it much more complicated to handle when
security patches don't cleanly apply for whatever reason.

Yes, but this consideration does not to apply in this case, since
options actually disable internally built stuff.

I send the-rays-of-cosmic-love your way for the time you put into
reviewing this PR, but I disagree.

:) That is okay, we're all pushing for a better NixOS.

Yay!

My proposal: read the old expression, read the new expression, ignore
new configurability, feel the difference (I think the level of space
required to correctly indent the expression usually correlates with its
craziness, this patch considerably reduces that metric), then think
about poor power users that want to run MirageOS on their credit-cards
with Xen and want all those options, reconsider.

Yeah, I have read the old expression. It is crazy. I do appreciate the
clean-up you have here, it is why I've tried so hard to explain my
position. My goal with this review was:
Don't push back so hard that it is a "no", but do push back hard enough
that it requires real justification on your part.
I think you've made a good case and justification for the work you've
done. In the veign of ffmpeg, have to ask: why offer -vanilla and
-slim at all? Why not just make our default and only expression the
-light version?

This is the question that bugs me too, yes. As I said before, ideally
xen would build only xen's binary and scripts, qemu and
qemu-traditional would be built with qemu's overriden expression.
But

the latter is not implemented yet, and I'm not sure -traditional
part is possible,
and I don't want to break something that seems to work.

(I'm not sure it works because I didn't test it with xen-dom0.nix
craziness, nor do I use xendomains to run VMs, but I assume it does.)
So, ideally, there would be only -light and two on-demand qemus. But
that's not possible right now.
Less ideally, there would be -light and -slim and single external
qemu. That is possible already. But having those two options and
dropping -vanilla seems strange, and also not friendly for migrating
users.
Maybe we can switch xen to be xen-light by default after awhile, but
I'm not comfortable doing it right away. My reasoning: people other than
me will see -light, test it in other setups, maybe fix bugs, and then
we could switch.
In short: I'm trying not to break things by doing a minimal change to
the current top-level xen derivation.

When Xen's XSAs include patches for qemuu and qemut, how should those be
handled? Are those patches fully included in the version of qemu we
already have, and Xen is distributing them only because they have their
own (old?) copy?

For now we need to add them into 4.5.nix as before. But, generally,
yes, qemuu is just an old version of top-level qemu with security
patches backported.
So, the simplest solution for getting XSAs for top-level qemu is to
update its src. Alternatively, we can patch it with the same XSA
patches since those XSAs would apply to it too (if it's old enough).
Okay. So what do you want me to do?

First some cc's to people who've updated xen stuff in the past: @ts468 @domenkozar
Disclaimer: I'm not a xen user, so not going to merge this. Sorry.

Firstly, I had a fair share of rather meh experiences in pushing changes to nixpkgs when people rejected them for years and then reimplemented them themselves (with toposorting of fileSystems people picked up my rejected branch #11484, which was nice, but most of the time things simply get reimplemented, e.g. I'm netbooting NixOS for >3 years now, and my original rejected proposals got reimplemented in netboot
installer ~30% ATM), most of the time reimplementations are rather 1:1 with my old changes. This is rather demotivating.

You are not alone. Even project members can have a hard time getting in useful changes. This is one of the biggest problems we're currently facing and we need a solution soon or we won't be able to scale our development efforts. More people would maintain forks instead of contributing new code. We need a useful and well-defined contribution process but first we need to solve other governance issues in the project,. For instance, we're not able to reach a decision on controversial issues other than "nobody has reverted that commit" or try to invoke our BDFL. This the big think I'd like to tackle in person with project members at the next Nixcon.
Regarding your fork, I can take a look at your work and can the very least merge in stuff that I know about. Do you have it on Github?

In short: I'm trying not to break things by doing a minimal change to the current top-level xen derivation.

Back to the topic of this PR: I think your changes are generally useful and using qemu from nixpkgs is the right choice, if it works. Most xen vulnerabilities seem to be in fact vulnerabilities in qemu, which we would fix anyway.
So from a security standpoint, I would welcome removing the vanilla version or let xen default to xen-light if it doesn't cause any immediate problems for users of the vanilla version (you mentioned some issues but I'm not sure how serious). As 17.03 is already branched off and will be released soon, this could be done for 17.09. Still half a year to go. :smile:
I agree with @grahamc that we should take care not to needlessly increase the complexity of the expression, but in this case it does seem useful to me.
So :+1: from me to go ahead with this and the other changes you proposed like fixing the xen-dom0 module. Unfortunately, I can't help you much with testing. Thanks for your work!

I'll merge it, though :) thank you for the additional eyes, @fpletz. Thank you again for the contributions, @oxij!

Graham Christensen<EMAIL_ADDRESS>writes:

I'll merge it, though :) thank you for the additional eyes, @fpletz. Thank you again for the contributions, @oxij!

I want to give @domenkozar an opportunity to review. I'll merge at
2017-03-08 00:00 CET with or without a review :)
Graham

Franz Pletz<EMAIL_ADDRESS>writes:

You are not alone. Even project members can have a hard time getting in useful changes. This is one of the biggest problems we're currently facing and we need a solution soon or we won't be able to scale our development efforts. More people would maintain forks instead of contributing new code. We need a useful and well-defined contribution process but first we need to solve other governance issues in the project,. For instance, we're not able to reach a decision on controversial issues other than "nobody has reverted that commit" or try to invoke our BDFL. This the big think I'd like to tackle in person with project members at the next Nixcon.

Good, then let me pour my frustrations here. Feel free to cite,
copy-paste or reuse in any other way anything I wrote here, into other
nixpkgs issues and nix-dev list for that discussion.
IMO one of the worst problems with nixpkgs process is that new hot stuff
gets merged (think systemd, hardening), while reworked for nix tested by
time borrowings from other distros don't.
I know people who now maintain their own forks (we exchange patches
privately from time to time, "nix underground movement", lol) after
hardening got merged in (because of #18995 and/or slowdowns, not
everyone has new shiny Ryzens and Kabby Lakes (or even wants them,
because of Intem ME and AMD PSP)), others switched from NixOS way back
after switch for systemd (and I'm tempted too, every second systemd
update breaks something new. Like recently why why!? did they break
utmp? Just why? Now a lot of old useful UNIX stuff doesn't work on
NixOS.  I'm like super tempted to switch to Gentoo with nix
on top because of that shit, or fork that part of nixpkgs too.).
My fork was started after the original #6968, which did what #12877
discusses the possible implementation of.
The fun fact is: original #6968 can solve #18995 with something like
nixpkgs.config.stdenv.disableHardining = [ "all" ]; or
nixpkgs.config.gcc_wrapper.disableHardining = [ "all" ]; in
configuration.nix. But nixpkgs.config is deprecated and
packageOverrides is the way to go, right?

Regarding your fork, I can take a look at your work and can the very least merge in stuff that I know about. Do you have it on Github?

Nah, it's very personal, because I patch a lot of software to my
preference my branch is a mess.
After #6968 I stopped using overrides. Everything I can do with my
nixpkgs.config on steroids I do using it. Everything else I "override"
directly in my nixpkgs fork. This way git produces a merge every time
something potentially breaking the override gets commited to master.
Which is way better than debugging packageOverrides.
But back to the topic. I'm not sure there's a way to solve controversial
issues like that without actually forking.
Same with hardening. You either have it on by default, slowdown everyone
and produce #18995, or you don't and you loose some potential benefits
on security breach. Potential because "control flow bending" gets around
hardening, so IMO it's pretty useless, compiling everything with
-fsanitize=address would've been more useful, but it's not trivial to
implement and it'll blow the size of binaries, and so that needs a
proper expert discussion (and I don't think I'm enough of an expert for
that, btw).
Or you implement useful nixpkgs.config like #6968 did and make
everything configurable. (Note, that packageOverrides are not an
option for that, because you don't want to override every single package
by hand.)
But that's a serious policy decision too, because just having a fork
that does that is not useful. You need to actually compile most of the
packages with and without hardening to prevent bitrot and to provide
binary caches.
In my case, I'm just too busy and too lazy to comb my fork and make it
something public can enjoy. But even if I waste some time and do the
combing, what number of people will use it?
I don't think most people have a couple of 8-cores with 32G of RAM
laying around just to compile nixpkgs for themselves. And if you have
less than 12G of RAM then compiling nixpkgs spills into swap (and takes
a week per mass rebuild) or turns into a maintenance job of fixing
compile-time options of every package to reduce RAM requirements. Even
16G is not enough to deterministically compile all of nixpkgs, e.g.
firefox and libreoffice, or libreoffice and Agda start compiling
together and you get OOM.
Also note that our BDFL @edolstra is either on opposite side on every
issue mentioned here, or does not care. Which is kinda depressing.
Also, since GitHub is not a mailing list this issue and discussion will
get forgotten as soon it will get closed. And most people are too lazy
to care for a new issue with "read discussion from #suchandsuch, but
continue that discussion here". (Sometimes I feel like new technology is
conspiring against the older more sane approaches, using network effects
to push better stuff of the market. I can think of a handful of
instances more from the top of my head.)

Back to the topic of this PR: I think your changes are generally useful and using qemu from nixpkgs is the right choice, if it works. Most xen vulnerabilities seem to be in fact vulnerabilities in qemu, which we would fix anyway.

My thoughts exactly.

So from a security standpoint, I would welcome removing the vanilla version or let xen default to xen-light if it doesn't cause any immediate problems for users of the vanilla version (you mentioned some issues but I'm not sure how serious). As 17.03 is already branched off and will be released soon, this could be done for 17.09. Still half a year to go. :smile:

Yep, it has some technical problems, though. Like I have no idea why
xen-light needs seabios and ipxe (which should be external too,
but xen's configure doesn't give such an option ATM).

a few hours late, but thank you for the discussion on this!

Thanks, everyone!


Fix clippy::use_self warning in generated derive(Serialize) output

Rust 1.61 seems to point out clippy::use_self inside macros again (#1993), which may or may not be an upstream problem that gets a hotfix release; it's probably easier and more concise to remove the variable #this expansion for hygiene reasons too.
Perhaps more functions can/should be updated to just use Self::, for simplicity.

Submitted this to see what the CI would think, looks like this approach is already breaking the serde(remote = "AnotherType") feature which can't use Self.

@dtolnay That's pretty lame as this has been a pretty useful lint thus far, even if this is an upstream compiler issue (macro-expanded code shouldn't be linted).
If it wasn't for the remote feature it'd have been better to use Self directly in macro code anyway instead of having to pass a this around.

It's a nursery lint because it has excessive false positives. Try again when it no longer has excessive false positives. It is not a good idea to put workarounds for that kind of lint into macro generated code because the workaround doesn't disappear after the false positive goes away, which is bad for maintainability.

I may have been misunderstood, so I'll try again. If you can use Self instead of coming up with some let this = &...this; followed by #this in quote expansion that's simpler and preferred in my eyes. But we can't because of the remote feature, so the second revision of this PR merely turned into a workaround instead of being - in my eyes - legitimate simplification.

Any chance to reanimate this? Or reimplement it if there's anything wrong with the diff itself?
(People do use nursery lints whether they're "recommended" or not, in a 50k line codebase the only 'excessive false positives' we've observed so far are those for serializing enum variants with serde, plus one or two other proc macro crates, but generally use_self is fairly useful and catches most of 'true positives'.)


Variable not Found in Licenses.html

ℹ Computer information

Spotify version: <IP_ADDRESS>5
Plan Spotify: Free
Windows version: Windows 11
PowerShell version: 7.2.5
Spicetify installed: No

📝 Description of the problem

Error

Was using the Automatic Full Installation (Code in Powershell)
📷 Screenshots

Screenshot

Can't reproduce this error.
You are installing over another version, perhaps this is the problem.
Does this error happen again with a clean install?

After a Full Clean Install there is no more Error now, maybe it was the Install Over the Old Spotify Version
But it seems the Error with the Licenses didn't effect the Patch itself
Still Thanks didn't test it with Complete Clean Installation from the beginning

Spotify version: <IP_ADDRESS>4.g07f1e963-9
Plan Spotify: Free
Windows version: 10.0.19044
PowerShell version: 7.2.5
Spicetify installed: No

I get "Didn't find variable $contents.HtmlLicMin2 in licenses.html" on a clean install.
When I check AppData\Roaming\Spotify\Apps\ I only have 2 files: Login.spa and xpui.spa. No licenses.html.

@GhostTwohand @koen-debacker
Managed to reproduce a bug that prevented the licenses.html file from being truncated.
fixed


Add google  sign in option

Add a custom button in sign in page

Hello, I am interested in working on this issue. Could you please assign it to me?


[AOTI] Remove the original model weights in Python deployment

Summary: Fixes https://github.com/pytorch/torchchat/issues/1302. Because AOTI-compiled model contains a copy of model weights, if we also keep the eager model weights, it will be a wast of GPU memory and even triggers OOMs. This PR releases the corresponding eager model weights in the AOTI-Python deployment path.

@Jack-Khuu , is there any known problems with CI? Many of the failures here seem unrelated to my change.

Yeah, we're checking with the DevInfra folk

Heads up CI is broken in pt/pt and at higher level
https://github.com/NVIDIA/nvidia-container-toolkit/releases/tag/v1.17.0

@desertfire it should be fixed now. We just need to churn through the tests

Summary: Fixes #1302. Because AOTI-compiled model contains a copy of model weights, if we also keep the eager model weights, it will be a waste of GPU memory and even triggers OOMs. This PR releases the corresponding eager model weights in the AOTI-Python deployment path.

In the long term we might ask whether there's a point in going to all the expense of building the model if we just need the config.  The model build process had a config_only bool argument with the intent of allowing suppression of actual model build (that was never implemented though).
This is particularly noteworthy (aka expensive because GGUF requires much more than weight mmaps) when building a model from GGUF, as @metascroy pointed out a long time ago when he implemented the GGUF reader.


android application does not receive UDP (broadcast and unicast)

Steps to reproduce
The SkyDemon Android application is not receiving UDP messages from the local network. I am broadcasting UDP messages from the same host where the SkyDemon app is running. This setup used to work approximately a month ago. I have verified that UDP messages are still being broadcasted by ensuring the SkyDemon app can receive them when running on a physical Android device within the same network.
✔ Expected Behavior
The Android app (SkyDemon) running under WSA should receive UDP multicast messages from the local area network.
❌ Actual Behavior
The Android app (SkyDemon) running under WSA is not receiving UDP multicast messages from the local area network.
Other Software
tested on:
SkyDemon for Android 3.16.9
SkyDemon for Android 3.16.6
Please specify the version of Windows Subsystem for Android
2306.40000.4.0

UDP works as expected with WSA 2303.40000.3.0 (my best guess when it was still working). So it appears to be some regression.

Yes Multicast is broken since the the "Local network access" replaced "Advanced networking" option and the refactor of networking that went with it. Also see #378

How can I prevent WSA from updating automatically to a broken version? I disabled autoupdate for Windows Store apps but it does not seem to stop WSA from updating. Every day I have to reinstall WSA and Android apps.

@kpx1894, thank you for taking the time to provide feedback.  Unfortunately, there is no way to prevent WSA from updating. As @bubbleguuum indicated we are working on improving the local network access feature.  Let's use #378, to track UDP support.


feat: add deleterr

Adds an app I developed recently to support deleting stale and inactive media automatically from Plex/Radarr/Sonarr

That looks great! Thank you :D


CupertinoContextMenu Brightness

Use case
The CupertinoContextMenu has hardcoded color values. It doesn't respect the current platform/theme brightness, so it'll appear light even if the platform/theme is dark.
Proposal
_CupertinoContextMenuActionState should be updated to:

expose these colors, and/or
use the platform/theme brightness to toggle between colors like in CupertinoActivityIndicator, which uses CupertinoDynamicColor.withBrightness.

P.S. I also noticed that the font is also hardcoded. It looks off when compared to the native implementation.


duplicate file names

ERP's built in file storage handles files with identical names by adding a random string to the file name and also including that in the file_url field.
However, with S3 this behavior is not the same. The URL is made unique by using the name of the File document, but the object storage key only uses the file_name field, not including the unique name field of the File document. Thus, while ERP stores two different File documents, S3 is writing to the same file for both of them.
The effect can be hidden by the cache for a time. If you've downloaded the first upload of a file, then it is cached using name. If you then upload another file with different content but the same file name, it will appear you can download the two different files with their own content correctly. But if you disable or clear the cache, then you can confirm that the first file has been overwritten by the second file.
The issue seems to stem from the dfp_external_storage_upload_file function:
                key = f"{frappe.local.site}/{self.file_name}"
		is_public = "/public" if not self.is_private else ""
		if not local_file:
			local_file = "./" + frappe.local.site + is_public + self.file_url

		try:
			if not os.path.exists(local_file):
				frappe.throw(_("Local file not found"))
			with open(local_file, "rb") as f:
				self.dfp_external_storage_client.put_object(
					bucket_name=self.dfp_external_storage_doc.bucket_name,
					object_name=key,
					data=f,
					length=os.path.getsize(local_file),
					# Meta removed because same s3 file can be used within different File docs
					# metadata={"frappe_file_id": self.name}
				)

			self.dfp_external_storage_s3_key = key
			self.dfp_external_storage = self.dfp_external_storage_doc.name
			self.file_url = f"/{DFP_EXTERNAL_STORAGE_URL_SEGMENT_FOR_FILE_LOAD}/{self.name}/{self.file_name}"

			if delete_file:
				os.remove(local_file)
			self.save()
		except Exception as e:

The key field is assigned the site name and file name, but not the unique name field. Then it is used as the object_name and saved as the dfp_external_storage_s3_key.
Could this be changed to include the unique name field in the key as well?

This is actually more serious than I first realized. I just did some tests, and anytime I upload an attachment to a document in ERP, it will silently overwrite every other attachment of the same name on any other ERP document. This means that file names must be GLOBALLY unique to avoid data loss, which is not a reasonable requirement, as I imagine you'd agree.

Hi @khoran thanks again for your feedback. It is solved in "develop" branch (check this commit that solves it  https://github.com/developmentforpeople/dfp_external_storage/commit/b7554d9441e5949eb07a926fa6ebad57ced9d7ae).
Kevin, could you recheck at your side, please?
Frappe uses this suffix suffix=self.content_hash[-6:] for same names. I initially thought about check with S3 if key exists, but finally decided to add the File ID as unique identifier for file:

file url: /file/7719b64f91/a.png
s3 key: bucket-name/a-7719b64f91.png

What do you feel about this solution, any other ideas to improve it? :) Thank you very much Kevin

@khoran FYI I published new version with fix. Compared with previous one: https://github.com/developmentforpeople/dfp_external_storage/compare/v1.1.1...v1.1.2
Please check at your side, but seems to work perfectly in all my tests :)

@developmentforpeople It kind of works, but requires a hack for my situation. If you set the content field before saving, then it works. But I need to upload a file by using the dfp_external_storage_upload_file function directly, so that the content is streamed rather than held in memory.
The catch is that name is not set at this point because the file has not been saved. That makes sense, but if I save the file first, I get a validation error from validate_file_on_disk because the dfp_is_s3_remote_file is false because dfp_external_storage_s3_key is not yet set because I tried to save it before calling dfp_external_storage_upload_file. So it's kind of a chicken and egg problem.
I can work around it by setting content to some string, like "dummy content", then save it, which works now, and then call dfp_external_storage_upload_file. But I don't think that would be clear to a new user.
You might   wonder how file_url was getting set correctly before, if name was not set. It actually was not getting set properly in the dfp_external_storage_s3_key function, but was getting 'fixed' by the hook_file_before_save function, so it still came out ok.

@khoran Hi Kevin, did you find a solution for the issue? I were looking for one and maybe you can set the self.file_name before uploading the file, checking at your side that is not duplicated in the ERP (in case you are worried for that) because is it needed the file content to get the content hash for that check. Here is how Frappe calculates the content hash:
			with open(file_path, "rb") as f:
				self.content_hash = get_content_hash(f.read())

And look for int within "content_hash" in all File docs. I paste here some code within Frappe used to generate the file name (check in file.py within doctype File):
			self.name = frappe.generate_hash(length=10)
...
			self.file_name = re.sub(r"/", "", self.file_name)
...
				self.file_name = generate_file_name(
					name=self.file_name,
					suffix=self.content_hash[-6:],
					is_private=self.is_private,
				)
...

Hope this helps you Kevin :) Have a good weekend!

@developmentforpeople Thanks for the info. I've so far just been appending a random string to my file names, but that is not guaranteed to be unique. I think what would be helpful is the ability to save the the file initially with no content at all, creating a unique File doc name. Then be able to call a function to set (and re-set later even) the contents of the file. Currently, it's not possible to call dfp_external_storage_upload_file a second time since it bails out if the storage key is already set. So maybe a differently name function could be created just for uploading from files (never from content). What do you think?

@developmentforpeople I still can't get past the issue I described above.
Here is a simple reproduction, which I think is not too uncommon:
   file_doc = frappe.get_doc({
                "doctype":"File",
                "file_name":"test_file.txt",
                "folder":"Home/Audio"  #an S3 folder
            })

    with tempfile.NamedTemporaryFile(delete=False) as temp_file:
        temp_file.write(b"test content")
        file_doc.dfp_external_storage_upload_file(temp_file.name)
    file_doc.save()

No error is produced, but it results in a File with the file_url set to /file/None/test_file.txt and dfp_external_storage_s3_key set to dev.galcom.local/test_file-None.txt. When you attempt to download the file, you get a 404 not found error. So the file is effectivly lost, but without any errors produced until retrieval is attempted.
At the very least, I think dfp_external_storage_upload_file should check for self.name being None and throw an error. Ideally, when name is None, it could save itself to produce a name, and then proceed. I'm not sure if that is possible in that context. If not though, another function with the ability to upload a new file, whether or not dfp_external_storage_s3_key is set would be needed.
Thanks.


What's the style idea?
We need to improve
assign this issue under GSSoC'24
Checklist

[X] I have checked the existing issues.
[X] I follow Contributing Guidelines of this project.
[ ] I am a IWOC'24 contributor.
[ ] I am a JWOC'24 contributor.
[ ] I am a SWOC'24 contributor.

Code of Conduct

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

i will try. Plz assign it to me. I'm a GSSOC contributer


Mean and Standard Deviation for the datasets while normalizing

Dear Authors,
Thank you for the dataset.
I am looking at the getting_started.ipynb, for pathmnist it is said that the normalization transform is the following - data_transform = transforms.Compose([     transforms.ToTensor(),     transforms.Normalize(mean=[.5], std=[.5])])
The values 0.5, 0.5 are being used. I have the following questions.

Does this value work for all the datasets in medmnist?
Is 0.5, 0.5 the correct mean and standard deviations, or are they just approximate numbers?
Is there a place where I can find datasets and their corresponding mean and standard deviation values so I can use them in my method?

Thanks for your time and help,
Megh

Thanks for your questions ;)
The MedMNIST is a dataset, we don't consider how users will use it. The current normalization and any other pre-processing are only something we show "well, you can do like this", but you can use your own pre-processing anyway.
So, to answer your questions.

Yes, in our code, but you can also use your own.
No. They are something we specify by experience.
You can calculate them by yourself. The data is there, isn't it?

Jiancheng


Update to Gradle 6

This commit updates the project build to use Gradle 6.
I didn't have the chance to test that publishing works on a real repository, but there's a task, publishMainPublicationToBuildLocalRepository which publishes to the build directory and where you can see the generated POM/Gradle files.
It seems that there were a lot of redundant work in the previous build file, probably because of copy/pasted code.

Thanks a lot for the PR
Looks good, but the uploading to Maven Central fails.
With the mavenPublish.gradle, this worked:
./gradlew --info -PrepositoryUrl=https://oss.sonatype.org/service/local/staging/deploy/mavenuploadPu
Deploying to https://oss.sonatype.org/service/local/staging/deploy/maven2
Uploading: com/gluonhq/strange/0.0.11/strange-0.0.11.jar to repository remote at https://oss.sonatype.org/service/local/staging/deploy/maven2/
with the new approach:
./gradlew --info -PrepositoryUrl=https://oss.sonatype.org/service/local/staging/deploy/maven publish
Execution failed for task ':publishMainPublicationToMavenRepository'.

Failed to publish publication 'main' to repository 'maven'
Could not PUT 'https://oss.sonatype.org/service/local/staging/deploy/maven/com/gluonhq/strange/0.0.11/strange-0.0.11.jar'. Received status code 403 from server: Forbidden

@melix Do you have a suggestion on how to do the upload to maven central (without the old mavenPublish.gradle we're currently still using)? I am very ok with this PR, but need to be able to push the artifact to maven central.

I probably made a mistake in how to get the credentials from env/system properties, I'll take a look.


Django sending Internal Server Error mails for storage checks

Hi, I have an issue similar to #202 but having to do with storage checks. Our app uses S3Boto3Storage and generally our health checks are successful, but a couple times a day since upgrading to Django 2.2 I've been getting error emails. We didn't get these when on Django 2.0.
It seems to have to do with DefaultFileStorageHealthCheck rather than S3Boto3StorageHealthCheck. Do you recommend just disabling DefaultFileStorageHealthCheck if we're using S3Boto3StorageHealthCheck? That doesn't resolve the root issue, though for our purposes it'd probably be fine. Any other thoughts/suggestions?
unavailable: Unknown exception

ServiceUnavailable
unavailable: Unknown exception

Django Version: 2.2.14
Python Executable: /opt/python/run/venv/bin/python3
Python Version: 3.6.10

...

Traceback:

File "/opt/python/run/venv/local/lib/python3.6/site-packages/health_check/storage/backends.py" in check_status
  70.             self.check_delete(file_name)

File "/opt/python/run/venv/local/lib/python3.6/site-packages/health_check/storage/backends.py" in check_delete
  62.             raise ServiceUnavailable('File was not deleted')

During handling of the above exception (unavailable: File was not deleted), another exception occurred:

File "/opt/python/run/venv/local/lib/python3.6/site-packages/health_check/backends.py" in run_check
  30.             self.check_status()

File "/opt/python/run/venv/local/lib/python3.6/site-packages/health_check/storage/backends.py" in check_status
  73.             raise ServiceUnavailable('Unknown exception')

Exception Type: ServiceUnavailable
Exception Value: unavailable: Unknown exception
Request data not supplied

Hi @jessemorton this is to be expected. If there is an error, you should get an email, if you set it up that way. I would personally recommend to use sentry, as it will save you from a flood of emails. To be clear, we don't send the emails, nor do anything to trigger them. We simply return a 500-error, which causes Django to trigger the admin email. The response type is correct and needed by status bots. I hope this answer your question. Best, Joe

Thanks @codingjoe for the pointer towards Sentry, I'll take a look.
Regarding the emails, I wasn't finding that itself odd, since as you say it's just the way that our logging is set up - what I was surprised about was why we'd start getting these errors with Django 2.2 when we didn't previously on 2.0 (in both cases having otherwise the same stack and the same logging). Where you didn't mention any potential reasoning behind that change in behavior, I take it that it's not some known issue with django-health-check and Django version compatibility but rather some difference in my setup now that I need to sort out.
Thanks

Hi @jessemorton,
No, we don't trigger any mails ourselves. Though I should mention, we do not officially support either Django 2.0 nor does the Djangoproject ;) Good thing you are upgrading.
Best,
Joe

Thanks @codingjoe, I fully understand you're not triggering emails within django-health-check, but rather an exception is occurring and my logging is configured to send me emails whenever such an exception occurs. :) My question was more why those errors would occasionally occur to begin with, in django-health-check when used with Django 2.2, when those errors didn't occur when used with Django 2.0. But it sounds like something I need to sort out on my end. Thanks for your replies.

@jessemorton that I don't know. Looking at the stack, it seems deleting files is the problem. It would be helpful if you could share your storage settings, if you want me to help you debug the problem. Best, Joe

@jessemorton Were you able to sort it out?

@codingjoe We're having similar problems with us either receiving OSError: [Errno 14] Bad address or File Not Deleted during health checks for storage backends. Our storage backend is AWS S3. Below is the complete stack trace. Could you shed some light into the possible reasons/fixes for this? We're currently using django-health-checks v 3.12.1.
OSError: [Errno 14] Bad address
File "health_check/storage/backends.py", line 65, in check_status
file_name = self.check_save(file_name, file_content)
File "health_check/storage/backends.py", line 42, in check_save
file_name = storage.save(
File "django/core/files/storage.py", line 51, in save
name = self.get_available_name(name, max_length=max_length)
File "storages/backends/s3boto3.py", line 694, in get_available_name
return super(S3Boto3Storage, self).get_available_name(name, max_length)
File "django/core/files/storage.py", line 82, in get_available_name
while self.exists(name) or (max_length and len(name) > max_length):
File "storages/backends/s3boto3.py", line 562, in exists
self.connection.meta.client.head_object(Bucket=self.bucket_name, Key=name)
File "storages/backends/s3boto3.py", line 365, in connection
self._connections.connection = session.resource(
File "boto3/session.py", line 384, in resource
client = self.client(
File "boto3/session.py", line 258, in client
return self._session.create_client(
File "botocore/session.py", line 831, in create_client
client = client_creator.create_client(
File "botocore/client.py", line 78, in create_client
service_model = self._load_service_model(service_name, api_version)
File "botocore/client.py", line 115, in _load_service_model
json_model = self._loader.load_service_model(service_name, 'service-2',
File "botocore/loaders.py", line 132, in _wrapper
data = func(self, *args, **kwargs)
File "botocore/loaders.py", line 383, in load_service_model
model = self.load_data(full_path)
File "botocore/loaders.py", line 132, in _wrapper
data = func(self, *args, **kwargs)
File "botocore/loaders.py", line 420, in load_data
found = self.file_loader.load_file(possible_path)
File "botocore/loaders.py", line 173, in load_file
payload = fp.read().decode('utf-8')
ServiceUnavailable: unavailable: Unknown exception
File "health_check/backends.py", line 30, in run_check
self.check_status()
File "health_check/storage/backends.py", line 69, in check_status
raise ServiceUnavailable('Unknown exception') from e

@jessemorton Were you able to sort it out?

@affan10 I haven't had the chance to prioritize it, so have simply been ignoring them in recent weeks.

Hi @affan10,
Thanks for reaching out. Honestly, this looks like an issue with the S3 setup or the Python client configuration. I'd recommend reaching out to the boto project or to django-storages. We simply use the high-level storage API defined by Django regardless of its underlying implementation.
Still, should you find a solution, feel free to share it here.
Best,
Joe


Change script to take multiple audiences and other claims from command line flag.

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: myidpt
Assign the PR to them by writing /assign @myidpt in a comment when ready.
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

security/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Looks good. Can you also update README.md to reflect the change?

@diemtvu: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-presubmit.sh
ed6b2cc638578b8cb26c7de5261f53867cab82fa
link
/test istio-presubmit

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

/test istio-presubmit

@diemtvu: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e-v1alpha3.sh
03ec6391cd2acc6ba5b2544873a537221a031c1e
link
/test istio-pilot-e2e-v1alpha3

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@diemtvu: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e-v1alpha3.sh
03ec6391cd2acc6ba5b2544873a537221a031c1e
link
/test istio-pilot-e2e-v1alpha3

prow/istio-pilot-e2e.sh
03ec6391cd2acc6ba5b2544873a537221a031c1e
link
/test istio-pilot-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@diemtvu: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e.sh
03ec6391cd2acc6ba5b2544873a537221a031c1e
link
/test istio-pilot-e2e

prow/istio-pilot-e2e-v1alpha3.sh
0830a7f4031f1a1ecbcd1613e9a819ac57cd1421
link
/test istio-pilot-e2e-v1alpha3

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@diemtvu: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e-v1alpha3.sh
0830a7f4031f1a1ecbcd1613e9a819ac57cd1421
link
/test istio-pilot-e2e-v1alpha3

prow/istio-pilot-e2e.sh
0830a7f4031f1a1ecbcd1613e9a819ac57cd1421
link
/test istio-pilot-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

New changes are detected. LGTM label has been removed.

@diemtvu: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e.sh
0830a7f4031f1a1ecbcd1613e9a819ac57cd1421
link
/test istio-pilot-e2e

prow/istio-pilot-e2e-v1alpha3.sh
1e483fbb515fe738fdcebfe49129e0e74ac59cab
link
/test istio-pilot-e2e-v1alpha3

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@diemtvu: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e-v1alpha3.sh
1e483fbb515fe738fdcebfe49129e0e74ac59cab
link
/test istio-pilot-e2e-v1alpha3

prow/istio-pilot-e2e.sh
1e483fbb515fe738fdcebfe49129e0e74ac59cab
link
/test istio-pilot-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[APPROVALNOTIFIER] This PR is APPROVED
Approval requirements bypassed by manually added approval.
This pull-request has been approved by:
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

security/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@diemtvu: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e.sh
1e483fbb515fe738fdcebfe49129e0e74ac59cab
link
/test istio-pilot-e2e

prow/istio-pilot-e2e-v1alpha3.sh
1e483fbb515fe738fdcebfe49129e0e74ac59cab
link
/test istio-pilot-e2e-v1alpha3

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.


Unable to run video:deploy

Describe the bug
I am following your README for CLI setup.  The deploy command doesn't work.
To Reproduce
Steps to reproduce the behavior:

Go to terminal
igorganapolsky@Igors-MacBook-Pro ~ % twilio rtc:apps:video:deploy --authentication passcode
Error:  » Error code 20003 from Twilio: Authenticate. See https://www.twilio.com/docs/errors/20003 for more info.
›   Error: EEXIT: 20003
›   Code: EEXIT

Expected behavior
It should start video:deploy
Video Android SDK (please complete the following information):

Version: (e.g. 5.1.0)

Additional context
Add any other context about the problem here.

Hi @IgorGanapolsky ! Thanks for reaching out. Have you tried the Possible Solutions outlined in that error code link above?

Verify the Account SID and Auth Token are correct
Verify the correct Account is being accessed
Ensure the Account is active, not suspended or closed
Ensure no extra characters or spaces are being included
Ensure JWT is only used with API Key

I believe I tried all of the above steps.  Question:

Ensure JWT is only used with API Key

What is that?

You only need to worry about that if you are using access tokens. Here's some documentation that goes into more detail on that subject. However, with this command you can work around that by setting the following environment variables:
export TWILIO_ACCOUNT_SID=your Account SID from twil.io/console
export TWILIO_API_KEY=an API Key created at twil.io/get-api-key
export TWILIO_API_SECRET=the secret for the API Key

Is this part of */twilio-cli/config.json *file?
On Fri, Apr 9, 2021 at 1:50 PM John Qualls @.***> wrote:

You only need to worry about that if you are using access tokens. Here's
some documentation
https://www.twilio.com/docs/video/tutorials/user-identity-access-tokens
that goes into more detail on that subject. However, with this command you
can work around that by setting the following environment variables:
export TWILIO_ACCOUNT_SID=your Account SID from twil.io/console
export TWILIO_API_KEY=an API Key created at twil.io/get-api-key
export TWILIO_API_SECRET=the secret for the API Key
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/twilio/twilio-video-app-android/issues/198#issuecomment-816851294,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABRD6OWAV76FTDTWFOIL23TH446HANCNFSM42RQRGEA
.

Hmm not sure about that config file. If you are using Mac OS you can set the environment variables in your ~/.bashrc file. That way they get set every time you start a new session in the terminal before you interact with the Twilio CLI. Hope that helps!

Hmm, my Mac doesn't even have a ~/.bashrc file.  All I have is ~/.bash_profile.  Do you use ~/.bashrc yourself?


Cannot use DevTools extension on example application

What happened?
I ran the example app from flame/examples: flutter run -d chrome. I opened DevTools from the link printed to the CLI and navigated to the flame tab to use the DevTools extension. Instead of the extension loading as expected, I saw this failure message:

This is because the example app uses package:flame from src due to the melos setup. The flame directory on disk does not have the flame/extension/devtools/build directory, because this is only included with the published package.
To resolve this issue for my local needs, I downloaded the latest flame package from pub.dev and copied the extension/devtools/build directory from there into my local flame/extension/devtools folder. After doing this and reloading DevTools, the flame extension loaded as expected.
What do you expect?
I expected to be able to use the DevTools extension on the example app.
How can we reproduce this?
Run the example app, open DevTools, and navigate to the flame tab.
What steps should take to fix this?
Is it possible to include the extension/devtools/build directory on GitHub? If not, perhaps the examples/README.md file could include documentation for an easy way to generate the extension build for local development.
Do have an example of where the bug occurs?
No response
Relevant log output
No response
Execute in a terminal and put output into the code block below
Output of: flutter doctor -v
Affected platforms
All
Other information
No response
Are you interested in working on a PR for this?

[ ] I want to work on this

Did you run melos bootstrap before running it?

Yes I ran melos bootstrap before running the example app. Is there an easy way to generate the devtools extension build? This would have been nice to be able to run before running the example app for the purpose of trying out the flame-provided tooling along with the example.

Ah right! I thought we built it as a hook on bootstrap, but apparently we only do on publish.
You can run melos devtools-build to build it.


Allow to control internal WiFi with the Network Manager

should eth network also be controlled by NetworkManager or systemd-networkd?


Update example in README.md to be copy-paste-go

I had to google what an async function is in python, now one can just copy paste.

Thanks Pietro!


Add WMT14 test-full.tgz to API

Description
The current API provides WMT14 http://statmt.org/wmt14/test-filtered.tgz. This pr adds another commonly used one http://statmt.org/wmt14/test-full.tgz. This is enabled by setting option full to True.
Checklist
Essentials

[x] Changes are complete (i.e. I finished coding on this PR)
[x] All changes have test coverage
[x] Code is well-documented

Changes

[x] Feature1, tests, (and when applicable, API doc)
[x] Feature2, tests, (and when applicable, API doc)

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

Job PR-177/3 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-177/3/index.html

Job PR-177/4 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-177/4/index.html


Review of data model hierarchy.

Bond now derives from BaseInterface instead of containing an interface.
The physical properties of an interface and the name are removed in BaseInterface thus a Bond don't have a name nor a bond master id.
The API also reflect this by making the bond a single flat JSON dictionary.

That's all i see to for now.   I would have put the model refactor in another PR, and the serialializing in even another PR.
We should try to keep PRs as small as possible, me the first i work on that.
So the main thing here is deprecation, since we're launching this we need to have a deprecation process with some guidelines, for now the only thing i require is that it is still unit tested

To support bond.interface.name I need to pass the "name" in the API.  Shall it be called "name" ? or "deprecated_name"?  Do we have to support using the API with an old version of the RemoteSwitch?
The whole idea of this pull request was to change the API and object representation of a Bond and remove the name of it as proposed in a comment https://github.com/internaphosting/netman/pull/30.

There is no caching for add_bond anymore and the API now has a version management mechanism:

The switch API client must provide the Netman-Max-Version header to limit the API version to this number.
The server will then answer with the Netman-Version header.

i think you're missing a cached test:
get_bonds() -> does the call
add_bond() -> no caching
get_bonds() -> does the call again caus one of the object is new

I like the API Versionning!

:+1:

:+1:


Is skywalking version 3.2.6 support motan verson 1.1.1?

Is skywalking version 3.2.6 support motan verson 1.1.1?
I notice that skywalking version 3.2.6 only support motan 1.1.0 and below, according to doc https://github.com/apache/incubator-skywalking/blob/master/docs/Supported-list.md.

We don't know :)  You can run some tests on your own env. I guess it is yes.

Yes, it did.
I checked code from motan1.1.0-1.1.1 to skywalking-3.2.6.
Then I think the key is : the process of these classes is same in motan that from 1.1.0 to 1.1.1 as below:
private static final String ENHANCE_CLASS = "com.weibo.api.motan.transport.ProviderMessageRouter";
private static final String ENHANCE_CLASS = "com.weibo.api.motan.rpc.AbstractReferer";
So it works, is this reason? Of course , I had run on my own env, it did work.
BTW, the java classes named in skywalking is some weird, I mean,
MotanProviderInstrumentation-->com.weibo.api.motan.rpc.AbstractReferer
MotanConsumerInstrumentation-->com.weibo.api.motan.transport.ProviderMessageRouter
It seems named reverse.

Yes. That is the reason. @ascrutae could you confirm whether the naming is wrong?

Yes. the class name is incorrect. Maybe you can send an PR to resolve this problem. @hepyu

@hepyu Please help us to fix this in 5.0(master branch)

Even no functional issue, naming is making people confused. Need to be fixed in RC release.

Thanks, I'm Interested. This weekend I'll have a try.

I will do release at Monday. Please, didn't be late.

Oh, I'm sorry, the time is tight, because my go experience is little, need much time to adjust env.
Although this change is very simple, I think only rename relative class name is ok.

But I have some obsession, still want to validate in my own running env, the time is not enough.
So maybe this change need your team fixing, sorry a lot.

We also encourage the community to fix the issue. This is an issue, but don't hurt anyone at runtime. So if you want to fix this, I can move this to GA, which will release one month later. Should be better?
You are right, every PR should be checked in your local env. :) Don't worry, you are doing the right thing.

Milestone changed. So don't worry about time.


Create Encoder Only Model

[ ] Create a basic version of the model
[ ] Train the model
[ ] Generate predictions
[ ] Test predictions
[ ] Hypertune model
[ ] Link GUI interface


Gateways created by istio manifest generate do not have unique pod selectors

istio version 1.5.2
$ istioctl version --remote
client version: 1.5.2
control plane version: 1.5.2
data plane version: 1.5.2 (3 proxies)

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.2", GitCommit:"52c56ce7a8272c798dbc29846288d7cd9fbae032", GitTreeState:"clean", BuildDate:"2020-04-16T11:56:40Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"15+", GitVersion:"v1.15.9-gke.24", GitCommit:"39e41a8d6b7221b901a95d3af358dea6994b4a40", GitTreeState:"clean", BuildDate:"2020-02-29T01:24:35Z", GoVersion:"go1.12.12b4", Compiler:"gc", Platform:"linux/amd64"}

given this operator:
apiVersion: install.istio.io/v1alpha1
kind: IstioOperator
metadata:
  namespace: istio-system
  name: istio-operator
spec:
  components:
    ingressGateways:
    - enabled: true
      k8s:
        env:
        - name: ISTIO_META_ROUTER_MODE
          value: sni-dnat
        hpaSpec:
          maxReplicas: 5
          metrics:
          - resource:
              name: cpu
              targetAverageUtilization: 80
            type: Resource
          minReplicas: 1
          scaleTargetRef:
            apiVersion: apps/v1
            kind: Deployment
            name: public-ingress
        resources:
          limits:
            cpu: 2000m
            memory: 1024Mi
          requests:
            cpu: 100m
            memory: 128Mi
        service:
          ports:
          - name: status-port
            port: 15020
            targetPort: 15020
          - name: http2
            port: 80
            targetPort: 80
          - name: https
            port: 443
          - name: kiali
            port: 15029
            targetPort: 15029
          - name: prometheus
            port: 15030
            targetPort: 15030
          - name: grafana
            port: 15031
            targetPort: 15031
          - name: tracing
            port: 15032
            targetPort: 15032
          - name: tls
            port: 15443
            targetPort: 15443
          - name: tcp
            port: 31400
        strategy:
          rollingUpdate:
            maxSurge: 100%
            maxUnavailable: 25%
      name: public-ingress
      label:
        app: public-ingress
        istio: ingressgateway
    - enabled: true
      k8s:
        env:
        - name: ISTIO_META_ROUTER_MODE
          value: sni-dnat
        hpaSpec:
          maxReplicas: 5
          metrics:
          - resource:
              name: cpu
              targetAverageUtilization: 80
            type: Resource
          minReplicas: 1
          scaleTargetRef:
            apiVersion: apps/v1
            kind: Deployment
            name: iap-ingress
        resources:
          limits:
            cpu: 2000m
            memory: 1024Mi
          requests:
            cpu: 100m
            memory: 128Mi
        service:
          ports:
          - name: status-port
            port: 15020
            targetPort: 15020
          - name: http2
            port: 80
            targetPort: 80
          - name: https
            port: 443
          - name: kiali
            port: 15029
            targetPort: 15029
          - name: prometheus
            port: 15030
            targetPort: 15030
          - name: grafana
            port: 15031
            targetPort: 15031
          - name: tracing
            port: 15032
            targetPort: 15032
          - name: tls
            port: 15443
            targetPort: 15443
          - name: tcp
            port: 31400
        strategy:
          rollingUpdate:
            maxSurge: 100%
            maxUnavailable: 25%
      name: iap-ingress
      label:
        app: iap-ingress
        istio: ingressgateway

All of the selectors use app: istio-ingressgateway rather than the name given here, so the pods in the two created deployments are not unique to the gateways.

It seems like what i was looking for was "labels" in GatewaySpec.

That didn't seem to work. I updated the original manifest in the issue description to match. Here is the template I'm referring to:
https://github.com/istio/istio/blob/master/manifests/charts/gateways/istio-ingress/templates/deployment.yaml
To me, it looks like it would use the labels from the GatewaySpec (though the field there is labels, not label..)

Ok, I think I found it... in this file:
https://github.com/istio/istio/blob/master/manifests/charts/gateways/istio-ingress/values.yaml
The labels are in there. Can i somehow reference a different values.yaml for the istio manifest generate command?

You can change labels field to other value for your values.yaml if you are intent to deploy extra gateway.

@morvencao that sounds great. thats actually my question: "Can i somehow reference a different values.yaml for the istio manifest generate command?"

If you want to handle additional Helm values.yaml input in your IstioOperator, you can add values stanza as explained here https://istio.io/docs/reference/config/istio.operator.v1alpha1/#IstioOperatorSpec.
You can check out this example IstioOperator definition which has similar setup.
https://github.com/istio/istio/blob/master/manifests/examples/multicluster/values-istio-multicluster-gateways.yaml
As to the other point you raised regarding app: istio-ingressgateway label, I have raised an issue https://github.com/istio/istio/issues/23303 and found out that the fix is pending on the new release (for both v1.6 and v1.5).
There are a few other potential concerns with multiple gateway handling which I detailed more in the aforementioned issue. It will likely be shipped with v1.6.1.


Add eager quantifiers

Exactly as the title says, no support for lazy quantifiers as lookbehind increases complexity significantly, and could cause issues with a future group implementation. I decided to separate these as whenever groups are implemented lazy quantifiers likely will have to be changed regardless.

Thank you, this PR is great!


Exhentai server "Not found Possible reasons: server offline"

set only source to exhentai, get this. Was working until a little while ago.
LOG:
[12:33:47.408][Info] Loading results...
[12:33:47.409][Info] [exhentai.org][Html] Loading page https://exhentai.org/?page=0&f_cats=0&f_search=maniacbox
[12:33:48.087][Info] [exhentai.org][Html] Receiving page https://exhentai.org/?page=0&f_cats=0&f_search=maniacbox
[12:33:48.088][Error] [exhentai.org][Html] Loading error: Unknown error (0)
[12:33:48.088][Warning] [exhentai.org] No valid source of the site returned result.
Not quite sure if this is a grabber problem or not, any ideas?

Forgot to mention I get success when I log in


Delaunay3D can produce overlapping tetrahedra

Tested versions
4.3.dev.custom_build.29b3d9e9e
System information
Godot v4.3.dev (248e58a4e) - Debian GNU/Linux trixie/sid trixie - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2080 (nvidia) - Intel(R) Core(TM) i7-9700K CPU @ 3.60GHz (8 Threads)
Issue description
Delaunay3D::tetrahedralize() when called directly from C++ or from GDScript via Geometry3D.tetrahedralize_delaunay() can produce overlapping tetrahedra (with points inside their circumsphere). All examples of two overlapping tetrahedra that I've seen have had two or three vertices in common and intersecting volumes.

It doesn't happen all the time and I don't know exactly what circumstances trigger it, but it seems that both of these must be true:

there are some points that are several hundreds distance units apart, and
there are some points that are only a couple of units apart

I noticed this problem while working on issues with LightmapGI probe triangulation and partitioning.
It's difficult to create an MRP that doesn't involve thousands of points and doesn't require extra tooling to find and visualize the few invalid cases. The attached MRP is the smallest and simplest thing I can do.
The MRP is just one scene with one node and one script. I'll dump the script here so you can check my code for obvious bugs without running the project.
@tool
extends MeshInstance3D

const points: PackedVector3Array = [
	Vector3(204.537, 2.64082, 0),
	Vector3(208.53, 2.64082, 0),
	Vector3(204.537, 2.64082, 2),
	Vector3(206.53, 2.64082, 2),
	Vector3(-501.4156, -2.415572, -501.4156),
	Vector3(501.4156, -2.415572, -501.4156),
	Vector3(-501.4156, 248.2922, -501.4156),
	Vector3(501.4156, 248.2922, -501.4156),
	Vector3(-501.4156, -2.415572, 501.4156),
	Vector3(501.4156, -2.415572, 501.4156),
	Vector3(-501.4156, 248.2922, 501.4156),
	Vector3(501.4156, 248.2922, 501.4156),
	Vector3(-250.7078, -2.415572, -501.4156),
	Vector3(-250.7078, 248.2922, -501.4156),
	Vector3(-501.4156, -2.415572, -250.7078),
	Vector3(-250.7078, -2.415572, -250.7078),
	Vector3(-501.4156, 248.2922, -250.7078),
	Vector3(-250.7078, 248.2922, -250.7078),
	Vector3(0, -2.415572, -501.4156),
	Vector3(0, 248.2922, -501.4156),
	Vector3(0, -2.415572, -250.7078),
	Vector3(0, 248.2922, -250.7078),
	Vector3(-501.4156, -2.415572, 0),
	Vector3(-250.7078, -2.415572, 0),
	Vector3(-501.4156, 248.2922, 0),
	Vector3(-250.7078, 248.2922, 0),
	Vector3(0, -2.415572, 0),
	Vector3(0, 248.2922, 0),
	Vector3(250.7078, -2.415572, -501.4156),
	Vector3(250.7078, 248.2922, -501.4156),
	Vector3(250.7078, -2.415572, -250.7078),
	Vector3(250.7078, 248.2922, -250.7078),
	Vector3(501.4156, -2.415572, -250.7078),
	Vector3(501.4156, 248.2922, -250.7078),
	Vector3(250.7078, -2.415572, 0),
	Vector3(250.7078, 248.2922, 0),
	Vector3(501.4156, -2.415572, 0),
	Vector3(501.4156, 248.2922, 0),
	Vector3(-501.4156, -2.415572, 250.7078),
	Vector3(-250.7078, -2.415572, 250.7078),
	Vector3(-501.4156, 248.2922, 250.7078),
	Vector3(-250.7078, 248.2922, 250.7078),
	Vector3(0, -2.415572, 250.7078),
	Vector3(0, 248.2922, 250.7078),
	Vector3(-250.7078, -2.415572, 501.4156),
	Vector3(-250.7078, 248.2922, 501.4156),
	Vector3(0, -2.415572, 501.4156),
	Vector3(0, 248.2922, 501.4156),
	Vector3(250.7078, -2.415572, 250.7078),
	Vector3(250.7078, 248.2922, 250.7078),
	Vector3(501.4156, -2.415572, 250.7078),
	Vector3(501.4156, 248.2922, 250.7078),
	Vector3(250.7078, -2.415572, 501.4156),
	Vector3(250.7078, 248.2922, 501.4156)]

var imesh: ImmediateMesh

func _ready() -> void:
	imesh = ImmediateMesh.new()
	mesh = imesh

	var tetrahedra: PackedInt32Array = Geometry3D.tetrahedralize_delaunay(points)
	assert(tetrahedra.size() % 4 == 0)

	# These two are just one example of overlapping tetrahedra
	draw_tetrahedron(tetrahedra, 129, Color.RED)
	draw_tetrahedron(tetrahedra, 130, Color.BLUE)

func draw_tetrahedron(tetrahedra: PackedInt32Array, index: int, color: Color) -> void:
	var vertices: Array[Vector3] = [
		points[tetrahedra[index * 4]],
		points[tetrahedra[index * 4 + 1]],
		points[tetrahedra[index * 4 + 2]],
		points[tetrahedra[index * 4 + 3]]
	]
	print(vertices)

	imesh.surface_begin(Mesh.PRIMITIVE_TRIANGLES)
	for i in 4:
		imesh.surface_set_color(color)
		imesh.surface_add_vertex(vertices[i])
		imesh.surface_add_vertex(vertices[(i + 1) % 4])
		imesh.surface_add_vertex(vertices[(i + 2) % 4])
	imesh.surface_end()

The two tetrahedra chosen here, and in the screenshot above, are:
[(250.7078, 248.2922, 0), (501.4156, 248.2922, 0), (250.7078, -2.415572, 250.7078), (250.7078, 248.2922, 250.7078)]
and:
[(250.7078, 248.2922, 0), (501.4156, -2.415572, 0), (250.7078, -2.415572, 250.7078), (250.7078, 248.2922, 250.7078)]
I just noticed that in this particular example, the two have exactly the same circumsphere, but that is likely a red herring since it is not true for many other cases (e.g. 117 together with 167).
Steps to reproduce
Load the MRP. It's a tool script and will show the issue directly in the editor.
Minimal reproduction project (MRP)
invalid-delaunay3d-mrp.zip

The problem is the constant used here:
https://github.com/godotengine/godot/blob/f47f4a02c87bb701452a621d254ad30c7be84faa/core/math/delaunay_3d.h#L176
When distances between points span a large range, from 2 to 350 units (meters) in my experiments, this constant is too big. Making it smaller by two orders of magnitude improves the situation a lot, but at the price of introducing some instability for smaller point ranges. I don't know where this instability comes from. Going to 1e-7 is pushing beyond the significant digits of Vector3, but the relevant calculations are all done with R128.


[hotfix] Do not use ExecutorService.submit since it can swallow exceptions

This commit changes the KubernetesLeaderElector to use ExecutorService.execute instead of submit
which ensures that potential exceptions are forwarded to the fatal uncaught exeception handler.
cc @wangyang0918

Thanks for the review @wangyang0918. Merging this PR now.


Add support to time the builds

A head function which wraps other functions under time.
similar to
time make Image


Simplify testing; Add connection type field

Taken from the tcp-impl branch of nedpals/vls2
Things to remember:

Most frequent functions such as log_message do not need to return string. Only for incoming request procedures except for initialized, shutdown, and exit.

Please don't remove the logic with send method. It enables asynchronous sending stuff. Like a command has to send two things, or it send's nothing. (I copied it from a typescript LS afaikr).
implement your tcp login in cmd/vls please.


Pandas copy-on-write

Pandas 2.0 has improved performance with copy-on-write.  Apparently we've been doing lots and lots of unnecessary copies.
@phofl I'm curious, what would we need to do to turn this on?  You've mentioned before that the test suite passes.  Are we good to go?

Should just need to turn on the config option in pandas like we did in https://github.com/dask/dask/pull/9942. I've opened / updated that PR to see if CI passes. Would probably want some minimum version checks, etc. Though @phofl would know better

For folks looking for more context there's this doc https://pandas.pydata.org/docs/dev/user_guide/copy_on_write.html

I'm looking forward to seeing this.  Thanks for rekindling the PR.

Yes turning on the option would functionally enable CoW. We should keep https://github.com/coiled/benchmarks/issues/808#issuecomment-1520491955 in mind. @crusaderky and I talked a bit about exposing some information whether an object is a view or a copy.

That sounds like a concern, but kind of like "oops, I didn't realize I had all of this money lying around and so my books are off" kind of concern.  It's a problem, granted, but a problem that I think we welcome with open arms?

Yes agree.
We should only turn it on for 2.0 and up, the flag became available in 1.5.0, but we didn't implement any optimisations at that point.

We need to review all slicing operations in dask.dataframe.
For example, I understand that pandas' default CoW behaviour re horizontal slicing is to retain the full original input. In this case dask would need to force a deep-copy in case the input of the task is free'd.

XREF https://github.com/dask/distributed/issues/5868 for the over-estimation of managed memory

Horizontal slicing shouldn't change between CoW and non-CoW. Vertical slicing now retains views as much as possible. Potentially keeping more memory alive. Similar to the following without CoW:
df = pd.DataFrame({"a": [1, 2, 3], "b": 1})
df = df["a"]

This also keeps the whole array alive.

We decided against doing this at import time since this is a not great UX, so there isn't anything to do for now. We will just wait until pandas 3.0 drops and we get it by default


Add an example of \NewDerivative in the documentation

A little thing, but I found that it could be interesting to have in the documentation a little example to illustrate practically how to use \NewDerivative.

I suggest looking at section 2 and section 3 for how the package uses them. But I will either make examples, or refer to those sections in the next release.


Add Opacity for web and Android, Support pinch/zoom for Android

Added a method to manage opacity for the camera preview, for web and Android.  Useful for AR type applications.
Also added pinch/zoom support for Android.
Works for both settings for CameraPreview.toBack (true and false)

Hi @johnbyrne7, thanks so much for this patch!
I'm working on getting Capacitor 3 support released first so there will be a little delay before this is merged in. In the meantime, could you rebase your pull request against the capacitor-community:capacitor-3 branch please, which will become master once released?

Sure thing.  This week sometime I think.
A question though.  Out of habit, I formatted the Android java code in
Android Studio, so a few minor formatting changes are also included.  Do
you want those formatting changes also, or just the changes needed for the
patch?
thanks,
John Byrne
On Sat, Nov 6, 2021 at 10:24 AM Peter Bowyer @.***>
wrote:

Hi @johnbyrne7 https://github.com/johnbyrne7, thanks so much for this
patch!
I'm working on getting Capacitor 3 support released first so there will be
a little delay before this is merged in. In the meantime, could you rebase
your pull request against the capacitor-community:capacitor-3 branch
please, which will become master once released?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/capacitor-community/camera-preview/pull/177#issuecomment-962459255,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAMX7N4IFAZKTBQBMFDVDN3UKU3DPANCNFSM5HHSC4OQ
.
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.

@johnbyrne7 I'd rather just the changes needed at this stage please. Once the Capacitor 3 version is released I'm going to get the code formatting standardised in the project. But waiting to do that until no PRs as it's makes so many changes!

Closing in favour of #178


Change schema description file to OpenAPI spec

Change job protocol schema description file to OpenAPI spec.

Since the protocol allow different version between job (root) and components (e.g. docker image, data in prerequisites), thus it is better to separate them to independent components
refer to #3