Upstream response time mismatch.

Discussed in https://github.com/Kong/kong/discussions/13300

Originally posted by ksgnextuple June 25, 2024
Hi Team
We are running few performance tests on of our apis and we see that the kong_upstream_response time reported on Prometheus is having a large difference as compared to the actual upstream latency.
Kong : 3.6
KIC: 3.1
The same test was run using Kong 2.8 and KIC 2.5 we saw that this difference was minimal which is expected behavior.
Has anyone faced such an issue?

Prometheus is having a large difference as compared to the actual upstream latency.
Could you give us some examples?

For the actual upstream latency, could you show us how you monitored this actual latency value?

We have instrumented the application using newrelic java agent, and also monitoring it using prometheus micrometer metrics. We saw that on both the places the latency were matching and much lower than the upstream reported by the prometheus plugin of kong. This was not the case with Kong 2.8, where the upstream latency reported by kong prom plugin was pretty close to latency observed on the above places.
The difference we see with kong 3.6 is around 100ms (sometimes more) whereas it was not more than 10ms with kong 2.8.

It is not impossible that there is a bug in how the Prometheus plugin response upstream latency that was introduced between 2.8 and 3.6.  To dig deeper, we'd need a way to reproduce.  Can you share details on what your custom plugin does, i.e. what functions it implements and what is done in them?

So the plugin intercepts every request made to kong and then makes a HTTP POST request to a custom auth service which has authorization logic. Can share the lua scripts here if required.  The script itself is based on this custom plugin -> https://github.com/pantsel/kong-middleman-plugin/tree/master
We have modified the above a bit and can share those scripts if required.

It would be useful to have a way to reproduce the problem, so if you can supply your plugin as well as a minimal configuration for reproduction, it'd help.

middleman-plugin-cm.zip
Attached the configMap as a zip
Below is the sample plugin yaml
apiVersion: configuration.konghq.com/v1
config:
  response: table
  timeout: 2
  url: http://<endpoint>/<some-api>
kind: KongPlugin
metadata:
  annotations:
    kubernetes.io/ingress.class: <className>
  name: middleman-plugin
plugin: middleman

@ksgnextuple Please provide the plugin in a ZIP containing the individual files, not as a YAML file.  Also, provide instructions to reproduce the problem.

@ksgnextuple Don't bother with the plugin files.  I was able to extract them from the YAML file and to set up a test environment.  I could not reproduce the issue, however.  With the kong:latest image, your plugin and the Prometheus plugin installed and the middleman http server responding after a 200ms delay, I see these metrics (elided):
# HELP kong_request_latency_ms Total latency incurred during requests for each service/route in Kong
# TYPE kong_request_latency_ms histogram
kong_request_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="250"} 24
# HELP kong_upstream_latency_ms Latency added by upstream response for each service/route in Kong
# TYPE kong_upstream_latency_ms histogram
kong_upstream_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="25"} 24

As you can see, the request latency for all 24 requests is below 250ms, and the upstream latency is below 25ms.  Hence, the upstream response time metrics don't include the time that the middleman server takes to respond.
From our perspective, it does not look like there is a problem in Kong Gateway here.  If you think different, please provide a self-contained way to reproduce the problem.

Here is my test environment in case you want to experiment with it yourself: middleman.zip

@hanshuebner Thanks for the inputs, will try this setup on our end and get back.

@hanshuebner  I setup the test environment, but I set it up on a k8s cluster as that's where we run our tests. First I ran a 15 min test and used the below Prometheus queries to record the response times (p95)
histogram_quantile(0.95, sum by(le) (rate(kong_kong_latency_ms_bucket{kubernetes_namespace="kong", service="default.echo-service.80"}[$__interval])))
histogram_quantile(0.95, sum by(le) (rate(kong_request_latency_ms_bucket{kubernetes_namespace="kong", service="default.echo-service.80"}[$__interval])))
histogram_quantile(0.95, sum by(le) (rate(kong_upstream_latency_ms_bucket{kubernetes_namespace="kong", service="default.echo-service.80"}[$__interval])))
These were the results:
Without Middleman
P95 Kong - 4.29ms
P95 Request - 17.9ms
P95 Upstream - 17.8ms
Then I ran a another test with Middleman Enabled, below are the results:
P95 Kong - 364ms
P95 Request - 485ms
P95 Upstream - 325ms
Pasting the k8s yamls for reproduction if required.
apiVersion: apps/v1
kind: Deployment
metadata:
  name: echo-deployment
spec:
  replicas: 1
  selector:
    matchLabels:
      app: echo-server
  template:
    metadata:
      labels:
        app: echo-server
    spec:
      containers:
        - name: echo-server
          image: test-middleman
          # command: [ "/bin/bash", "-c", "--" ]
          # args: [ "while true; do sleep 30; done;" ]
          resources:
            requests:
              cpu: 500m
              memory: 100Mi
            limits:
              cpu: 500m
              memory: 100Mi
          ports:
            - name: http-port
              containerPort: 80

---
apiVersion: v1
kind: Service
metadata:
  name: echo-service
spec:
  ports:
    - name: http-port
      port: 80
      targetPort: http-port
      protocol: TCP
  selector:
    app: echo-server
---

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    konghq.com/plugins: middleman-plugin
  name: echo-service
spec:
  rules:
    - host: "poc.example.com"
      http:
        paths:
          - path: /
            pathType: ImplementationSpecific
            backend:
              service:
                name: echo-service
                port:
                  number: 80

Do let me know if the plugin yaml is required for the setup on k8s cluster.

The latency numbers that you report don't tell me that the Prometheus plugin includes the upstream time, but rather that the plugin has drastic negative effects on Kong Gateway's overall performance. The reason most likely is that it includes a http client that is not interacting with OpenResty well, causing proxy performance to be massively affected. I would recommend rewriting the plugin to use lua-resty-http. This will greatly simplify the code and likely solve your performance issues.

I was having a look at the response headers which gives the X-Kong-Upstream-Latency and X-Kong-Proxy-Latency.
The X-Kong-Proxy Latency is actually very consistent, around below 220ms which is expected, but the upstream latency > 500ms. This is with the middleman plugin. I also updated the httpclient.lua to make use of the lua-resty-http and still similar results. Trying to modify the plugin a bit by adding keepalive pool will check if that helps.

Please provide us with a complete, self-contained way to reproduce.  We're not normally using Kubernetes for our Gateway testing, so we either need a reproduction setup that includes instructions how to set up K8s locally or that uses docker compose.

@hanshuebner Update from my end, looks like it is a plugin issue, updated the script to use cjson instead of the json script I had shared and made use of the neturl library instead of the https://github.com/lunarmodules/luasocket/blob/master/src/url.lua module and things seem to be good now.
But one small thing I observed is the P95 of the Kong Proxy Latency is showing up as more than the P95 Request Latency.
I could share the updated plugin script here.
We can make use of docker compose itself, but I am kind of doing a load test on the api - 10tps currently.

If there is anything you want us to look at, please share reproduction steps and relevant scripts / configuration files.  Thank you!

Attaching the updated plugin scripts here. You could continue to use the same test env that was previously setup. All I want to check is whether the metrics reported by the prometheus plugin is valid. I ran a 10 TPS test with the test environment that was shared above. To me it seems like the kong proxy latency shown by the plugin is not too accurate.
plugin.zip

I ran your updated plugin to see whether I can reproduce the results.  I ran wrk to generate some load and then queried the Prometheus metrics endpoint to determine the latency information:
cadet 1190_% docker compose exec -u 0 -it client bash
root@4186f90bf3fe:/# wrk -c64 -t64 http://kong:8000/
Running 10s test @ http://kong:8000/
  64 threads and 64 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   220.86ms   23.93ms 325.92ms   86.42%
    Req/Sec     4.02      0.66     5.00     59.86%
  2880 requests in 10.10s, 0.88MB read
Requests/sec:    285.17
Transfer/sec:     89.50KB
root@4186f90bf3fe:/# http kong:8100/metrics
HTTP/1.1 200 OK
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Type: text/plain; charset=UTF-8
Date: Mon, 01 Jul 2024 07:51:46 GMT
Server: kong/3.7.1
Transfer-Encoding: chunked
X-Kong-Admin-Latency: 6

# HELP kong_kong_latency_ms Latency added by Kong and enabled plugins for each service/route in Kong
# TYPE kong_kong_latency_ms histogram
kong_kong_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="200"} 2
kong_kong_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="500"} 2943
[...]
# HELP kong_request_latency_ms Total latency incurred during requests for each service/route in Kong
# TYPE kong_request_latency_ms histogram
kong_request_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="250"} 2627
kong_request_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="400"} 2943
[...]
# HELP kong_upstream_latency_ms Latency added by upstream response for each service/route in Kong
# TYPE kong_upstream_latency_ms histogram
kong_upstream_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="25"} 2643
kong_upstream_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="50"} 2936
kong_upstream_latency_ms_bucket{service="example-service",route="example-route",workspace="default",le="80"} 2943

My takeways from this are:
The kong_latency metric reports the time from when the connection was opened until after the load balancer has been invoked to determine the upstream to send the request to.  It is uncertain to me how useful this metric is in practice, as connections may be reused.
The request_latency is taken from nginx's $request_time variable, which is defined as follows:

request processing time in seconds with a milliseconds resolution; time elapsed between the first bytes were read from the client and the log write after the last bytes were sent to the client

The upstream_latency metric reports the time from when the upstream was selected and forwarding began to when Kong Gateway received the first bytes of the response.
The accuracy of the latency measurements can be influenced by the load that is put onto Kong Gateway in some situations.  In particular, the reported upstream response time includes the time that the underlying software (i.e. nginx) needs to react to data received on the socket and scheduling the request handling process.
Let us know if any questions remain, otherwise please close the issue.


[ZEPPELIN-5966] Support Spark 3.5

What is this PR for?
Spark 3.5.0 was released a few weeks ago, this PR tries to add Spark 3.5 support in Zeppelin.
https://spark.apache.org/releases/spark-release-3-5-0.html
What type of PR is it?
Improvement
What is the Jira issue?
ZEPPELIN-5966
How should this be tested?
Update GitHub workflow to cover Spark 3.5 tests
Screenshots (if appropriate)
Questions:

Does the license files need to update? No
Is there breaking changes for older versions? No
Does this needs documentation? Yes, updated correspondingly.

BTW, Spark 3.5 drops support for Python 3.7
https://spark.apache.org/docs/latest/api/python/getting_started/install.html#python-versions-supported

will rebase once https://github.com/apache/zeppelin/pull/4668 get in

@Reamer @jongyoul this is ready for review (the previous attempts already passed CI), also cc @zjffdu

could the committer help trigger the failed checks? I suppose all failures are caused flaky tests

kindly ping @jongyoul

@Reamer @jongyoul I think it's ready to go, would you please take another look?

@Reamer thanks for the review and approval


Cygwin CMake build issue

