Error when using

Stacktrace is:
2019-07-17 20:56:06,825 :: bw_plex :: INFO :: plex.py:1021 :: Connectiong to Chrome
2019-07-17 20:56:06,826 :: bw_plex :: ERROR :: plex.py:85 :: There was an exception in client_action
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/requests/models.py", line 379, in prepare_url
    scheme, auth, host, port, path, query, fragment = parse_url(url)
  File "/usr/local/lib/python3.6/dist-packages/urllib3/util/url.py", line 234, in parse_url
    raise LocationParseError(url)
urllib3.exceptions.LocationParseError: Failed to parse: http://::ffff:<IP_ADDRESS>:32400/resources

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/app/bw_plex/bw_plex/plex.py", line 80, in inner
    return func(*args)
  File "/app/bw_plex/bw_plex/plex.py", line 1022, in client_action
    correct_client.connect()
  File "/usr/local/lib/python3.6/dist-packages/plexapi/client.py", line 91, in connect
    data = self.query(self.key, timeout=timeout)
  File "/usr/local/lib/python3.6/dist-packages/plexapi/client.py", line 160, in query
    response = method(url, headers=headers, timeout=timeout, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 546, in get
    return self.request('GET', url, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 519, in request
    prep = self.prepare_request(req)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 462, in prepare_request
    hooks=merge_hooks(request.hooks, self.hooks),
  File "/usr/local/lib/python3.6/dist-packages/requests/models.py", line 313, in prepare
    self.prepare_url(url, params)
  File "/usr/local/lib/python3.6/dist-packages/requests/models.py", line 381, in prepare_url
    raise InvalidURL(*e.args)
requests.exceptions.InvalidURL: Failed to parse: http://::ffff:<IP_ADDRESS>:32400/resources

Something to note is that <IP_ADDRESS> is the IP of the client, not the server.

I know very little about ip adresses, but that done dont seem to be valid. It seems to mix ipv6 and ipv4. have you enabled support for ipv6 on your plex media server?

It isn't valid. ipv6 support was enabled, so I have disabled it.  I'm still getting the same issue though.
I don't understand why it's attempting to connect to that particular IP & port, as plex doesn't run on <IP_ADDRESS>, that's where I'm watching the show from.

I never expected the URL to be invalid, but ill catch the error and proxy the request using the server later.

Where does this address come from? Does plex provide it?
Thu Jul 18 12:01:28 GMT+01:00 2019 Hellowlol<EMAIL_ADDRESS>
I never expected the URL to be invalid, but ill catch the error and proxy the request using the server later.
—
You are receiving this because you authored the thread.
Reply to this email directly,  view it on GitHub [https://github.com/Hellowlol/bw_plex/issues/97?email_source=notifications&email_token=ABO2DWT6GMJSE3VWNRSNFOTQABEQPA5CNFSM4IEUBAB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2ID5AA#issuecomment-512769664] , or  mute the thread [https://github.com/notifications/unsubscribe-auth/ABO2DWVOHG2UMLYBFBKGVO3QABEQPANCNFSM4IEUBABQ] . [https://github.com/notifications/beacon/ABO2DWSFVYKI74BAET2DIJDQABEQPA5CNFSM4IEUBAB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2ID5AA.gif]

Tes

I was experiencing this on firefox/chrome browsers fwiw.

The info i use comes from http://ip:port/clients btw. The latest commit should catch the invalid url exception.

I'll take a look at that wheeling and see what information it sends out.
It seems odd to me that you're connecting to the client on 32400 though, that's just a web browser. There must be something amiss.

It’s the pretty common, the clients seems to be using different ports on clients iOS fix (at least on my iPhone)is 32500  I don’t think the port is wrong.

Closing this as i dont think there is anymore i can do to fix this. Let me know if any issues should arise.


can't copy file path

When clicking on a file, the full path shows correctly, like the gif works, but can't copy, even it shows copy to clipboard when hover

I'm unable to reproduce this issue. Can you paste in your ActiveFileInStatusBar settings and let me know what OS are you using, and are you using any clipboard altering plugins?

Having this issue in Ubuntu 18.04 LTS. I have no settings set for ActiveFileInStatusBar (defaults). To my knowledge, I have no clipboard altering plugins.


oztec register the management server

The oztec should be able to register  to the management server by using  simple command: oztec --register server_url
after what:

write into config file the server configuration
generate the certificate
push the certificate request to the management server
get management server default agent policy

registration procedure:

Optional: The security officer can register an employee in oztes and generate the the profile to employee's agents.

a. the join link contains the identifer (unique id) to access the employee profile and register agents.
b.  the config file.

User use agent menu (or command line arguments --register URL) provide the URL or the oztes endpoint.
Agent contact the management server endpoint to get default server policy (for registration procedure) and user profile.
the agent generate PKI key pair  and  request for certificate (CSR) openssl (howto) in line the p1. requirements. The list of used attributes to fill in:

commonName  - (mandatory) host name
countryName -  (optional) provided by oztes registration policy
localityName -  (optional) provided by oztes registration policy
organizationName - provided by oztes registration policy
organizationUnitName  - (optional) provided by oztes registration policy
stateOrProvinceName - (optional) provided by oztes registration policy
emailAddress - (mandatory) corporate email
name  - (mandatory) employee name
surname - (mandatory) employee surname
givenName -(mandatory)
USER_ID (optional) provided by oztes registration policy
telephoneNumber - optional) provided by oztes registration policy
etc

sent the CSR to the oztes  to  register the agent with information regards to the workstation.

checksum of application
platform information:

MAC
CPU
OS version

Security officer checks the request and authorise them.  with authorization the oztes:

issue the agents certificate.
register create record in DB related to the registred agent. (linked to the employee object)
agent status changed to the registered.
push the applied security policy with certificate

Agent get the certificate move in Platform default store. Apply the security policy


[SCB-1501] Improvement UnitTest for Travis CI

The tearDown method is used to clean up the data after each test case is executed. But the compensation method is executed asynchronously, so it is possible to execute after the tearDown method.
I used other ways to get the globalTxId of the current test case.

Coverage increased (+0.07%) to 80.792% when pulling c4670458f4f6468bda0cf3d6d02b3f91fcbd0953 on coolbeevip:SCB-1501 into ed6fa9d4beb24c2dfab448f0be6dd53aed9caa56 on apache:master.


Require newer SciPy in a test

Following up #7511 and #7518.
We can see some signal filter fixes on SciPy 1.9.0 release.
https://docs.scipy.org/doc/scipy/release.1.9.0.html
This fixes https://ci.preferred.jp/cupy.linux.cuda115/128715/

/test full

/test full


[stable/prometheus] add apiVersion

Adding the apiVersion for chart.yaml
to fix the issue: https://github.com/helm/charts/issues/13763
Checklist

[x] DCO signed
[x] Chart Version bumped
[x] title of the PR contains starts with chart name e.g. [stable/chart]

/assign @mattfarina

/test pull-charts-e2e

/lgtm


Fragment launching without an ActionBar

As I was testing this sample, while running the test containing the launchFragmentInContainer method, I can see that the ActionBar is clearly visible.
However, running it on my own app, the fragment takes up the entire space and does not display the ActionBar. Any idea why this behavior might be arising because I need to test an action on the ActionBar.
The following are my dependencies. Any idea why I might be getting this behavior? My dependencies are the same as those in this project.

try to set support action bar .
check whether it is androidX or android


Can a Res<EguiContext> be used in two parallel systems?

This is more of a question than an issue. In my application, I currently have various parallel systems showing different windows. Most times they work just fine, but they occasionally provoke a startup crash, so I wonder if this is a limitation or a bug.
The first few lines of the startup crash error message:
thread 'main' panicked at 'already mutably borrowed', C:\Users\vi\.cargo\registry\src\github.com-1ecc6299db9ec823\atomic_refcell-0.1.7\src\lib.rs:151:23
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/9bc8c42bb2f19e745a63f3445f1ac248fb015e53\/library\std\src\panicking.rs:493
   1: core::panicking::panic_fmt
             at /rustc/9bc8c42bb2f19e745a63f3445f1ac248fb015e53\/library\core\src\panicking.rs:92
   2: core::panicking::panic_str
             at C:\Users\vi\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\panicking.rs:57
   3: atomic_refcell::AtomicRefCell<egui::memory::Memory>::borrow_mut<egui::memory::Memory>
             at C:\Users\vi\.cargo\registry\src\github.com-1ecc6299db9ec823\atomic_refcell-0.1.7\src\lib.rs:151
   4: epaint::mutex::Mutex<egui::memory::Memory>::lock
             at C:\Users\vi\.cargo\git\checkouts\egui-5e4507fa4153be06\effd3c7\epaint\src\mutex.rs:144
   5: egui::context::Context::memory
             at C:\Users\vi\.cargo\git\checkouts\egui-5e4507fa4153be06\effd3c7\egui\src\context.rs:378
   6: egui::context::Context::style
             at C:\Users\vi\.cargo\git\checkouts\egui-5e4507fa4153be06\effd3c7\egui\src\context.rs:443
   7: egui::containers::window::{{impl}}::show_impl::{{closure}}
             at C:\Users\vi\.cargo\git\checkouts\egui-5e4507fa4153be06\effd3c7\egui\src\containers\window.rs:255
   8: core::option::Option<egui::containers::frame::Frame>::unwrap_or_else<egui::containers::frame::Frame,closure-0>
             at C:\Users\vi\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\option.rs:427
   9: egui::containers::window::Window::show_impl
             at C:\Users\vi\.cargo\git\checkouts\egui-5e4507fa4153be06\effd3c7\egui\src\containers\window.rs:255
  10: egui::containers::window::Window::show<closure-0>
             at C:\Users\vi\.cargo\git\checkouts\egui-5e4507fa4153be06\effd3c7\egui\src\containers\window.rs:236

Hi @OfficialURL! Oh, when I looked at egui's Context, I saw it wrapped its fields into mutexes, so I assumed it's totally fine to be used in parallel out of the box. Just now I've noticed that it uses its own wrapper around parking_lot's Mutex, which panics if it's built with egui/single_threaded feature (which is the default one). Please try to disable this one, enable egui/multi_threaded and see whether it works.
We should probably build egui with multi_threaded by default since the current API implies using the context in multiple threads. And it would be nice to introduce a feature in bevy_egui to switch back to the default egui/single_threaded, also documenting that our context should be accessed only via ResMut<EguiContext> when using it.

I decided not to add the feature for the following two reasons:

Enabling the single-threaded mode would require a user to disable all the default features.
Egui doesn't quite advertise multi-threaded usage, so I think that a safer default is the single-threaded mode.

From Egui's documentation:
/// Almost all methods are marked `&self`, `Context` has interior mutability (protected by mutexes).
/// Multi-threaded access to a [`Context`] is behind the feature flag `multi_threaded`.
/// Normally you'd always do all ui work on one thread, or perhaps use multiple contexts,
/// but if you really want to access the same Context from multiple threads, it *SHOULD* be fine,
/// but you are likely the first person to try it.

I tried to improve our documentation instead, to mitigate the confusion: 83b84cec364075a893d256a28da0c1aada374f68.
Please feel free to reopen the discussion if you disagree or aren't quite happy with the decision. I'm open to other arguments and might still reconsider it.

Thoughts on revisiting this? It looks like the egui is multi-threaded by default and the context can be freely cloned now.
https://github.com/emilk/egui/commit/15254f8235e5905b1c61c9f29aff30ce22db8141
https://github.com/emilk/egui/commit/8ce0e1c5206780e76234842b94ceb0edf5bb8b75#diff-6593d7c63c5e66f9c0c1cd121186d9ebbde6eb4fc86b0396d5ffd93ac9788377R125

@frewsxcv you can use the immutable_ctx feature to be able to access the context in parallel. Though I usually encourage people to keep relying on the Bevy scheduler, since you can't really use Egui context truly in parallel anyway, as its internals is hidden behind RwLock. Accessing the Egui context by two systems at once will just cause one of the systems busy waiting for the lock to be released. By accessing the context with ResMut you'd let the Bevy scheduler do other useful work.


backend order document search

1. Why is this change necessary?
allows searching for orders via their order document numbers
2. What does this change do, exactly?
changes the order backend search indexer for ES and the default dbal search to include the order document numbers
3. Describe each step to reproduce the issue or behaviour.
add a document to a order and try to search the definied document id.
4. Please link to the relevant issues (if any).
https://issues.shopware.com/issues/SW-22627
5. Which documentation changes (if any) need to be made because of this PR?
none
6. Checklist

[ ] I have written tests and verified that they fail without my change
[X] I have squashed any insignificant commits
[ ] This change has comments for package types, values, functions, and non-obvious lines of code
[X] I have read the contribution requirements and fulfil them.

Hey @jochenmanz,
can you please squash your commits? :-)

commits squashed :)

