Try building on windows

Checklist

[ ] Used a personal fork of the feedstock to propose changes
[ ] Bumped the build number (if the version is unchanged)
[ ] Reset the build number to 0 (if the version changed)
[ ] Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@conda-forge-admin, please rerender

@conda-forge-admin, please rerender

Since flang seems to be used for Windows builds, this is not going to work right now given the unfinished state of the flang compiler and the shaky frontend. I know it works with GFortran and MinGW, if those are available a Windows build here could have a chance, but if flang is our only choice I don't see this happening too soon.

Since flang seems to be used for Windows builds, this is not going to work right now given the unfinished state of the flang compiler and the shaky frontend. I know it works with GFortran and MinGW, if those are available a Windows build here could have a chance, but if flang is our only choice I don't see this happening too soon.

I think MinGW is not an option with conda-forge, so no reason to pursue this further.

I think MinGW is not an option with conda-forge, so no reason to pursue this further.


Removes test requirements installation from qemu tests

Description
Removes the installation of tests/requirements.txt. If this file contains requirements for which there are no existing Raspberry Pi wheel files, a build from source will be triggered. This can cause CI timeouts. From now on, the python requirements for qemu tests should to be managed separately.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.

[x] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
[x] Changes are complete (i.e. I finished coding on this PR)
[x] All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
[x] Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
[x] To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Very interesting findings.

@marcoabreu could you please merge this? ^^

I don't agree with the approach because it removes transparency and instead makes the project rely on a provided blob (aka the OS image used in the Emulator). Pedro chose this approach specifically to keep up reproducibility.
From what I can tell, we are trying to circumvent a different problem here, which is that windows as well as qemu use the same test requirements.
So I'd either propose to generate a new OS image (preferred solution) or split the requirements.

I agree that this is the preferred solutions. But, a few things to keep in mind:

this is blocking the windows tests
it takes a lifetime to compile scipy to get it in the OS
there is no automation around creating the image
the one test we run inside the emulated box does not require scipy
we get some transparency by specifying that the mxnet_requirements.txt are what's in the box

Good points @perdasilva

For the sake of maintaining the mental model for future eyeballs to look at.  Leaving breadcrumbs in expected places with the right information to show the err of this way makes for a cleaner knowledge trail.  Though it is not pragmatic... it is keeping good hygiene for the future.

@gavinmbell @marcoabreu I've changed the approach slightly. Is this more agreeable?

Nit: You might wanna adapt the title.

Lets put this to bed Mr nit :-)

@mxnet-label-bot add [CI, pr-awaiting-review]
@perdasilva could you have a look at the CI failure?


Overriding optional bosh link consumers in the deployment manifest doesn't seem to work

The docs at https://bosh.io/docs/links/#explicit state:

Optional consumers may be specified as nil in the deployment manifest to block consumption of any providers.

In kubecf we have done this to disable link resolution for postgres for the credhub instance group. Looking at desired-manifest-v1 shows:
      jobs:
      - consumes:
          postgres: null
        name: credhub

However, ig-resolved.credhub-v1 still references the autoscaler database:
          quarks:
            consumes:
              postgres:
                address: asdatabase

We've ultimately worked around this with an ig_resolver patch, modifying the job.MF of the credhub job, but I think the explicit link resolution should have worked as well.

Ping @manno in case you missed this. I see that the Pivotal Tracker issue was not scheduled yet.


TensorFlow and keras version

Hi,
First of all, thanks for this work.
I am trying to run both models, but I think that I have some problems with the versions of the libraries.
Which versions did you use ?
In CUHK03 dataset, you divide the dataset into 'a' and 'b', according to the views of the cameras, they don't say nothing like this in the paper. I am newbie with this dataset, is it normal ?
Thanks!!!

Hi~
I have received many emails to ask for the version that I used for this
project. Actually, I could tell the true version, but I think the most
efficient way is to update the code to a new version such as TensorFlow 1.1
support. So I have make a plan to update the whole project code to a
totally new version using TensorFlow 1.1. If you could wait for two weeks,
you will see the new code.
For the CUHK03, the paper said nothing about that. After imshow several
images of CUHK03, I found by myself that the index 0-4 and 5-9 are
correspond to two cameras.
Regards,
Ning
2017-05-01 2:05 GMT+08:00 MarcosPieras<EMAIL_ADDRESS>
Hi,
First of all, thanks for this work.
I am trying to run both models, but I think that I have some problems with
the versions of the libraries.
Which versions did you use ?
In CUHK03 dataset, you divide the dataset into 'a' and 'b', according to
the views of the cameras, they don't say nothing like this in the paper. I
am newbie with this dataset, is it normal ?
Thanks!!!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/Ning-Ding/Implementation-CVPR2015-CNN-for-ReID/issues/16,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKTknIq_QmYAgoAJcJpcgsy4e4EYsx5hks5r1M17gaJpZM4NMpJS
.


NAN_PROPERTY_SETTER signature mismatch

Hi, I'm developing a native extension in Node 0.12.7 (from NodeSource PPA on Debian Jessie) using nan 0.8.4. I get the following error when trying to use NAN_PROPERTY_SETTER with SetAccessor:
error: invalid conversion from ‘void (*)(v8::Local<v8::String>, v8::Local<v8::Value>, const v8::PropertyCallbackInfo<v8::Value>&)’ to ‘v8::AccessorSetterCallback {aka void (*)(v8::Local<v8::String>, v8::Local<v8::Value>, const v8::PropertyCallbackInfo<void>&)}’ [-fpermissive]

The relevant code is:
// VideoPlayer.h:
class VideoPlayer : public node::ObjectWrap
[...]
static NAN_PROPERTY_GETTER(GetPosition);
static NAN_PROPERTY_SETTER(SetPosition);

// VideoPlayer.cpp:
Local<FunctionTemplate> tpl = NanNew<FunctionTemplate>(New);
[...]
tpl->InstanceTemplate()->SetAccessor(NanNew<String>("position"), GetPosition, SetPosition);

The getter works fine, and I have other properties which only have a getter and no setter which work fine. If I manually define the method signature as the error says, it compiles and works, so this seems to be a bug in nan.

#382 should be correct.

Actually, something fishy is going on here. There are so many similarly named things. NAN_PROPERTY_SETTER is a named property setter and the original signature appears correct for that. I need to investigate this some more.

Your code is wrong (probably due to bad documentation on my part). You should use NAN_SETTER and NAN_GETTER, they are accessor callbacks, for use with SetAccessor.
NAN_PROPERTY_GETTER and NAN_PROPERTY_SETTER are named property interceptor callbacks, for use with SetNamedPropertyHandler.
NAN_INDEX_GETTER and NAN_INDEX_SETTER are indexed property interceptor callbacks, for use with SetIndexedPropertyHandler.

Ah, that makes a lot of sense. Thanks!
Yeah, I struggled to find many examples of getters and setters, and whatever getter example I found used NAN_PROPERTY_GETTER with SetAccessor, which "worked" since the signatures happen to be identical. I just extrapolated to NAN_PROPERTY_SETTER which as we know didn't work. In hindsight the embedder's guide explains the distinctions pretty well, so even just adding those links to the documentation for those macros should be very helpful.

The documentation is getting a rewrite for the next major version of NAN. I'll make sure to address this as well. It also comes with an updated test suite that should serve as a future example.


go build ./  报错

~/go/src/bzppx-codepub (master ✔) ᐅ go build ./
bzppx-codepub
ld: warning: text-based stub file /System/Library/Frameworks//IOKit.framework/IOKit.tbd and library file /System/Library/Frameworks//IOKit.framework/IOKit are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//Security.framework/Security.tbd and library file /System/Library/Frameworks//Security.framework/Security are out of sync. Falling back to library file for linking.

@chenyongze 贴出你的 go 环境

同样报错：麻烦看下
(base) arthur-wxys-MacBook-Pro:bzppx-codepub arthurw$ go build ./
main.go:4:2: cannot find package "bzppx-codepub/app" in any of:
/usr/local/go/src/bzppx-codepub/app (from $GOROOT)
/Users/arthurw/go/src/bzppx-codepub/app (from $GOPATH)
main.go:5:2: cannot find package "github.com/astaxie/beego" in any of:
/usr/local/go/src/github.com/astaxie/beego (from $GOROOT)
/Users/arthurw/go/src/github.com/astaxie/beego (from $GOPATH)

@chenyongze 贴出你的 go 环境

go的版本是：
(base) arthur-wxys-MacBook-Pro:bzppx-codepub arthurw$ go version
go version go1.14.7 darwin/amd64


将环线的线路图显示为环形

Is your feature request related to a problem? Please describe.
no
Describe the solution you'd like
如题
Additional Information
none

@jonafanho

He said "Change the circuit diagram display of the loop to a circle-like display instead of a straight line"

@jonafanho
所以你能用谷歌翻译吗？他们都不懂中文
so can you use Google translate?The group can't understand Chinese.

jonathanqwq is saying to display circular lines (lines that have the same start and end stations) in a loop instead of a straight line. It's a good idea, I just never had the time to do it.


Account Payable invoice should be copy to credit note