Originally discussed in #4499.
Quoting from @yellowdolphin :
Concerning the built and cygwin support, the code is now somewhat inconsistent: I looked into the fopen64 issue and found that dmlc-core already considers cygwin and quite a list of other OSs where fopen64 is not present. But in rabit (rabit/include/rabit/internal/utils.h), only for FreeBSD and apple fopen64 is replaced by fopen (if glic is used). I wonder if this does not break the build for other OS's as well.
I guess the quick solution would be to apply the same logic in rabit/dmlc-core whether to use fopen64. On the other hand, I read that fopen64 is obsolete and one should instead set
_FILE_OFFSET_BITS=64
which then for 32-bit linux would map fopen to fopen64 to allow reading >4GB files.
https://www.gnu.org/software/libc/manual/html_mono/libc.html#Opening-Streams
https://stackoverflow.com/questions/4357570/use-file-offset64-vs-file-offset-bits-64
So, would it be safe to get rid of fopen64 altogether?
I can prepare a PR for whatever you prefer, but for the 2nd solution, some testing might be appropriate and I don't have a 32-bit system with 4GB mem...
Originally posted by @yellowdolphin in https://github.com/dmlc/xgboost/pull/4499#issuecomment-496037203
And pointed out by @RAMitchell , we should add a test for cygwin users.

We have tests on Action.


Add documentation to GLOBAL scope in HystrixCollapser

Ran into an issue where HystrixCollapser with GLOBAL scope was used and HystrixRequestContext was incorrect.
At a minimum, I need to add documentation of this option.
After reviewing the code, I will likely need to make changes to how GLOBAL collapsing works w.r.t. request state. My guess is that the first thread to submit work to a GLOBAL collapser attaches its request  context for the duration of the JVM.  Will confirm and then suggest a fix.

Yikes !  I would think a global anything would have exactly no request
context. :)
-- Keith

Don’t listen to me, I play with Christmas lights
http://www.christmasoutloud.com/ all year long.
On Mon, Jul 27, 2015 at 2:58 PM, Matt Jacobs<EMAIL_ADDRESS>wrote:

Ran into an issue where HystrixCollapser with GLOBAL scope was used and
HystrixRequestContext was incorrect.
At a minimum, I need to add documentation of this option.
After reviewing the code, I will likely need to make changes to how GLOBAL
collapsing works w.r.t. request state. My guess is that the first thread to
submit work to a GLOBAL collapser attaches its request context for the
duration of the JVM. Will confirm and then suggest a fix.
—
Reply to this email directly or view it on GitHub
https://github.com/Netflix/Hystrix/issues/842.

Added GLOBAL/REQUEST discussion to Wiki here: https://github.com/Netflix/Hystrix/wiki/How-To-Use#request-collapsing

There are valid usecases where a batch command may be created with multiple HystrixRequestContexts, so I don't want to remove HystrixRequestContext from a globally-scoped collapser.  However, this option may NOT be used when the batch command is unaware of the work is has to do to create a new HystrixRequestContext out of multiple constituent HystrixRequestContexts.
Does this sound right?


SSH Agent integration

Is there any interest in providing ssh agent integration, similar to that provided by Mina? This requires some sort of native library to access the unix socket (Mina use the apache runtime I believe), so would probably need to be made optional.
Any thoughts?

+1 we'd really like this to work

Agreed, would be neat to have. https://github.com/mcfunley/juds is also an option for unix domain sockets.

This seems to be solved? Correct, if so, closing it soon...

I can confirm that the combination with the jsch-agent-proxy is working. I propose to close this issue.

Closing :)


Refactor collection type extractor into TypeExtensions

Also avoids reflection on type name.

Why is this needed? This code is only used in one spot as far as I can tell?

Agreed on all review points, should be fixed now and rebased against master.

Awesome, thanks!


asynchronous sampler crashes with access violation in fn interp_lin when using chunksize of 1

i got a access violation exception calling resampler.process_into_buffer(&self.resampler_input, &mut self.resampler_output, Some(&self.channel_mask)) ;
using rubato = "0.14.1"
using FastFixedOut = FastFixedOut::new(96000.0/44100.0, 100.0, PolynomialDegree::Linear, 1, 8);
using  Resampler::input_buffer_allocate and Resampler::output_buffer_allocate(); methods to preallocate the io buffers.
this happened after a few successful calls to proccess_into_buffer() so i'm completely lost on what could be happening.

after testing this exception only occurs with a chunk_size of 1. if this isn't intended, here is a test to reproduce this error:

#[test]
fn test_rubato() {
    // new resampler
    let parameters = SincInterpolationParameters {
        sinc_len: 32, f_cutoff: 0.95, oversampling_factor: 32, interpolation: rubato::SincInterpolationType::Nearest, window: rubato::WindowFunction::Blackman
    };
    const CHUNK_SIZE:usize = 4;
    let resamplerx: SincFixedOut = rubato::SincFixedOut::new(44100.0/44100.0, 10.0, parameters,
        CHUNK_SIZE, 2).ok().unwrap();
    let mut resampler_input: Vec>= Resampler::input_buffer_allocate(&resamplerx, true);
    let mut resampler_output: Vec>= Resampler::output_buffer_allocate( &resamplerx, true);
    let mut resampler: SincFixedOut = resamplerx;
let delay = Duration::from_micros(3);
for _ in 0..4000 {
    sleep(delay);
    Resampler::process_into_buffer(&mut resampler, &resampler_input, &mut resampler_output, None).unwrap();
}

}

if this is intended then id be happy if an assertion or note in the docs is added. thank you

Thanks for the report!
Running with a chunksize of 1 will be extremely inefficient, so it's not something to recommend ever doing. But it should still work. I'll fix it in the next release.
Try to use a chunksize of at least 100 (ideally a bit larger, like 1000) if you want good performance.

my goal was sample accurate resampling related to resample ratio. but you're right thats probably overkill. nonetheless in my case chunksize has to be small in order to get low latency.
thank you for your work and quick response! :)

A fix is included in https://github.com/HEnquist/rubato/pull/78


Automated Package Upgrade

:tada: This PR is included in version 2.2.40 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Create a screencast showing how to use the tool

I will produce a short screencast for my YouTube channel, Code Cast

Would love to get this done for 1.0.0-beta

Are you going to use a script? If so, I can translate it in French.

@aloisdg Probably not. It will only be short.

I can still make a french cover, but I will have to be good. :smile:


the mount command behaves incorrectly

The traditional way to remount a filesystem is with:
# mount -o remount,rw /mountpoint

However this does not work in toybox.
There is a workaround, though.
# mount -o rw,remount /mountpoint

Any thoughts as to why the "correct" way doesn't work? You can find the "remount,rw" ordering in tons of documentation, yet it doesn't work in toybox.

On 09/17/2016 02:39 AM, Joshua J. Drake wrote:

The traditional way to remount a filesystem is with:
|# mount -o remount,rw /mountpoint |
However this does not work in toybox.
There is a workaround, though.
|# mount -o rw,remount /mountpoint |

blink blink That should totally not matter. If it does, it's a
bug. (You didn't say what "does not work" means in this context, lemme
see what I can dig up...)

Any thoughts as to why the "correct" way doesn't work? You can find the
"remount,rw" ordering in tons of documentation, yet it doesn't work in
toybox.

That would be a bug. The fact that remount doesn't work without an
absolute path is also a bug. Hmmm...
(I haven't put together a proper mount test suite yet because it
requires root access and a known host environment, meaning it's gated by
a dependency on a qemu-based test environment, which is gated on my
taking my aboriginal linux project apart and putting it back together
again as described on that list. Working on it. But mostly I have to get
back to Austin and a decent work environment again so my development
bandwidth for these projects isn't so severely constrained...)
Rob

another report of this bug today: https://github.com/android-ndk/ndk/issues/380#issuecomment-306905516

On 07/10/2017 02:56 PM, Elliott Hughes wrote:

another report of this bug today: android-ndk/ndk#380 (comment)
https://github.com/android-ndk/ndk/issues/380#issuecomment-306905516

Blah. Sorry about the delay. (The underfunded startup I work at can't
afford to replace people it lost over the past year, so the rest of us
gained more plates to spin. Very distracting. Yes, I am catching up on
this at 4am.)
The simple fix is:

remount = (void *)(long)comma_scan(opts, "remount", 1);

remount = (void *)(long)comma_scan(opts, "remount", 0);

The problem is if comma_scan removes "remount" from the option string
during early parsing (setting the "remount" flag that changes the
behavior to do /proc/mounts lookup to find existing flags to delta
against), then we never set the remount flag when collecting the set of
flags to apply as a delta, meaning it tries to do a new mount over the
existing mount, which either gives you -EBUSY or stacks a new mount on
top of the old mount (for tmpfs).
The deeper question is "why the heck does rw,remount work then"? And the
answer is that comma_scan() is failing to remove entries that match at
the end of the list. (Ok, fixed. And that failure is why it passed my
initial testing way back when. Sigh...)
Hmmm... if /proc/filesystems says that a filesystem type is nodev, I
don't have to require two arguments to mount it. I.E. there's no reason:
mount -t tmpfs sub
Can't work? Try the fstab lookup first, but if it's not there just use
basename of sub as the device?
Hmmm... that works for tmpfs, ramfs, and devtmpfs, and ntfs claims to
need a device, but cifs and fuse are nodev and I'm pretty sure they
care. (Use the "device" field as other info).
Meanwhile the insane v9fs expects the address to live in the options as
special arguments in the csv, because IBM. Sigh... (I need to write
nfsmount, cifsmount, and v9fsmount commands. For two of those I could
even do a reasonable server. It's on the todo list. But not at this time
of morning. :)
Rob

i think this was fixed by https://github.com/landley/toybox/commit/d4adb3f8e2ec39674e34ed518983ed6e85560c91 and can now be closed.


Remove old docs

New docs are now listed at https://github.com/globus-gladier/gladier-patterns-examples-2022

Yes, good. Let's remove to avoid confusion.


List all parked vehicles, seed data into db and created another repository

Added another repository for vehicles
Seed garage and vehicles data into db
New migrations
List all parked vehicles in Vehicle/Index.cshtml

FYI @suny-am
Made some minor changes to the GarageRepository that you built
You should be able to apply the new migrations by running Update-Database either in package manager console or your terminal I guess. If you run into issues my recommendation would be to drop (delete) the database and then run the command again, this should rebuild the db with the latest changes. 🙂
Last thing to mention, I renamed our tables from Garages to Garage and ParkedVehicles to ParkedVehicle (singular) to try to follow better naming conventions. This is of course something we can change, if we still prefer the default table names.
https://stackoverflow.com/questions/338156/table-naming-dilemma-singular-vs-plural-names

Looks good! Only some small things to check out!

Great! Will look through your suggestions and fix the code :)

Looks good! Only some small things to check out!

Great! Will look through your suggestions and fix the code :)

Awesome!
Also just want to add: Exemplary commit messages!


Some packages are missining in k8s.io/client-go/

Hi,
I am using k8s client-go and I imported some of them:
"k8s.io/client-go/pkg/api/errors"
"k8s.io/client-go/pkg/api/resource"
"k8s.io/client-go/pkg/api/unversioned"
"k8s.io/client-go/pkg/runtime"
But I found they are missing now:
https://github.com/kubernetes/client-go/tree/master/pkg/api
Can you tell me where are they and why they are missed?
If possible, is there any solution that I can use them again?
Thanks!