Hello,
thank you for creating this pull request.
I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/SW-22765
Please use this issue to track the state of your pull request.

Hi @jochenmanz, thank you for your awesome contribution! 💙


Update conda yaml files to be more consistent and bug fix

PR Summary:

As is, environment.yml does not work because the requests package is missing.
environment-win.yml is really a development environment environment-dev-win.yml. This file was renamed.
There is no windows equivalent of environment.yml. This has been added.

Right now, creating an development environment with environment.yml cause the GAFF foyer tests to error: AttributeError: module 'foyer.forcefields.forcefields' has no attribute 'load_GAFF'. At first I thought this was because GAFF foyer is installed before foyer. Wasn't there some issue with this @justinGilmer @umesh-timalsina. However, I tried re-installing GAFF foyer before foyer so now I am less certain what has gone wrong. If possible, I would also like to get that fixed before we merge this PR.
Edit:  The problem is related to the installation order of editable installs with entry points. If the foyer installation is editable, it appears that antefoyer needs to be installed after foyer. If the installation is not editable, the tests fail to even run with the following error:
ImportError while loading conftest '/Users/ryandefever/repos/mosdef/foyer/foyer/tests/conftest.py'.
_pytest.pathlib.ImportPathMismatchError: ('foyer.tests.conftest', '/Users/ryandefever/software/miniconda3/envs/foyer-dev/lib/python3.8/site-packages/foyer/tests/conftest.py', PosixPath('/Users/ryandefever/repos/mosdef/foyer/foyer/tests/conftest.py'))

I think you also need to update the azure-pipelines.yml to use the new windows-dev yml

This PR is ready for review. All tests are passing except for codecov.


Changing the Depth setting doesn't alter the diagram for hidden blocks

Description:

Changing the Depth setting doesn't alter the diagram for hidden blocks.
Steps to reproduce:
Affected Versions:
1.0.0-RC1
OS, DB, other environment details and versions:
Related Issues (optional):

Suggested Labels (optional):

Suggested Assignees (optional):

Issue is no longer valid for Swan Lake release. Hence, closing it.

Issue is no longer valid for Swan Lake release. Hence, closing it.


Update pooling layer

Implement AVG pooling.
Allow for window of size 3.

Implemented avg pooling option. Next is to edit extract_architecture and generate_cpp to incorporate these modes into pooling calls.

Incorporated AVG pooling into function calls.

Successfully accepting pooling kernel size of 3.

Only working for cifar-10 and mnist. Fix it.

Removed hard-coded bug!!!


shapely 2.01 parallel_offset different behaviour from 1.8.4

With a "complex linestring", 1.8.4 parallel_offset used to return a MultiLineString composed in my example of 3 distinct Linestrings, each of them completely separated from each other
With shapely 2.0.1, I only obtain 1 single LineString, only the first one given in 1.8.4. The two others are missing.
Windows 2.0.1 installed from PyPI using pip

Similarly as in the other issue, this is probably due to a change in the implementation in GEOS 3.11. See https://github.com/libgeos/geos/issues/816 for some discussion.
Do you have a reproducible example? Then I would be happy to test it with the latest GEOS main.

Yes sure
wkt.txt
from shapely import wkt
import shapely

import matplotlib
import matplotlib.pyplot as plt

def plot(linestring):
    plt.figure()

    x = linestring.coords.xy[0]
    y = linestring.coords.xy[1]
    plt.plot(x,y)
    plt.show()

fp = open("wkt.txt", "r")
ww = fp.read()
fp.close()

ww

linestring = shapely.wkt.loads(ww)
linestring

plot(linestring)

offset = linestring.parallel_offset(0.25, 'left', resolution=16)
offset

print( "shapely version ", shapely.__version__, "offset type :", offset.geom_type)

# shapely 1.8.4 -> MultiLineString, 3 lines
if shapely.__version__ == '1.8.4':
    print(" shapely 1.8.4: nb lines = ", len(list(offset.geoms)))

    for line in offset.geoms:
        plot(line)

# shapely 2.0.1 -> LineString, 1 line, 2 others missing
if shapely.__version__ == '2.0.1':
    plot(offset)

Many thanks.
XM