Account Payable invoice should be copy to credit note (It's already there in account receivable)

Done by Joe


bug: Call to a member function inGroup() on null after logging out

I have declared the following under my Auth.php file:
public function logoutRedirect(): string
{
    $url = auth()->user()->inGroup('admin')
        ? 'login'
        : setting('Auth.redirects')['logout'];

    return $this->getUrl($url);
}

And get a Call to a member function inGroup() on null error when navigating to /logout.
After some debugging I realized the logout action function under LoginController.php logs the user out before redirecting, causing the redirect() helper to get an invalid value.
To overcome this, I first capture the URL in a variable and pass it to the redirect function after the auth is logged out:
public function logoutAction(): RedirectResponse
{
    $url = config('Auth')->logoutRedirect();

    auth()->logout();

    return redirect()->to($url)->with('message', lang('Auth.successLogout'));
}

This is my first time contributing to CodeIgniter, I'm not sure if I did this correctly so please let me know if I should change something, or even if this should not be considered an issue as it's not something CI Shield sets up out of the box.
Thank you and have a good one!

Thank you for the contribution!
This PR makes sense to me.
We need your GPG sign to your commits. Please sign.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#signing
and https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits

Thank you, I didn't know about that. Just signed the commit and pushed the update, let me know if anything else is needed

@cornejobarraza Thanks, can you write the php unit test for this PR?
see :
https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests#resources

@cornejobarraza Thanks, can you write the php unit test for this PR?
see : https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests#resources

Hi @datamweb, I think I ran the tests correctly and some files were generated, but those seem to be in the .gitignore file. I'm new to this, so please enlighten me on how to proceed, thank you!

I think I ran the tests correctly

Running a test is different from writing a test.

but those seem to be in the .gitignore file.

We have:
https://github.com/codeigniter4/shield/blob/ca7335c717efcd4fadfc219cdb2c0a3bae758ae2/.gitignore#L53-L56
so this issue does not create a problem for the new test. I did not understand your point in this regard.
When you execute the test command, the existing tests are executed.
Therefore, you should write a new test related to this PR.
Have you written a test for this PR? If not, see here.

I'm new to this!

Welcome. Don't worry, this PR will remain open until you write the corresponding test.

@datamweb The code is covered by existing test code.

So I think this PR is okay with no additional test case.
If we write a test case that reproduces the bug, we need to write an anonymous class which extends Config\Auth and override the logoutRedirect(), and inject it to the Factories.
I don't think we need such a test because the change is very small.
And it is not easy for test beginners.

So I think this PR is okay with no additional test case.

I agree., my emphasis was because of label teste needed.


demo does not work

Hi. What exactly don't work?

it just doesn't load. Empty screen. Only the header on the blue background and that's it.
In the console there is an error:
Error: TypeError: format_errors_1.formatErrors is not a function(…)
    (anonymous function) @ bogdan1975.github.io/:33

Component was recompiled, using Common JS modules. So, now it is compatible with both Webpack and SystemJS.
Try new version, please. Maybe problem was in module system ...

I think there might be confusion. I'm not downloading anything to try it. It's your demo that's not working. I'm just clicking on the provided demo link on the gihub project page.
Does it work for you?

Yes, it works for me.
https://youtu.be/OJUMffBtaJI
What browser do you use?

I tried on Chrome, Firefox, and IE Edge. All give the same result. Is it possible you have some browser extension installed that I don't?

No, any extension is not needed.
Send me chrome devtool screenshot with error, please.

The error points to the
System.import('app').catch(function(err){ console.error(err); });


cmd/cover: Some code are missed by Go coverage profile

What version of Go are you using (go version)?

$ go version
go version go1.20.3 darwin/amd64

Does this issue reproduce with the latest release?
Yes. Also, go1.19.8 works fine.
What operating system and processor architecture are you using (go env)?
darwin, amd64
What did you do?
Run:
$ git clone https://github.com/rumtid/parser.git
$ cd parser

$ /usr/local/go/bin/go test -timeout 30s -coverprofile=/var/folders/ry/vp2b73b17vq3_w50fsqt1xp80000gn/T/vscode-goaU9l71/go-code-cover -run ^TestMe$ github.com/rumtid/parser -coverpkg=all

What did you expect to see?
The coverage profile marks some covered lines in yyParse().
What did you see instead?
Despite yyParse() is invoked, it is not covered according to the profile.

Thanks for the report. I will take a look.

OK, I think I see what's going on here. I will send a fix shortly.
What's happening here is a side effect of -coverpkg=all, in which we gather coverage data from everything, including the Go runtime and a lot of other low-level packages. This in turn means that the underlying coverage counters can be changing even as the data is being written out, which is the source of this particular bug.
As a workaround, I recommend picking out just the packages that you are actually interested in and passing those to -coverpkg; doing that won't expose you to this particular problem.

I see, thanks a lot.


G&P | On the Progress Reports list show that a report was Skipped and allow users to view the reason it was skipped

First thought is strike through the report label and have paper tooltip showing the reason.


Dynamic is not playing nice with EF+

Good day,
I happened upon an obscure error today. I currently cannot use Dynamic and EF+ side by side using EF Core 3.1.
EDIT: Currently utilizing the latest versions of both libraries from nuget.
I get the following error:
"Oops! Select projection are not supported in EF+ Query IncludeFilter For more information, contact us<EMAIL_ADDRESS>The interesting element here is that I'm not explicitly using the IncludeFilter method from EF+
Here is a short version of what will cause the issue to occur inside my repository:
//expression where filter is Expression<Func<T, bool>> utilizing base EF Core framework ("ex. (x => x,Id == Id))
query = query.Where(filter);
//string expression where include is runtime value utilizes EF+ (ex. "Project.Image")
query = query.IncludeOptimizedByPath(include);
//string expression where orderby is runtime value utilizes Linq.Dynamic (ex. "Created desc")
query = query.OrderBy(orderby);
These calls happen in linear succession.
Is there anything I can try to get around this and or is there a fix that can be built for this. I wanted to reach out before attempting to circumvent or workaround this issue.
Thank you.

We may be able to close this issue out. I've uncovered that reordering the execution of the API calls seems to alleviate the collision between libraries.
You must utilize the Dynamic library prior to Utilizing the EF+ library of EF+ gets confused about what's going on.
This whole issue may be able to be copied over to the EF+ library.

Hello @hdusten ,
Thank you for reporting,
In this case, we will probably close it as we are not sure how we can really fix it (as you noticed, both libraries don't always work well together).
Best Regards,
Jon

Performance Libraries
context.BulkInsert(list, options => options.BatchSize = 1000);
Entity Framework Extensions • Entity Framework Classic • Bulk Operations •  Dapper Plus
Runtime Evaluation
Eval.Execute("x + y", new {x = 1, y = 2}); // return 3
C# Eval Function • SQL Eval Function


Beeswarm plot

We were discussing alternatives to the boxplot figure type. Something that's very similar to what was described verbally in the room is called a "beeswarm" plot. For example from https://www.excelnaccess.com/beeswarm-chart/:

The nice thing about these types of plot is that the circles are not allowed to overlap. Instead, they're pushed to the side. This makes it possible to use (much) larger circles without them obscuring each other.
Another option then is to scale the circles with the inverse of the number of ensemble members for each method. If you do that, the total area for each method is equal.

Nice idea, but I think we dismissed this idea (since we are not very interested in highlighting how many ensemble members contribute). So I will close this for now, but feel free to open it again if you still think it would be worthwhile in pursuing.

Even if you don't use it for highlighting individual members, I still think this type of figure could be more suitable than the boxplots or standard scatter points we have now, because you can make much bigger dots without the overlap

Hello! As I was left in charge of making this plot, I'm updating this issue a bit. First, the beeswarm plot using the seaborn library for the Pakistan case looks like this:

I took the average of the different ensemble members for those method providing more than one, and used similar colors for similar methods. The size of the points is chosen such that there is not overlap (otherwise the library gives a warning). Given that this figure is really different from that of the link you provided, I also made it with PowerBI, using the same template, and looks like this:

The trick is that in this case the value represented on the Y-axis can be quite different from the real one, so that much larger bubbles can be used. I would say it is not very rigorous, but I can explore this option further. Anyway, by the moment I will leave the new Python beeswarm figure with the box plots on top of it in the powerpoint file.

Thanks for looking into this! Is the python code for this plot available somewhere?

Ahh, found it. It's this one, right?
https://github.com/WAM2layers/Moisture_tracking_intercomparison/blob/4232cf6cfa4a66da21650efada27be7e9784367d/BeeswarmAndBoxPlots_AllCases.ipynb

Nice work! Looking again at these plots, I was thinking that there's a bit of a conflict in the way we represent different things:

Comparing moisture contributions from different regions
Comparing the agreement between different methods

Since most source regions are near-zero, and one of them is very high, we get really dense clusters near zero for most regions, and it's hard to see method-to-method differences in this way.
In this light, I think the "trick" used in PowerBI is acceptable. It still clearly conveys message 1, but it also makes it slightly easier to see if some methods are consistently at the top/bottom of the cluster (it seems that's not the case).


Missing and incorrect items.  Needs complete review.

This page has a number of missing and incorrect items.  The entire page needs a review.

Document Details
⚠ Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

ID: 28461613-82d6-e8f7-7f12-da24a594775d
Version Independent ID: d7758211-e43d-9fca-4ca2-52be17a7cd63
Content: Projects and Solutions, Options dialog box - Visual Studio (Windows)
Content Source: docs/ide/reference/projects-and-solutions-options-dialog-box.md
Product: visual-studio-windows
Technology: vs-ide-general
GitHub Login: @TerryGLee
Microsoft Alias: tglee

@graymatter00 -- Tony, thank you for your feedback.  Which items are inaccurate?  Meanwhile, please consider these resources:

Ask a question or find an answer on the Microsoft Q&A page for Visual Studio.
Check the Visual Studio MSDN Forum.
Stack Overflow
Reddit

@WilliamAntonRohm Hi.  I've attached screenshots of the web page in question and the relevant Options page in Visual Studio, as well as a screenshot of the About window to confirm the version of Visual Studio 2022.
Web page: The red crosses denote items that are not in the Options Window, and the "Tip" callout is in the wrong position.

Options window: The red crosses denote items that are not mentioned in the web page.

About window

@graymatter00 -- Tony, thank you for your detailed followup.  It is very helpful.
@TerryGLee -- Terry, please look into this issue.

@graymatter00 Thanks for letting us know; this is in the queue to be updated soon.

@graymatter00 Thanks again for bringing this to our attention and for the detailed info you provided. We've updated the page. We'll close this issue now, with our thanks for your help and interest in improving the Visual Studio documentation!

@TerryGLee Additional minor correction...
Visual Studio Options page...

Website page...

@graymatter00 Ah, thanks again for your eagle eye! It's now fixed. :)


Eymard

Not present in Missale Romanum 1962
Present in Missale Romanum 2002
Present in Italian Missal 1983
Was the memorial present in the Missale Romanum 1975? 1970? 1971?

Eymard was added in the 2002 edition, see the decree https://www.vatican.va/roman_curia/congregations/ccdds/documents/rc_con_ccdds_doc_20020327_card-medina-estevez_it.html


Please enable ipv6 by default

It's strange to see service with ipv6 disabled nowadays. Please enable it by default.

I'd like to do this, but I think it's going to be best to defer it until after we've moved the xrdp/sesman interface to a local socket (which we're working on). This will let us remove some workarounds in the code related to localhost which will simplify testing.
Most package managers build the package with IPv6 enabled. If you're building it yourself, it's a simple matter to add the switch.


Home Assistant Not Restarting in Docker Image Z-Wave Issue

Home Assistant release with the issue:
0.85.1
Last working Home Assistant release (if known):
Operating environment (Hass.io/Docker/Windows/etc.):
Unraid Docker
Component/platform:
Z-Wave
Description of problem:
When I updated to 0.85.0 and 0.85.1 I have begun to have massive Z-Wave problems.  I am using the Aeotec Z-Wave stick.  I have hand numerous problems with devices being stuck in the probe state.  Also now when I restart the docker image it fails.  I get and execution error stating that tty/ACM0 cannot be found so to fix it I have to reboot the PC.  It has become quite the issue.
Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

Traceback (if applicable):

Additional information:

For Aeotec (and other Sigma based sticks), avoid headaches by using /dev/serial/by-id/usb-0658_0200-if00 instead of /dev/ttyACM0. It will not change.

Is this just for Docker or will this work with VirtualBox setups too?  I'm running into the exact same issue with my Aeotec stick bouncing between /dev/tty/ACM0 and /dev/tty/ACM1 on VM machine restarts.

Never got a response, but /dev/serial/by-id/usb-0658_0200-if00 seems to work in Virtualbox in case it helps anybody.

@grangemd have you tried this solution:

Because it is ignored. The path is configured in .storage/core.config_entries not configuration.yaml.
For Aeotec (and other Sigma based sticks), avoid headaches by using /dev/serial/by-id/usb-0658_0200-if00 instead of /dev/ttyACM0. It will not change.

I had the same issue using an Aeoteck USB stick in Virtualbox that I just set up. I was really scared that my whole zwave network would need to be readded. But thankfully I found this and made the change! I'm still having a weird issue where randomly my most recently added entities are saying "entity not available" and I can't find their zwave information anywhere. But for now I at least have half of them back online


Strange with keep-alive

Hi, Boaz!
It's again i)
At first off all, i create database  prototype based on your library: https://github.com/recoilme/okdb
But some strange things found with flag keep alive(
Then i test with keep-alive - all looks good:
ab -n 1000 -c 200 -k http://<IP_ADDRESS>:8888/
Requests per second:    86452
But if i disable keep-alive:
ab -n 1000 -c 200 http://<IP_ADDRESS>:8888/
Requests per second:    33.85
I test in on mac and debian with same results.
May be i do something wrong with headers? https://github.com/recoilme/okdb/blob/master/okdb.c (sophia not used for this request)
And i dont may reproduce right now - but i see one time, then i send long value (around 1024 bytes) without keep-alive from curl it dont work
Example request:
curl -v -H "Connection: keep-alive" -X PUT -d "01234567890123456789012345678901234567890123456789012345678901234567890123456901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456701234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012" http://<IP_ADDRESS>:8888/hello
* Hostname was NOT found in DNS cache
*   Trying <IP_ADDRESS>...
* Connected to <IP_ADDRESS> (<IP_ADDRESS>) port 8888 (#0)
> PUT /hello HTTP/1.1
> User-Agent: curl/7.38.0
> Host: <IP_ADDRESS>:8888
> Accept: */*
> Connection: keep-alive
> Content-Length: 1021
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 1021 out of 1021 bytes
< HTTP/1.1 200 OK
< Content-Length: 2
< Connection: keep-alive
< Keep-Alive: 1;timeout=5
<
* Connection #0 to host <IP_ADDRESS> left intact

And last - then i try to reproduce - looks like server down:
./tmp/okdb

Initialized libsock for 512000 sockets, each one requires 32 bytes.
overall ovearhead: 16384000 bytes.
Initialized packet pool for 1024 elements, each one 16424 bytes.
overall buffer ovearhead: 16818176 bytes.
=== Total: 33202176 bytes ===

* Listenning on port 8888
* Running 19 processes X 1 threads.
* [23163] Running.
* [23164] Running.
* [23161] Running.
* [23162] Running.
* [23165] Running.
* [23166] Running.
* [23167] Running.
* [23168] Running.
* [23169] Running.
* [23175] Running.
* [23177] Running.
* [23179] Running.
* [23181] Running.
* [23183] Running.
* [23185] Running.
* [23187] Running.
* [23154] Running.
* [23189] Running.
* [23191] Running.
Segmentation fault

But then i grep - i see some running procces and server proccesed request.
ps aux|grep okdb
vkuliba+ 23161  0.1  0.0 3591948 33572 pts/0   Sl   14:08   0:15 ./tmp/okdb
vkuliba+ 23164  0.1  0.0 3591948 33572 pts/0   Sl   14:08   0:16 ./tmp/okdb
vkuliba+ 23165  0.1  0.0 3591948 33572 pts/0   Sl   14:08   0:17 ./tmp/okdb
vkuliba+ 23167  0.1  0.0 3591948 33572 pts/0   Sl   14:08   0:15 ./tmp/okdb
vkuliba+ 23168  0.1  0.0 3591948 33572 pts/0   Sl   14:08   0:16 ./tmp/okdb
vkuliba+ 23175  0.1  0.0 3591948 33572 pts/0   Sl   14:08   0:16 ./tmp/okdb
vkuliba+ 24072  0.0  0.0  14644  1612 pts/0    S+   16:43   0:00 grep okdb

Segfault - my mistake, iIdon't check  on null body_string

I'm a bit busy today, I'll try to be short and fast, if I don't answer everything, I'll write more when I can.
I think you encountered a known issue with the HTTP protocol that was probably (maybe) made worst because your previous code was writing directly to the sockets instead of using an http_response_s object (this object will optimize parts of the response and make sure it's formatted correctly).
One of the issues in the HTTP/1.1 protocol is that it is VERY expensive to open and close network connections. There's a whole lot of processing required by the kernel as well as a TCP/IP feature called "slow start".
It's even worst if the same client sends a lot of requests (requiring a lot of new connections). Each client has a "cool-down" period for each connection port (about two minutes on many systems) and there's a theoretical limit on how many connection ports each client can use (~64K ports per client for each listening socket)... This is system related, not much we can do about it.
This is a serious issue if you have many requests from a single client.
In normal HTTP settings, Keep Alive should improve performance drastically.
It's also why HTTP/1.1 is being replaced with HTTP/2, why Websockets are often used instead of AJAX and why memcache was using persistent connections instead of HTTP.
The better the server is designed (the server overhead is low), the higher the performance gain for Keep Alive (the more you notice the issue with the TCP/IP layer).
For facil.io, Keep-Alive should improve performance at a X20 rate (%2000)! This is similar to nagging and it's partly because facil.io is relatively fast with a small overhead. On other systems you might see smaller improvement (they have higher overhead).
HOWEVER, I suspect some of the delays were because of the way the old code you used in the test was handling response issue.
In the old code you wrote to the socket directly. This can cause HTTP errors in relation to the Content-Length header...
...When this happens, the client (curl) might wait for a disconnection before it realizes that the response is complete. Unless you manually close the socket, you are actually waiting for the HTTP timeout to occur (the default is 5-6 seconds).
Some of the issues you handle using http_response_s (i.e. herehttps://github.com/recoilme/okdb/blob/cf8ab9368f4c7d6e48a9c66c2a2fab6ec0d01df2/okdb.c#L87-L96), but

As to the segfault, I'm not sure where it's coming from.
However, would you mind experimenting with me?
If you add -DASYNC_USE_SENTINEL=1 to the CFLAGS in the makefile (or update the value in libasync.c), than the server will try to listen to error signals  and it will try to recuperate after some errors without crashing. It will also print an error report with a backtrace of the stack, so you have more information about what happened.

Oh, I just noticed you know what happened... forget about it :-)

Thank you Boaz!  Your code very well commended,  i will read it!

You're very welcome.
If you find that the keep-alive is slower than it should be or anything else, please let me know.
If there's anything wrong I would like to fix it. Having false alarms is better than not knowing :-)
Thank you for opening the issues.


Enhancement: Retrieve Plug Status via p100.getDeviceInfo() ?

Thanks for the Node - excellent !!
It would be useful to be able to query a plug's status (on or off).  Might this be done via p100.getDeviceInfo() and included in the Node ?

Hi, Thank you for using "node-red-contrib-tplink-tapo-connect-api".
It will be supported in the next update.

Add "255: Get device information" to the payload input.
[INPUT]
msg.payload = 255
You can receive the following results:
[OUTPUT]
msg.payload = {
device_id: xxxx,
device_on: <boolean [false or true]>,  <<<< true:on, false:off
...,
}
You can tell if the device is on or off by getting "device_on".

Thanks !
Currently upgrading my PC so may be a few days before I can test but I appreciate the effort :-)

On 19 Aug 2021, at 16:13, sanlike @.***> wrote:

Add "255: Get device information" to the payload input.
[INPUT]
msg.payload = 255
You can receive the following results:
[OUTPUT]
msg.payload = {
device_id: xxxx,
device_on: <boolean [false or true]>, <<<< true:on, false:off
...,
}
You can tell if the device is on or off by getting "device_on".
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

Excellent - works a treat!  I've now got a button on my SteamDesk that can toggle my room lights on and off!
I'm new to Node-Red so there may well be an easier way of doing this but here's my flow.  What would have been really handy is to have the Node itself have a toggle option, e.g 0 for Off, 1 for On, and "-1" for toggle state which incorporated the steps between the two http bits down into a single operation.  But that's just me being lazy but thought I'd mention it in case you were thinking similar :-)
Once again, thanks for your efforts !

Thank you, too.
Thank you for your wonderful hint.
I will add "toggle mode" to the next version.

What would have been really handy is to have the Node itself have a toggle option

Well then, this issue is closed.
Thank you very much.


⚠ Secret Website 3 has degraded performance

In b5f7c96, Secret Website 3 ($SECRET_WEBSITE_3) experienced degraded performance:

HTTP code: 200
Response time: 2660 ms

Resolved: Secret Website 3 performance has improved in 8fef72b.


Fix error that prevents Comments

Description
We were testing against an object and JS didn't know what to do so it gave a falsy value
Type of change

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation update
[ ] Chore: cleanup/renaming, etc
[ ] RFC

Notes

Acceptance

[ ] I have read the Contributing guidelines
[ ] I have read the Code of Conduct

Not sure if this is the main goal of this PR or not, but doesn't fix the error I get.

No, unrelated.
Your dev environment is broken.

Not sure if this is the main goal of this PR or not, but doesn't fix the error I get.

Not sure if this is the main goal of this PR or not, but doesn't fix the error I get.

No, unrelated. Your dev environment is broken.

As you can see in the code, @baristaGeek, this PR fixes an error that compared an object to a number.

Any updates @baristaGeek ?


Spellsliger skill gem does not exist

trying to theory craft a build i was surprised that the skill gem spellslinger is not there, also the name of the skill kinetic bolt is wrong its written kinetic blast, thank you for your efforts

Hello,
this is not an issue "per se".
Kinetic Blast is another skill gem.
Kinetic Bolt and Spellslinger are very new and not yet supported and will be added soon into PoB. Please be a bit patient while the community prepares these items and OpenARL puts them into the program.

Aaaah okay, i thought it was a mistake or something, im not in hurry or
anything, Keep up the good work
On Tue, Mar 17, 2020, 1:34 AM AkumaNoTsubasa<EMAIL_ADDRESS>wrote:

Hello,
this is not an issue "per se".
Kinetic Blast is another skill gem.
Kinetic Bolt and Spellslinger are very new and not yet supported and will
be added soon into PoB. Please be a bit patient while the community
prepares these items and OpenARL puts them into the program.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/Openarl/PathOfBuilding/issues/1890#issuecomment-599819205,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AO3AFBSKUHGXPCDMSOSNAPDRH3APTANCNFSM4LMQNWYQ
.


MongoDB Atlas's URI does not work

Could you provide a bit more detail? Version, anonymized URL

Could you provide a bit more detail? Version, anonymized URL

I am using the latest version of Node.js and MongoJS (v2.6.0) and the URI is used to connect to the cluster is mongodb+srv://<username>:<password>@test-rhvos.mongodb.net/test?retryWrites=true&w=majority.
However, when I pass this URI to the MongoJS, it returns an error: [MongoError]: failed to connect to server [localhost:27017] on first connect [Error: connect ECONNREFUSED <IP_ADDRESS>:27017.
I tried the same URI with the official driver and it worked perfectly fine.

@saintedlama, could you take a look at the reply I sent?

Fixed in commit https://github.com/mafintosh/mongojs/commit/106af3ec16f72a71b705983c1f5c2ab680179ecb

Major version update 3.0.0 released to npm so that fix is available if you update the package


[Issue]: Batch size scaling does not show expected benefits

Problem Description
I'm training a FastSpeech2 model on my 7900 XTX, using the latest rocm/pytorch:rocm6.0_ubuntu22.04_py3.9_pytorch_2.0.1 docker image.
What I've noticed is that increasing batch size does not necessarily translate to shorter epoch times. Adding simple instrumentation showed that increasing batch size has also increased time required to compute each of the steps in the pipeline.
Times for all of forward_pass, calc_loss and backward_pass have lengthened.
For forward_pass and calc_loss roughly inline with batch size increase (4x), and for backward_pass increase was 6x!
Is this expected behaviour?
Batch size = 16

Batch size = 64

Operating System
Gentoo Linux (kernel 6.6.8)
CPU
AMD Ryzen 9 3900X 12-Core Processor
GPU
AMD Radeon RX 7900 XTX
Other
No response
ROCm Version
ROCm 6.0.0
ROCm Component
ROCm
Steps to Reproduce
No response
(Optional for Linux users) Output of /opt/rocm/bin/rocminfo --support
ROCk module is loaded
=====================
HSA System Attributes
=====================
Runtime Version:         1.1
System Timestamp Freq.:  1000.000000MHz
Sig. Max Wait Duration:<PHONE_NUMBER>3709551615 (0xFFFFFFFFFFFFFFFF) (timestamp count)
Machine Model:           LARGE
System Endianness:       LITTLE
Mwaitx:                  DISABLED
DMAbuf Support:          YES

==========
HSA Agents
==========
*******
Agent 1
*******
  Name:                    AMD Ryzen 9 3900X 12-Core Processor
  Uuid:                    CPU-XX
  Marketing Name:          AMD Ryzen 9 3900X 12-Core Processor
  Vendor Name:             CPU
  Feature:                 None specified
  Profile:                 FULL_PROFILE
  Float Round Mode:        NEAR
  Max Queue Number:        0(0x0)
  Queue Min Size:          0(0x0)
  Queue Max Size:          0(0x0)
  Queue Type:              MULTI
  Node:                    0
  Device Type:             CPU
  Cache Info:
    L1:                      32768(0x8000) KB
  Chip ID:                 0(0x0)
  ASIC Revision:           0(0x0)
  Cacheline Size:          64(0x40)
  Max Clock Freq. (MHz):   3800
  BDFID:                   0
  Internal Node ID:        0
  Compute Unit:            24
  SIMDs per CU:            0
  Shader Engines:          0
  Shader Arrs. per Eng.:   0
  WatchPts on Addr. Ranges:1
  Features:                None
  Pool Info:
    Pool 1
      Segment:                 GLOBAL; FLAGS: FINE GRAINED
      Size:                    32790352(0x1f45750) KB
      Allocatable:             TRUE
      Alloc Granule:           4KB
      Alloc Alignment:         4KB
      Accessible by all:       TRUE
    Pool 2
      Segment:                 GLOBAL; FLAGS: KERNARG, FINE GRAINED
      Size:                    32790352(0x1f45750) KB
      Allocatable:             TRUE
      Alloc Granule:           4KB
      Alloc Alignment:         4KB
      Accessible by all:       TRUE
    Pool 3
      Segment:                 GLOBAL; FLAGS: COARSE GRAINED
      Size:                    32790352(0x1f45750) KB
      Allocatable:             TRUE
      Alloc Granule:           4KB
      Alloc Alignment:         4KB
      Accessible by all:       TRUE
  ISA Info:
*******
Agent 2
*******
  Name:                    gfx1100
  Uuid:                    GPU-f002f7e2741b7d34
  Marketing Name:          AMD Radeon RX 7900 XTX
  Vendor Name:             AMD
  Feature:                 KERNEL_DISPATCH
  Profile:                 BASE_PROFILE
  Float Round Mode:        NEAR
  Max Queue Number:        128(0x80)
  Queue Min Size:          64(0x40)
  Queue Max Size:          131072(0x20000)
  Queue Type:              MULTI
  Node:                    1
  Device Type:             GPU
  Cache Info:
    L1:                      32(0x20) KB
    L2:                      6144(0x1800) KB
    L3:                      98304(0x18000) KB
  Chip ID:                 29772(0x744c)
  ASIC Revision:           0(0x0)
  Cacheline Size:          64(0x40)
  Max Clock Freq. (MHz):   2482
  BDFID:                   4096
  Internal Node ID:        1
  Compute Unit:            96
  SIMDs per CU:            2
  Shader Engines:          6
  Shader Arrs. per Eng.:   2
  WatchPts on Addr. Ranges:4
  Features:                KERNEL_DISPATCH
  Fast F16 Operation:      TRUE
  Wavefront Size:          32(0x20)
  Workgroup Max Size:      1024(0x400)
  Workgroup Max Size per Dimension:
    x                        1024(0x400)
    y                        1024(0x400)
    z                        1024(0x400)
  Max Waves Per CU:        32(0x20)
  Max Work-item Per CU:    1024(0x400)
  Grid Max Size:<PHONE_NUMBER>(0xffffffff)
  Grid Max Size per Dimension:
    x<PHONE_NUMBER>(0xffffffff)
    y<PHONE_NUMBER>(0xffffffff)
    z<PHONE_NUMBER>(0xffffffff)
  Max fbarriers/Workgrp:   32
  Packet Processor uCode:: 528
  SDMA engine uCode::      19
  IOMMU Support::          None
  Pool Info:
    Pool 1
      Segment:                 GLOBAL; FLAGS: COARSE GRAINED
      Size:                    25149440(0x17fc000) KB
      Allocatable:             TRUE
      Alloc Granule:           4KB
      Alloc Alignment:         4KB
      Accessible by all:       FALSE
    Pool 2
      Segment:                 GLOBAL; FLAGS:
      Size:                    25149440(0x17fc000) KB
      Allocatable:             TRUE
      Alloc Granule:           4KB
      Alloc Alignment:         4KB
      Accessible by all:       FALSE
    Pool 3
      Segment:                 GROUP
      Size:                    64(0x40) KB
      Allocatable:             FALSE
      Alloc Granule:           0KB
      Alloc Alignment:         0KB
      Accessible by all:       FALSE
  ISA Info:
    ISA 1
      Name:                    amdgcn-amd-amdhsa--gfx1100
      Machine Models:          HSA_MACHINE_MODEL_LARGE
      Profiles:                HSA_PROFILE_BASE
      Default Rounding Mode:   NEAR
      Default Rounding Mode:   NEAR
      Fast f16:                TRUE
      Workgroup Max Size:      1024(0x400)
      Workgroup Max Size per Dimension:
        x                        1024(0x400)
        y                        1024(0x400)
        z                        1024(0x400)
      Grid Max Size:<PHONE_NUMBER>(0xffffffff)
      Grid Max Size per Dimension:
        x<PHONE_NUMBER>(0xffffffff)
        y<PHONE_NUMBER>(0xffffffff)
        z<PHONE_NUMBER>(0xffffffff)
      FBarrier Max Size:       32
*** Done ***

Additional Information
No response

@xelibrion  - Thanks for opening this github issue. I tried training FastSpeech2 model on 7900XTX (also on W7900) + latest rocm/pytorch docker and I do not see any performance inversion.
(Increasing the batch size from 16 to 64 increases the throughput and reduces the epoch elapsed time, as expected)
Batch size = 16

Batch size = 64

Can you please provide the exact steps/instructions (and any modified/instrumented code you used) for us to repro your result?
Thanks

@smalekta thanks for looking into this. I published my changes at https://github.com/xelibrion/FastSpeech2
The issue I see is that you have quite a low baseline for epoch times in the screenshot for batch_size = 16.
This is what I observe:

As you might notice, the run time for the first epoch is roughly inline with what you see, but then it quickly drops until it settles around 3 minute mark.
Compare it with the screenshot for batch_size = 64, and you see the same (or in fact slightly longer) run times of around 3 minutes. Therefore we did not get any benefit at all from increasing batch size, epoch still takes the same amount of time to compute.

To take performance advantage of larger batches, hardware must have enough resources for command processor to dispatch all the queued kernels for parallel processing. In case of Navi31, larger number of dispatched kernels will just stay longer in HW queue. For experiment we tried to run FastSpeech2 training on 3 different platforms:
Navi31,
Number of GPUs:       1
Per GPU:
Compute Unit:            96
SIMDs per CU:            2
Shader Engines:          6
Observation:
Batch16 training is almost always overperforming Batch64, some epochs have similar durations.
MI300,
Number of GPUs:        1
Per GPU:
Compute Unit:            240
SIMDs per CU:            4
Shader Engines:          24
Observation:
Batch64 only overperforming Batch16 when executing convolution kernels. Then epoch durations are almost the same.
MI300,
Number of GPUs:    4
Per GPU:
Compute Unit:            228
SIMDs per CU:            4
Shader Engines:          24
Observation:
Batch64 training is always overperforming Batch16.
As for much lower performance for first few epochs, during first time training some time-consuming convolution kernels are executed. During that MIOpen creates Find-Db database, where convolution data gets cached, which drastically increases performance for later epochs.
https://rocm.docs.amd.com/projects/MIOpen/en/docs-5.0.2/finddb.html
https://rocm.docs.amd.com/projects/MIOpen/en/docs-5.0.2/perfdatabase.html
So, using batch16 for FastSpeech2 training on Navi31 is preferable, and you should not expect higher performance using batches with larger sizes.

Hi @xelibrion, it is an expected behavior.  You should not expect higher performance using batches with larger sizes.

@vstempen thank you for the detailed explanation, closing the issue now.


Support option for ignoring paths

Our hapi servers export a /healthz it would be nice if we could omit those requests from the logs.
Would you be open to a PR for this functionality?
I know another option would be to pipe to a filter but I wanted to see the appetite for this built in.

Definitely, you might want to use find-my-way to match the routes.

related: https://github.com/pinojs/pino-http/issues/40 (@mcollina - perhaps not implementation but the idea.. path ignoring)

I need to silence our /healthz polling logs as well and tried to use the new ignorePaths feature in PR #30. However, it's not flexible enough for my use case since it matches the routes against request.url.path which includes the request's query params but my query params can have dynamic values.
Just in case it helps someone in the future, I ended up silencing our logs with this micro hapi plugin:
/**
 * A minimal plugin that silences the `hapi-pino`
 * response log for the `/healthz` route.
 */
exports.plugin = {
  register: async (server, options) => {
    server.ext('onPreResponse', (request, h) => {
      if (request.url.pathname === '/healthz') {
        request.logger.level = 'silent'
      }
      return h.continue
    })
  },
  name: 'health-log-silencer',
  version: '1.0.0'
}

The ignorePaths option is useful for sure (another typical use case would be paths relative to static files). But it could be a bit more useful if the path could be given with a wildcard. Exemple:
ignorePaths: ['/static*']

In this case the comparison could be done with startsWith instead of equality.

I was still struggling with this and this worked well for me in the pino-http or express-pino-logger (which just uses pino-http) library:
const expressLogger = expressPino({
  autoLogging: {
    ignorePaths: ["/_next"],
    getPath: (req) => {
      if (/_next/.test(req.url)) {
        return "/_next"
      }
      return req.url
    }
  },
});

I was still struggling with this and this worked well for me in the pino-http or express-pino-logger (which just uses pino-http) library:
const expressLogger = expressPino({
  autoLogging: {
    ignorePaths: ["/_next"],
    getPath: (req) => {
      if (/_next/.test(req.url)) {
        return "/_next"
      }
      return req.url
    }
  },
});


.set with {merge: true} doesn't work with timestamps when document already exists

Environment details

OS: Linux Mint 20.2 5.11.0-27-generic 29~20.04.1-Ubuntu SMP Wed Aug 11 15:58:17 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Node.js version: Chrome 91.0.4472
npm version:
@google-cloud/firestore version: v8.10.0

Steps to reproduce

Create a doc (e.g. "test") with one decimal field plan set to 20 (the code below uses collection "coll").
Run the following code:

do("test");
async function do(docName) {
      const docRef = this.db.collection("coll").doc(docName);
      const doc = await docRef.get();
      let plan = 0;
      let created = firebase.firestore.FieldValue.serverTimestamp();
      if (doc.exists) {
        let data = doc.data();
        console.log("Document data:", data);
        if (data) {
          plan = typeof data.plan === "undefined" ? plan : data.plan;
          created = data.created || created;
        }
      }
      // don't overwrite created if it exists, but do write created if
      // we only set the plan and "created" is undefined
      await docRef.set(
        {
          plan,
          created,
        },
        { merge: true }
      );
}

It's expected that the created field be set to the timestamp, instead it does not get defined!

@mikob I ran the following code snippet and the printed data shows that the created field is defined.
async function main() {
      const docRef = firestore.collection('coll').doc();
      await docRef.set({plan: 20});
      const doc = await docRef.get();
      let plan = 0;
      let created = FieldValue.serverTimestamp();
      if (doc.exists) {
        const data = doc.data();
        console.log('Document data:', data);
        if (data) {
          plan = typeof data.plan === 'undefined' ? plan : data.plan;
          created = data.created || created;
        }
      }
      await docRef.set(
        {
          plan,
          created,
        },
        {merge: true}
      );
      const newDoc = await docRef.get();
      console.log('Data:', newDoc.data());
}

Resulting output:
Document data: { plan: 20 }
Data: {
  plan: 20,
  created: Timestamp { _seconds:<PHONE_NUMBER>, _nanoseconds: 931000000 }
}

@thebrianchen
I think I found the issue. It seems to be my permissions, which were:
match /users/{userEmail} {
      allow get: if request.auth.token.email == userEmail;
      allow create: if request.auth.uid != null;
}

(Actual collection was "users" and not "coll)
This was a simplified example. The real version didn't use the admin account, it uses an authenticated user.
So I think that adding the created field wouldn't be supported by the above permissions. Would have thought that it would throw an error though?


Crash when focus is lost during changing name of node that's decorating external scene

We have inherited scene, or we are editing instance of external scene with 'Editable children'
We are adding new scene instance to one of the children possessed by inherited scene.
Click on recently added scene, in the way like we would like to change it's name.
Click with your mouse somewhere else to lost 'change name' focus
Godot will crash.

Note that we can normally change the name of this node if we will accept it with keyboard 'enter' key
Tested in e4cb104e6daa9c2e5781535ac8790f406ae53f08 commit
Video for quick judge: https://youtu.be/orM0doUT_JU
Sample project: CrashOnNameChange.zip

mhh, I can't see to be able to reproduce this using your example project. Renaming the "TestCube" node using focus loss did not crash for me.

@Hinsbart are you on the newest Godot?

@Hinsbart Thanks for the tip, to recreate this bug we need to actually click on other node in scene tree

@kubecz3k ok, that did it. Can confirm it's crashing when clicking on another node


Fix treatment of Assume when loading constants from .cfg

[ ] Tests added for any new code
[ ] Ran make fmt-fix (or had formatting run automatically on all files edited)
[ ] Documentation added for any new functionality
[ ] Entry added to UNRELEASED.md for any new functionality

Closes #1623

Codecov Report

Merging #1628 (037e756) into unstable (cb11ded) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##           unstable    #1628      +/-   ##
============================================
- Coverage     75.16%   75.07%   -0.09%
============================================
  Files           356      356
  Lines         11568    11583      +15
  Branches        553      548       -5
============================================
+ Hits           8695     8696       +1
- Misses         2873     2887      +14

Impacted Files
Coverage Δ

...apalache/tla/pp/passes/ConfigurationPassImpl.scala
0.00% <0.00%> (ø)

...he/io/annotations/parser/CommentPreprocessor.scala
96.70% <0.00%> (+1.09%)
: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 cb11ded...037e756. Read the comment docs.


[BEAM-14478] Fix missing 'projectId' attribute error

Fix 'RuntimeValueProvider' object has no attribute 'projectId' error in _CustomBigQuerySource.split
Please add a meaningful description for your change here

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[ ] Choose reviewer(s) and mention them in a comment (R: @username).
[ ] Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
[ ] Update CHANGES.md with noteworthy changes.
[ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

R: @pabloem


style: 🛁 linting

Description
Linting.
Type of change

[x] Linting

How Has This Been Tested?

[x] cargo test run with all tests passing

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes
[x] Any dependent changes have been merged and published in downstream modules

Codecov Report
Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.91%. Comparing base (6395325) to head (6fd7c6e).

Files with missing lines
Patch %
Lines

src/main.rs
0.00%
1 Missing :warning:

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #6   +/-   ##
=======================================
  Coverage   96.91%   96.91%
=======================================
  Files           7        7
  Lines        1331     1331
=======================================
  Hits         1290     1290
  Misses         41       41

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


Ensure times at which chpi are fitted are valid samples

I encountered this while maxfiltering 40 subjects. Some subjects failed, claiming Head position time points must be greater than first sample offset, but found 500.0005 < 500. A bit of a misleading error, as it was not the first time point that caused the problem, but the final time point.
When the continuous head position is fitted through compute_chpi_amplitudes, the first column of the result contains the times at which each fit was performed. However, these times were not necessarily aligned on the times of the original raw object, meaning some times fell in between two samples. This caused problems later on in maxwell_filter as a check is done whether the times are all valid, as the final timestamp can be slightly larger than the final timestamp of the raw object.
In this PR, compute_chpi_amplitudes is changed to always produce times that are aligned with those in the original raw object.

the final timestamp can be slightly larger than the final timestamp of the raw object... In this PR, compute_chpi_amplitudes is changed to always produce times that are aligned with those in the original raw object.

I actually don't mind the fractional samples too much as it accurately reflects the center of the window (except at the edges, but this is by design I think). Instead the problem is maybe that fit_idxs can be chosen such that the center of a window is outside the bounds of the object. In other words, the fix should probably be in the calculation of fit_idxs rather than in rounding later.
WDYT? If you agree I feel free to give it a shot or I can push a commit to try it

I simplified a bit in my explanation.
The errors occurs in mne.utils.numerics._time_mask, which does not get the raw object passed into it, but just sfreq. Since the timestamps are not aligned on samples, in some cases, the include_tmax=True behavior can fail. This behavior is programmed as:
sfreq = 1000
tmax = times[-1]  # 810.6805
tmax = int(round(tmax * sfreq)) / sfreq  # 810.68
tmax += (0.5 if include_tmax else -0.5) / sfreq  #<PHONE_NUMBER>999999999, uhoh!

maybe round should become ceil?

The errors occurs in mne.utils.numerics._time_mask

To me this is just a symptom of the problem. We shouldn't have any time in quats[:, 0] that is greater than raw.times[-1]. It means that we centered a window at a point that is beyond the limit of the raw data. So I think this still points to the problem being that we had one too many points in fit_idxs, probably because we keep mixing float and int arithmetic all over the place in that function. I'd rather see if it's possible to stick with int arithmetic based on n_window and len(raw.times) until the very end and compute times directly from from fit_idxs (maybe with a +0.5 if we have an even number of window samples -- but the inte-based fit_idxs computation could account for adjusting stop in an arange call if that's the case).

... or we just use floor arithmetic for the window length, then for even-length windows instead of getting 500.5 you get 500 and instead of going beyond len(raw.times) - 1 by 0.5 you land right at len(raw.times) - 1. That to me seems like the easiest solution. Then the fit_idxs that includes the last time point means you've used exactly half the window in your fit (which is what we do at the beginning, too, I think).

Clearly I am a bit conflicted on the right approach here as I am contradicting myself in these discussions...
I think I'm happy with merging this as is, and at some point I might try to simplify the calculations so that there are fewer rounds and casts going on. If we can stick with ints longer I think the behavior and code will be clearer.

I agree and will see if I can fix it higher up.

This can actually not be fixed higher up. This is due to rounding errors. Perhaps we should just merge this because it works?

Okay in it goes, someday maybe I'll try to simplify the logic but I don't think it should matter if we have to change by a sample or two. Thanks @wmvanvliet !


Update feedback vanity URLs for 2023

Summary
Update the forms for SR2023
Code review
Do not merge until the forms are finalised.
Testing

[ ] applied the configuration locally
[ ] manually validated the new behaviour

Links
https://github.com/srobo/tasks/issues/1147

@RealOrangeOne is this still a draft or are you looking for review here?

It's reviewable, but not mergable

What makes this not mergeable?

@RealOrangeOne I think we should aim to get these forms live as soon as possible given the aim we had of sharing them with teams ahead of the competition.


Markdown stopped working in IE, Firefox

As of 9/24/2019 the Markdown app stopped working in our O365 tenant.  We have invested a large amount of time into this application and have a large user base.  Any resolution would be greatly appriciated.
The error details are below
**Something went wrong
If the problem persists, contact the site administrator and give them the information in Technical Details.
[SPLoaderError.loadComponentError]:
***Failed to load component "1431c215-2c2a-479c-a483-51b474c7bc88" (Markdown). Original error: ***Failed to load URL 'https://spfx40fantastics.azureedge.net/spfx40fantastics/showdown_b94bc96e28da2032a19583819bff8da6.js' for resource 'showdown' in component '1431c215-2c2a-479c-a483-51b474c7bc88' (Markdown). There was a network problem. This may be a problem with a HTTPS certificate. Make sure you have the right certificate.
***INNERERROR:
***Failed to load URL 'https://spfx40fantastics.azureedge.net/spfx40fantastics/showdown_b94bc96e28da2032a19583819bff8da6.js' for resource 'showdown' in component '1431c215-2c2a-479c-a483-51b474c7bc88' (Markdown). There was a network problem. This may be a problem with a HTTPS certificate. Make sure you have the right certificate.
*CALLSTACK:
t@https://spoprod-a.akamaihd.net/files/sp-client-prod_2019-09-13.009/sp-pages-assembly_en-us_ec9244e0470ebea1d6ad4db7f56d00c0.js:4:53495
t@https://spoprod-a.akamaihd.net/files/sp-client-prod_2019-09-13.009/sp-pages-assembly_en-us_ec9244e0470ebea1d6ad4db7f56d00c0.js:20:48986
jx2P/s</e.buildErrorWithVerboseLog@https://spoprod-a.akamaihd.net/files/sp-client-prod_2019-09-13.009/sp-pages-assembly_en-us_ec9244e0470ebea1d6ad4db7f56d00c0.js:20:112759
jx2P/s</e.buildLoadComponentError@https://spoprod-a.akamaihd.net/files/sp-client-prod_2019-09-13.009/sp-pages-assembly_en-us_ec9244e0470ebea1d6ad4db7f56d00c0.js:20:108790
e/</<@https://spoprod-a.akamaihd.net/files/sp-client-prod_2019-09-13.009/sp-pages-assembly_en-us_ec9244e0470ebea1d6ad4db7f56d00c0.js:20:45200

Same issue here

We're seeing the same HTTPS cert error too.

Same here as well. I really hope this can be fixed soon

Does anyone know if this is being supported still?   We want to know if our organization needs to look for an alternative solution.

CDN was down, webparts are now available

Thank you @OlivierCC very much appreciate your quick support.


Unit test fails with error at Array.reduce

Hi! Following the instructions on how to write unit tests, I get the following error running one:
My Code:
beforeEach() {
  this.container.lookup('service:flash-messages').registerTypes(['success', 'warning']);
}

TypeError: Reduce of empty array with no initial value
    at Array.reduce (native)
    at get (http://<IP_ADDRESS>:4200/assets/vendor.js:107066:45)
    at Descriptor.ComputedPropertyPrototype.get (http://<IP_ADDRESS>:4200/assets/vendor.js:24630:26)
    at get (http://<IP_ADDRESS>:4200/assets/vendor.js:30476:19)
    at null._setInitialState (http://<IP_ADDRESS>:4200/assets/vendor.js:106678:49)
    at Object.sendEvent (http://<IP_ADDRESS>:4200/assets/vendor.js:26846:25)
    at new Class (http://<IP_ADDRESS>:4200/assets/vendor.js:47853:25)
    at Function.ClassMixinProps.create (http://<IP_ADDRESS>:4200/assets/vendor.js:48274:14)
    at _newFlashMessage (http://<IP_ADDRESS>:4200/assets/vendor.js:106777:38)
    at add (http://<IP_ADDRESS>:4200/assets/vendor.js:106732:24)

I recently got this error, too. I ended up working around this by adding
  danger: Ember.K
}```

to the test subject. I'm using version `1.3.8`.

Will dig into this, thanks for reporting

I had this same issue. It turned out in my case that my test helper was not set up correctly.
It was saying
FlashObject.reopen({ _destroyLater: null });

which apparently comes from an old version of this addon.
I changed this to
FlashObject.reopen({ setInitialState: null });

which is from version 1.3.4 that I'm using. If on a newer version, the code should probably be changed to reflect the current blueprint which is again somewhat different.

The unit test instructions were updated in: https://github.com/poteto/ember-cli-flash/pull/137
container.lookup is deprecated in favor of getOwner. @poteto Good to close?


fixed broken install instruction links

links to install docker on your computer had an extra docs/ in the path.
Removed the extra path.

Thanks for this @yufengg 👍


Loading line item extension from custom cdn behind firewall

What type of issue is this?
Question
What SharePoint development model, framework, SDK or API is this about?
💥 SharePoint Framework
Target SharePoint environment
SharePoint Online
What browser(s) / client(s) have you tested

[ ] 💥 Internet Explorer
[ ] 💥 Microsoft Edge
[ ] 💥 Google Chrome
[ ] 💥 FireFox
[ ] 💥 Safari
[ ] mobile (iOS/iPadOS)
[ ] mobile (Android)
[ ] not applicable
[ ] other (enter in the "Additional environment details" area below)

Additional environment details

browser version
SPFx version
Node.js version
etc

Issue description
Hello,
Is it possible to fail early, when trying to load the js of a line item extension from a custom cdn, that is fire walled?
The idea is to not block the list of line items if the asset can not be loaded for some users and only allow the extension to load for certain users, that are white listed?
It looks like this was working before, but now the whole list of line items is blocked until the assets are loaded and thus for some users the list is unusable.
Best Regards

Absolutely, we will take a look at this. The list of items should still be rendered even if an extension fails to load.

Hey,
Has this been confirmed as a bug and, if so, would you happen to know what the timelines for fixing it would be?

Hi there @ivaylomitrev ,
I talked this over with the team, and this is by-design behavior. For better or worse, it's a better practice for us to fail to render the list item to give the developer/user some feedback that the list item was not rendered, rather than to fail silently if the list item extension failed to load.
However, there should be some UX for this. Can you share a screenshot of what you are seeing in the list UX when this issue occurs?

Hello @nick-pape and thank you for the feedback.
I will attach additionally screenshots.
At the moment, the whole list / library is not accessible if there is  a line item extension that can not be loaded.
I think that if there is a timeout during loading the extension assets, the client and the developer can be notified in a way that will keep the list still usable and the data visible.
The user could be additionally notified by flash message for example.

Hello again @nick-pape,
Just checking if there is any new information regarding this issue? :)
Best Regards,
Plamen


Fix Markdown in Object Types

Because of a missing empty line after <summary> tag, code formatting inside of collapsible section was not applying and the text was shown as is, with ` symbols

Agree, thanks!


[$4000] Text selection options disappears abruptly

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Expected Result:
Text formatting option should be visible properly
Actual Result:
Text formatting option disappears abruptly
Workaround:
unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[x] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.2.91-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/43996225/228638165-b77fbc3a-e80b-4e43-9da0-811918508c04.MP4
https://user-images.githubusercontent.com/43996225/228638187-fd727292-37f0-4b56-8cc9-04fee22e92dc.MP4
Expensify/Expensify Issue URL:
Issue reported by: @dinaljivani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680084554334659
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01d391e71cd8a32e4c
Upwork Job ID:<PHONE_NUMBER>567663104
Last Price Increase: 2023-06-01

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Bug0 Triage Checklist (Main S/O)

[ ] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)

If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead

[ ] This bug is reproducible using the reproduction steps in the OP. S/O

If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
If the reproduction steps aren't clear and you determine the correct steps, please update the OP.

[ ] This issue is filled out as thoroughly and clearly as possible

Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.

[ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Double click on amount to select -> Look for option above text.

@DinalJivani and @kavimuru , I don't know this to be expected behavior.  I always tap and hold to select.  I tried to double-tap text in Safari on multiple pages and nothing happens, I have to tap and hold.  Am I missing something? Can either of you point me to documentation that states double tapping should select text?

@mallenexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@DinalJivani and @kavimuru , can you reply to the above?   If not (which is fine) I'll close this once it goes overdue again

@mallenexpensify
Basically the thing is when it’s static text it will be selected on tap and hold but when its text box (which is in our case) you can double tap to select the text inside or triple tap to select the whole text present inside the text box!
Check below video for same!
https://user-images.githubusercontent.com/58871336/230178266-fe5b6662-bded-4799-ae68-d1e6228b07a5.mp4

Contributor details
Your Expensify account email<EMAIL_ADDRESS>Upwork Profile Link: https://www.upwork.com/freelancers/~01a356bfeda21bbb61?s=996364627857502209

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

Job added to Upwork: https://www.upwork.com/jobs/~01d391e71cd8a32e4c

Current assignee @mallenexpensify is eligible for the External assigner, not assigning anyone new.

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

Triggered auto assignment to @madmax330 (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@madmax330, @s77rt, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@madmax330, @s77rt, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Doesn't look like we have any proposals for this yet

Doubled price to get more 👀 and proposals.

Hi I'm Nicolay from Callstack - expert contributor group - will start investigation in this area

@madmax330 @s77rt @mallenexpensify @narefyev91 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Proposal
Please re-state the problem that we are trying to solve in this issue.
Default mobile text selection menu disappears when trying to select text by clicking on it
What is the root cause of that problem?
Still same issue which we have for this TextInput for this screen - not correct using TextInput prop - selection
What happened - when we typing any value, we move cursor programmatically to the last position of the element.
When we try to select all text - by clicking - we also fire re-render a state - which cause re-render of TextInput component (because technically selection is changed for example from {start: 6, end: 6}  to {start: 0, end: 6}. That's why we see default menu for selection start showing transition (or may even not be shown) and disappears
Example in the video:
https://user-images.githubusercontent.com/28352309/231500920-b1b7d95d-3a4d-44f6-a0b8-b3be7024ff32.mp4
What changes do you think we should make in order to solve the problem?
Still suggesting to remove this selection prop - we already have some issues on hold for IOS which related to this prop - related issue with bug for the same prop https://github.com/Expensify/App/issues/15856
What alternative solutions did you explore?
cc @madmax330 @s77rt @mallenexpensify @mountiny

@narefyev91 Thanks for the proposal. I agree with you that the bug is related to selection prop. Selection-controlled TextInput is buggy on RN. We have many issues that revolve around the same root cause such as https://github.com/Expensify/App/issues/12854 and https://github.com/Expensify/App/issues/8349. Ultimately we want to fix that weird buggy behaviour upstream (on RN). But even if we wanted to go with some sort of a workaround on E/App we can't just simply omit selection prop. It's used to control selection and we still need to do so. An alternative is to control the selection directly (and render free) as proposed here.

@mountiny selection prop is used for getting start and end for selecting text

But on some pages we still use it for manipulate with cursor
@s77rt correct me if i'm wrong - but we do not really needed setting selection for TextInput itself in current situation (no matter by ref, or by state) - we need selection result only to know last cursor position to insert next number. If we fully remove selection={this.state.selection} from here src/components/TextInput/BaseTextInput.js - it will work as expected (for current page 100% on IOS). Also as i see in the message above - there are a lot of issues with Android - and how selection is working inside it. And fixes for android should be done on TextInput level but not in each particular case

Could we try to fix this upstream?

@narefyev91

we need selection result only to know last cursor position

This is done using onSelectionChange and not selection. We use selection to set the cursor position. Thus we can't just remove that prop.
Regression of removing the prop:
https://user-images.githubusercontent.com/16493223/231914448-17ba8e25-4d54-4e0b-9797-35eb297ef1fc.mp4

@s77rt thanks for video and regression preview - in that case we either should fix this somehow on react native upstream or change logic from state manipulation of selection to ref manipulation. Regarding upstream - not sure that it's related really on RN. It's mostly how we manipulate with component itself (jumping cursor from start to end, not possible to select numbers on android, and more other items). And most of this items coming from changing state between manipulation with text input. In my view we should fix it as @s77rt suggested (investigate and adapt proposal for current TextInput). BTW I tested using setSelection and removing selection state - working fine, but has some issues with Android - will be needed platform separations. @mountiny

@narefyev91

Regarding upstream - not sure that it's related really on RN

If we can fix that using setSelection then it's a confirmation that the bug is on RN.

I tested using setSelection and removing selection state - working fine, but has some issues with Android

Can you please elaborate on Android issues?

@narefyev91 Any updates here?

Ideally we could fix this upstream, its probably fine with us if that would take you a lot of time to investigate. But if you dont really want to go that path I understand that

@s77rt i will post updates here during the day - but not sure which kind of updates to be honest - if we agree to fix it based on setSelection for cross-platform solution - i will continue development and raise a PR. If we want to fix upstream - i will not sure that i could do it. We still have a lot of issues and mostly 0 possible solution to fix upstream (mostly for android). @mountiny i still not see the issue which really related to RN IOS platform - basic text input without custom control position (which we have now) - working fine.

I think we will be going with setSelection here https://github.com/Expensify/App/pull/11684 and possiblly here https://github.com/Expensify/App/issues/12854 too.. So I think we can use setSelection to fix the bug here as well. @narefyev91 Can you please update your proposal based on that
cc @mountiny

I guess if we are going with that solution in other places it makes sense to use it here too if upstream is a long shot for us here.
Isnt this something fabric could help with? I am not sure, but I heard it will improve some issues with the text fields. cc @s77rt

Unfortunately I also dont have any links in hand right now, I remember these discussions from ECX.

@s77rt @mountiny I updated proposal for using setSelection directly. Also based on what i read in comments here - https://github.com/Expensify/App/issues/15856#issuecomment-1480287140 - fabric does not resolve issue with TextInput

@narefyev91 I was looking for that issue :sweat_smile: I asked here https://github.com/Expensify/App/issues/15856#issuecomment-1514352885 about Fabric.
cc @mountiny

@madmax330 @s77rt @mallenexpensify @narefyev91 this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

@MelvinBot We are close, just getting some confirmations about the use of setSelection, seems okay for now. And asked about whether Fabric architecture would fix the selection issue, seems as a no (no hold then). So we will probably get this going very shortly.

Thanks @narefyev91 and @s77rt lets go with the setSelection if that works as we need cc @madmax330

@mountiny Let's hold for a while till we get some :eyes: on https://expensify.slack.com/archives/C01GTK53T8Q/p1681938841671029

Cool responded there

@mountiny Do you think we should hold till we see how the tests goes for https://github.com/Expensify/App/pull/17687

yeah lets do it

Still on hold. Maybe we should change this to weekly

Also yeah tagging @hannojg as this also might be covered in your proposal

Still on hold.

Still on hold https://github.com/Expensify/App/issues/12854

Same ^

Hey,
This has been first put on hold for a now closed and dismissed PR. Afterwards it has been put on hold for another PR (from #12854), but this PR is only addressing an android issue.
So I'd say this issue can be taken off hold, and we can again look into proposals!

Thanks @hannojg .  Took off hold, doubled price to $4k, added Help Wanted and Daily.
🎲🎲

Discussed with Hanno, he is going to work on this one

❌ Problem
Right now, neither on android or iOS the context menu shows when selecting text in text components rendered with BaseTextInput.
In general you can say that this issue happens when you update the value or selection prop of a TextInput to something different, after text has been selected.
When looking at the code of BaseTextInput's code, we see that it can receive value and selection as prop. However, we don't use the prop values directly, but put it in the state.
This is fine, however, we are making the update to the state in componentDidUpdate():
https://github.com/Expensify/App/blob/5ff32c3d8cec682ab25107ce5db1319c7d89f51c/src/components/TextInput/BaseTextInput.js#L73-L81
This causes the underlying text input to render twice. Once when the prop changes and thus the component re-renders. And then another time when we update the value and state state.
This re-rendering of the TextInput component causes the native text input view to re-apply the text. When we re-apply the text in the native view the input's selection menu will get dismissed.
💡 Proposal
As per this comment, we can't just use the prop value directly. To my understanding this is because all text component would need to become controlled and it would break the autoGrow behaviour.
So, instead of doing the state update in componentDidUpdate we should use static getDerivedStateFromProps. getDerivedStateFromProps is called before render and in turn prevents the re-render of the input component.
This is how the updated code would roughly look like:
    static getDerivedStateFromProps(props) {
        return {selection: props.selection, value: props.value};
    }

    // TOOD: i think we have to compare by prev state
    componentDidUpdate(prevProps) {
        // Activate or deactivate the label when value is changed programmatically from outside
        const inputValue = _.isUndefined(this.props.value) ? this.input.value : this.props.value;
        if ((_.isUndefined(inputValue) || this.state.value === inputValue) && _.isEqual(prevProps.selection, this.props.selection)) {
            return;
        }

        // In some cases, When the value prop is empty, it is not properly updated on the TextInput due to its uncontrolled nature, thus manually clearing the TextInput.
        if (inputValue === '') {
            this.input.clear();
        }

        if (inputValue) {
            this.activateLabel();
        } else if (!this.state.isFocused) {
            this.deactivateLabel();
        }
    }

Here is a video of it working.

⚠ This won't fix the flickering of the cursor on iOS. This is something that can be fixed on the native side.

Android
iOS

thanks Hanno! Look good, @narefyev91 @s77rt would you want tot share your thoughts about the above proposal?
@hannojg @janicduplessis any idea when the https://github.com/Expensify/react-native/pull/55 should be released in RN?

Giving this one to @mountiny

thanks Hanno! Look good, @narefyev91 @s77rt would you want tot share your thoughts about the above proposal?
@hannojg @janicduplessis any idea when the Expensify/react-native#55 should be released in RN?

Looks good to me, we need to check that this changes will not break web, mobile web and also if it will not introduce any jumping cursor issues

@hannojg Thanks for the proposal. Your RCA makes sense. However the way you are using getDerivedStateFromProps is basically the same as just passing the props directly. What is interesting is that even passing the props directly the autoGrow feature seems to work. If there is no need for a state after all then let's rewrite the BaseTextInput to use props directly (value and selection).
cc @rushatgabhane Maybe I'm doing something wrong so tagging you regarding https://github.com/Expensify/App/pull/8232#issuecomment-1077282006
Let's also make sure we don't break this https://github.com/Expensify/App/pull/10147#discussion_r1199505029

I think we'll loose support for uncontrolled BaseTextInputs. Will double check in a few hours!

Should we convert this to functional component before making these changes?

@s77rt direct passing prop will work, but it may cause jumping cursor issues on web. Needs also to double check this

I just tested using props directly in <BaseTextInput /> and can confirm that it breaks the autoGrow behaviour when the component is uncontrolled, i.e. no selection nor value prop being set.

@mountiny As we want to migrate everything to function components I think it can make sense to do it here already, yes.
However, function components don't really have a good replacement for getDerivedStateFromProps replacement.
So in that case it might be a solution to agree that when using TextInput from BaseTextInput it always has to be controlled (e.g. needs a value prop), so that we can use the prop directly in the function component.

@narefyev91

but it may cause jumping cursor issues on web

Why would that happen?

But again, that might not be an option, but we might just require the value prop on the BaseTextInput, also in hindsight of the function component refactor it would make sense.

@hannojg Thanks! I think we just need to pass the new value to the hidden Text. We can use onChangeText callback which is what I accidentally was doing :sweat_smile:. So the plan is:

Pass props directly
OnChangeText save new text in state
Pass the saved text to the hidden Text for measurement

Does that sound good?
There may be an alternative solution which is to re-implement the autoGrow feature in a different way.

Pass props directly
OnChangeText save new text in state
Pass the saved text to the hidden Text for measurement

This is kind of what the component is doing already. For your proposed changes, think about the event that the parent changes the value (e.g. by using the Big number pad). Then you'd still need to update your local state from the props to the state. Which is what we have right now. I see the current road map more like this:

Refactor to function component
Make the value and onChangeText prop mandatory for components using the text input (after all I think its just two components that make use of it)
Use props directly, don't relay on internal state at all

re-implement the autoGrow feature in a different way.

Yes, I think that code is pretty old and can be improved. There is an issue for that here which I am working on separately:

https://github.com/expensify/app/issues/19961

@hannojg

Make the value and onChangeText prop mandatory for components using the BaseTextInput

So this is TextInput only? Won't this increase the complexity a bit for instances where TextInput is used and value is passed. Also we will still use state in this case (just moved it from BaseTextInput to TextInput).

My thoughts here that we should for sure:

migrate to functional component
We only need to have value in state to measure correctly the width of text input for auto-grow feature (but his feature is already has a bug which i was managed to fix (but we still in some decision https://github.com/Expensify/App/pull/18579/files)) - maybe after re-writing to functional we can revisit logic of measurement and re-implement it
using useLayoutEffect to calculate width of input based on coming data from props (which will be the same as getDerivedStateFromProps logic)

Also we will still use state in this case (just moved it from BaseTextInput to TextInput).

Using state is not the enemy here, the enemy is re-rendering the component twice with different values for the text input, which removes the selection context menu (which is what we are trying to fix in this issue).
Strictly speaking the state isn't in TextInput either, but rather in the components that make use of TextInput.

Won't this increase the complexity a bit for instances where TextInput is used and value is passed.

I don't think so, in almost all cases of <TextInput /> component usage in Expensify the text input is controlled, meaning a value and onChangeText prop is being passed.

We only need to have value in state to measure correctly the width of text input for auto-grow feature

We don't need this imo, when just using the props directly we don't need an internal state. The component that make use of the autoGrow props are already controlling the <TextInput /> component with a value prop anyways, so no changes needed in the implementing components.

using useLayoutEffect to calculate width of input based on coming data from props (which will be the same as getDerivedStateFromProps logic)

I see what you mean, but interestingly it won't be "the same logic". Even though using useLayoutEffect here is a good idea and will provide good results in the web environment, it still causes a state update, and a second time re-render, so it will have the same bug as the current implementation (at least on mobile).

So for now I'd still go with the roadmap I proposed 🤔
[I am not trying to be anti or anything, just sharing my thoughts, let me know if you disagree (or agree)! 😊 ]

Also we will still use state in this case (just moved it from BaseTextInput to TextInput).

Using state is not the enemy here, the enemy is re-rendering the component twice with different values for the text input, which removes the selection context menu (which is what we are trying to fix in this issue). Strictly speaking the state isn't in TextInput either, but rather in the components that make use of TextInput.

Won't this increase the complexity a bit for instances where TextInput is used and value is passed.

I don't think so, in almost all cases of <TextInput /> component usage in Expensify the text input is controlled, meaning a value and onChangeText prop is being passed.

We only need to have value in state to measure correctly the width of text input for auto-grow feature

We don't need this imo, when just using the props directly we don't need an internal state. The component that make use of the autoGrow props are already controlling the <TextInput /> component with a value prop anyways, so no changes needed in the implementing components.

using useLayoutEffect to calculate width of input based on coming data from props (which will be the same as getDerivedStateFromProps logic)

I see what you mean, but interestingly it won't be "the same logic". Even though using useLayoutEffect here is a good idea and will provide good results in the web environment, it still causes a state update, and a second time re-render, so it will have the same bug as the current implementation (at least on mobile).
So for now I'd still go with the roadmap I proposed 🤔
[I am not trying to be anti or anything, just sharing my thoughts, let me know if you disagree (or agree)! 😊 ]

Maybe we can fully remove setState and migrate to setting value and width using refs (making TextInput uncontrolled)? Maybe that will helps. (Also just an idea)

Controlling the width via ref could be an interesting solution when changing the autoGrow implementation, but this will happen here:

https://github.com/Expensify/App/issues/19961

Let's keep this conversation focused on agreeing on a roadmap to fix this bug.
I wouldn't recommend controlling the value of the textinput via a ref. I experimented with it in https://github.com/Expensify/App/pull/11684 and there were a lot of regressions.
I still think the simplest and most straightforward solution would be:

Refactor to function component
Use props directly, don't relay on internal state at all (check every use case in the app, but I believe almost all text inputs are controlled already)

Controlling the width via ref could be an interesting solution when changing the autoGrow implementation, but this will happen here:

[$1000] Request money component is jumpy when entering the digits #19961

Let's keep this conversation focused on agreeing on a roadmap to fix this bug. I wouldn't recommend controlling the value of the textinput via a ref. I experimented with it in #11684 and there were a lot of regressions.
I still think the simplest and most straightforward solution would be:

Refactor to function component
Use props directly, don't relay on internal state at all (check every use case in the app, but I believe almost all text inputs are controlled already)

Plan sounds good to me!

Looks good to me as well! Let's do it! :rocket:

Alright, will draft a PR 😊

Looks we have a PR targeting this issue already https://github.com/Expensify/App/pull/20186

Not overdue. I see the PR still being worked on (and looks soon to be merged)

YEs we will merge this after SNH

@parasharrajat is eligible for payment here for the C+ role as I didn't review the PR fixing this

Assigned @parasharrajat as C+ and removed @s77rt .  (let me know if that isn't right)

Not deployed just merged, not sure why this got closed

@parasharrajat , can you please complete the above checklist?  Also, can you please accept the job
https://www.upwork.com/jobs/~01a98bd9a501a5056a

Rajat is paid in product

@mallenexpensify @mountiny
I see this Upwork job is not available to apply for reporting bonus.

@mallenexpensify will be able to help with that

@DinalJivani invited! Can you please accept and comment here when you have? https://www.upwork.com/jobs/~01a98bd9a501a5056a

Rajat is paid in product

Thanks for the reminder @mountiny !

@mallenexpensify
Offer Accepted

@parasharrajat for payments via NewDot for C+, Anu will take care of them so there's no action needed.
@DinalJivani is paid, thanks!
TestRail update https://github.com/Expensify/Expensify/issues/300616

Approved $4000 to Rajat based on the last comment


pandexo run_test doesn't run

Hi, I'm trying to install pandexo via github clone. The "pip install ." command runs successfully, but when I try to run the the run_test.py code it crashes with the following traceback:
Traceback (most recent call last):
  File "run_test.py", line 30, in <module>
    jdi.run_pandexo(exo_dict, ['NIRSpec G140H'], save_file=False)
  File "/home/hobson/Data/pandexo/pandexo/pandexo/engine/justdoit.py", line 402, in run_pandexo
    results =wrapper({"pandeia_input": inst_dict , "pandexo_input":exo}, verbose=verbose)
  File "/home/hobson/Data/pandexo/pandexo/pandexo/engine/pandexo.py", line 36, in wrapper
    return compute_full_sim(dictinput, verbose=verbose)
  File "/home/hobson/Data/pandexo/pandexo/pandexo/engine/jwst.py", line 85, in compute_full_sim
    i = InstrumentFactory(config=conf_temp)
  File "/home/hobson/venv_pandexo/lib/python3.8/site-packages/pandeia/engine/instrument_factory.py", line 50, in InstrumentFactory
    cls = inst_map[instrument](mode=mode, config=config, webapp=webapp, **kwargs)
  File "/home/hobson/venv_pandexo/lib/python3.8/site-packages/pandeia/engine/jwst.py", line 70, in __init__
    JWSTInstrument.__init__(self, mode=mode, config=config, webapp=webapp, **kwargs)
  File "/home/hobson/venv_pandexo/lib/python3.8/site-packages/pandeia/engine/jwst.py", line 33, in __init__
    telescope = JWST()
  File "/home/hobson/venv_pandexo/lib/python3.8/site-packages/pandeia/engine/config.py", line 47, in __init__
    all_config = merge_data(self._get_config(), config, dict(**kwargs))
  File "/home/hobson/venv_pandexo/lib/python3.8/site-packages/pandeia/engine/telescope.py", line 33, in _get_config
    self.ref_dir = os.path.join(default_refdata_directory, self.tel_name, "telescope")
  File "/home/hobson/anaconda3/lib/python3.8/posixpath.py", line 76, in join
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

Any idea what could be going wrong?

Solved, I hadn't sourced the bash profile so it wasn't finding the refdata paths!


Texture parameter errors

Minor ignorable error when rendering, but I noticed there was a "int filename" parameter put into the rib stream when using a PxrTexture node.  It doesn't cause a problem, but there's already of course a "string filename" parameter to PxrTexture so it throws an error like this:
PRMan: R10043 {WARNING} filename, unknown or mismatched input parameter of PxrTexture (PxrTexture)".
Not sure where the other parameter is get getting in there?  Maybe from the txmake options?

Yea, This is coming from txmake options. I'll take care of that tonight.

Fixed awaiting merge.

Anyway this is in devel (and works well) and a merge request is active for the master branch. I am calling this fixed since I have not seen this error after implementing the fix.