Those look like the ones moved to k8s.io/apimachinery.
On Mon, Jun 12, 2017 at 6:30 AM, zhangtbj<EMAIL_ADDRESS>wrote:

Hi,
I am using k8s client-go and I imported some of them:
"k8s.io/client-go/pkg/api/errors"
"k8s.io/client-go/pkg/api/resource"
"k8s.io/client-go/pkg/api/unversioned"
"k8s.io/client-go/pkg/runtime"
But I found they are missing now:
https://github.com/kubernetes/client-go/tree/master/pkg/api
Can you tell me where are they and why they are missed?
If possible, is there any solution that I can use them again?
Thanks!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/client-go/issues/207, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAnglgul1g6rvOgRdSrDjORmUWhxMnoCks5sDT2BgaJpZM4N3HjA
.


Update to v2.7.3

Checklist

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

Note: Upstream states that python 3.12 is not yet supported:

Mainly because of distutils deprecation and some packages not being compatible with it, we should
Limit airflow to < 3.12 until those dependencies are ready and until we can support Python 3.12

Even so, I think it's better for us to keep the python 3.12 release since one was already built for 2.7.2 and packages seem to be available.  We can mark them as broken later if we have to.

Okay, darn it!  pip check isn't going to let python 3.12 happen.

@conda-forge-admin, please rerender


Updates e2e test to use extended cirros image

Changes default cirros image location to github image
Fixes incompatibilities with old tests
Some simplification of test code

This change is

Reviewed 10 of 10 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

This PR appears to include a commit from #463 ?

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@ivan4th I think so but #463 is merged now so doesn't matter for now I think.

@pigmej no, it does matter as the commit has different id here. We'll get duplicate commits in the history.

@ivan4th then it certainly does matter.

Ok, I looked at the wrong commit id.

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable


Added function headings in multiple files of the client directory

Hey there,
I just added function headings in multiple files of the client directory like lxd_oidc.go, lxd_storage_volumes.go and util.go.
Now the client directory is completed. Now I will work on other directories in LXD to add their function documentation.
Thank you!

Please can you break this into a commit per file to aid with backporting.

@tomponline I don't know how to break the commit per file. It's something new to me. I will check it.

I am going to send another PR with required changes.


[release-1.10] Upgrade to latest dependencies

cve -dprotaso
/cc knative-extensions/serving-writers
/assign knative-extensions/serving-writers
Produced by: knative-extensions/knobots/actions/update-deps

this did updates from the wrong branch


Saving/Loading data is not working