Yes, I can confirm that this seems fixed with GEOS main (https://github.com/libgeos/geos/issues/816).
Plotting the original line and the offset for shapely 1.8 using GEOS 3.10.1, shapely 2.0 using GEOS 3.11.1 and shapely 2.0 using GEOS 3.12.dev main:
GEOS 3.10:

GEOS 3.11:

GEOS 3.12:

import shapely

with open("../Downloads/wkt.txt", "r") as fp:
    linestring = shapely.from_wkt(fp.read())

offset = linestring.parallel_offset(0.25, 'left', resolution=16)

from shapely.plotting import plot_line
plot_line(linestring)
plot_line(offset, color="C2")


Very simple fix for #1153

Preventing the deletion of  Caddy-Rewrite-Original-URI from header allows php fastcgi apps to correctly log the url requested.
With my basic testing seems to fix #1153
I do not have enough knowledge to know if the other lines in the if statement are relevant or should also be removed.

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Thanks for the PR @tobya! As long as keeping the header there will not impact any upstream PHP applications, then this change should be fine. I'm waiting for at least one reliable confirmation in the linked issue before merging this.
Maybe we could accept this change either way if we skip this header field when forming the env vars that go to the PHP app (in the loop a bit lower with // Add all HTTP headers to env variables). That might be the cleanest, surest way to resolve this.

I will test it with a lumen php app served by caddy over fastcgi

As suggested I ensured that the  Caddy-Rewrite-Original-URI isnt added as a HTTP ENV value passed to fastCGI.
I had to convert both variable to Uppercase before comparison as it turns out the constant is

Caddy-Rewrite-Original-URI

but the field value in the Header was actually

Caddy-Rewrite-Original-Uri

I thought comparing both uppercase would be more resilent than changing the constant to  Caddy-Rewrite-Original-URI
I am new to GO so let me know if I've broken anything.

The change I've made seems to be preventing any ENV variables from being created according to Travis, I don't see how though

There is an issue here beyond my issue.
There seems to be a problem with the linter wanting a function to be called

blockHTML

rather than

blockHtml

but the blackfriday render is expecting blockHtml

I must admit I'm not sure where to start with writing tests that would do this.    Would I add another request at the bottom of `TestBuildEnv' in fastcgi_test.php?
I also have to admit I'm not sure how to run the test suite (or is that done by travis?).  I'll look at it tomorrow but any pointers would be appreciated, or links to resources.

Ok I think I can see how to add a test, however how do I setup the neccessary caddyfile directive to have the rewrite happen?
Is there any docs on setting up and writing tests for caddy?

@tobya You found the right place; your test should be in the TestBuildEnv test function. All you have to do is add a test case (like this one, but maybe after it or at the end) where you add a header to the request (try r.Header.Add()) with the field name in question with some arbitrary value. Then make sure to set the expected env vars to NOT include that header field, but then check that after the call to buildEnv that the header still exists on the request (r). The test should fail without your changes, and pass with them.

I have created tests for this PR.  I check that the header still exists and that the ENV variable HTTP_CADDY_REWRITE_ORIGINAL_URI does not.
I had to add a default header value into the call to newReq.  I tested it and it failed without changes and passes with changes.

Hey @tobya, thanks again. You've handled this really well; I'm going to give you collaborator status so you have push privileges to the repo. This way, you can help review pull requests, submit PRs on a branch instead of having to fork, etc. If you accept the invite, please read the contributing guidelines so you know what kind of clean code we expect. :)

@mholt Thanks.


Migrating to WebGPU

I'm currently working on an implementation of regl-gpu-lines for WebGPU.
regl-gpu-lines was the only thing keeping me from switching to WebGPU, because it's so amazing, but I've decided to bite the bullet and switch to WebGPU.
Right now, I'm working on a WebGPU version of this library. I've read the articles at the bottom of the README a few times over but I'm still struggling to grasp the implementation.
Have you guys thought about doing some sort of write up on how regl-gpu-lines works under the hood?
Thanks!!

Sorry I missed this; I've been out of the office for the last few weeks. Yes, this is something I'd very much like to accomplish! If I'm being honest, I'm moderately happy with the sophistication with which this library accomplishes its task, but I'm moderately unhappy with how complicated the library ended up, which makes it much harder to extend or translate the code.
The very short, inadequate answer about how it works is that the input data is the same attribute buffer, passed four times with different offsets so that the geometry of a line segment together with the joins at either end can be computed in the vertex shader. For triangulation, each line segment together with two half joins on either end are a single instance. Each instance is rendered as a triangle strip, and the instanced attribute, rather than vector offsets, is an integer index which is translated inside the shader to a particular position on the instance. The debug page has a numbering debug mode which shows instance vertex numbers. When there's a gap in the line, it stretches the half-join all the way around into an end cap. At the end of the day, it's really just fancy connect-the-dots, but I had to try a number of different strategies before figuring out how to number the vertices and use degenerate triangles to avoid problematic corner cases.

Sorry I don't have a better answer than this for now, but putting together a writeup and managing to translate this to WebGPU is definitely something I'd like to accomplish!

Thank you for the answer. Right now I'm just running the library locally and poking around at it. It's really comprehensive and educational picking it apart. I'm sure, if I can convert it to raw Webgl, then I can convert it to Webgpu. Although, Webgl global state might be a headache. Gonna try my luck!
Again, thanks for making this open source. It's been really useful and educational along with all your other Observablehq notebooks!


PR https://github.com/speedb-io/speedb/pull/368 breaks public API compatibility

PR https://github.com/speedb-io/speedb/pull/368 breaks public API compatibility, some applications using rocksdb do not compile because of the public api changes

currently paused until after the rebase

@udi-speedb , are you aware? https://github.com/speedb-io/speedb/pull/368 has been removed from the current main

updated implementation after 8.1 rebase does not change existing RocksDB public API -
https://github.com/speedb-io/speedb/pull/620


Unable to find dependencies when injecting @injectable classes that have the @registry annotation from another file.

Describe the bug

Unable to find dependencies when injecting @injectable classes that have the @registry annotation from another file.
To Reproduce

The following sample code works and can be seen running here: https://stackblitz.com/edit/typescript-ys8qjv?file=index.ts
The issue problem I'm facing is the same code doesn't work when subclasses eg Abc, Foo and Baz are defined in different files
import "./style.css";
import "reflect-metadata";
import { injectable, registry, container } from "tsyringe";

class RestController {}

//Imagine all these classes are in their own files
@injectable()
@registry([
  {
    token: "RestController",
    useToken: RestController
  }
])
class Abc extends RestController {}

@injectable()
@registry([
  {
    token: "RestController",
    useToken: RestController
  }
])
class Foo extends RestController {}

@injectable()
@registry([
  {
    token: "RestController",
    useToken: RestController
  }
])
class Baz extends RestController {}

const controllers: RestController[] = container.resolveAll<RestController>(
  "RestController"
);

const appDiv: HTMLElement = document.getElementById("app");
appDiv.innerHTML = `<h1>Injected Dependencies ${controllers.length}</h1>`;

Expected behavior

I expect to be able to inject classes defined in different files without needing to explicitly add them to the registry in the class using them. It would also be awesome if we didn't need the registry annotation and could maybe inject it based on the type only or add the token to the @injectable annotation similar to how SpringBoot handles the bean or component annotations.
Version:
4.4.0

Can you share your example where it doesn't work? I'm wondering if the registry() decorators are running or not.

Hi @MeltingMosaic I updated the example: https://stackblitz.com/edit/typescript-wjjeyq?devtoolsheight=33&file=index.ts
Basically the same code except it's moved to seperate files but doing that results in this error being thrown: Attempted to resolve unregistered dependency token: "RestController"

Got it, so what I think is happening here is that in index.ts it doesn't know of the existence of Abc Foo and Baz. The registry() decorator runs (and the type gets registered) when a module is imported, and there are no imports for those types. This is a little different than coming from Java or .NET where the dependency injector can introspect the entire package and find injectables.
Here's a quick fix (not sure this will meet your needs)
import "./style.css";
import "reflect-metadata";
import { injectable, registry, container } from "tsyringe";
import RestController from "./RestController";
import "./Abc";
import "./Baz";
import "./Foo";

const controllers: RestController[] = container.resolveAll<RestController>(
  "RestController"
);

const appDiv: HTMLElement = document.getElementById("app");
appDiv.innerHTML = `<h1>Injected Dependencies ${controllers.length}</h1>`;

Thank you! That makes sense I think your suggestion should work for now.


Readme refers to Codeplex?

README.md says:
Add the Quick Converter namespace to your xaml files so you can reference it.
xmlns:qc="http://QuickConverter.CodePlex.com/"

Should it really still point to Codeplex?
Thanks

While it works, I guess so.

You should refer in your XAML to
xmlns:qc="clr-namespace:QuickConverter;assembly=QuickConverter"


Signing root reqs

We found "block root" is ambiguous in Trinity implementation - now we use custom type HashTreeRoot and SigningRoot to distinguish them. (In Python, they are HashTreeRoot = NewType("HashTreeRoot", Hash) and SigningRoot = NewType("SigningRoot", Hash). Do you think this kind of annotation would be helpful to the networking spec?

The name Hash is from the main spec, I think we should stay consistent with that (ie change this or change that) - not sure why we ended up using HashTreeRoot here (historical?).
I guess  the problem is signing_root itself which has a pretty awkward definition, which leads to these name games. In practise we never use hash_tree_root with blocks, so there's no de facto ambiguity (or is there?) - one alternative would be to unify hash_tree_root and signing_root, or remodel the ssz objects so that the signature is external to the data being signed.
Having types for different hashes in the spec feels a bit over the top - they're the same kind of hash but over different data, really - though this is not an opinion I hold strongly, and I guess will depend on context.

Agree that we don't really need an additional type here. Let's keep as is with the note for now

Changed PR to new branch v09x to stage for release in next minor release

@arnetheduck @djrtwo I don't feel strongly about using new types, but I think there are still some unclarity of the spec. :)
https://github.com/ethereum/eth2.0-specs/blob/v09x/specs/networking/p2p-interface.md#beaconblocksbyrange

Requests count beacon blocks from the peer starting from start_slot on the chain defined by head_block_root (= signing_root(BeaconBlock)).

and
https://github.com/ethereum/eth2.0-specs/blob/v09x/specs/networking/p2p-interface.md#messages
In /eth2/beacon_chain/req/status/1/

head_root: The block hash tree root corresponding to the head of the chain as seen by the sending node.

To make sure, is it hash tree root?
If so, the client needs to keep both hash tree roots and signing roots for lookup? (That's what we do in Trinity, because we follow the v08 spec which uses hash tree root for block root.)
If so, it would be nice to distinguish both fields more precisely. e.g., head_beacon_signing_root and head_beacon_hash_tree_root. Or using the same way to describe hash tree root, e.g.:

Requests count beacon blocks from the peer starting from start_slot on the chain defined by SSZ signing root head_beacon_block_root (= signing_root(BeaconBlock)).
head_beacon_block_root: The block hash tree root (hash_tree_root(BeaconBlock)) corresponding to the head of the chain as seen by the sending node.

Also, it's odd to have both bytes32 and Hash for roots at the spec. I'd say align it with bytes32, otherwise, we should apply all custom types (Slot, Epoch... ) here, which seems to cross the design scopes.

Good catch @hwwhww!

The intent when we were discussing it was signing root here as well, it is another oversight / naming confusion
I'd say let's keep blocks to signing root always - less confusion, fewer indices, easier to copy paste things in logs
Also fixed by always using signing root - the names were meant to be derived from / aligned with the main spec while conveying their special meaning. Clearly there's copy-writing to do here :)

Follow up: #1459


[WIP] Convert checked chart ids to filter scope

CATEGORY
Choose one

[ ] Bug Fix
[x] Enhancement (new features, refinement)
[ ] Refactor
[ ] Add tests
[ ] Build / Development Environment
[ ] Documentation

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

[ ] Has associated issue:
[ ] Changes UI
[ ] Requires DB Migration.
[ ] Confirm DB Migration upgrade and downgrade tested.
[ ] Introduces new feature or API
[ ] Removes existing feature or API

REVIEWERS

Codecov Report

Merging #8516 into scope-selector-modal will increase coverage by 0.03%.
The diff coverage is 93.02%.

@@                   Coverage Diff                    @@
##           scope-selector-modal    #8516      +/-   ##
========================================================
+ Coverage                 66.75%   66.79%   +0.03%
========================================================
  Files                       467      468       +1
  Lines                     22848    22891      +43
  Branches                   2467     2484      +17
========================================================
+ Hits                      15253    15290      +37
- Misses                     7434     7440       +6
  Partials                    161      161

Impacted Files
Coverage Δ

.../src/dashboard/util/getFilterScopeFromNodesTree.js
93.02% <93.02%> (ø)

superset/db_engine_specs/sqlite.py
54.83% <0%> (-16.13%)
:arrow_down:

superset/dataframe.py
93.7% <0%> (-0.79%)
:arrow_down:

superset/utils/core.py
88.41% <0%> (-0.17%)
:arrow_down:

...assets/src/dashboard/util/getDashboardFilterKey.js
100% <0%> (+66.66%)
: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 67694c0...7027248. Read the comment docs.


[Tables] Passing empty sort={} to a find is enough to trigger the "no pageState allowed" error

When running e.g. the second page of a find command, thus passing a pageState: <string> to the options,
it is sufficient to provide an empty sort: {} to trigger the following error (the find being otherwise working correctly):
    "errors": [
        {
            "message": "Request invalid: field 'command.options.pageState' value
 \"FindCommand[filterClause=FilterClause[logicalExpression=LogicalExpression
{logicalRelation='AND', totalComparisonExpressionCount=2, logicalExpressions=[],
comparisonExpressions=[ComparisonExpression{path='p_ascii', filterOperations=
[ValueComparisonOperation[operator=EQ, operand=JsonLiteral{type=STRING,
value(String)=A}]], dbFilters=null}, ComparisonExpression{path='p_bigint',
filterOperations=[ValueComparisonOperation[operator=EQ, operand=JsonLiteral
{type=NUMBER, value(BigDecimal)=100}]], dbFilters=null}]}], projectionDefinition=null,
sortClause=SortClause[sortExpressions=[]], options=Options[limit=null, skip=null,
pageState=AAgABAAAABMBAPB////r8H///+s=, includeSimilarity=false, includeSortVector=false]]\"
not valid. Problem: pageState is not supported with sort clause.",
            "errorCode": "COMMAND_FIELD_INVALID",
            "id": "f18f73b3-3fa5-4d3c-ad17-ccfe21affd9b",
            "family": "REQUEST",
            "title": "Request invalid",
            "scope": "EMPTY"
        }
    ]

@hemidactylus this is a bug we need to fix, do you have an example of what the find was ?

Sure, I just found this is enough even on an empty table and with a made-up pageState:
{
    "find": {
        "sort": {},
        "options": {
            "pageState": "x"
        }
    }
}

Related to #1736


Add a redirect jaxrs resource.

retest this please

retest

@chirino can you rebase?

Dont think we need this yet.  Will keep in on my personal branch in case we ever do need it.


N-API support for Node.js 8

Are there any plans to support N-API? I think it can drastically simplify binary packages deployment and management ;)

https://medium.com/the-node-js-collection/n-api-next-generation-node-js-apis-for-native-modules-169af5235b06

I would like to second this. I think it would solve a lot of open issues and allow people to use the most current version of node.

This issue has been closed because it has been inactive for > 180 days.
You may reopen this issue if it has been closed in error.


[BUG] ops.GroupBy after ops.Filter fails to group correctly, and produces unexpected NaNs

Describe the bug
Applying ops.GroupBy(...) after ops.Filter(...) causes some weird behaviour. Some rows are filled with lists of nans, and rows are not groupped correctly. It seems like the problem is with indexes.
A bug related to https://github.com/NVIDIA-Merlin/NVTabular/issues/1767
Steps/Code to reproduce bug
Sample code:
import pandas as pd
import nvtabular as nvt

# dummy data
_event_id = [0, 1, 2, 3]
_session = ["a", "a", "a", "b"]
_category = ["x", "x", "x", "y"]
_event_type = ["start", "start", "stop", "start"]
input_df = pd.DataFrame(
    {"event_id": _event_id, "session": _session, "category": _category, "event_type": _event_type}
)
print(input_df.head())

# graph
cat_feats = ["category"] >> nvt.ops.Categorify()

features = ["event_id", "session", "event_type"] + cat_feats

features = features >> nvt.ops.Filter(f=lambda df: df["event_type"] == "start")

groupby_features = features >> nvt.ops.Groupby(
    groupby_cols=["session"],
    aggs={
        "event_id": "list",
        "category": ["list", "count"],
        "event_type": ["list"],
    },
)

processor = nvt.Workflow(groupby_features)
dataset = nvt.Dataset(input_df)

output_df = processor.fit_transform(dataset)
print(output_df.head())

input_df looks like this:
   event_id session category event_type
0         0       a        x      start
1         1       a        x      start
2         2       a        x       stop
3         3       b        y      start

And output_df (after filter and groupby):
  session    event_id_list    category_list        event_type_list     category_count
0       a  [0.0, 1.0, 3.0]  [3.0, 3.0, 4.0]  [start, start, start]                  3
1       b            [nan]            [nan]                 [None]                  0

Expected behavior
Expected output_df should look like this:
  session event_id_list category_list       event_type_list  category_count
0       a        [0, 1]        [3, 3]        [start, start]               2
1       b           [3]           [4]               [start]               1

The event  with event_id ==  3 should be assigned to the session b, not a.
Dtype of columns event_id_list and category_list should be lists of ints not floats
Environment details (please complete the following information):

Environment location: docker container (from nvidia/cuda:11.8.0-devel-ubi8)
Method of NVTabular install: mamba
nvtabular version: 23.8.0

Additional context
Related issue https://github.com/NVIDIA-Merlin/NVTabular/issues/1767 was about TypeError. In the output_df you can see, that the category_list column contains lists of floats (categories should be ints after ops.Categorify ) so they were converted in order to avoid TypeError.
I believe, that only the symptom of a bug was fixed there and not the cause. I think TypeError was an indirect result of the bug I describe in this issue. Since GroupBy causes some rows to be nans, there was a type conflict between original values (ints) and the nans (floats).  But the real problem is that GroupBy after Filter messes up indexing and create some empty rows.

I'm also really interested in this issue. I've seen that @oliverholworthy was involved in the linked problem! Dear @oliverholworthy do you think the above is a pressing issue?

Hey again! I've tried to reproduce the above using two of Merlin's Docker images. Interestingly, it turns out the issue:

does show up for a pretty recent image [nvcr.io/nvidia/merlin/merlin-pytorch] (from September 25, 2024) (https://catalog.ngc.nvidia.com/orgs/nvidia/teams/merlin/containers/merlin-pytorch)
does not show up for an old image [nvcr.io/nvidia/merlin/merlin-training] (from April 7, 2022) (https://catalog.ngc.nvidia.com/orgs/nvidia/teams/merlin/containers/merlin-training)


UI Custom Service form and flow

Feature

https://github.com/open-metadata/OpenMetadata/issues/4698

Describe the task
The UI will need to support the handling of the custom services and connectors properties.
Let's wait until the resolution of https://github.com/open-metadata/OpenMetadata/issues/4699 to see how the new services need to be structured.

@nahuelverdugo can you verify if we need anything else to do to support custom service through the UI, if not lets close this

@harshach this one is still open https://github.com/open-metadata/OpenMetadata/issues/7480


[Improvement][Task] Split ExecutionStatus for workflow and task respectively

Search before asking

[X] I had searched in the issues and found no similar feature requirement.

Description

As discussed in https://github.com/apache/dolphinscheduler/pull/11128#discussion_r928928590, currently ExecutionStatus is both for workflow and task.
We need to split it for workflow and task respectively and remove those deprecated states.

https://github.com/apache/dolphinscheduler/blob/011040f90edd3afffd286a98cbef550c2c90d7d7/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/enums/ExecutionStatus.java#L27-L68
Are you willing to submit a PR?

[X] Yes I am willing to submit a PR!

Code of Conduct

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

This issue has been fixed on dev branch.


Monitor the response time of NATS jetstream

Should be a standard quantity to be included in the Grafana dashboard.

Maybe not relevant in the case of the event queue, since the subscribers are passively subscribing not requesting.


500 error accessing dashboard after upgrade to 1.16.1 from 1.15.10, caused by mismatched SQLite colmun type

Gitea Version
1.16.1
Git Version
2.25.1
Operating System
Ubuntu Server 20.04 LTS
How are you running Gitea?
Running as service with binaries (gitea-1.16.1-linux-amd64).
Database
SQLite
Can you reproduce the bug on the Gitea demo site?
No
Description
After upgrading from 1.15.10 to 1.16.1 I get a 500 error when accessing the dashboard.
Other pages seem to work correctly if I open them directly (repos, issues, ...).
(the same issue happened a week ago when I tried to upgrade from 1.15.10 to 1.16.0 but I haven't opened an issue at that time due to lack of time, sorry)
Log contains only this relevant line:
.../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax

Database version before update was 189, after first run it was 209.
I've reverted back to 1.15.10 (both binaries and DB).

Could you paste more logs before this log and after?

And could you paste your database table action's structure?

This is the first log line in gitea.log as of today, repeated more times (I hit F5 morte than once).
Then another appeared, maybe related.
The last ones are my attempt to get back to 1.15.10 before restoring the DB:
2022/02/07 08:22:41 .../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax
2022/02/07 08:22:43 .../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax
2022/02/07 08:22:44 .../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax
2022/02/07 08:22:54 .../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax
2022/02/07 08:23:47 ...s/context/context.go:290:PlainTextBytes() [E] PlainTextBytes: Not found.
2022/02/07 08:23:47 ...s/context/context.go:290:PlainTextBytes() [E] PlainTextBytes: Not found.
2022/02/07 08:25:22 .../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax
2022/02/07 08:25:43 .../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax
2022/02/07 08:26:48 ...ations/migrations.go:414:Migrate() [F] Downgrading database version from '209' to '189' is not supported and may result in loss of data integrity.
        If you really know what you're doing, execute `UPDATE version SET version=189 WHERE id=1;`

This is the required table structure (in v1.15.10 DB):
CREATE TABLE [action] (
    id           INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
    user_id      INTEGER,
    op_type      INTEGER,
    act_user_id  INTEGER,
    repo_id      INTEGER,
    comment_id   INTEGER,
    is_deleted   INTEGER DEFAULT 0 NOT NULL,
    ref_name     TEXT,
    is_private   INTEGER DEFAULT 0 NOT NULL,
    content      TEXT,
    created_unix INTEGER
);
CREATE INDEX IDX_action_user_id ON [action] (user_id);
CREATE INDEX IDX_action_act_user_id ON [action] (act_user_id);
CREATE INDEX IDX_action_repo_id ON [action] (repo_id);
CREATE INDEX IDX_action_comment_id ON [action] (comment_id);
CREATE INDEX IDX_action_is_deleted ON [action] (is_deleted);
CREATE INDEX IDX_action_is_private ON [action] (is_private);
CREATE INDEX IDX_action_created_unix ON [action] (created_unix);

The error is coming from the sqlite DB itself and it indicates that one or more of the rows of action has a string value in one of its integer columns.
How this has happened I do not know. Gitea shouldn't have done this directly.  Have you restored from a cross db backup previously?
The problem is that this cannot be automatically fixed. You're going to have to look at the sqlite DB yourself using sqlitebrowser or sqlite3. My suspicion is going to be that one of the is_ prefixed columns contains "true" and/or "false" when they should be 1 or 0.

Maybe related to

https://github.com/go-gitea/gitea/issues/17748#issuecomment-1032241333

The SQLite database was corrupted

The database worked correctly up to 1.15.10.

Have you restored from a cross db backup previously?

My steps:

stopped 1.15.10
backed up the DB file
downloaded 1.16.1 binary and started it
got 500 error
reverted to 1.15.10 and restored DB
restarted 1.15.10... it still works

My suspicion is going to be that one of the is_ prefixed columns contains "true" and/or "false" when they should be 1 or 0.

This is a good starting point, thanks.
I'll check it out as soon as I can stop the service 👍

There should be no need to stop the service to just read the data - sqlite will not take a lock unless you start writing to the db and once you write the lock will be released.
The issue would come if you start editing data but don't write it immediately as that will lock the table and gitea will hold-up until you release that lock.

I've checked actions table content and none of the is_* columns contain text, only 0 and 1.
The first 5 records in the table (id 118 ->122) have some differences with all the others:

they are the only one with is_private == 0
4 of them they have repo_id == 0
ref_name contains a number instead of branch/tag names like "master", "integration", ...
created_unix column  is empty
next id after these 5 rows is 359 (no IDs from 123 to 358...)

id
user_id
op_type
act_user_id
repo_id
comment_id
is_deleted
ref_name
is_private
content
created_unix

118
2
2
22
0
0
0
0
0

119
0
2
19
0
0
0
0
0

120
2
2
19
0
0
0
0
0

121
9
2
29
0
0
0
2
0

122
7
2
29
17
0
0
0
0

359
4
1
4
11
0
0

1

1549289674

360
1
1
4
11
0
0

1

1549289674

361
4
1
4
12
0
0

1

1549292689

362
1
1
4
12
0
0

1

1549292689

...(other 14000+ similar records, with consecutive IDs)

Could it be that those 5 leading records are causing the issue?

You can try to deleting them after you have a backup.

You can try to deleting them after you have a backup.

It worked: after deleting those 5 records I can now access toe dashboard without errors, thanks!
Just wondering what I've lost by deleting them and if there's a way to fix and re-adding them back...

It seems that empty created_unix causes the bug. SQLite doesn't have data type inside.
Is your database created by a very old Gitea?

These records are not important, they are used used to show user activities, so you can safely delete them

Ok, I've suspected that... can live without them 😉

Is your database created by a very old Gitea?

Yes it is.
I don't remember the version, but it surely was one of the very first ones...

But I think the created_unix should be there when action table created.

But I think the created_unix should be there when action table created.

I think so, but it didn't cause any issue till 1.15.10

But I think the created_unix should be there when action table created.

No, I have reviewed the whole history, it changed a lot.

The previous commit of v1.0.0 doesn't have that field. https://github.com/go-gitea/gitea/blame/0c9a616326ba096a2ff6c058cc96950f68c0fa6e/models/action.go
And I am not sure whether there are something wrong occurs during SQLite migrations since the field type/index all changed a lot. Well, this is just a guess.
@nicorac can you help to check what's the earliest created_unix in your action table?

what's the earliest created_unix in your action table?

It's<PHONE_NUMBER>, which means GMT: Monday 4 February 2019 14:14:34

what's the earliest created_unix in your action table?

It's<PHONE_NUMBER>, which means GMT: Monday 4 February 2019 14:14:34

Well, not that early 😂  I have no idea

should be fixed at #19629 ?

Yes, https://github.com/go-gitea/gitea/pull/19629 will fix the problem of the rows that have a repo_id of 0 but not the one with repo_id = 17 if the root cause of this particular error was indeed the empty created_unix field.

A null created_unix field cannot explain the error
2022/02/07 08:22:41 .../web/feed/profile.go:22:RetrieveFeeds() [E] GetFeeds: Find: converting driver.Value type string to a int64: invalid syntax

Figuring out the root cause would require more information:

A copy of the database to repeat the problem, or
An error message from XORM that provides a copy of the row that caused the error

What is puzzling is that there are no fields in Action that Find would need to convert from string to int64...

should be fixed at #19629 ?

I do not think they are the same problem. The problem in this issue is caused by incorrect database value, it might be caused by incorrect database operations (eg: incorrect upgrade/downgrade, incorrect dump/restore, etc) or unknown bugs.
There is a chance that #19629 could bypass the problem, but the data in SQLite database is still incorrect.
I would propose to close this issue since there is no clue.
And this problem can be bypass manually by truncating action table or deleting the incorrect data in action table.

Other databases won't be affected by this problem either.

Why ?

Log: converting driver.Value type string to a int64: invalid syntax

SQLite doesn't have data type inside.

I do not understand this sentence. How does that explain why other databases won't be affected by this problem?

SQLite doesn't have data type inside.

I do not understand this sentence. How does that explain why other databases won't be affected by this problem?

In SQLite, you can have created_unix='' or created_unix=null or created_unix=123
In others, you can only have  created_unix=null or created_unix=123
created_unix='' will cause the converting driver.Value type string to a int64 error, others won't.

I did not know that, thanks for explaining. Since this problem can still exist for Gitea instances that run on SQLite, I propose this issue is left open.

With these improvements to gitea doctor that replace created_unix='' with created_unix=0 address this issue.


The Polygon which coordinates cross 180,-180 display bug in Scene2D

Thanks for the report @Lemonjay! Would you be able to include a Sandcastle example that replicates the issue? That would help us give us more context to determine the issue. Thanks!

I'm closing this issue due to inactivity. If you believe this is still an issue, please feel free to re-open. Thanks!


phone:mobile=* and similar contact fields shouldn't suggest data

If I add the key phone:mobile then ID will suggest data for the value. It shouldn't suggest any data, as that data is invariably going to be the contact details for someone else. This is is common to all or nearly all contact fields, e.g. contact:twitter and contact:linkedin, but not contact:email.
Certain contact keys do not suggest data, e.g. website=, email=, phone=, fax= and this needs to be replicated.

See #7485 for discussion about this functionality. You're right that iD shouldn't show suggestions, but the problem is there are hundreds of tags like this and it's unrealistic to manually exclude them all. We could theoretically do this intelligently if iD knew the data type for each tag, but for now I'm closing this as low impact. If you care a lot about it then feel free to open a PR.


Add file name to chunk metadata and insert into formatted chunk

Context
REDBOX-433. The filename is important metadata for the LLM. This stores it in Elastic then inserts it into the formatted retrieved document.
Changes proposed in this pull request

Change document loader to add a new metadata field: file_name
Change document formatter to pick up file_name if it exists

Guidance to review

Confirm you're happy with my changes to the document XML formatting

Things to check

[x] I have added any new ENV vars in all deployed environments
[x] I have tested any code added or changed
[ ] I have run integration tests

LGTM


Fix Style/RedundantParentheses with hash literal as first argument to super

Currently, Style/RedundantParentheses finds an offense in super ({foo: bar}) but not baz ({foo: bar})because its internal hash_literal_as_first_arg doesn't account for super invocations. This PR adds support for the super case.
I couldn't figure out how to write one def_node_matcher that covers both cases -- not sure if that's preferable.

Thanks!


Update to v2

xref https://github.com/conda-forge/menuinst-feedstock/pull/23
xref https://github.com/conda/menuinst/releases/tag/2.0.0

I'll close and re-open to see if we can get the CI to be triggered.

Umm nope, it didn't work. I guess our system prevents people outside Anaconda from trigerring our CI. I'll check with our people to see what could be done.

Alright, so CI can't run if the source is coming from a fork. @jaimergp I'll create a branch and will cherry-pick you commits and I'lll create a new PR.

Thanks @JeanChristopheMorinPerso!

See https://github.com/AnacondaRecipes/menuinst-feedstock/pull/7


Add 101-jenkins quickstart template script & 'Jenkins On Azure' landing page

The scripts will be used in the new 'secured Jenkins' quickstart templates.
Here's a sample: http://clguiman-test-secure2-jenkins9.eastus.cloudapp.azure.com/

@clguimanMSFT,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@EricJizbaMSFT I fixed all the reported issues


Not pulling from postman API

When I use
        with:
          apiKey: ${{ secrets.POSTMAN_API_KEY }}
          collection: ${{ secrets.POSTMAN_COLLECTION_URL }}
          reporters: cli

This doesn't work. It succeeds but I know it's not finding the collection.
I generate the collection url and api key when I hit share in postman

But I have no luck. It isn't finding the collection.

I figured it out:
Created a PR to fix this here: https://github.com/anthonyvscode/newman-action/pull/432


File Input stream not closed, Too Many Open Files exception

When using a file based configuration, our server runs into Too Many Open Files exceptions after some while.
It seems when opening a config file, it is not properly closed:
try (InputStream input = new FileInputStream(path.toFile())) {
        PropertiesProvider provider = propertiesProviderSelector.getProvider(path.getFileName().toString());
        properties.putAll(provider.getProperties(input));
} catch (IOException e) {
      throw new IllegalStateException("Unable to load properties from file: " + path, e);
}

Taken from org.cfg4j.source.files.FilesConfigurationSource, Lines 124 ff
Presumably, function close() should be called on the input stream in a finally clause.

@gerdriesselmann please have a look to the try-with-resources statement. There is no need to explicitly call the close method.
I guess your problem lies somewhere else in your code.

True.
I stepped into the issue again, and the problem seems to be, that too many threads are reading properties at the same time. The the file is loaded on each access. Which in turns opens the config file too many times at the same time.
I noticed that version 4.4.0

does not create cached configuration sources when invoking ConfigurationProviderBuilder.build(), unlike the current Git branch, but uses SimpleConfigurationProvider
That SimpleConfigurationProvider just passes requests to its sources
That FilesConfigurationSource always loads the file whenever a property is accessed

Is this correct?


set cookie expire to match token exire date

Expire date now matches the expire date from the BEARER token.

This would prevent an expired JWT token to be used in case the session has not been closed (expire=0 means the cookie is valid for this session)

Resets cookie on logout. Invoke now sends an empty cookie as header, which overrides and removes the previous cookie.


chown: fix #2060

fixes #2060

https://github.com/uutils/coreutils/pull/2063/checks?check_run_id=2311940222 is failing

https://github.com/uutils/coreutils/pull/2063/checks?check_run_id=2311940222 is failing

The failed tests don't seem to be related to the change in this PR.
For me it looks like whoami is doing some shenanigans on the testing environment (whoami doesn't return anything and chown is called with : only).
This is tricky to debug since I don't have failed tests on my development boxes.
The tests in chown need to be refactored anyway.
I will do that and write some more exception handling for the whoami calls and see if it changes anything.

I retriggered the tests

This needs #2068 to get merged first.

Should be okay now.
(there's a failed check on macos not coming from test_chown but from test_cp::test_cp_parents, which looks random to me)


Refactor AvroIterable and prevent multiple iterators

@vrozov, can you give more context for why you'd like to make this change? It is strange to make an Iterable produce only one iterator and I don't think there is a compelling reason to do this.

The change helps to avoid resource shortage: as it is AvroIterable that is Closeable, not the Iterator, resources acquired in the iterator() won't be closed until AvroIterable itself is closed and in the case where multiple iterators are obtained from the same AvroIterable, there will be multiple file handles/sockets/http connections acquired and pointing to the same file/bucket that can't be closed individually.
Note that org.apache.avro.file.DataFileReader follows a similar pattern. It is not possible to get multiple iterators from DataFileReader as it always returns the same iterator instance (itself).
@rdblue What will be a use case were support for multiple iterators is necessary and that can't be covered by creating multiple AvroIterable instead?

@vrozov, all of the iterators are closeable, so we should be able to close them as they are consumed to free up resources. We do this in ParallelIterable for example.
The close method in CloseableIterable is really intended to be a second catch-all to avoid eventual leaks. Plus, it allows using try-with-resources:
try (AvroIterable records = Avro.read(...).build()) {
  for (R record : records) {
    ...
  }
}

@rdblue While iterators returned by AvroIterable.iterator() are Closeable, they are unlikely to be closed. In your example, it is AvroIterable.close() that closes Iterator that java implicitly obtains from records and there is no implicit or explicit call to the iterator close() method. In ParallelIterable it is Iterable that is closed (assuming that you refer to https://github.com/vrozov/incubator-iceberg/blob/master/core/src/main/java/org/apache/iceberg/util/ParallelIterable.java#L63) not the iterator.
The PR prevents constructs like
try (AvroIterable records = Avro.read(...).build()) {
  for (R record : records) {
    ...
  }

  for (R record : records) { // <-not allowed
    ...
  }
}

@vrozov, I see what this PR does, but I don't think there is a good argument for making Iterables that can only be used once. Both iterables and iterators can be closed. If you'd prefer to close iterators earlier, then I think the right thing to do is to update code to do that, instead of disallowing reasonable uses.

@rdblue What is the use case where AvroIterable needs to be used more than once? At minimum, there is no such use case in the current code base as the PR does not break existing functionality. Code to close iterators earlier will be extremely verbose and I would prefer not to do that.

The expectation is that iterables can be used more than once. Breaking that expectation should be worth the trade-off and I don't see a reason to change it. All of these can be closed. If you want an iterator closed earlier, then we can close it earlier.

There is no such assumption in org.apache.avro.file.DataFileReader and there is no such expectation in the existing iceberg code. Ability to get multiple iterators from AvroIterable will promote code that leads to performance degradation: AvroIterable iterator does file I/O, iterating twice like in the code example I provided, should be discouraged in favor of a single iteration over avro file.

@rdblue Does the above provide sufficient justification not to support multiple iterators? If not, can you please provide a use case where such support is necessary so that I can take it into account and modify the code accordingly?

@vrozov, no. There is no way for Iterable to signal that it only supports a single iteration, and most iterables, like lists, maps, sets, etc. support iteration multiple times. I think that there isn't much point to using Iterable if you only intend to support a single iteration.
The goal you want to accomplish should be done by making sure that resources are closed. I think a better approach would be for an iterator to close underlying resources automatically when it is exhausted.

@rdblue Please take a look at DataFileReader. It supports only one iterator. AvroIterable also supports only one iterator and it provides the signal (IllegalStateException) should somebody try to obtain a second iterator.
There is a difference between iterating over in-memory collection and iterating over records in file: list, map, set iterators do not need to be closed and there is no significant cost associated with the second iteration over in-memory collection.
The point of using Iterable even with a single iterator() is

support "for-each loop"
ability to pass Iterable and delay creating Iterator instance.

I considered the approach to close an iterator when hasNext() returns false, but rejected it as it is not necessary to exhaust iteration (even though most of the time iterator will be fully exhausted), so the resource may be left open.
The PR does not fix any existing resource leakage or shortage. The goal of the PR is to prevent such a possibility for future modifications.

I get the argument, and I know that Avro has a class that does something else. I'm saying that Iceberg should not violate a common assumption about iterables without a good reason. I don't think your justification for this is sufficient. If you want to clean up resources more quickly, then the right way to do that is in the iterators returned by these iterables.

iceberg already violated the common assumption about iterator in two ways

Iterator returned by the iterator() is "usually" (personally I don't know any) not Closeable as the primary use case for the iterator() method is "for-each loop", so making Iterator Closeable is not well justified as close() is not called in "for-each loop".
AvroReuseIterator always returns the same reference in the next() method. This is a much bigger "violation" of a common Iterator behavior.

I do not say that I see a problem with those "violations" and I see why those approaches were taken, but I don't see why it is a problem to introduce Iterable that allows only single iterator() in favor of less error-prone code and that does not break any existing assumptions in iceberg.

@vrozov: there are better ways to solve the problem. Closing resources in iterators is the right solution to this.

@rdblue It is not a better way to solve the problem and I already explained why it does not work for "for each loop". All I see is multiple excuses to reject a contribution that goes from violating common assumptions (while other violations are somehow acceptable) to a "better way" that leads to resource leak or more error-prone code.

To be clear, the problem is that this violates common assumptions that people make about iterables. In Iceberg, we make sure that iterables can be used multiple times and, although we don't use this internally, these are part of the guarantees in the public API.
I also disagree with the justification for making this change. Preventing resource leaks is not a valid justification because there are better ways to handle it, like closing when an iterator has been consumed.
I'm sorry to hear that this sounds like multiple excuses to you, but I think my position has been consistent. This would make iterables have surprising behavior, which is bad, and doesn't have a strong enough justification for that surprising behavior because I don't think it would be more effective than other changes.

It goes in a circle. I already explained that people make other assumptions about Iterables and Iterators, but somehow it is OK to violate those assumptions and not OK to violate another assumption. Would you agree that iterating twice over the same file should be discouraged in favor of single iteration and when necessary it is possible to get Iterable without any extra performance penalty? In rear cases where multiple iterators are necessary, it is easy to go from
try (AvroIterable records = Avro.read(...).build()) {
  for (R record : records) {
    ...
  }

  for (R record : records) {
    ...
  }
}

to
try (AvroIterable records = Avro.read(...).build()) {
  for (R record : records) {
    ...
  }
}
...
try (AvroIterable records = Avro.read(...).build()) {
  for (R record : records) {
    ...
  }
}

There is also no better way. Iterators are not expected to be Closeable and it is very easy to lose Closeable iterator during Iterable transformation. For example CloseableIterable.transform() does not preserve Closeable and guava Iterables won't preserve it either.
The only consistency I see in this PR review is the denial of the contribution. Arguments provided changed from "not a common violation" to "there is a better way". When I asked for a "use case" it was never provided.
In any case, the PR is closed and there is no need to discuss this further. I am working away from contribution to the iceberg project.

I think you misunderstand my suggestion. What I see as a better solution is removing the need to explicitly close Iterator if the iterator exhausts its contents. That would alleviate the need to call close on the iterator in most cases.

I already explained that people make other assumptions about Iterables and Iterators, but somehow it is OK to violate those assumptions and not OK to violate another assumption

Your examples were Iterator not typically implementing Closeable, and reusing objects in AvroReuseIterator:

It is true that Iterator typically doesn't implement Closeable and it is hard to keep track of when using for loops. That's why CloseableIterable can be used in a try-with-resources block. I'm also suggesting that iterators should close underlying resources when finished iterating, which would solve this problem earlier if iterator is completely consumed.
AvroReuseIterator is not used by default. Callers must use reuseContainers on the read builder, which I think is a reasonable compromise. The performance benefit and the steps we've taken to guard against accidental use justifies this change.

Would you agree that iterating twice over the same file should be discouraged in favor of single iteration and when necessary it is possible to get Iterable without any extra performance penalty?

I agree that what you're suggesting would work. But it would be surprising behavior. Callers would not know until runtime that they should have used the second of your two code blocks instead of the first. The choice here is to avoid throwing exceptions at runtime when we can leave resources around for a bit longer and avoid a failure.

@rdblue Closing resource in an iterator when it (iterator is exhausted) is not a "better" solution, it is the mitigation for the resource leak/shortage issue. Otherwise, it would not be necessary for AvroIterable to be CloseableIterable. Such a solution still leaves a possibility for the resource shortage. It will also require a wrapper around DataFileReader as DataFileReader iterator does not close the underlying resource when it is exhausted. So, all CloseableIterable still needs to be closed either explicitly or by using a try-with-resources block. Note that a common expectation for Closeable or AutoCloseable used in a try-with-resources block is that there is a single resource acquired and that is guaranteed to be closed, not multiple resources as in the case of CloseableGroup. Additionally CloseableGroup' is a subject of resource leak: should close()for one ofCloseableincloseablesthrow anIOExceptionorRuntimeException, all other Closeable` won't be closed.
It sounds that the major disagreement is whether it is OK to keep resources a little bit longer and allow multiple iterators or it is better to ensure that a resource is released asap but fail multiple iterators request. After spending a significant effort in isolating iceberg resource leak/shortage in our environment, my strong preference and recommendation is to fail fast when resources are not released asap. In an environment where resources are limited, the first behavior will still lead to run-time errors.

Would closing when an iterator is exhausted solve the resource leak you found? What caused the leak you found? Having more concrete information about your situation might help.

It was the same leak as was fixed in #150. It is not a question of how to fix an existing leak. The PR goal is to prevent introducing such leaks in the future and helping with troubleshooting resource leak/shortage conditions.

Okay, do you agree that closing the iterator when exhausted would help avoid such leaks in the future?

It will mitigate such possibility, but will not eliminate it completely. For example, it won't work in case of
try (AvroIterable records = Avro.read(...).build()) {
  for (R record1 : records) {
    for (R record2 : records) {
      ...
    }
  }

I think in the first case, closing the iterator would do the right thing. Each inner iterator would be closed before the next one is opened, and the outer iterator must be left open the entire time because it is still in use.
For the second case, I agree that the first iterator's resources are left open until the end of the try-with-resources, block. I think that's worth the trade-off, and callers can always close the iterator if they choose. So I'm okay with the trade-off of using the automatic close at the end of the try block.

I already explained and, I believe, you already agreed that callers

won't be able to choose to close an iterator as Closeable may be easily lost
the code by itself does not imply that there are multiple resources acquired. When a person reads such code, she likely will assume that there is a single resource acquired in try-with-resources.

Both cases are examples where logic needs to be modified. For example instead of
try (AvroIterable records = Avro.read(...).build()) {
  for (R record : records) {
    ...
    break;
  }
  for (R record : records) {
    ...
  }
}

it should be written as
List<Record> recordsCached = new ArrayList<>();
try (AvroIterable records = Avro.read(...).build()) {
  Iterators.addAll(recordsCached, records.iterator());
}

for (R record : recordsCached) {
  ...
  break;
}
for (R record : recordsCached) {
  ...
}

ability to get multiple iterators promotes less code in favor of less performant code. It should be explicit for callers that they are iterating over a file resource that needs to be closed and that iterating twice over a file is much slower compared to an in-memory collection. Making it implicit will lead to resource leaks and a significant effort in isolating such resource leaks. It is a trade-off for allowing only one active (note additional changes) iterator.

won't be able to choose to close an iterator as Closeable may be easily lost

If the caller cares about the case where a partially consumed iterator is left open until the end of the try block, then they can close it and use a while loop instead of the for loop. It is easy to do this, but most people won't care.
Your rewrite assumes that it is reasonable to keep records in memory. That's not necessarily the case.

I see no point to discuss this further.


[MSVC][permissive-] xgboost failed to build with error C3779 under /permissive- on MSVC

Hi all,
The MSVC team recently added xgboost  as part of RWC testing. But we found error C3779 when building xgboost with '/permissive-'. Could you please take a quick look at the issue? Thanks in advance.
Repro steps:

git clone https://github.com/dmlc/xgboost.git F:\gitP\dmlc\xgboost
cd F:\gitP\dmlc\xgboost & mkdir build_amd64
set CL= /permissive-
cd F:\gitP\dmlc\xgboost\build_amd64
cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_SYSTEM_VERSION=10.0.18362.0 -DGOOGLE_TEST=ON -DUSE_DMLC_GTEST=ON ..
msbuild /m /p:Platform=x64 /p:Configuration=Release xgboost.sln /t:Rebuild

Expected Behavior
xgboost  could build successfully with '/permissive-'.
Actual Behavior
build.log
test_regen.cc
F:\gitP\dmlc\xgboost\tests\cpp\tree\test_regen.cc(69,30): error C3779: 'xgboost::anonymous-namespace'::RegenTest::Iter': a function that returns 'auto' cannot be used before it is defined F:\gitP\dmlc\xgboost\tests\cpp\tree\test_regen.cc(60): note: see declaration of 'xgboost::anonymous-namespace'::RegenTest::Iter'
F:\gitP\dmlc\xgboost\tests\cpp\tree\test_regen.cc(63,10): note: This diagnostic occurred in the compiler generated function 'size_t xgboost::`anonymous-namespace'::RegenTest::TestTreeMethod(std::string,std::string,bool) const'

I don't currently have a windows machine at hand, would you like to submit a fix?

@trivialfis No. We recently failed to build xgboost with '/permissive-'. So, I reported a new issue here.

@xiaoxiao-Xu Other C++ compilers, such as GCC, is able to compile XGBoost successfully. So we will not prioritize this issue for a while. Thanks for your understanding.

Ok. Thanks for your first look.
If the issue was fixed, please let me know.

New errors detected under /permissive- and /std:c++latest.
F:\gitP\dmlc\xgboost\tests\cpp\data\test_sparse_page_dmatrix.cc(93,75): error C2039: 'value': is not a member of 'std' [F:\gitP\dmlc\xgboost\build_amd64\testxgboost.vcxproj]
F:\gitP\dmlc\xgboost\tests\cpp\tree\test_regen.cc(69,30): error C3779: 'xgboost::anonymous-namespace'::RegenTest::Iter': a function that returns 'auto' cannot be used before it is defined [F:\gitP\dmlc\xgboost\build_amd64\testxgboost.vcxproj]


What is fiznum1?

Hey, it's the maintainer of pooltool, here. This sounds really interesting! It reminds me of the Computational Pool Olympiad that Stanford hosted until ~2010.
Any info on what this is all about? I'd love to get involved!

Hi, thanks for reaching out. 'fiznum1' is the Hungarian shorthand for Numerical Methods in Physics I, which is a course for first year physicist at Eötvös Loránd University. In this course we teach the basics of python and numerical computation using numpy, scipy and etc. We are trying to organize an AI pool contest to provide an interesting challenge for the students. The plan is that at the end of this semester we go to a bar and have the contenders' AI scripts play against each other to determine the winner.
I plan to make some customized rule sets for 8-ball or 9-ball (we haven't fixed the rules yet). For this I created a fork of pooltool. Currently the server and viewer clients are functioning, however, they probably could be improved. Any input on this would be greatly appreciated.

Hi @karacsm. Thanks for your explanation. I think that's a fantastic project and I hope it works out. If you need help along the way, please let me know. And of course, I hope you upload all the bots so I can see the tournament for myself!
If you end up making custom rulesets, I hope you consider merging them into the main pooltool project. I have seen the quality of your code in this pool server repo, and I am impressed!


[Performance] Optimise navigation

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

What performance issue do we need to solve?
Startup time can be improved
What is the impact of this on end-users?
app boot time
List any benchmarks that show the severity of the issue
boot time in debug mode improved by ~400ms after optimising AppNavigator so expect much better results once every navigators/conditionally rendered screens are optimised in a similar way
Proposed solution (if any)
before

after

What it does is basically postponing evolution of the js code until it's actually used.
Of course you need to get rid of corresponding  imports from the beginning of the file.
How to do it in navigators?
<Stack.Screen
  name="Profile"
  getComponent={() => require('./ProfileScreen').default}
/>

instead of
<Stack.Screen
  name="Profile"
  component={ProfileScreen}
/>

Such solution should be used for every navigator/screens the app has.
List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)
Note: These should be the same as the benchmarks collected before any changes.
Platform:
Where is this issue occurring?

[ ] Web
[x] iOS
[x] Android
[ ] Desktop App
[ ] Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
View all open jobs on Upwork

Agree that we can, but I don't know if our bundle size is big enough to be problematic yet. Last I checked it was about 6MB

I can hop on the review for this when a PR is up.

Looks like we've got a PR up with a healthy number of reviews: https://github.com/Expensify/App/pull/10986

@Szymon20000 are we all done here and can close this issue out?

yeh, seems like this is on production now.


Improved WaveDrom header/footer handling, added config handler

Previous method did not have full compatibility with WaveDrom standard.
All header or footer arguments were passed as an an object in the "text" field.
See:
http://wavedrom.com/tutorial.html
That would work fine in (most of) the last example of Step 8, for example, but would be incomplete in the immediately preceding example, where there are arguments outside "text"
This method allows more control on the user side, e.g.:

log.info(waves.dumpj(header = {'text':'WaveDrom example', 'tick':0}))
waves.write('wavedrom.json', header = {'tick':0}, config = {'hscale':3})

Worth noting this will break backwards compatibility if folks are using header/footer kwargs.  I'm happy with that, just thought I'd give a chance for anybody to object if they're using it.
We have an open action item to improve versions / releases, so when we merge this we should uprev a version number to indicate a backwards incompatible change.

Yeah, I was thinking about this more, too.
Having the "skin" parameter as a default isn't bad functionality at all - A good portion of projects will only need to use that option.
This commit checks to see if a dictionary was passed, and only uses the newer method is so. Otherwise, it passes the string to the "skin" field. Backwards compatibility is thus conserved.

Added the example kwargs to the repo's test docs, should allow anyone to easily see how to use this feature.
Worth noting that backwards compatibility will not be broken (falls back on previous functionality).
Is there anything that needs to be resolved to merge this PR?

Merging now to avoid bitrot even though comments were not addressed, can be improved in the future.


Add documentation to download via curl or wget

To avoid going via the webbrowser, which could be unreliable.

fixed: https://github.com/spine-generic/data-multi-subject#download-zip-package-recommended


resolved hotkey +S conflict on WASD mode

Shift + S in WASD mode is supposed to jump the cursor to the bottom of the screen, but due to a hotkey conflict it instead enters Quick Insert. This PR moves +S:: DoubleClickInsert() into the #If (NORMAL_MODE && WASD == false) statement.

looks right to me


Device Agent UI

closes #83
Description
UPDATED 2023-06-02 for renaming of webmin to UI
Structure

Adds subfolder ./frontend

Adds frontend/server.js - the web server (uses node:http)
Adds frontend/routes.js - the routeing logic
Adds frontend/home.html - the single HTML page served by the webserver at /home
Adds frontend/assets/

favicon.ico
favicon-16x16.png
favicon-32x32.png

Adds subfolder ./test/unit/frontend

Adds routes.spec.js
Adds server.spec.js

That accounts for 8 of the "Files Changed" in this PR
Apart from them, out of the other 10 "Files Changed":

3 are for linting and test fixes
1 is the read me
1 is the package (add scripts for test running)
the remaining 5 are for integration and/or facilitating testing. please see "Context" below

Features:

Supports following command line options:

Web UI Options

  -w, --ui            Start the Web UI Server (optional, does not run by default)
  --ui-host string    Web UI server host. Default: (<IP_ADDRESS>) (listen on all interfaces)
  --ui-port number    Web UI server port. Default: 1879
  --ui-user string    Web UI username. Required if --ui is specified
  --ui-pass string    Web UI password. Required if --ui is specified
  --ui-runtime mins   Time the Web UI server is permitted to run. Default: 10

Displays basic status info (5s ajax refresh)
Displays basic config info (5s ajax refresh)
Basic Auth enforced (will not run without username/password)
No default user/pass set (forces user to chose own username/password)
Supports file drop, file browser or text entry for entering config
Accepts a dedicated device config (for one-offs) or a provisioning config (for larger roll outs)
Validates the entered config using the same routines as the agent does to load a device config
Displays a "Web Server Offline" status bar after encountering 3 timeouts (rough n ready MVP)
The FlowForge header logo gets its href updated when a device config is applied
meaning the user can click it navigate quickly to the (correct) associated forge platform

Endpoints (all new):

GET / - ensures login or returns 401 + REALM
GET /home - loads an retuns the home.html page. redirects to / if not authenticated (protected)
GET /assets/* - loads the requested assets. NOTE: only image/pdf/test/json permitted (active files like .js are inhibited)
GET /status - returns the agent status and some other info to help the UI user understand what is going on. (protected)
POST /submit - accepts JSON { "config": "the yaml config" } (protected)

TESTS:
Device Agent Web Server Routes (API)
✔ all endpoints (except assets) return 401 if not authorized
✔ /home returns html content
✔ /assets returns content without auth
✔ /status returns expected items
✔ /submit with bad yaml results in 400
✔ /submit with good yaml writes file
Device Agent Web Server
✔ by default, Web UI is not enabled
✔ quits if config is bad AND UI is not enabled
✔ quits if config is missing AND UI not enabled
✔ fails to run web server if user or pass is not specified
✔ starts web server if a user and pass are specified
✔ omitted ui CLI options have correct defaults
✔ ui CLI options are set correctly
✔ ui CLI rejects invalid ui-runtime value
✔ server auto closes after runtime expires
Context

In order to unit test the CLI parts, the main entrypoint (index.js) needed to
become an importable object (only when Env Var NODE_ENV === 'test').
In order to verify config entered by user, the config.js config function had to be broken up
into separate functions so that a device config could be verified as a string (i.e. avoid saving and
loading temp files)

Known issues / gotchas / areas for improvement:

No real logout (logout is not part of basic auth spec) (user needs to close browser)
If there is no device file, the agent shuts down after webmin ends. This is because there
is no webserver or active component to keep the message loop running. This is no different to
how the agent acted prior to the webmin being added (user would get an error about missing device
file), now though, with the web min, the user has a chance to set the device config.
Only http is supported in this initial offering
Uses browser alert() for user feedback (not ideal)

A better solution would be to use a modal dialog.
This one
looks good, is very lightweight and no dependencies (could simply copy the dist files
into the assets folder and reference them from the html)

The webmin runtime is NOT turned off immediately when a new config is applied. This permits
the user to monitor the agent status for a short period after applying a config. Also keeping
the webmin running after a config is applied means that the user can rectify any mistakes in
the config without having to restart the agent.

Related Issue(s)
#83
Checklist

[x] I have read the contribution guidelines
[x] Suitable unit/system level tests have been added and they pass
[x] Documentation has been updated

[x] built in readme
[ ] Upgrade instructions
[ ] Configuration details
[ ] Concepts

[ ] Changes flowforge.yml?

[ ] Issue/PR raised on flowforge/helm to update ConfigMap Template
[ ] Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

[ ] Backport needed? -> add the backport label
[ ] Includes a DB migration? -> add the area:migration label

Given webmin is a real thing that provides browser based configuration for systems, then I'm hesitant to use webmin as our cli option given this isn't webmin.
Could we just use ui?
As a separate thought, we could support a second config file (eg config.yml) that can contain these options along with any others the platform doesn't otherwise provide it is device.yml - that would save the user from having to merge changes when getting a new device.yml. When loading we'd load config.yml first (if it exists), then apply device.yml (if it exists) over the top.
I'll raise a separate issue for that.

Could we just use ui?

no bother. with fix up. should be relatively quick thing (~20 mins to catch em all)

Updated with CLI flag naming suggestion.
Ready for review 🤞

Have run through a couple times, all appears working for me.
One comment... the logging has two lines saying much the same thing:
[AGENT] 6 Jun 2023, 18:53:14 info: Web UI listening on port 1879
[AGENT] 6 Jun 2023, 18:53:14 info: Web UI started on port 1879

Would be nice to have a clickable URL as well.

Have run through a couple times, all appears working for me.
One comment... the logging has two lines saying much the same thing:
[AGENT] 6 Jun 2023, 18:53:14 info: Web UI listening on port 1879
[AGENT] 6 Jun 2023, 18:53:14 info: Web UI started on port 1879

Would be nice to have a clickable URL as well.

I'll make that logging singular.
What would you use for the host part of the URL Nick? localhost?

Updated console entry for Web UI started in c9c2a7b

Command line
Log Entry

node index.js --ui --ui-user user --ui-pass pass
Web UI Server now listening at http://<IP_ADDRESS>:1879

node index.js --ui --ui-user user --ui-pass pass --ui-host ::
Web UI Server now listening at http://[::1]:1879

node index.js --ui --ui-user user --ui-pass pass --ui-host <IP_ADDRESS>
Web UI Server now listening at http://<IP_ADDRESS>:1879

node index.js --ui --ui-user user --ui-pass pass --ui-host ::1 --ui-port 54321
Web UI Server now listening at http://[::1]:54321

@knolleary ready for (re)review please.


internal_stats: astnode (sizeof)

What are the reasons/motivation for this change?
Requested by @povik. We'd like to see how much of the memory usage of Yosys between passes is directly due to AstNodes. This doesn't count overhead of initialized vectors and such.
Explain how this is achieved.
Adding a counter adjusted on AstNode construction/destruction
If applicable, please suggest to reviewers how they can test the change.
yosys -p "read_verilog tests/sim/dffe.v; internal_stats -json"

This should have used the scratchpad...

Wouldn’t that be inefficient accessing the scratchpad with every construction/destruction?

I'm not proposing astnodes is a scratchpad variable, I'm proposing that astnodes_count() is a scratchpad variable.

But scratchpad is a static key-value store. We would need to periodically update it, or extend what scratchpad is.
In any case we can go back and redo this.


Using instance props with new lifecycle methods

This question is similar to https://github.com/reactjs/reactjs.org/issues/828 but the suggested answer by @gaearon isn't feasible for me.
I currently use a class field to store an instance of TreeManager (which is another class that helps manage various forms and functions of the tree):
https://github.com/dowjones/react-dropdown-tree-select/blob/423a5e47fa644c460c3c36dcf85addd333a1fd44/src/index.js#L70-L78
I don't use state to store this instance because:

This is not something that is rendered
I do not want state to update/change if I change the reference to this field (e.g., even if you do a new TreeManager but the underlying data is same, it should not cause a re-render)
If I store it in state, I'd have to do a deep compare to prevent unwanted renders and that's not desirable since deep compares can get expensive for large trees.

I could store it in props but that's a leaky encapsulation. Callers have no business seeing this property or having access to it.
However, with the new lifecycle methods gDSFP and CDU, I don't see a clean and easy pattern to get around this. It is forcing me to store the treeManager in state or props, neither of which is a desirable or the proper place to store it:
  static getDerivedStateFromProps({ data, mode, showDropdown, showPartiallySelected, searchPredicate }, prevState) {
    const treeManager = new TreeManager({
      data,
      mode,
      showPartiallySelected,
      rootPrefixId: prevState.clientId,
      searchPredicate,
    })
    // Restore focus-state
    const currentFocusNode = prevState.currentFocus && treeManager.getNodeById(prevState.currentFocus)
    if (currentFocusNode) {
      currentFocusNode._focused = true
    }

    return {
      showDropdown: /initial|always/.test(showDropdown) || prevState.showDropdown === true,
      ...treeManager.getTreeAndTags(),
      treeManager,
    }
  }

  componentDidUpdate(prevProps, prevState) {
    if (this.state.treeManager) this.treeManager = this.state.treeManager
  }

Seems like I need to break down TreeManager to individual functions that require all instance props of TreeManager be passed as arguments.
So is there a pattern/practice here that let's me keep using the TreeManager class with the new lifecycle methods?

Ok, the treeManager is a private data object that's not React state and that's perfectly fine. What I think should happen is that you read data prop only initially and build the treeManager out of it.
const [treeManager] = React.useState(createTreeManager(props.data))

Don't get alarmed by useState here. It's a small trick on how to keep expensive objects around without recreating it on every render. But since you it won't utilize setter, it won't cause rerender. The alternative way would be with useRef and emulate laziness.
In case someone wants to use new data, it's simple as passing different key prop to the component which forces React to remount the component. There is nothing you need to do for that.
const treeManager = React.useRef()
// use this function whenever you need to something based on treeManager
const getTreeManager = () => {
  if (!treeManager.current) {
    treeManager.current = createTreeManager(props.data)
  }
  return treeManager.current
}

Personally, I choose the first option as it's less verbose, but the second one is officially recommended and more under the control.

The next step is to decide what prop changes should cause some update to existing treeManager and that's where useEffect comes in.
React.useEffect(() => {
  updateTreeManager(props.mode)
}, [props.mode])

What's a little caveat here that useEffect will also run first time after mount, not only with the change. For that, the second approach with useRef is also better and you can use getTreeManager defined above to avoid recreating multiple times.
Similarly with an update based on focus. It's generally better to have several useEffects focused on the particular change.
const [currentFocus, setFocus] = React.useState()
React.useEffect(() => {
    const currentFocusNode = getTreeManager().getNodeById(currentFocus)
    if (currentFocusNode) {
      currentFocusNode._focused = true
    }
}, [currentFocus])

The problem with both approaches is that they fail when dealing with multiple instances of the component on the same page.

Uh, I don't follow. Refs are isolated in the component, how could they be a problem?

Uh, I don't follow. Refs are isolated in the component, how could they be a problem?

But the imported module is the same instance, so you're not creating multiple unique refs.

I'm of course, assuming this is a module and not wrapped in some method:
const treeManager = React.useRef()
// use this function whenever you need to something based on treeManager
const getTreeManager = () => {
  if (!treeManager.current) {
    treeManager.current = createTreeManager(props.data)
  }
  return treeManager.current
}

Maybe I'm missing something...

You cannot call useRef just anywhere, only in a component or custom hook. There is no module wide ref :)

@FredyC ah, you're right. I forgot hooks can only be called inside a function component body. This seems like a feasible path, just gotta watch out for the gotchas (avoid mutating it in render phase). Gonna give it a try. Thanks!

Closing since this seems resolved

@threepointone can we reopen this? We never found a clear path towards this.

@mrchief You should probably elaborate first what is the problem then. From your last comment, it seemed was merely your misunderstanding of the concept.

Questions like these probably belong on stack overflow, they’re not directly related to enhancing the docs.

@FredyC It's more than that. I'll get back to you on that.
@threepointone from docs perspective, I think it'd be valuable to have something like https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-getderivedstatefromprops for instance props as I think that's a key element to migrate away from classes, but it could be a blogpost as well. The former is preferable for obvious reasons but I see your perspective as well.

@FredyC Thanks for your edit but it's alright, you can say that and I've no issues admitting that I find adopting these class methods hooks super confusing. A lot of time it just feels like shoehorning - something that is so elegantly expressed in Classes becomes so convoluted with hooks. And if I'm having these doubts, I'm sure there are others which is why a clear direction from the core team will help.


cleanup

rework attributes (distinguish nodes with same value on all nodes verse node-specific ones)
Use standard java recipe instead of jenkins::java (here's when I realized I needed to do the above rework)
working Vagrantfile
factor out jenkins init for master
fix default['repos']['private']['pr-snap']
clarify readme

this is all laying the groundwork for moving to a new instance for scala-ci that also hosts artifactory (+ EBS storage for repo & jenkins jobs)

post-factum review by @lrytz  -- going to push this into prod and make sure it doesn't break anything (mostly a refactor)

lgtm - in the sense, didn't spot anything obvious. it's good to have me review changes, helps getting used to the codebase :)


boards: silabs: removed FLASH_BASE_ADDRESS

Removed FLASH_BASE_ADDRESS configuration from various boards' Kconfig. The only thing needed in order to do this was to update the relevant dtsi files so that the flash0 node has its reg property configured properly.

I notice efr32mg24b020f1536im40.dtsi is not updated. I believe this PR should also take care of sparkfun_thing_plus_matter_mgm240p (which use efr32mg24b020f1536im40.dtsi).

you got it!

https://github.com/zephyrproject-rtos/zephyr/pull/82737 waz merged so update xg23 too


Jest issues

Having a hard time tracking down the change that triggered this because your release notes aren't very detailed but it looks like you introduced some strange module export patterns or something along those lines. Could you just look through this error output and point me in the direction of something you changed since 0.7.3 that might have triggered it? Our jest transformers already account for typescript and tsx so i'm thinking it has something to do with your module pattern.
Jest encountered an unexpected token

Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

Can you give me more info about your dev environment?

OS
Runtime

node
deno
bun

jest version

The more info you can give me, the easier it will be to fix this.

OS: Mac Sanoma 14
Runtime: Node 20.11.1
Jest version: "jest": "29.7.0"

I'm not sure what else to provide you. When upgrading from 0.7.3 to your latest version I started seeing these errors.

It seems this issue is caused by the fact that I'm using bun now to build. But bun build doesn't support cjs yet. https://bun.sh/docs/bundler#format. So until they do, you will have to use 0.7.3. Is that ok with you?

absolutely. Just wanted to see the path forward. I appreciate you looking into it.

Great, thanks


Bug: Pressing ctrl + shift + v hangs the program

Pressing ctrl + shift + v causes the program to hang and start behaving in a weird way. The CPU usage also spikes up.

Reason:
After every key input event, the program queries the current cursor position. The response of this query is read from stdin. However when we paste some text at the prompt, the response of the cursor position query comes after the pasted text, and the correct cursor position is not read.
This needs to be fixed.

#12 uses a better escape sequence to draw the line. Eliminating the need to call get_cursor_position repeatedly.