Xcode 8.1 Swift 3
You can recreate this issue just by using the basic example:
try! Locksmith.saveData(["some key": "some value"], forUserAccount: "myUserAccount"
let dictionary = Locksmith.loadDataForUserAccount("myUserAccount")
dictionary is always nil for me.
I debugged the read function and found the problem here: (Line 79 in Locksmith.swift)
let status = closure(requestReference, &result)
This return a status of -25300 which corresponds to errSecItemNotFound.
So the problem could also be that something goes wrong when saving the data.
Although it is not throwing any errors when saving.

Alright found the solution after some more digging in the issues.
@PawelLukasiak mentions (in #97) the correct solution:

I have solution how to fix that problem in iOS 10.
Chose your target i project, then select Capabilities -> Keychain Sharing to ON.
xCode will create for you entitlements file.
This is all !

#97 is from December 2015, the solution should be inside the projects README.

Fixed with #160


hide worksheet and reorder sheets

Hello,
Would be useful if I could hide/unhide a worksheet.  Currently couldn't see an option to do so.
Maybe as some property:
var sheet = workbook.addWorksheet('My Sheet', {properties: {hidden: true}});
Along with some function to check hidden status
workbook.getWorksheet('My Sheet').IsHidden();  // true or false
And to also toggle hidden status on the fly:
workbook.getWorksheet('My Sheet').hidden = true;  // or false...
Would also be useful to re-order sheets in the workbook programmatically.  I guess you could technically just "add" sheets in the correct order then circle back around and add data as necessary... but reordering sheets would allow you to not need to do that.

Up Vote (the reordering part)!  Would love to have a way to reorder sheets after they are added, particularly with streaming.  I'm streaming in the first place because I'm working with large datasets.  I have to create new sheets as I encounter certain types of data while streaming, which will occur randomly, but this means I can never control the final order of the sheets.  I don't commit sheets until the very end anyway.  If they haven't been committed, I really need a way to programmatically re-order the sheets (e.g., alphabetize by name with a Summary sheet as first one) just before doing the final commits.

+1 on the hiding of sheets also

@guyonroche Any thoughts about it? Need this actually, especially hiding of worksheet.
Also would be great to export class Worksheet and add ability to add worksheet as parameter with addWorksheet method. I think this is must have thing in order to create Workbook from custom worksheets in defined order.

@dvsoukup @king612 @LogansUA
npm install my branch
"exceljs": "thingnario/exceljs#feature/DeploySheetAddState",
the following will hide the second_sheet
const wb = new Excel.Workbook();
wb.addWorksheet('first_sheet');
wb.addWorksheet('second_sheet', { state: 'hidden' });

if you want to hide the first_sheet, you need to specify the activeTab to second_sheet
const wb = new Excel.Workbook();
wb.views = [{ activeTab: 1 }];
wb.addWorksheet('first_sheet', { state: 'hidden' });
wb.addWorksheet('second_sheet');

Published @Hsinfu 's fix in v1.5.0

const wb = new Excel.Workbook();
wb.views = [{ activeTab: 1 }];
wb.addWorksheet('first_sheet', { state: 'hidden' });
wb.addWorksheet('second_sheet');
this code is not working

Hide all rows and column which doesn't have data when writing to excel.
Want to hide rows and columns which does have data as highlighted in Screenshot 1

Want to achieve Screenshot 2


(vue) useQuery pause option is not reactive

Describe the bug
When a reactive property (a ref, computed or reactive) variable is used in the pause property, it keeps using the initial value.
Reproduction
https://stackblitz.com/edit/github-uroxw1-qpmhta?file=package.json
Urql version
Bug introduced again in >= v1.3.1
Validations

[X] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
[X] Read the docs.
[X] Follow our Code of Conduct

Experiencing the same issue.


Compiled JS is bigger than the one of TinyMCE

📝 Provide detailed reproduction steps (if any)
Build the project (yarn && yarn build) and compare the size of the js/tinymce/tinymce.js and js/tinymce/tinyme.min.js files with<EMAIL_ADDRESS>and<EMAIL_ADDRESS>✔ Expected result
After building the project, the resulting js/tinymce/tinymce.js and js/tinymce/tinymce.min.js files should have about the same size as<EMAIL_ADDRESS>and<EMAIL_ADDRESS>❌ Actual result
After building the project, the resulting js/tinymce/tinymce.js and js/tinymce/tinymce.min.js files are around 600/200 KB bigger than the ones from<EMAIL_ADDRESS>and<EMAIL_ADDRESS>❓ What to do now
Find out if TinyMCE at commit https://github.com/hugemce/hugemce/commit/282f1ebb24e6dc86da3bf259e0c031ca44518cba also presents this issue; the cause might be the removal of the lockfile.

TinyMCE at commit https://github.com/hugemce/hugemce/commit/282f1ebb24e6dc86da3bf259e0c031ca44518cba doesn't present this issue, so I may have to downgrade some dependencies.

Hmm, bringing back the lockfile (letting yarn fix it by disabling frozen-lockfile in .yarnrc) doesn't help.

@mikkorantalainen You're actually right that it's something about the Adt class although that's already present in the official TinyMCE 6 script

As you can see in the bar at the very right of the screenshot, there are differences (blue) along everything, but it's mostly about something like find$1 instead of find. However, there's one green part so most of the garbage must be there. One thing that's there is const uid = isSketchSpec(spec) of Memento.js, which only belongs into the silver/theme.js but somehow now also goes into the main tinymce script.

Guess what?
find . -type f -name "*@hugemce*" ! -path "./.git/*" -exec rename 's/@hugemce/@ephox/g' {} +
grep -rl --exclude-dir=.git '@hugemce' . | xargs sed -i 's/@hugemce/@ephox/g'

Now everything's back to normal!
Huh, why? That's the question.

I have it! @ephox/oxide-icons-tools which is still a dependency as it's not in this monorepo depends on @ephox/katamari so we have both @ephox/katamari and @hugemce/katamari in our code, everything doubled. I'll probably fork oxide-icons-tools. Thanks to @mikkorantalainen for his vigilance to the size of the bundle!

Although...this still doesn't clearly explain why some code is in files here where it totally isn't in TinyMCE, as you can see on the screenshot in one of my above messages. Hmm. Will have to re-check this after fixing.

I have it! @ephox/oxide-icons-tools which is still a dependency as it's not in this monorepo depends on @ephox/katamari so we have both @ephox/katamari and @hugemce/katamari in our code, everything doubled. I'll probably fork oxide-icons-tools. Thanks to @mikkorantalainen for his vigilance to the size of the bundle!

Great find! I don't yet understand the effects of monorepo vs normal(?) repo so I couldn't have figured that out this fast.

 Took out the package of node_modules, published it on https://github.com/hugemce/oxide-icons-tools, now there's no tsconfig.json :/

Looking if I can just patch the package.json to use @hugemce/katamari instead of @ephox/katamari

What? Version 3.1.1 doesn't even depend on @ephox/katamari

So maybe that isn't even the reason..

Debugging with yarn list and grep, don't find the reason yet. What I thought previously doesn't appear the reason.

Will continue to investigate tomorrow. First start with TinyMCE and replace ephox by something like efoks (same length)

Started today, will do more tomorrow

So I'll have to document every step and make this carefully. I'll first try the ephox > hugemce transition again, possibly with different commands (I'm not that good at more complex bash scripting, and letting AI write all the commands for me):

git clone git@github.com:tinymce/tinymce.git
cd tinymce
git reset --hard 282f1eb
This time, I'm not going to use a terminal command for replacing the folder names. Rather, I open up nemo file manager, search recursively and case-sensitively (only ephox, not Ephox) for ephox. All I get are folders that lie somewhere in subfolders of modules. So I select everything (Ctrl + A), right-click, rename, Bulky opens, I choose to replace ephox by hugemce. All runs smoothly.
Now, I have to replace ephox by hugemce in all files. I got find . -type f -not -path '*/.git/*' -exec sed -i 's/ephox/hugemce/g' {} + from ChatGPT, hope this is okay.
git status verifies big changes.
Now, one important thing is to convert all occurrences of some packages in the @ephox namespace that are not contained in this monorepo from hugemce back to ephox:find . -type f -not -path '*/.git/*' -exec sed -i 's|hugemce/swag|ephox/swag|g' {} +
find . -type f -not -path '*/.git/*' -exec sed -i 's|hugemce/bedrock|ephox/bedrock|g' {} +
find . -type f -not -path '*/.git/*' -exec sed -i 's|hugemce/dispute|ephox/dispute|g' {} +
find . -type f -not -path '*/.git/*' -exec sed -i 's|hugemce/oxide-icons-tools|ephox/oxide-icons-tools|g' {} +

Now, git add . && git commit -m "Brute-force replace ephox by hugemce except where not applicable" the current state. Just a quick local commit.
Now, try it again. yarn && yarn build. Oh, forgot one thing.
find . -type f -not -path '*/.git/*' -exec sed -i 's|hugemce/wrap-promise-polyfill|ephox/wrap-promise-polyfill|g' {} +
git add . && git commit -m "Migrate hugemce/wrap-promise-polyfill back to ephox/wrap-promise-polyfill"
yarn && yarn build again.
eslint prints some warnings, but it the process does not abort. The problem is now suddenly at the rollup:core task:

Running "rollup:core" (rollup) task
SOURCEMAP_ERROR Error when using sourcemap for reporting an error: Can't resolve original location of error. file<EMAIL_ADDRESS>line 3:15
Warning: The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten file<EMAIL_ADDRESS>line 3:15
1: import * as Differ from './Differ';
2: import * as TestError from './TestError';
3: var __assign = this && this.__assign || function () {
                  ^
4:   __assign = Object.assign || function (t) {
5:     for (var s, i = 1, n = arguments.length; i < n; i++) {

 Use --force to continue.

Aborted due to warnings.
error Command failed with exit code 6.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: "tinymce-grunt" exited with 6.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I'll have to take a granular approach: For each module replace @ephox/[name] by @hugemce/[name], commit, do the same again without the @ (I could do it all without the @, but I'd like to see where ephox/something is used without the @), commit again, build and see after the replacement of which module it fails, then maybe skip it, do the other modules first and see what to do at the end. Not forget to replace the appropriate folders named ephox and always delete node_modules before or after a replacement.
You shall hear from me at the weakend.

I've started investigating with a bash script; building after replacing ephox by hugemce works fine (script is 1.3 MB instead of 1.2, I hope that's not because of much...I'll diff later) for all packages before katamari, then it seems to fail even if I skip it and continue replacing, first mcagar. I'll do more investigations around the root cause tomorrow.

So it even fails after replacing the ephox of agar.

 I'll leave it open as planned to fix long-term but now I've more important stuff to do. I'll revert the replace...and work on the other things in #1. As all this is in a monorepo, it shouldn't harm building it while the namespace is still called ephox. The only thing I cannot do is publishing the modules of the monorepo at npm, although ephox is doing it. I'll just publish the final HugeMCE build.

Running "rollup:core" (rollup) task
SOURCEMAP_ERROR Error when using sourcemap for reporting an error: Can't resolve original location of error. file<EMAIL_ADDRESS>line 3:15
Warning: The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten file<EMAIL_ADDRESS>line 3:15
1: import * as Differ from './Differ';
2: import * as TestError from './TestError';
3: var __assign = this && this.__assign || function () {
^
4:   __assign = Object.assign || function (t) {
5:     for (var s, i = 1, n = arguments.length; i < n; i++) {

I think this warning should simply be ignored so at least that part of the commit bdf8d2e should be okay to include. The warning is caused by old style JS code that shouldn't be used nowadays but as long as the build process interprets it correctly it should be okay. I think this old syntax was needed for NodeJS CommonJS modules.
As TinyMCE / Tiny Inc never publicly explained how they supposedly generated the distribution files from their source, we cannot know if they apply some secret fixes to get the publicly visible source to build correctly.
The above code var __assign = this && this.__assign || function is interpreted according to the above warning as just function which seems perfectly acceptable for me.
However, I don't understand the build process well enough to understand how the build is supposed to collect only live code from all the dependencies. This is called tree-shaking in general if I've understood correctly, but I have no idea how it's supposed to work in case of TinyMCE or HugeMCE because it's really poorly documented anywhere.
It appears to me that for whatever reason, the tree-shaking fails for the Adt class but I have no clue about why.

Here's an article about tree-shaking with different syntaxes: https://dev.to/lukasbombach/how-to-write-a-tree-shakable-component-library-4ied
However, I don't understand TinyMCE/HugeMCE build process well enough to know if this applicable or not.

I still fail to understand why the TinyMCE source code has so much unneeded source code. When I do following:
$ cd hugemce/modules
$ find -type f -name "*.ts" | xargs du -cb | awk '/total$/ {sum += $1} END {print sum}'

The output is 15324143 meaning that all the TypeScript source code combined is over 15 MB! Why did Tiny Inc bother maintaining all that code when obviously only about 10% is actually needed? I guess the tree-shaking process is expected to throw about 90% of all the source code away.

20. Everything builds perfectly 👍 without warnings 👍 the bundle is only 1.2 MB.

Have you been able to reproduce this part? I tried to remove the whole node_modules hierarchy and re-did yarn && yarn build and still ended up with all the extra code in tinymce.js.
I didn't modify the source code before running yarn, though.

I tried to bisect the code from the original TinyMCE code that was still licensed as MIT to find the patch that increases the size of tinymce.js over 1.5 million bytes:
git bisect start
git bisect bad d486f0087e55a911c02b4fb59a5df672733a0922
git bisect good 282f1ebb24e6dc86da3bf259e0c031ca44518cb
git bisect run bash -c 'set -x; (yarn || exit 125) && (yarn build || exit 125) && test "$(stat -c %s modules/tinymce/js/tinymce/tinymce.js)" -lt 1500000'

but this results in every commit after commit 51051d8e0d201000ec464e4aa1ad79ba0393e26f failing to build (and being skipped) until the versions with my commit to ignore warnings and keep going (commit bdf8d2e74112543a2f215b0f3b0c32b876c069c9) which results in too big emitted tinymce.js after completing the build.
I would guess the commit 51051d8e0d201000ec464e4aa1ad79ba0393e26f does something that causes the filesize to increase but it's impossible to tell without a successful build.
The git bisect log gives you the full status.
You can restore your working tree git bisect reset after you're done bisecting.

Interestingly I get a bit less problems building the project if I remove the .yarnrc file. There seems to be rule --install.frozen-lockfile true which may be related to the issue. I think removing both this rule and yarn.lock file might be the least problematic way to proceed.

I think this warning should simply be ignored so at least that part of the commit bdf8d2e should be okay to include. The warning is caused by old style JS code that shouldn't be used nowadays but as long as the build process interprets it correctly it should be okay. I think this old syntax was needed for NodeJS CommonJS modules.

But there is no warning if I don't replace ephox by something else (like hugemce).

As TinyMCE / Tiny Inc never publicly explained how they supposedly generated the distribution files from their source, we cannot know if they apply some secret fixes to get the publicly visible source to build correctly.

The source builds fine if I don't replace ephox by something else.

Have you been able to reproduce this part? I tried to remove the whole node_modules hierarchy and re-did yarn && yarn build and still ended up with all the extra code in tinymce.js.
I didn't modify the source code before running yarn, though.

It builds perfectly if I don't replace ephox by hugemce, but I did this in the main branch, so probably you're building using the replaced version.

Interestingly I get a bit less problems building the project if I remove the .yarnrc file. There seems to be rule --install.frozen-lockfile true which may be related to the issue. I think removing both this rule and yarn.lock file might be the least problematic way to proceed.

Does the build still get big then or not?

The build result was still ~1.8 MB instead of expected ~1.2 MB after removing file .yarnrc. Removing that file simply allowed me to successfully build the source for versions that failed if I had .yarnrc in the working directory.
Do you know what technically happens to the build if you try to replace ephox with hugemce? Why would that result in increased build results?

I can successfully build 1.2 MB modules/hugemce/js/hugemce/hugemce.js when I do following:
git reset --hard 282f1eb
find -type f -not -path '*/.git/*' -exec perl -np -i -e 's/(?<!@)tinymce/hugemce/g; s/TinyMCE/HugeMCE/g' {} +
find modules/* -depth -type d -name 'tinymce*' -exec bash -c 'mv "$0" "${0//tinymce/hugemce}"' {} \;
yarn
yarn build

The build result was still ~1.8 MB instead of expected ~1.2 MB after removing file .yarnrc. Removing that file simply allowed me to successfully build the source for versions that failed if I had .yarnrc in the working directory.
Do you know what technically happens to the build if you try to replace ephox with hugemce? Why would that result in increased build results?

Please don't think removing .yarnrc and the lockfile is a feasible solution, I just did something stupid with the lockfile during the replacement or whatever, I added a working version of it back later.
I do not know why the replacement resulted in these errors. Maybe something like @ephox/bedrock-server/main/ts/hugemce somewhere? But I do not know. I want to publish on Saturday if possible, fixing this issue is something for later.

I can successfully build 1.2 MB modules/hugemce/js/hugemce/hugemce.js when I do following:

git reset --hard 282f1eb

find -type f -not -path '*/.git/*' -exec perl -i -pe 's/(?<!@)tinymce/hugemce/g; s/TinyMCE/HugeMCE/g' {} +

find modules/* -depth -type d -name 'tinymce*' -exec bash -c 'mv "$0" "${0//tinymce/hugemce}"' {} \;

yarn

yarn build

Note that the flag -i of perl means "replace file inplace" instead of "incasesentive search" and this flag cannot be combined with other flags. That is, replace all occurrences of tinymce with hugemce (case sensitive) unless prefixed with @. Then replace all occurrences of TinyMCE with HugeMCE (case sensitive).
Then find all directories in hierarchies modules/* and rename all fragments containing tinymce with hugemce in depth order.
After that the build seems to work correctly and the results say HugeMCE everywhere.
There are still dependencies to @ephox and @tinymce packages but I guess that's good enough for now. It might be good to include source code for those in the monorepo, too, to make sure this project is buildable even if Tiny Inc removed those packages from npm repositories in the future.

I'll look at it, thanks! However, main problem with ephox is that most monorepo packages themselves are prefixed @ephox (and some @tinymce). After replacing ephox by hugemce, I have to revert that for the external ephox packages.. maybe I'm missing something there and that's the cause of the build failure...yeah, maybe I could make them monorepo part later. However, some are Apache-licensed...monorepo won't be fully MIT...but hope that's okay.

Apache license is compatible with MIT so there's no problem with that.
You might want to add info about this to LICENSE.TXT to be clear. Something like
"This project has been licensed under the MIT license but portions of the project may have different compatible license such as Apache 2.0. See the LICENCE and NOTICE files in subdirectories of this project for details."
followed by the full MIT license in the top level LICENSE.TXT. The file extension is not needed but it shouldn't hurt either.
Note that if some piece of code is licensed under Apache 2.0 and contains file called NOTICE you should also include that in the generated ZIP files unless that's happening already. See the license text https://opensource.org/license/apache-2-0 section "4. Redistribution" list item 4 for details. Note that this part doesn't allow calling such file NOTICE.TXT.

I want to publish on Saturday if possible

Are you going to publish HugeMCE to npm or yarn repositories?

I can successfully build 1.2 MB modules/hugemce/js/hugemce/hugemce.js when I do following:
git reset --hard 282f1eb
find -type f -not -path '*/.git/*' -exec perl -i -pe 's/(?<!@)tinymce/hugemce/g; s/TinyMCE/HugeMCE/g' {} +
find modules/* -depth -type d -name 'tinymce*' -exec bash -c 'mv "$0" "${0//tinymce/hugemce}"' {} \;
yarn
yarn build

Note that the flag -i of perl means "replace file inplace" instead of "incasesentive search" and this flag cannot be combined with other flags. That is, replace all occurrences of tinymce with hugemce (case sensitive) unless prefixed with @. Then replace all occurrences of TinyMCE with HugeMCE (case sensitive).
Then find all directories in hierarchies modules/* and rename all fragments containing tinymce with hugemce in depth order.
After that the build seems to work correctly and the results say HugeMCE everywhere.
There are still dependencies to @ephox and @tinymce packages but I guess that's good enough for now. It might be good to include source code for those in the monorepo, too, to make sure this project is buildable even if Tiny Inc removed those packages from npm repositories in the future.

Thank you for these commands. I hope you'll be fine with HugeRTE, I've decided to do it and already registered a domain and GitHub repo. Now I can reproduce that your commands work fine, but they do not anymore on the latest commit (if I don't run the git reset part. I'll have to bisect that...

 Let me see if I get it work...

Okay, your commands worked now. Doing a bisect right now.

Unfortunately, I'm not going to publish this today. I'll have to do what git bisect does manually.  I hope to publish today.

Unfortunately, now more problems. No icons, more replacement, task emojis not found. I'll continue tomorrow. Sorry for all the delay.

I investigated rewriting everything from TinyMCE to HugeRTE and here's my take on it. It also fixes the modules/hugerte/tools/tasks/emojis.js file which would get dropped because of some error in the original .gitignore file. I didn't fix the .gitignore line in this branch but I think the file modules/hugerte/.gitignore is the culprit because it has emojis*.js which is probably intended to match something else but I don't know what exactly. I noticed this accidentally while running git status --ignored so maybe look for similar problems in the future.
https://github.com/mikkorantalainen/hugemce/tree/hugerte-fix-8
As usual, you can simply do following on command line:
git fetch "https://github.com/mikkorantalainen/hugemce.git" hugerte-fix-8
git checkout -b fresh-start FETCH_HEAD

See the git commit message for details. Basically TinyMCE just had huge variety of spellings. I think it would make sense to normalize everything to HugeRTE but my patch includes the minimal changes possible.
After checking out that code I'd recommend running
git ls-tree -r HEAD | grep -Ei 'mce|tiny'

and reviewing if you still think some of these files would need to be renamed, too.
I'd throw away the whole .changes directory but I leave that decision for you. I think change log should be generated from Git commits instead. You can always generate changelog with git shortlog 282f1ebb24e6.. or if you tag releases, it gets even easier because you can do stuff like git shortlog v6.9..
And yes, I'd recommend checking out my commit (branched from last TinyMCE revision that was still MIT licensed) and then re-apply the correct commits after that to avoid having lots of reverted commits in the history.
I'd do this as git cherry-pick $COMMIT_SHA repeated once for every commit you want to pick. Or if you use gitk --all you can simply right click to commit you want to include and select "Cherry-pick this commit". You may need to use git mergetool to workaround the changed path names in case Git isn't smart enough to do it fully automated but the changes needed should be pretty simple. The final resulting history should be clean linear history with zero reverted commits after completing all that.
And I'd recommend trying to run yarn build after every cherry-pick to verify the results but that gets tedious fast unless you have super fast computer.

What?? https://github.com/tinymce/tinymce/blob/main/modules/tinymce/.gitignore is not present in modules/hugerte on my last local state ?!

Oh, but it exists on GitHub. https://github.com/hugerte/hugerte/blob/main/modules/tinymce/.gitignore

Thank you for your comment, @mikkorantalainen. I've now fixed the emojis stuff. Your message was very helpful to find out the root cause. However, unfortunately, I'm not going to apply your commit. I've already committed some stuff locally yesterday (including more replacements, like TinyMce -> HugeRte) and now everything works. I have excluded the .changes directory of some replacements and only applied some replacements. I shouldn't throw it away. Although it shouldn't be the case if everything was always in an optional scenario, Git commits aren't always that clean (especially now, where there are problems, problems, and I don't want to force-push). I could have a stable branch and always cleanly rebase and squash commits from main into it, then take it for generating the changelog...maybe...but for now, I'll continue going with changie.

I'll have to patch @ephox/oxide-icons-tools, I think (ideally it'd get integrated into the monorepo later, but patching is easier for now and the source code isn't publicly available, only the built stuff on npm), because right now the editor looks simply awful.

Now I behave embarrassingly. I fixed everything and the build runs fine, but now I don't know how to publish to npm. I have npm publish in the modules/tinymce directory (and removed the private: true), but it just published the source files instead of building the project. I think the contents of dist/hugerte_1.0.0_component.zip are those who have to be published (including files are mostly the same as in https://www.npmjs.com/package/tinymce?activeTab=code). Now when running lerna publish, it assumes all packages have changed...how can I just publish the right HugeRTE build? Are you familiar with this stuff?
Note: I've unpublished the wrong hugerte from npm right away. ChatGPT says I now have to wait 72 hours...okay. It'd be really great if you could tell me how to do it right in the meantime, though I don't know if you are familiar with this stuff. :disappointed:

I think the easiest workaround would be to extract the component zip, add package.json and README.md if they don't already exist and then publish from that directory directly.

I don't know how npm publish is supposed to work. I know that it's possible to publish anything and the published stuff doesn't need to match the GitHub repository so I would guess there's an API that simply takes whatever binary to push to it.
I much prefer the Canonical PPA system where the only way to publish anything is to push source code and their system will build binary files from the published source code. That enforces that the published source code always matches whatever you get from the distribution system.

Here are some simple instructions from ChatGPT-4o. Hopefully this helps...
https://chatgpt.com/share/66fcf75d-3e38-8005-8977-bfabb664a2b6

Thank you. I'll take the contents of dist/hugerte_1.0.0_component.zip, they're exactly what I need.

I still fail to understand why the TinyMCE source code has so much unneeded source code. When I do following:
$ cd hugemce/modules
$ find -type f -name "*.ts" | xargs du -cb | awk '/total$/ {sum += $1} END {print sum}'

The output is 15324143 meaning that all the TypeScript source code combined is over 15 MB! Why did Tiny Inc bother maintaining all that code when obviously only about 10% is actually needed? I guess the tree-shaking process is expected to throw about 90% of all the source code away.
And even if you exclude all test and demo code, you still have ~7.3 MB:
$ find -type f -name "*.ts" -not -path "*/test/*" -not -path "*/demo/*" | xargs du -cb | awk '/total$/ {sum += $1} END {print sum}'
7239672

But ~7.7 MB IS what hugerte.js and the contents of icons, models, plugins, skins and themes in the hugerte-dist repo actually are. (Well, these include minified and non-minified versions, but that may not matter that much.)


InternVL2-40B-AWQ+lmdeploy video infer speed  is very slow

I am using the InternVL2-40B-AWQ model and performing video inference according to the multi-images inference paradigm. Each video is sampled into 24 frames, and the prompt is as follows. My questions are:
1.When performing video inference, is the video processed according to the multi-images paradigm? Is this method correct? Is there a specific prompt format for video inference? On the official website(https://internvl.readthedocs.io/en/latest/internvl2.0/deployment.html#), I only see inference for single images and multi-images, but I haven't found inference code specifically for videos.
2.During video inference (following the multi-images paradigm) with num_segments=24, I encounter an error: lmdeploy - ERROR - Truncate max_new_tokens to 128 when session_len=8192. Through repeated testing, I found that session_len=65536 is necessary for correct inference. I want to ask, how many tokens does a single image actually consume before being input to the language model? According to this issue, the model's max_seq_length is at most 8192 (https://github.com/OpenGVLab/InternVL/issues/381), so why can I set session_len=65536? The related link is: https://github.com/InternLM/lmdeploy/issues/2382.
3.During video inference (following the multi-images paradigm) with num_segments=24, I noticed that inference with lmdeploy takes about 60 seconds, while transformers inference takes around 9 seconds. Why is this? Is there room for optimization? Why is the inference speed of lmdeploy significantly slower than that of PyTorch?
code demo
from lmdeploy import pipeline, TurbomindEngineConfig
from lmdeploy.vl import load_image
from lmdeploy import GenerationConfig
import time

target_images=["2.jpg","6.jpg","11.jpg","15.jpg","20.jpg","24.jpg","28.jpg","33.jpg","37.jpg","41.jpg","46.jpg","50.jpg","54.jpg","59.jpg","63.jpg","68.jpg","72.jpg","76.jpg","81.jpg","85.jpg","90.jpg","94.jpg","98.jpg","103.jpg"]

# 2,   6,  11,  15,  20,  24,  28,  33,  37,  41,  46,  50,  54,
#         59,  63,  68,  72,  76,  81,  85,  90,  94,  98, 103

pixel_values_list=[]
for ele in target_images:
    img=load_image(f"./image/{ele}")
    pixel_values_list.append(img)

question="Frame1: {IMAGE_TOKEN}\nFrame2: {IMAGE_TOKEN}\nFrame3: {IMAGE_TOKEN}\nFrame4: {IMAGE_TOKEN}\nFrame5: {IMAGE_TOKEN}\nFrame6: {IMAGE_TOKEN}\nFrame7: {IMAGE_TOKEN}\nFrame8: {IMAGE_TOKEN}\nFrame9: {IMAGE_TOKEN}\nFrame10: {IMAGE_TOKEN}\nFrame11: {IMAGE_TOKEN}\nFrame12: {IMAGE_TOKEN}\nFrame13: {IMAGE_TOKEN}\nFrame14: {IMAGE_TOKEN}\nFrame15: {IMAGE_TOKEN}\nFrame16: {IMAGE_TOKEN}\nFrame17: {IMAGE_TOKEN}\nFrame18: {IMAGE_TOKEN}\nFrame19: {IMAGE_TOKEN}\nFrame20: {IMAGE_TOKEN}\nFrame21: {IMAGE_TOKEN}\nFrame22: {IMAGE_TOKEN}\nFrame23: {IMAGE_TOKEN}\nFrame24: {IMAGE_TOKEN}\nIs a person in the video?Answer Yes or No in one word."

model_path = '/media/star/disk2/pretrained_model/InternVL/InternVL2/InternVL2-40B-AWQ'
#model_path = '/media/star/disk2/pretrained_model/InternVL/InternVL2/InternVL2-1B'
model = pipeline(model_path, backend_config=TurbomindEngineConfig(session_len=65536))
start=time.time()
response_l = model([(question, pixel_values_list)],gen_config=GenerationConfig(max_new_tokens=1024,top_p=1))
print(response_l[0].text)
print(f"run time is {time.time()-start}")

images
image.zip

@czczup @whai362 @ErfeiCui @hjh0119 @lvhan028 @Adushar @Weiyun1025 @cg1177 @opengvlab-admin @qishisuren123 @dlutwy Could you please take a look at this issue?

@AmazDeng
For video understand with internvl2, you can refer this docs (the video multi-round conversation part)
To see how many input token used, you can turn the logging on with pipe = pipeline('...', log_level='INFO'). For internvl2, single image with one patch will use 256 tokens. By default, a single image will have 1~13 patches depending on its aspect ratio.  Reducing the max_dynamic_patch will speed up the inference and the official video demo also use one patch for single image in video clip.
related issue https://github.com/InternLM/lmdeploy/issues/2260

In my test, InternVL2-1B (which can not turbomind backend and will fallback to pytorch backend) takes an average of 5.4s.  InternVL2-2B takes an average of 1.8s.

In my test, InternVL2-1B (which can not turbomind backend and will fallback to pytorch backend) takes an average of 5.4s. InternVL2-2B takes an average of 1.8s.

@irexyc
May I ask if you are testing the code and images that I provided before?
It takes 50 seconds for the InternVL2-1B I tested to run. What could be the reason for this?
test code
from lmdeploy import pipeline, TurbomindEngineConfig
from lmdeploy.vl import load_image
from lmdeploy import GenerationConfig
import time

target_images = ["2.jpg", "6.jpg", "11.jpg", "15.jpg", "20.jpg", "24.jpg", "28.jpg", "33.jpg", "37.jpg", "41.jpg",
                 "46.jpg", "50.jpg", "54.jpg", "59.jpg", "63.jpg", "68.jpg", "72.jpg", "76.jpg", "81.jpg", "85.jpg",
                 "90.jpg", "94.jpg", "98.jpg", "103.jpg"]

pixel_values_list = []
for ele in target_images:
    img = load_image(f"./image/{ele}")
    pixel_values_list.append(img)

question = "Frame1: {IMAGE_TOKEN}\nFrame2: {IMAGE_TOKEN}\nFrame3: {IMAGE_TOKEN}\nFrame4: {IMAGE_TOKEN}\nFrame5: {IMAGE_TOKEN}\nFrame6: {IMAGE_TOKEN}\nFrame7: {IMAGE_TOKEN}\nFrame8: {IMAGE_TOKEN}\nFrame9: {IMAGE_TOKEN}\nFrame10: {IMAGE_TOKEN}\nFrame11: {IMAGE_TOKEN}\nFrame12: {IMAGE_TOKEN}\nFrame13: {IMAGE_TOKEN}\nFrame14: {IMAGE_TOKEN}\nFrame15: {IMAGE_TOKEN}\nFrame16: {IMAGE_TOKEN}\nFrame17: {IMAGE_TOKEN}\nFrame18: {IMAGE_TOKEN}\nFrame19: {IMAGE_TOKEN}\nFrame20: {IMAGE_TOKEN}\nFrame21: {IMAGE_TOKEN}\nFrame22: {IMAGE_TOKEN}\nFrame23: {IMAGE_TOKEN}\nFrame24: {IMAGE_TOKEN}\nIs a person in the video?Answer Yes or No in one word."

model_path = '/media/star/disk2/pretrained_model/InternVL/InternVL2/InternVL2-1B'
model = pipeline(model_path, backend_config=TurbomindEngineConfig(session_len=65536))
start = time.time()
inter_num=10
for _ in range(inter_num):
    response_l = model([(question, pixel_values_list)], gen_config=GenerationConfig(max_new_tokens=1024, top_p=1))
    print(response_l[0].text)
print(f"run time is {round((time.time() - start)/10,2)} seconds")

@AmazDeng
The test_code.py is different from the code you post before. In your previous code you set max_dynamic_patch. I test InternVL2-1B and InternVL2-2B with your previous code.
I am not sure the actual patch size for each image. If the patch size is large, the time of vision model may be large.  You can enable logging by pipe = pipeline(..., log_level='INFO') and see the logs.
There will be lines like
2024-09-05 13:04:11,044 - lmdeploy - INFO - ImageEncoder received 24 images, left 24 images.
2024-09-05 13:04:11,044 - lmdeploy - INFO - ImageEncoder process 1 images, left 23 images.
2024-09-05 13:04:11,149 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.105s
2024-09-05 13:04:11,149 - lmdeploy - INFO - ImageEncoder process 1 images, left 22 images.
2024-09-05 13:04:11,251 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.102s
2024-09-05 13:04:11,251 - lmdeploy - INFO - ImageEncoder process 1 images, left 21 images.
2024-09-05 13:04:11,345 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.094s
2024-09-05 13:04:11,346 - lmdeploy - INFO - ImageEncoder process 1 images, left 20 images.
2024-09-05 13:04:11,455 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.109s
2024-09-05 13:04:11,455 - lmdeploy - INFO - ImageEncoder process 1 images, left 19 images.
2024-09-05 13:04:11,554 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.099s
2024-09-05 13:04:11,554 - lmdeploy - INFO - ImageEncoder process 1 images, left 18 images.

@AmazDeng
The test_code.py is different from the code you post before. In your previous code you set max_dynamic_patch. I test InternVL2-1B and InternVL2-2B with your previous code.
I am not sure the actual patch size for each image. If the patch size is large, the time of vision model may be large. You can enable logging by pipe = pipeline(..., log_level='INFO') and see the logs.
There will be lines like
2024-09-05 13:04:11,044 - lmdeploy - INFO - ImageEncoder received 24 images, left 24 images.
2024-09-05 13:04:11,044 - lmdeploy - INFO - ImageEncoder process 1 images, left 23 images.
2024-09-05 13:04:11,149 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.105s
2024-09-05 13:04:11,149 - lmdeploy - INFO - ImageEncoder process 1 images, left 22 images.
2024-09-05 13:04:11,251 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.102s
2024-09-05 13:04:11,251 - lmdeploy - INFO - ImageEncoder process 1 images, left 21 images.
2024-09-05 13:04:11,345 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.094s
2024-09-05 13:04:11,346 - lmdeploy - INFO - ImageEncoder process 1 images, left 20 images.
2024-09-05 13:04:11,455 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.109s
2024-09-05 13:04:11,455 - lmdeploy - INFO - ImageEncoder process 1 images, left 19 images.
2024-09-05 13:04:11,554 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.099s
2024-09-05 13:04:11,554 - lmdeploy - INFO - ImageEncoder process 1 images, left 18 images.

@irexyc
I carefully tested the two pieces of code I uploaded. The first code performs inference according to the multi-frame paradigm provided by InternVL2's official implementation（https://internvl.readthedocs.io/en/latest/internvl2.0/deployment.html）, while the second follows the video paradigm as specified by lmdeploy's official documentation（https://lmdeploy.readthedocs.io/en/latest/multi_modal/internvl.html）.

The multi-frame paradigm takes a long time for inference, up to 50 seconds. The video paradigm inference time is normal and similar to your test results. For 40B-AWQ, inference on a single video takes about 7 seconds.

By checking the log files, I found that the ImageEncoder processing time for images in the multi-frame paradigm is more than twice that of the video paradigm. Additionally, the multi-frame paradigm requires setting session_len=65536 (which is not needed in the video paradigm), causing the steps in lmdeploy to take longer.

multi-frame paradigm
2024-09-06 11:00:50,221 - lmdeploy - INFO - ImageEncoder received 24 images, left 24 images.
2024-09-06 11:00:50,221 - lmdeploy - INFO - ImageEncoder process 1 images, left 23 images.
2024-09-06 11:00:50,362 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.140s
2024-09-06 11:00:50,362 - lmdeploy - INFO - ImageEncoder process 1 images, left 22 images.
2024-09-06 11:00:50,501 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.139s
2024-09-06 11:00:50,502 - lmdeploy - INFO - ImageEncoder process 1 images, left 21 images.

[TM][INFO] ------------------------- step = 55620 -------------------------
[TM][INFO] ------------------------- step = 55630 -------------------------
[TM][INFO] ------------------------- step = 55640 -------------------------
[TM][INFO] ------------------------- step = 55650 -------------------------
[TM][INFO] ------------------------- step = 55660 -------------------------
[TM][INFO] ------------------------- step = 55670 -------------------------
[TM][INFO] ------------------------- step = 55680 -------------------------
[TM][INFO] ------------------------- step = 55690 -------------------------

video paradigm
2024-09-06 11:36:34,196 - lmdeploy - INFO - ImageEncoder received 24 images, left 24 images.
2024-09-06 11:36:34,196 - lmdeploy - INFO - ImageEncoder process 1 images, left 23 images.
2024-09-06 11:36:34,268 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.071s
2024-09-06 11:36:34,268 - lmdeploy - INFO - ImageEncoder process 1 images, left 22 images.
2024-09-06 11:36:34,338 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.070s
2024-09-06 11:36:34,339 - lmdeploy - INFO - ImageEncoder process 1 images, left 21 images.
2024-09-06 11:36:34,411 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.071s
2024-09-06 11:36:34,411 - lmdeploy - INFO - ImageEncoder process 1 images, left 20 images.
2024-09-06 11:36:34,479 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.068s
2024-09-06 11:36:34,480 - lmdeploy - INFO - ImageEncoder process 1 images, left 19 images.
2024-09-06 11:36:34,551 - lmdeploy - INFO - ImageEncoder forward 1 images, cost 0.071s

[TM][INFO] ------------------------- step = 6370 -------------------------
[TM][INFO] ------------------------- step = 6380 -------------------------
[TM][INFO] ------------------------- step = 6390 -------------------------
[TM][INFO] ------------------------- step = 6400 -------------------------
[TM][INFO] ------------------------- step = 6410 -------------------------
[TM][INFO] ------------------------- step = 6420 -------------------------
[TM][INFO] ------------------------- step = 6430 -------------------------
[TM][INFO] ------------------------- step = 6440 -------------------------

I also noticed that in lmdeploy, the image processing by the ImageEncoder seems to be done serially. If my assumption is correct, this part of the logic could actually be optimized for multi-threading using a C++ shared object (SO) file. Of course, I need to check the code to confirm if my assumption is correct.

I feel that the official instructions provided by InternVL2 might mislead users, especially for video inference. Moreover, when I used the multi-frame paradigm, the results were incorrect（there is only one person in the video!）, while the video paradigm produced the correct results. If what I'm saying is accurate, the InternVL2 team might consider revising the instructions.

prompt:Describe this video in detail.
multi-frame paradigm
The video features two women standing in an indoor setting, each holding a sign with text in Chinese. The first woman is wearing a green jacket and has long dark hair, while the second woman is wearing a yellow jacket and has long black hair. Both women are looking directly at the camera with serious expressions.

The first woman is holding a sign with the text "鸭鸭男装官方旗舰店" (Duckman's Men's Clothing Official Store) and the second woman is holding a sign with the text "鸭鸭男装官方旗舰店" (Duckman's Men's Clothing Official Store) as well. The signs have a similar design, with the text in Chinese characters and a logo at the top.

The background of the video shows a modern, minimalist interior with large, gray marble columns and some green plants hanging from the ceiling. The lighting is bright and artificial, creating a clean and professional atmosphere. The women appear to be standing in a lobby or entrance area of a building, possibly a shopping center or a department store.

The video seems to be a promotional or advertising video for the Duckman's Men's Clothing Official Store, showcasing the store's branding and the products they offer. The women are likely trying to convey a sense of professionalism and reliability, as they are both dressed in business attire and holding signs that clearly state the store's name.

video paradigm
The video features a woman standing in a modern, minimalist building with a large, rectangular sign in front of her. The sign is predominantly white with black and red text, and it reads "鸭鸭男装官方旗舰店" in both Chinese and English. The woman is holding the sign with both hands, and she is wearing a green jacket with a hood. She has long, dark hair and is looking directly at the camera with a serious expression.\n\nThe background of the video is a modern, sleek interior with large, gray marble walls and a few plants hanging from the ceiling. The lighting is bright and evenly distributed, creating a clean and professional atmosphere. The woman appears to be standing in a spacious area, possibly a lobby or a waiting area, as there are no other people visible in the shot.\n\nThroughout the video, the woman remains in the same position, holding the sign and looking directly at the camera. She does not move or interact with anything in the scene. The video seems to be a promotional or advertising shot for the "鸭鸭男装官方旗舰店," which is likely a clothing store or brand. The overall tone of the video is professional and focused on presenting the store\'s brand and offerings clearly and effectively

I am now planning to study the source code to see if I can further accelerate the inference process. I have a question: is the lmdeploy code fully open-source, or is it only partially open-source, similar to TensorRT-LLM?

1 & 2

The internvl docs doesn't set max_dynamic_patch, one patch for a image will cost 256 tokens. A image will cost 256 * 13 tokens if it has 13 patched. And if input_tokens is large, the prefill stage will cost much time. In your log, you can see large  step = 55620 as input_tokens is large.

3

By default, the vision part has running batch of 1 image. You may enlarge the default value(pipe = pipeline(..., vision_config=VisionConfig(max_batch_size=4) but currently we found that the benefits of batching vision model are not significant with pytorch backend.

1.is the lmdeploy code fully open-source, or is it only partially open-source, similar to TensorRT-LLM?

full open-source.

2.Can you provide an example of batch inference?

out = pipe([messages, messages], gen_config=GenerationConfig(top_k=1)) will do batching inference. You may see logs like [TM][INFO] [Forward] [0, 2), dc=1, pf=1, sum_q=5433, sum_k=5432, max_q=5432, max_k=5451 (batch is two)
But there are something you should know. The full pipeline of vlm is vision + llm. The vision part is time consuming if one request have many image input. If you wan't to batching two request in llm, the first request shouln't complete the llm part before the second request complete the vision part.
I recommand using multi-thread to do the batching inference with pipeline.stream_infer api and each thread do one request, the engine will automatically batch llm part if the above conditions are met. The reson why I recommand this way is that different request may need different time to complete. And if you give two requests in one function call, you need to wait for both requests to complete.

I recommand using multi-thread to do the batching inference with pipeline.stream_infer api and each thread do one request, the engine will automatically batch llm part if the above conditions are met. The reson why I recommand this way is that different request may need different time to complete. And if you give two requests in one function call, you need to wait for both requests to complete.

@irexyc
Does this sentence mean:

Using multithreading to simultaneously request a service?
Starting a thread pool in a service, with each thread handling a request (batch=1)? For instance, 8 video inferences are sent in one request, and then the model processes each video inference in 8 threads, with the final results combined? Is the model's handling of multithreaded inference thread-safe? Which interpretation is correct?
which one above is correct?

This is a sample code I wrote. I'm not sure if it's correct. If it's not, could you provide a sample code to illustrate?
example code
def infer_video(request_json_data: Dict,model):
    res=[]
    with ThreadPoolExecutor(max_workers=4) as executor:
        futures = [executor.submit(infer_single_video, video_url,prompt,model) for (video_url,prompt) in request_json_data["video_urls_prompt"]]
        for future in futures:
            res.append(future.result())
    return res

def infer_single_video(video_url: str,prompt, model):
    # infer single video
    model.stream_infer(...)

if __name__ == '__main__':
	request_json_data={...}
	model = pipeline(model_path, backend_config=TurbomindEngineConfig(session_len=8192), log_level='ERROR')

	res=infer_video(request_json_data,model)

In my opinion, using multithreading or threadpool is the same thing. The truth of the matter is that each thread processes one request
It is worth noting that in your code, you do not take advantage of the streaming feature of stream_infer api. If you doesn't need streaming output, you can just use model.(...)

In my opinion, using multithreading or threadpool are the same thing. The truth of the matter is that each thread processes one request
It is worth noting that in your code, you do not take advantage of the streaming feature of stream_infer api. If you doesn't need streaming output, you can just use model.(...)

Alright, I will conduct tests on the multithreaded code. If there are any questions, I will continue to communicate with you. Thank you for your consistent support and detailed answers.

@irexyc
I wanna do offline video batch inference with InternVL2-40B-AWQ+lmdeploy similar to @AmazDeng .
According to your conversation context, May I assume that if the streaming feature is not utilized, the speed of sending requests in parallel using multithreading is close to that of native batch inference?

@hkunzhe
The streaming feature has little impact on performance.
Using multi-thread have one advantage. Since each request has different input / output token length, It takes different amout of time. If a request is finished, we can start a new thread and process the next request. But if you using batch api, you have to wait all requests be finished before you send next batch.

@irexyc Thanks for your quick reply! BTW, when using InternVL2-40B-AWQ+lmdeploy, I noticed that
Flash Attention 2.0 only supports torch.float16 and torch.bfloat16 dtypes. No dtype was provided, you should run training or inference using Automatic Mixed-Precision via the `with torch.autocast(device_type='torch_device'):` decorator.

Will this affect the speed of inference? My environment:
lmdeploy                  0.6.0
torch                     2.3.1+cu118
transformers              4.37.2

Multi-threading is clearly faster than the native batch inference and also has a higher GPU utilization. @AmazDeng provided a nice demo code, but need to add VisionConfig(thread_safe=True) when initializing the pipeline and use concurrent.futures.as_completed to handle asynchronous tasks.

我想使用 InternVL2-40B-AWQ+lmdeploy 进行离线视频批量推理，类似于 。根据您的对话上下文，我是否可以假设，如果不使用流式处理功能，使用多线程并行发送请求的速度接近本机批量推理的速度？

我想问一下， InternVL2-40B-AWQ启动大概需要多大的显存？

使用多线程并行发送请求的速度接近本机批量推理的速度？

需要A100 80G版本，可以启动

我想使用 InternVL2-40B-AWQ+lmdeploy 进行离线视频批量推理，类似于 。根据您的对话上下文，我是否可以假设，如果不使用流式处理功能，使用多线程并行发送请求的速度接近本机批量推理的速度？

我想问一下， InternVL2-40B-AWQ启动大概需要多大的显存？

需要A100 80G版本，可以启动


attempt to fix a race where new LS starts before previous LS stopped.

From crash report, we found a possible race situation where new Pylance is started before old Pylance is disposed causing duplicate commands to be registered and error out by LSP client.
this is an attempt to fix the race by making sure LSP server stop is awaited before calling start eliminating all weird cases and we no longer assume that languge client will be reused (start/stop and start again).

by the way, does anyone know when settings.json is changed, which code path in core extension call onDidChangeConfiguration on us?

What do you mean? Who fires the onDidChangeConfiguration event?

figured out, it was LSP client itself raising the event. by the way, found issue on LSP itself as well
https://github.com/microsoft/vscode-languageserver-node/pull/929

alright, it looks like whatever fix I made here in proxy won't fix the race. it turns out unlike before (< 2022.4.x), now, when interpreter is changed, completely new language server is created (sorry, I just found out once I couldn't figure out how the state change it is having happened). so, race can't be fixed in proxy, but must be fixed in
https://github.com/microsoft/vscode-python/blob/main/src/client/languageServer/watcher.ts#L137 where it stops and create new LS. (and probably other places, I saw multiple code paths that end up restart server)
it looks like the change happened in this PR (https://github.com/microsoft/vscode-python/pull/18884)
any objection on just adding stop: Promise or make dispose async on proxy and all callers to make it simple and remove any chance of race?

any objection on just adding stop: Promise or make dispose async on proxy and all callers to make it simple and remove any chance of race?

I vote for the first option, not so sure about making dispose async for everything.

do you guys use cancellation? I don't see code path involved here use any cancellation. for example, if someone flip thorough different version of interpreters, in current form, every selection must start server and stop them one by one. rather than all the intermediate one gets cancelled if server is not started yet and only last selection start server?

@kimadeline can you take a look?

Nope, we don't use cancellation, although it would make sense to use it. Do you think that would be a big change to the current code?

Also, there will be merge conflicts depending on which PR gets merged in first between this one and https://github.com/microsoft/vscode-python/pull/19087.


Can flutter access device heading & pitch, similar to how Google Sky Map works?

I'd like to access the devices heading and pitch data similar to how Google Sky Map works. Is Flutter able to access that data (with user permissions of course)?
I'm sorry for asking this in the issues forum, but Flutter is new enough that I can't seem to find anything about this already out on the net.
Thank you!

If there is a plugin in https://pub.dartlang.org that provides that, you can use that or
you can make native functionality available to flutter by
https://flutter.io/developing-packages/#plugin
Please consider asking support questions in one of the other channels listed at http://flutter.io/support .


RunwayRoutes trait throws a "Maximum call stack size" exception

Description
If a model uses the trait RunwayRoutes, an existing (also a not existing) attribute slug cannot be accessed without crashing the entire application.
With PHP 8.2.15 the application simply exits without an error message.
With PHP 8.3.0 the exception  Error  Maximum call stack size of 8355840 bytes reached. Infinite recursion? is displayed.

Steps to reproduce

Install a fresh new Statamic project with the latest version 4.47.0
Install the latest statamic-rad-pack/runway package with composer (6.0.5)
Add the RunwayRoutes trait to the existing User model
Start Tinker and instantiate a new user with $user = new App\Models\User()
Call $user->slug

Environment
Environment
Application Name: Statamic
Laravel Version: 10.43.0
PHP Version: 8.2.15 / 8.3.0
Composer Version: 2.5.5
Environment: local
Debug Mode: ENABLED
URL: statamic-runway.test
Maintenance Mode: OFF
Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: NOT CACHED
Drivers
Broadcasting: log
Cache: statamic
Database: mysql
Logs: stack / single
Mail: smtp
Queue: sync
Session: file
Statamic
Addons: 1
Antlers: runtime
Stache Watcher: Enabled
Static Caching: Disabled
Version: 4.47.0 Solo
Statamic Addons
statamic-rad-pack/runway: 6.0.5

Are you using the {{ nocache }} tag anywhere?

No, I'm not using any {{ nocache }} tag.
I'm using Laravel Herd and installed with the statamic/cli a fresh new Statamic project. The only installed Addon is statamic-rad-pack/runway.
Without configurating anything I've created a new Model with only these lines of code:
<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;
use StatamicRadPack\Runway\Routing\Traits\RunwayRoutes;
use StatamicRadPack\Runway\Traits\HasRunwayResource;

class Dog extends Model {
    use HasRunwayResource, RunwayRoutes;
}

The runway configuration has only one resource:
<?php

return [
    'resources' => [
        App\Models\Dog::class => [
            'name'        => 'Dogs',
            'route'       => '/dogs/{{ slug }}',
            'template'    => 'dogs/show',
        ],
    ],
    'disable_migrations' => true,
];

I open Tinker straight after that:

In my real project I do have a slug column, but here in my example with the freshly created Statamic project and the Dog class, there is no migration. But in both cases I'm getting the exception.

Do you get the same error when creating models with the Dog::create([]) syntax?

If I try to create the Dogmodel with Dog::create([]) then I get a different error:

I've tested it now with Laravel Sail, to check if Laravel Herd is the problem. But the same exception 😞
It's really strange. Especially as it's a completely fresh Statamic installation.

After some tests I found out that the exception only occurs if the slug is unset. If it has a value, everything's fine.
In my case the user has the possibility to set the slug manually. If it's not set, in the life cycle event creating the value is generated by another field, e.g. title.
But the content of the slug field cannot be checked, because if it's empty, the exception is thrown.
I have to use this workaround: Get all attributes with toArray() or getAttributes() and check if the slug is set 🙃

Thanks for doing some digging!
Runway's RunwayRoutes trait contains a slug() method (which is required by Statamic's routing contracts).
The slug method attempts to get the current slug for the model, using Eloquent's getAttribute method. Since, it can't find a value for slug on the model, it finds that there's a slug() method on the model and tries to use that instead. That's where the infinite loop happens.
We do need the slug method in the RunwayRoutes method or the feature wouldn't work at all. However, it obviously needs to be re-worked to avoid the infinite loop. I'll have a think about the best way to fix this 🤔


THRIFT-4395: fix rust build on xenial

I found this while doing due diligence on another issue (C++ HTTP) - I could no longer run cross test because builds fail in lib/rs.

shoot... will resubmit without THRIFT-3877


Add analyzer of C/C++ project source and build files

Example scenario:

Developer wants to use the “foo” library.
However, to make it “easy to install” and not depend on the version (if any) installed on the system, she creates a “foo” subdirectory and copies the entire foo library into that subdirectory.
Further, the “foo” subdirectory has a “configure.in” file, and that file tells us the version of foo.
Then, her top-level build system will cd into the foo library, and build, when needed.  (Typically with a recursive make).  The final executable will the embed the “.o” file in this “foo/” subdirectory.

The proposal here is to create an Analyzer that that is able to scan for some common ways of embedding version information in C/C++ build files. Further, items like the license and copyright comment headers can be mined for name and vendor information.
I'm planning on implementing this in a little while.

I know several people have asked for C/C++ support. I think this area of research is definitely worth-while. However, I am concerned about the false positive rates an analyzer like this might produce.
--Jeremy


Don't maintain full brightness when viewing Info

Is your feature request related to a problem? Please describe.
When the Settings > Viewer > Maximum Brightness is active, max brightness is maintained even when viewing the item's Info page (which takes up the entire screen), making it painful to read.
Describe the solution you'd like
Even when the Maximum Brightness setting is active, the Info page should be displayed at system brightness
Describe alternatives you've considered

Disabling Maximum Brightness completely, which has obvious downsides.
Squinting

Squinting

Haha, nice.


Add fields for accepting redirect url

xendit:
success: http://example.com/imrenagi/calendar-slug/events/event-id/payments_success
failed: http://example.com/imrenagi/calendar-slug/events/event-id/payments_success
midtrans: http://example.com/imrenagi/calendar-slug/events/event-id/payment_statuses

done with snap and xeninvoice. next: check every wallet implementation


Remove duplicate FROM in job exporter Dockerfile

Remove duplicate FROM in job exporter Dockerfile.

Coverage decreased (-0.01%) to 53.244% when pulling 31fbe1446a253faacd2a07c1e52132afe86e7b28 on xiongyf/fix-job-exporter into 4edb3054de0256de2cdd615f8fb1df3de2c5d393 on master.

Coverage decreased (-0.01%) to 53.244% when pulling 31fbe1446a253faacd2a07c1e52132afe86e7b28 on xiongyf/fix-job-exporter into 4edb3054de0256de2cdd615f8fb1df3de2c5d393 on master.


agent: Allow writing CNI configuration when ready

This can be used to generate the CNI configuration in a location outside of the
cni-conf-dir and then copy the CNI configuration into the config directory once
the agent is up. This avoids the node being marked ready before the agent is
ready to schedule pods.

This change is

Coverage increased (+0.03%) to 41.987% when pulling 96d4dc1cf1cedff7a73c80727c6f66c680403f48 on pr/tgraf/write-cni-conf into 5337067cbf52d39039329db4f33dc6359ff225e5 on master.

test-me-please


Define new component: Content Navigation

Checklist

[X] Name
[X] About this component
[X] Description
[X] Examples (ca.gov)
[ ] Examples (design systems)
[X] Features
[X] Requesting agency

Name
What should we call this component in conversation?
Content Navigation
About this component
Please tell us more about the component.
The content-navigation is a stand-alone web component that accepts a target selector, finds any h2-h6 tags, and builds a list of anchor links. If tags to not have ids, ids are created dynamically so that on clicking the link, it will scroll to that section.
Description
Describe how this component will be used.
This component will be used to help page viewers find content on the page.
Examples from ca.gov properties
(Can be screenshots, a few examples is great. Please include URLs if the examples are live.) This helps everyone understand what you are referring to.
The covid19.ca.gov website used jump links at the top of pages to help reads find relevant information.
Examples from other design systems
If this component is commonly found in other design systems, please link to it. This helps everyone understand real-world examples of this idea.
Not aware of any, requires research.
Features
Critical features to support initially.

Read page headers and build links to the headers

Requesting agency
Who is requesting this component?
Office of Digital Innovation & Department of Cannabis Control

cannabis.ca.gov trackback: https://github.com/cagov/cannabis.ca.gov/issues/53


Don't create duplicated genotype on Chado load

Example of a duplicate:
select g.name, g.uniquename, a.name, a.uniquename
  from feature g
  join feature_relationship r on g.feature_id = object_id
  join feature a on r.subject_id = a.feature_id
where g.type_id in (select cvterm_id from cvterm where name = 'genotype') and a.name = 'prp10delta';

@kimrutherford is this high priority?

is this high priority?

It's not a big deal so not high priority.  It isn't causing problems for Ensembl loading, it's just a bit ugly.

Val provided a good example in #611.

This is still not fixed.  I'm going to leave it until we decide how we plan to store genotype background.

Is this still current?

Is this still current?

Yes, it's still a problem.

pinging this because it seems related to more recent issues...

pinging this because it seems related to more recent issues...

Which related issues?
This is only a problem in Chado.  The website code should merge all identical genotypes for display.

I think this isn't worth fixing since it's an edge case in Chado only.  The website is fine.
Also the example in the first comment:
https://github.com/pombase/pombase-chado/issues/497#issue-107835012
isn't a duplicate because the background is different.


Teams Admin API

List all teams
See members of each team
Assign users to teams
Remove users from teams
Create teams

Requested features were implemented as part of the auth v2 project. Closing


uniextract2: Add version 2.0.0-rc.2

Official homepage for UniExtract2: https://github.com/Bioruebe/UniExtract2
Please let me know if there are any problems. Thanks.

In general, do we want to allow -rc builds? If so, perhaps we should modify scoop, instead of processing them manually?

For UniExtract2, we need RC build, since its stable version (maybe 2.0.0) is still not within the foreseeable future.


Add parameter checking for login command

This change introduces parameter checking for the login command.
If the parameters are not as required, an error message is printed
to STDERR and the exit status is set to 1.
The checks performed are:

Ensure that at least one parameter (the URL) is provided
Ensure that a password is provided if the username is provided

This implementation makes use of panic and recover within the
commands.(*Login).Run function.  However, it may be better to use
a globally defined error reporting function to ensure consistency
of handling for all of the commands.

LGTM, merging


Bump k8s dependencies to 0.27.7

This is the latest release in the 0.27 branch, which is the branch we're tracking for 2.9.

/cc @tpantelis @dfarrell07 @vthapar


Fix fragment arguments

(fix created with @EuAndreh)
Closes https://github.com/alumbra/alumbra/issues/15.
@xsc should I make a PR on alumbra/alumbra bumping to this version?

@mverardo Thanks a lot! I made some adjustments – mostly ensuring that only the variables actually available via operation-name are processed – and pushed your changes to master (ceb2dd24). I'll release momentarily and bump the dependency at the main repo.
May I ask how you became aware of alumbra? I haven't done a lot of PR, so I'm intrigued that people seem to be trying it out already.
Also, I'll need to release a stable version soon to at least have a changelog accompanying potentially breaking changes.

@xsc no problems!
I tried out a couple of graphql implementations in clojure for a personal project, and found alumbra to be the codebase I could understand the best.
A stable version would be really helpful!


task build-run dependencies installation

Running task build-run does not seem to launch the npm install command for the web-frontend folder
Steps to reproduce the behavior:

Run task build-run on a computer that does not have the required dependencies

Expected behavior
Npm install should be ran
Screenshots

WalletVersion
Latest
Desktop (please complete the following information):

OS: Windows 11

@ASAPSegfault npm install is a go:generate command, to execute it, please run task generate


removing files, moved to wiki and the pipeline folder

unfortunately the wiki is not great for organization

The title of the Pull Request needs to be changed.
Please follow https://www.spinnaker.io/community/contributing/submitting/#commit-message-conventions

The title of the Pull Request needs to be changed.
Please follow https://www.spinnaker.io/community/contributing/submitting/#commit-message-conventions


OS import error

Missing import os from score_explanations.ipynb (https://github.com/EleutherAI/sae-auto-interp/blob/main/examples/score_explanations.ipynb)

Thank you, fixed


[FEATURE] Add support for app_home_opened

Description of the problem being solved
Hello! I was curious if there is a plan to support Slack App Home's in workflow automation.
Alternative solutions
I assume it's not as simple as just adding the app_home_opened event, as it would also require enabling the app home in the app's feature configuration.
Requirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

Hi, @JVenberg! Thanks for submitting this request 😄
As far as I know, there are currently no plans on the roadmap to add in this functionality for Automations, as mentioned in this issue (https://github.com/slackapi/deno-slack-sdk/issues/182). However, it is definitely something that has been requested and I know it's been in discussions, but doesn't seem like there are any plans to tackle it as of right now. In the meantime, I recommend using a regular App with the app_home_opened event to accomplish what you need.
It would be helpful to get your insight into what use case you're trying to use the app home for in automations - this can help give the team insight for future plans :)

Since we're already tracking this in https://github.com/slackapi/deno-slack-sdk/issues/182, I will close this issue. Feel free to post additional comments / use cases / needs in that issue thread.


Add a menu for extension commands

https://github.com/Microsoft/vscode/pull/6000 introduces a new top level menu for extension commands. We should discuss if such a menu is desired or not.

@Microsoft/vscode for input

my 2 cents: I find this menu a workaround for not being able to allow extensions to fill in commands anywhere in the top level menu. I would rather want to provide explicit API so that extension writers can hook into existing menus. It is up for debate if we would want to allow for creating new top level menus or not.

duplicate of #365