PivotTable corrupts Excel file when it contains ~50000 records, it is not if it contains ~25000 records

Hello! First of all, thanks for the excellent library.

Description

Steps to reproduce the issue:

Generate an Excel file with 25000 or 50000 records of data * 13 columns. I inserted the whole thing using NewStreamWriter.
Use AddPivotTable to create a Pivot Table based on existing data.
Open the Excel file.

Describe the results you received:

While opening Excel with 25000 data, it works just fine.
While opening Excel with 50000 data, Excel says the document is corrupted. Excel asks to repair it, but when successfully repaired, it actually removed the Pivot Table.

Describe the results you expected:
No matter the data size, or at least the limitations specified by Microsoft, the Excel file should be able to be opened normally.
Output of go version:
go version go1.21.1 darwin/arm64

Excelize version or commit ID:
github.com/xuri/excelize/v2 v2.8.0

Environment details (OS, Microsoft Excel™ version, physical, etc.):
OS/Hardware: MacBook Air M1, Mac OS Sonoma
Excel: 16.77.1 from Office 365

Thanks for your issue. There are two kinds of functions in the excelize library: normal mode functions and stream mode functions. The AddPivotTable function is a normal mode function, and the NewStreamWriter is a stream mode function. These two kinds of functions can not be used mixed. The stream mode function is used to generate a worksheet with the amount of data in lower resource usage, this library doesn't support create pivot table in stream mode currently, so please set the cell data with normal mode function SetCellValue or SetSheetRow instead of the stream mode function.

As the documentation says, the normal mode function and stream mode functions can not be used mixed. If you reopen the workbook generated by the stream mode function and then use the normal mode function for this workbook, the memory usage is almost the same as using the normal mode function, so the stream mode generation is no longer required.

Yes thank you for your advice. I just tried my own method above and it is solving my problem. However I will considering using normal SetCellValue/SetSheetRow as you suggest if I want to take memory consumption into consideration.
Thanks!


Auto retry for "The specified table is being deleted. Try operation later."

We are using the latest version.
If would be great if ASE could retry automatically if it encounters a "specified table is being deleted" error e.g. during a paste of a table to another storage account.

Please provide the following when posting new issues:

Storage Explorer version
Platform
Architecture
Regression
Repro steps
Expected behavior
Actual behavior

Closing due to lack of response. Please open a new issue if you are still having problems.


Renaming a file does not update imports

Repo: https://github.com/andraaspar/vue-lang-tools-rename-issue
VS Code 1.83.0
The only extensions installed are:

TypeScript Vue Plugin (Volar) v1.8.18
Vue Language Features (Volar) v1.8.18

I have created a new project using yarn create vue and answered Yes only to TypeScript.
I have the following 3 files:
renameMe.ts
export function renameMe() {
  return 'Hello World!'
}

ThenCheckMe.vue
<script setup lang="ts">
import { renameMe } from './renameMe';
</script>

<template>
  {{renameMe()}}
</template>

thenCheckMeToo.ts
import { renameMe } from './renameMe';

export function thenCheckMeToo() {
  return renameMe()
}

I right click renameMe.ts and select Rename..., and make it renameMe2.ts.
If TypeScript and JavaScript Language Features is enabled
ThenCheckMe.vue becomes:
<script setup lang="ts">
import { renameMe } from './renameMe';

</script>

<template>
  {{renameMe()}}
</template>./renameMe2

❌ Note the import did not change, but the new file name was appended to the final line.
thenCheckMeToo.ts becomes:
import { renameMe } from './renameMe2';

export function thenCheckMeToo() {
  return renameMe()
}

✅ This is correct.
If TypeScript and JavaScript Language Features is disabled (takeover mode)
ThenCheckMe.vue becomes:
<script setup lang="ts">
import { renameMe } from './renameMe';

</script>

<template>
  {{renameMe()}}
</template>

❌ Note the import did not change.
thenCheckMeToo.ts becomes:
import { renameMe } from './renameMe';

export function thenCheckMeToo() {
  return renameMe()
}

❌ Note the import did not change.

Still an issue

For the first case fixed by https://github.com/volarjs/volar.js/commit/d4fbbd7d9cbbbdd8b12981b7ce05da85b2e24c3e.
The second case may be due to your vue.updateImportsOnFileMove.enabled setting not being enabled, which will be enabled by default in the next version.
Thanks for the report!


ISPN-16716 New client should separate CompletableFuture from HotRodOp…

…eration
https://issues.redhat.com/browse/ISPN-16716

Thanks, @wburns!


refactor(ast)!: replace Modifiers with declare on Function

[!WARNING]
This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

#3844  👈
#3841
#3839
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Boshen and the rest of your teammates on  Graphite


evaluator: incorrect behaviour when choosing defaults

What version of CUE are you using (cue version)?

$ cue version
cue version v0.0.0-20240712164527-719893f23850

go version go1.22.3
      -buildmode exe
       -compiler gc
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS linux
             vcs git
    vcs.revision 719893f23850172d224720e6d1257586179ac895
        vcs.time 2024-07-12T16:45:27Z
    vcs.modified false
cue.lang.version v0.10.0

Does this issue reproduce with the latest release?
Yes
What did you do?
! exec cue export x.cue
stderr 'b: 2 errors in empty disjunction'
stderr 'b: conflicting values 4 and 1'
stderr 'b: conflicting values 5 and 1'

-- x.cue --
a: *5 | int
if a == 5 {
    b: 1
}
b: a | *4

What did you expect to see?
A passing test (for some variation perhaps of the error messages).
What did you see instead?
> ! exec cue export x.cue
[stdout]
{
    "a": 5,
    "b": 1
}
FAIL: /tmp/testscript4268956037/repro.txtar/script.txtar:1: unexpected command success

The output of b: 1 cannot be correct given that a: 5, and b is constrained such that b: a | *4.
To confirm: the exact same behaviour is seen with CUE_EXPERIMENT=evalv3, i.e. the bug exists in both old and new evaluator implementations.
Another observation: there is nowhere in the spec where we can refer to try and reason about what the behaviour should be here.

So, I think we have this situation, following the details of https://cuelang.org/docs/reference/spec/#default-values
a: int | *5    <int | 5,  5>
b: a | *4      <int | 4 | 5, 4 | 5>

a resolves to 5. The problem seems to be what then happens: presumably at this point we should actually do:
b 🡒 <5, 5> & <int | 4 | 5, 4 | 5> 🡒 <5 & (int | 4 | 5), 5 & (4 | 5)> 🡒 <5, 5>

but I think this step isn't happening. I.e. we don't seem to be tracking that b depends on a, at least not sufficiently.
The result is that we pass into the if a == 5 block and add a <1,1> unification without having eliminated the int. Consequently we get:
b 🡒 <1, 1> & <int | 4 | 5, 4 | 5> 🡒 <1 & (int | 4 | 5), 1 & (4 | 5)> 🡒 <1, ⊥>

whereas we should be attempting <1, 1> & <5, 5> which should lead to ⊥.

My hunch is that the following happens.
Some time after evaluation starts, we realise can't make any more "progress". At that point, we have the following facts (I leave out the if comprehension on purpose to focus more on what we know about a and b):
a: *5 | int
b: a | *4

At this point, my intuition is that what we should do is choose a default for a but not b, and leave the "reference" from b to a intact. Reason being, a is "not done yet" and the existence of the a to b dependency means that after choosing defaults we might know more. I can't see anything in the spec however which would confirm this rule of ordering however.
Instead what I think currently happens is that we "erase" the reference to a too early (much like you suggest). Indeed that must happen before we have chosen the default for a because otherwise we would know that a: 5. So we end up with, in effect:
a: *5 | int
b:  *5 | int | *4

Then the default for a is being chosen leaving us at:
a: 5
b: *5 | int | *4

At which point we can evaluate the original if comprehension which leaves us at:
a: 5
b: *5 | int | *4
b: 1

This reduces down to:
a: 5
b: 1

without us even needing to revert to choosing a default for b.
Even if something else is happening here, I think the lack of spec clarity with respect to when defaults are chosen and when references are "erased" is a problem.
Here is an interesting follow-up example on the sequencing of default selection:
c: *5 | d
d: c | *4
if c == 5 {
	d: 1
}

In this case we should again get an error as the result, but we would choose defaults for a and b at the same time. i.e. we would get to a point in evaluation where we know:
c: *5 | d
d: c | *4

and not be able to make any further progress. Because of the mutual references, neither default selection orders before the other, so we choose defaults for both simultaneously. At which point we know:
c: 5
d: 4

Now we can evaluate the if comprehension and end up with:
c: 5
d: 4
d: 1

and hence error.

Even if something else is happening here, I think the lack of spec clarity with respect to when defaults are chosen and when references are "erased" is a problem

For me, this is the key problem: it is not clear (to me?) what b: a | *4 means: it could mean that b gets a deep copy of a before a has been evaluated at all. I think this is similar to referencing a definition - you're copying it and each copy is unrelated and so can be unified in different ways. I believe that if this is the case, you can get to the a: 5, b: 1 result. Alternatively, b: a | *4 is just a reference to a's disjunction and so a and b share part of the graph, and so any evaluation of that graph should affect them both. In which case the only possible result (I think) is a: 5, b: 5.

In which case the only possible result (I think) is a: 5, b: 5.

... but clearly, if a is 5, then b must be 1 (because of the if) and hence the above is not a valid result.

Alternatively, b: a | *4 is just a reference to a's disjunction and so a and b share part of the graph, and so any evaluation of that graph should affect them both.

FWIW my understanding is that this is (or should be) the case.
In support of this, consider:
#x: {
	a: *5 | int
	b: a
}

x: #x
x: a: 1

If b just made a deep copy of a, then I think that it would end up as 5, because it wouldn't be influenced by the eventual value of a. But it ends up as 1, which seems right to me.
In general if I see a reference, ISTM that it should always refer to the final value of the thing referred to.
Anything else seems like it's going to be less intuitive and useful.
Thus I believe that the original example should be an error of some kind (whether conflict or cycle I'm not quite sure).

In which case the only possible result (I think) is a: 5, b: 5.

... but clearly, if a is 5, then b must be 1 (because of the if) and hence the above is not a valid result.

Ah yes, sorry - too sleepy this morning and I'm leaving out the if ... when it suits me.

With the exception of references to definitions, yes?

No. Definitions are special only in that they recursively close their value on use of the definition, i.e. a reference to it. Otherwise, all fields (regardless of definition or not, hidden or not) unify in the same way, and a reference to a field is to the resulting/ultimate constraints "gathered" by that field.
I think one of the confusing points with this issue as I mentioned in https://github.com/cue-lang/cue/issues/3296#issuecomment-2246898444 is that it appears that a "copy" of a is happening in the reference from b before we get to the ultimate value of a. But that's exactly the bug: we shouldn't be doing that.

Yep, after much discussion and thought, I'm happy I'm in a less confused state wrt definitions etc.

Defaults are not picked by taking a reference. So b is equal to 5 | int | *4, which after unifying results in 1.
Binary operators resolve a references to a scalar, so in this case the reference to a is resolved to 5.

@rogpeppe

In general if I see a reference, ISTM that it should always refer to the final value of the thing referred to. Anything else seems like it's going to be less intuitive and useful.

This is definitely not the case. The & and | operations would not be very useful if this were the case. For instance, you would not be able to override a default value. In general, a reference results in a local copy of what it refers to. Logically at least. There are optimizations such as structure sharing to prevent actual copying.

Not to properly respond to any of the points above, rather just note down an observation based on my reading of the thread: namely that if I see:
b: a

there are at least two possible "interpretations" for the reader:

We are templating b from the constraints gathered against a
We are constraining b to be the value of a, where the phrase "the value of a" to mean the observed value of a, i.e. the result of cue export.

Per Marcel, the spec and implementation is currently adhering to 1.
My observation is that because of these two "interpretations" for the reader, then we at least have a visual ambiguity as to what it means to write b: a. An ambiguity that is compounded/made more confusing by b: a + 0 doing something else.
As such, I'm wondering if we need to tease apart these two situations with different syntax/other.
I leave this very much as an incomplete thought/comment... mainly to not forget where I got to so that I can come back to it later.


Showcase open source projects we are contributing to

@eskombro did a PR on a DO repo if I'm not wrong 😇

I did this issue and solved it in this PR
Is this the one you have in mind @curquiza ?

I only made a PR on https://github.com/actix/actix-web
@Kerollmops, @MarinPostma ?

Hum... there is a lot of PRs for me but here are the biggest ones: roaring-rs (doing set operation fast)!, fst (to store a lot of keys in a small amount of space, ordered), heed (even if its mine, a Rust wrapper around one of the most popular key-value store: LMDB), and Rust itself.

Should be added at https://github.com/meilisearch/devrel/blob/main/opensource.md

Now at https://www.notion.so/meilisearch/b59f89b748e84644bb089a52f0ca7513?v=a6c6db68f86e4fb98ffacf6ee4fe478e


LF-50: Integrate LiFi Contract with nxtp on staging

When activated by specifying supported chains in REACT_APP_LIFI_CONTRACT_ENABLED_CHAINS_JSON, the returned routes will be checked if they can be executed using our contract. The user then only has to submit a single transaction to perform swaps and a nxtp based transfer.

.

Merged in new implementation


GEOMESA-2956 Set deserialize charset in CqlTransformFilter

set the charset to UTF-8 in CqlTransformFilter#deserialize instead of using the JVM's character set as the same
as CqlTransformFilter#serialization.
Signed-off-by: xxxxs<EMAIL_ADDRESS>

merged as 00dbdf4c5dd327936b7491c4e52c1fa7137067ba


Multiple entries on TitleDB

I just updated and when I go to TitleDB to get some new games or apps and stuff it shows multiple versions of the app. I can count 33 TWLoaders on this which I can tell is not normal at all. My friend says it's because old versions are being shown and if that's the case I hope it gets fixed.

It's clearly being fixed look at commits

This has already been taken care of.


Proposal: Directory Restructuring

Hello everyone,
this is a proposal for the directory restructuring. I did most of it last week, so it's not 100% aligned with the main branch. Also there are probably still some minor mistakes in it, so don't expect you will be able to run everything after executing the script. But the general idea, where all the folders would be moved to, should be clear.
Please have a look at the proposed directory structure and tell me what you think. You can also always copy the script into your project root directory and run it yourself to see the changes.
Signed-off-by: Dominik Pinsel<EMAIL_ADDRESS>

Is this still a thing? It could be closed now I suppose.


[Core] Add case-insensitive String::containsn

Realized the potential need for this when reviewing:

https://github.com/godotengine/godot/pull/91598

There aren't extremely many cases, and not all String methods have a case-insensitive version, but I felt this one was a prime candidate.
There were a few cases of to_lower().contains cases that I left alone as they already operate on a lowe-case search term so probably faster to use that format
~Optionally converted some uses of find with contains where applicable, for readability~
Separated these to their own PR
~Edit: Will add exposing the method in an optional commit later today~

Thanks!

Thank you!


FEAT: MAKE SOCIAL ANCHORS ACCESSIBLE ON FOOTER 💡

Is your feature request related to a problem? Please describe.

This feature is related to the aspect of accessibility problem

Describe the solution you'd like

The solution is to add important attributes like aria-label for social anchors (links like Github) which are placed on Social media section.

it is important to note that we want to add the title attribute also. This is because the title attribute is intended to provide additional information to screen reader users.

The aria-label attribute is a way more important for providing accessibility information to screen reader users. The aria-label attribute is an Accessible Rich Internet Applications (ARIA) attribute that can be used to provide a label for an element. This label is then read aloud by screen readers, providing additional information about the element to the user.

In general, the aria-label & title attributes are preferred for providing accessibility information to screen reader users.

You can consider to assign this issue to me to make the site accessible for screen readers

Describe alternatives you've considered

There's no alternative's for accessibility improvements

Additional context

I'm a GSSoC`23 Contributor

Code of Conduct

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

Hey there @AshAman999 👋🏻,
Please make sure to assign this issue to me as soon as possible to make the work done quickly.
Thank you!


[CI] Add jobs to test static frameworks

Testing...

/azp run

Marking as draft till this is actually worked on.


Cmake fails to fail for new TEST_GROUP() on Travis CI

See here: https://travis-ci.org/arstrube/cpputest/builds/61118786
I added a test group Utest with a failing test to UtestTests.cpp in my fork, in order to figure out an unrelated problem with the AppVeyor build.
The Cmake build does not fail, apparently because the new test group is not included in the list of test groups, and thus not run.

Oh excellent, less build failures :)

hehe.

@mmidor any idea?

I created PR #640 with two tests that fail because of this. (The problem actually is that the name of one test group is part of the name of another one.
The same thing can happen (though far less likely) with Group.name as well.

TestRegistryTest.cpp:384: error: Failure in TEST(TestRegistry, shouldListTestNamesWhereOneIsContainedInAnother)
expected <MyTest.test HisTest.test Test.test>
but was <MyTest.test HisTest.test>
difference starts at position 24 at: <sTest.test >
^
TestRegistryTest.cpp:350: error: Failure in TEST(TestRegistry, shouldListGroupNamesWhereOneIsContainedInAnother)
expected <GROUP_1 GROUP>
but was <GROUP_1>
difference starts at position 7 at: < GROUP_1 >
^

@mmidor -- is there actually even a chance that the same group or group.test occur more than once in the TestRegistry? If not, you could simply omit the test for contains().
If there is the chance, might it not be better to run the same group more than once, rather then omitting it altogether?

Test groups can cause some trouble for junit I know, because if a group isn't contiguous, then the file gets overwritten.  However, I think that for a given group.name, there's no way that you could ever have a duplicate without causing a linker error.
A while ago there was discussion over breaking out the test groups into an actual class, which would help junit I know and might help this.  If the test group is a class that then executes its tests, we would know that a group will always be executed as a single block instead of intermixed with other tests from other groups.  I suspect it would also mean that a group can only be registered to test registry once since otherwise it would cause an error with a type mismatch.  Granted, namespaces may cause some unexpected corner cases with that.

Hmm. We know that any given group.name will be unique. But I am actually not so sure what happens if you define a test group in two compilation units, perhaps each with different test names. So maybe we need to play safe, after all.

Yes, I made this change on a later stage because I noticed I have groups listed more than once.
I don't remember details now...
But your observation about contains() is correct - this conditional should be corrected.
Now, if group named Group1 was registered first, the Group12 might be omitted.

I'll take your word for it. Any idea on your side how we can deal with the "containment" issue?

Please, take a look at #642

Neat. Thanks :-)

Your welcome :) Sorry for delay, I didn't realized quickly what is all about

This is fixed as #642 has been merged.


Update README.md

Updating to reflect change for 1.5 being default version

ping @NOX73 this seems like an easy merge

👍


glReadBuffer problem

hi dear author ,sorry to disturb you again.
when I run bgfx code on android platform, it's should be use openglEs platform render it.
but the app will crash after  call the readTexure method due to
if (!BX_ENABLED(BX_PLATFORM_EMSCRIPTEN)
					&&  !BX_ENABLED(BX_PLATFORM_IOS))
	{
			GL_CHECK(glReadBuffer(GL_COLOR_ATTACHMENT0) );
	}

I have searched the opengses doc but can't found the method can be used.
I remember it could only be used on OpenGL not OpenglEs,  is right ?
can you tell me why ?
Is it should be this ? I can use the readTexture method after change it as below.
if (!BX_ENABLED(BX_PLATFORM_EMSCRIPTEN)
					&&  !BX_ENABLED(BX_PLATFORM_IOS)
                                        &&  !BX_ENABLED(BX_PLATFORM_ANDROID))
	{
		GL_CHECK(glReadBuffer(GL_COLOR_ATTACHMENT0) );
	}

or

if (BX_ENABLED(BGFX_CONFIG_RENDERER_OPENGL))
	{
		GL_CHECK(glReadBuffer(GL_COLOR_ATTACHMENT0) );
	}

Brs

can you help check this


Feature request - detect Chromecast

Currently Chromecast is detected as Chrome Linux.  It would be helpful to have it detected as Chromecast, at least in the OS.  If there are no objections, I'll create a PR.

Here's a sample UA string: 'Mozilla/5.0 (X11; Linux aarch64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.84 Safari/537.36 CrKey/1.22.74257
The CrKey part denotes Chromecast.

Added upstream in ua-parser/uap-core#211 .  I'll create a PR to update useragent now.

Cool, closing this issue.


Error message

Where else have you looked for answers to this question? Github
Share your question with as much detail as possible? When we try to take a quests on a pnj, we have this message : "The quest may not be started via command." We have the latest release of plugin quests, our server is 1.13.2 with paper.
Thank you in advance for your answer.

Not sure what a "pnj" is, but make sure allow-command-questing: true is set in your config.yml file.


feat: optimism config review

Description
Optimism config review

Review Error for AshishYelekar @ 2023-08-24 05:02:24 UTC
User failed mfa authentication, public email is not set on your github profile. see go/mfa-help


Client connection and performance issue, while Neo4j Desktop Browser has no issues

Describe the bug
Server Log: Fatal error occurred when handling a client connection, remote peer unexpectedly closed connection
Client Exception: A connection attempt has failed because the remote station did not respond correctly after a certain period of time, or the established connection was faulty because the connected host did not respond [translated from German exception]
Queries are very slow or do not execute at all with the exception above.
Server has no indication of performance or connection issues. Access over Neo4j Desktop Browser is working as expected.
Versions:

Neo4jClient 5.1.15
Neo4jDriver 5.18.1
BoltGraphClient
.NET 4.8
Neo4j Server 4.4.2

To Reproduce
Cannot give you any advice to reproduce.
Expected behaviour
Stable and fast connection.

Hi.
Whilst I appreciate this is annoying, there is absolutely nothing I can do with this.
Without a test db, example queries, expected performance etc there is literally nothing that can be resolved.
Feel free to reopen with a way to reproduce, or with expected timings based on your code.
All the best (& sorry!)
Charlotte


Pin frontend package versions

Make sure frontend package versions are pinned. Also update the "app running" message for NextJS 13.5.4

I'm testing this with the main branch of reflex-web and am consistently getting a hydration error.

Agreed I've been seeing that hydration error today even without my PR, we will have to root cause it before the release.


[WIP] More tests

retest this please

Closing for now, will leave branch around for when I write more tests


Add e2e tests with jest and puppeteer

Fixes #215. This should help us make sure that certain critical flows (such as user signup, login, export etc.) continue to work and don't regress.

This isn't a priority right now -- and what's set up in #384 will give us pretty good coverage on the frontend part.


Add a UI element to make "attach" easier.

The experience of attaching the debugger to a running process is not great.  (For that matter, neither is starting the process separately first...)  It requires fiddling around in the launch config, which isn't an obvious step (at least not for newer VSC users).
When I tried attach I was expecting to find a UI element (e.g. button, command) that would trigger a dialog box where I would enter the info needed for attach.  I found no such thing.  Instead I had to figure out about the launch config and puzzle out what info I needed.  Having the expected UI element would have improved my experience.
Environment data

VS Code version: 1.22.1
Extension version (available under the Extensions sidebar): 2018.3.1
OS and version: Ubuntu 16.04
Python version (& distribution if applicable, e.g. Anaconda): n/a
Type of virtual environment used (N/A | venv | virtualenv | conda | ...): n/a
Relevant/affected Python packages and their versions: n/a

Actual behavior
see above
Expected  behavior
see above
Steps to reproduce:

start the process (separately)
open launch.json
find the appropriate launch config (default: "Python: Attach")
edit "host", "port", and "secret" (as appropriate)
start the debugger as normal

Closing this issue as we have separate issues to track each.

Selecting a local process for debugging is tracked in #1078
Improvements to remove debugging is tracked here #2493

If #2493 does not address your concerns, then please update #2493 with your suggestions.


[Feature] Added variable management menu to team admin role #1849

What changes were proposed in this pull request
Issue Number: close #1849

Brief change log

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:

Does this pull request potentially affect one of the following parts

Dependencies (does it add or upgrade a dependency): (yes / no)

@1996fanrui I added this as you said yesterday, I have tested with team admin and developer account respectively，please see if that's the problem?

@macksonmu Thanks for your contribution.
LGTM.
@wolfboys please help double check, thanks.

done


View preview
Checklist
Preflight

[x] Prefix the PR title with the slug of the package or component - e.g. accordion: Updated padding or docs: Updated header links
[x] Describe the changes clearly in the PR description
[x] Read and check your code before tagging someone for review
[x] Create a changeset file by running yarn changeset. Learn more about change management.

Testing

[x] Add any necessary unit tests (HTML validation, snapshots etc)
[x] Run yarn test to ensure tests are passing. If required, run yarn test -u to update any generated snapshots.

Documentation

[x] Create or update documentation on the website
[x] Create or update stories for Storybook
[x] Create or update stories for Playroom snippets

unit test error:

You have a conflict with Drawer.tsx


Remove AsyncLocal from HeaderPropagation middleware

Request scope HeaderPropagationValues rather than storing it in an AsyncLocal
Contributes to: https://github.com/aspnet/AspNetCore/issues/9813

This is the opposite of how we want this to work 😆

This is the opposite of how we want this to work 😆

Hmm?

This is the opposite of how we want this to work 😆

Oh? It could work.
I recall injecting into Invoke is slow because it uses ActivatorUtilities.

I recall injecting into Invoke is slow because it uses ActivatorUtilities.

O RLY? Challenge accepted...

I recall injecting into Invoke is slow because it uses ActivatorUtilities.

O RLY? Challenge accepted...

Certainly slower than constructor injection. DI does a lot of optimizations and caching for constructor injection, but Invoke injection is just GetRequiredService in a loop. Invoke injection is also per request which makes the effect far more significant.

Oh? It could work.
I recall injecting into Invoke is slow because it uses ActivatorUtilities.

We decided to use AsyncLocal because of the thread safety and fire-and-forget issues. A few people tried implementing this using IHttpContextAccessor and ended up injuring themselves.

I understand, but they were trying to access HttpContext inside SendAsync. Doing it in the handler constructor does look safer, that should only be created once per request, correct?

No, the handler is cached for an arbitrary amount of time.

A few people tried implementing this using IHttpContextAccessor and ended up injuring themselves.

IHttpContextAccessor isn't used to get headers from HttpContext; its just used to get the RequestServices container, because DefaultHttpClientFactory sets up a subscope which hides it

IHttpContextAccessor isn't used to get headers from HttpContext; its just used to get the RequestServices container, because DefaultHttpClientFactory sets up a subscope which hides it

The message handler will outlive the request scope, and will be reused across many requests.

No, the handler is cached for an arbitrary amount of time.

Ah, should be able to tweak for that.

No, the handler is cached for an arbitrary amount of time.

Ah, should be able to tweak for that.

The way I see this we have two places to run code:

In the message handler constructor (bad because cached)
In the SendAsync method (bad because threading and fire-and-forget)

Can solve this except in one scenario; if SendAsync is called after the request has finished.
i.e. is it

fire and forget of SendAsync or
a whole independent Task.Run that's fire and forgotten, and at some point in the future calls SendAsync?

Yes. These are the reasons why we decided on this solution.

@benaadams - we intentionally went with async-local as a solution so that this can support fire-and-forget work and multithreaded work that's spawned from a request. Users that tried to build header propagation on their own was a common source of people trying to use HttpContext in an unsupported way, which is what led us here 😆
If you (or someone like you) plans to use this and wants to make it work without the async local, I'd be happy to accept a PR like this with an option as long as the current "works all of the time" behaviour is the default.
I'm still not totally convinced by this PR because it relies on IHttpContextAccessor. So if you're not already using that, you just traded one dependency on async-local for another.

There is a cost per async local; both in allocations from in and out of a setting async method for the changed immuatable async local dictionaries and the new ExecutionContexts to support them; and in the increased cost of restoring the contexts; though as long as they don't have change notifications its not too bad.
However; I think it needs a more holistic approach for all the ASP.NET Core AsyncLocals, rather than looking at one in isolation. So closing this 😄

However; I think it needs a more holistic approach for all the ASP.NET Core AsyncLocals, rather than looking at one in isolation. So closing this 😄

OK, looking forward to what you come up with!

Related allocations 132MB and 3.4M objects for 312k requests with just 2 AyncLocals (Activity and logging scope) and the growth as you add them isn't linear.


Decision post on validation/verification packages

Include details about why frictionless wasn't good for us.

This is a duplicate to #132 and #131


Cannot Show/Hide Popup on Mouse Hover

Hi there,
I am trying to show asset positions on a map. I am using HTML markers as they let me use HTML and CSS to customise the asset's appearance e.g., a car gets a car icon, a train gets a train icon, a stationary asset looks different to a moving one, etc. I want to show high level details in a tooltip when the user hovers their mouse over the marker. The user can then either move their mouse off the marker or click it. If they move their mouse off the marker, the tooltip disappears. If they click the marker, a panel on the left appears with more details. It's basically the Google Maps experience.
The React Azure Maps documentation has a Popup example so I tried to use this as the basis of the solution, but I can't get it to work with mouse hover. It works ok on click (ok as in most clicks work, but not all), but, as mentioned above, I want a different experience on click.
There are two problems:

The HtmlMarkerEvents interface does not declare all events. mouseenter and mouseleave are missing and I think these are the ones I want. They are documented in the Azure Maps documentation and logging shows that they work: https://learn.microsoft.com/en-us/azure/azure-maps/map-events#interact-with-html-marker. This isn't really a problem with this repo as this type comes from azure-maps-control, but it may be related so thought I would mention it.
The visibility of the popup does not update when the state updates. It starts off correct, then gets out of sync.

Consider the below example code:
import {
  AzureMap,
  AzureMapDataSourceProvider,
  AzureMapsProvider,
  IAzureMapOptions,
  AzureMapHtmlMarker,
  useCreatePopup
} from "react-azure-maps";
import { AuthenticationType } from "azure-maps-control";
import { useState } from "react";

const option: IAzureMapOptions = {
  authOptions: {
    authType: AuthenticationType.subscriptionKey,
    subscriptionKey: "xxx",
  },
  center: [133.76866774216998, -25.151382729660657],
  zoom: 4,
};

export default function MapWithPopup() {
  const [visible, setVisible] = useState(false);
  return (
    <AzureMapsProvider>
      <AzureMap options={option}>
        <AzureMapDataSourceProvider id={'map AzureMapDataSourceProvider'}>
          <AzureMapHtmlMarker
            markerContent={

}
            isPopupVisible={visible}
            options={{
              position: [151, -34],
              popup: useCreatePopup({
                options: {
                  closeButton: false,
                  pixelOffset: [0, -50]
                },
                popupContent: (
visible = {visible.toString()}
),
              })
            }}
            events={[
              {
                eventName: 'mouseenter',
                callback: () => {
                  console.log('mouseenter');
                  setVisible(true);
                }
              },
              {
                eventName: 'mouseleave',
                callback: () => {
                  console.log('mouseleave');
                  setVisible(false);
                }
              }
            ]}
          />
        </AzureMapDataSourceProvider>
      </AzureMap>
    </AzureMapsProvider>
  );
};

This should render a map, focused on Australia, with a pink box near Sydney, and should show a popup on hover. What you experience is as follows:

The popup is not visible by default. Correct.
When you hover your mouse over the pink box, the mouseenter event fires, the visible state is set to true, the popup appears. All correct.
When you move your mouse off the pink box, the mouseleave event fires, the visible state is set to false. Correct. However, the popup is still visible. Why?
When you move your mouse back over the pink box, the mouseenter event fires, the visible state is set to true. Correct. However, the popup disappears. Why?
You are now stuck in a situation where mouse hover does the opposite of what is desired, despite the state being correct.

At this point I completely lost faith in the Popup functionality and abandoned the approach. Instead I have implemented the tooltip experience by including the tooltip HTML inside the marker itself, then using CSS to show/hide the tooltip on hover.
I feel like I have a fairly simple and common use case here, what am I doing wrong? Or is there a bug somewhere?

@and-rej, thanks for your detailed example. Yes, I believe it is a bug within the AzureMapMarkers component. The logic here seems to be the opposite of what is should be...
if (isMarkerPopupOpen && isPopupVisible) {
  markerRef.getOptions().popup?.close()
} else if (isMarkerPopupOpen !== undefined) {
  markerRef.getOptions().popup?.open()
} else if ((isPopupVisible && isMarkerPopupOpen) || isPopupVisible) {
  markerRef.togglePopup()
}

We will address this issue shortly. Thank you!

Fixed in https://github.com/Azure/react-azure-maps/releases/tag/0.4.7


Can't Install K

When running both "mvn install" and "mvn verify", the installation process hangs after "main:". There were also some errors when installing, such as "java.io.IOException: Cannot run program "kast" (in directory ...)" but this did not stop the installation process.

Could you provide a few more information please?
OS, Java version, make sure you have a recent maven version.
If these things don't work, I could also have a look directly at your computer. It's working fine for most of us, so such problems are interesting.

@radumereuta from the error he mentions I suspect that K is not in his PATH. You might test mvn verify that way and see if it reproduces the issue.

Hmm... true. Also, running the examples could fail if you run the tests on windows. The tests are set up to call kast, but on windows you have to call kast.bat. It's just some weird thing in how java executes commands.

It seems as though @dwightguth was right, I just added K to my PATH. I'm running mvn verify now to make sure but it looks like it's already gotten farther than it had before. Thanks for your help.

For my part, I never run more than mvn package in general, and on  Windows in special.
If K is added to the path, then mvn package seems enough for most practical purposes (unless one wants to stress their own computer by running the mighty K test suite).

I usually run mvn package -DskipTests. Even faster. It just builds the tool in order for me to run definitions, skipping all unit tests.

Guys, can we do something to avoid such silly errors?  I know you will say that this is not urgent and that we have something else which is more important or more intellectually interesting to do, but in the end these minor things matter a lot.  @lucaspena was really motivated, so he bothered to report the issue and try to make it work, but others will simply give up and never look back.  They would rightfully say that if they have problems installing the thing, likely they will also have lots of problems running it, so why bother.  So please, let us stop for awhile from what we do and try to find a clean solution to this (which hopefully does not involve asking users to read in detail some README in order to do something that is typically not done when installing a Java app from sources).

I think that ktest and mvn package | verify | ... should not use PATH for any of the K related binaries/libraries. @dwightguth is that hard to achieve?

The problem is that we currently use external parsers in order to invoke kast from krun. as a result of this, the command that gets invoked in krun is hard-coded in config.xml files. so that would be kind of tricky. But what we can do is make ktest add the bin directory to the path of every process it spawns.

Fixed in #1494.

@lucaspena please reopen if this does not fix your issue.


Please create a node to make Adaptive Guidance to work with Ksampler

Hi @blepping
Can you make a node to make Adaptive Guidance to work with Ksampler similar to what you do with Detail Daemon
https://github.com/asagi4/ComfyUI-Adaptive-Guidance
Adaptive Guidance help us to use negative prompt in Flux in the the first few steps and only sacrifice generate speed for a few early steps however it's not working well with Detail Daemon. My plan is make negative prompt work for a few step to create the good image with good hand, feet and style then Detail Daemon for the rest of steps to make image for detail. However it's only work with Ksampler which can separate step period. Can you support us ?
Here is the image of Adaptive Guidance

Thank you in advance

Can you make a node to make Adaptive Guidance to work with Ksampler similar to what you do with Detail Daemon

Hmm, it might be possible to do this as a sampler wrapper that forces a certain guider for a sampler (so you'd still need to use it with the bleh preset stuff). However I'm not sure you actually need this feature.

However it's only work with Ksampler which can separate step period.

What do you mean? It's possible to do start/end steps with a custom sampler also using a SplitSigmas node (built in). For example:

high_sigmas here is the first 10 steps, low_sigmas is the last 10 steps. Perhaps I misunderstood the issue.

Can you make a node to make Adaptive Guidance to work with Ksampler similar to what you do with Detail Daemon

Hmm, it might be possible to do this as a sampler wrapper that forces a certain guider for a sampler (so you'd still need to use it with the bleh preset stuff). However I'm not sure you actually need this feature.

However it's only work with Ksampler which can separate step period.

What do you mean? It's possible to do start/end steps with a custom sampler also using a SplitSigmas node (built in). For example:

high_sigmas here is the first 10 steps, low_sigmas is the last 10 steps. Perhaps I misunderstood the issue.

Hi @blepping
Thank for your quick answer. So how do I use high sigma and low sigma.
For example I want Adaptive Guidance work with first 10 steps of high sigma, now I want Detail Daemon work with last 10 steps of low sigma. Can you give me an example workflow for it.

Here's an example of how you could use different guiders/samplers for different steps with SamplerCustomAdvanced:

I don't have AdaptiveGuider installed, PerpNegGuider is the one that is used for the first 10 steps there, so that's the one you would want to swap out, probably.
If that looks complicated, it really just boils down to this setup:

And you are free to use different samplers/guiders for each SamplerCustomAdvanced. Using BlehDisableNoise because because ComfyUI has a bug regarding non-deterministic generations ( https://github.com/comfyanonymous/ComfyUI/pull/4518 ) which apparently will not be fixed in our lifetime despite being known for close to a year, multiple people submitting fixes (including myself) and even comfy himself saying he was going to accept it. Grumble grumble.

Here's an example of how you could use different guiders/samplers for different steps with SamplerCustomAdvanced:

And you are free to use different samplers/guiders for each SamplerCustomAdvanced. Using BlehDisableNoise because because ComfyUI has a bug regarding non-deterministic generations ( comfyanonymous/ComfyUI#4518 ) which apparently will not be fixed in our lifetime despite being known for close to a year, multiple people submitting fixes (including myself) and even comfy himself saying he was going to accept it. Grumble grumble.

Thank you so much. By can you give me an example of how to use your BlehSetSamplerPreset
I try to use load model or latent into anyinput of your node but still ComfyUI still said I haven't define bleh_preset_0 yet
Thank in advance

can you give me an example of how to use your BlehSetSamplerPreset

Sure. The main thing to keep in mind is the BlehSetSamplerPreset node has to run before anything uses that preset. ComfyUI runs nodes based on how their results are required. Just for example, in the little example image above, the first SamplerCustomAdvanced has to run before the second one, because the second one is using its output. Nodes also have to get run again when their inputs change.
Of course, you can't always know the order for sure. As another example, for the first SamplerCustomAdvanced, will the RandomNoise node run first or will the KSamplerSelect node run first? We don't know - the other those nodes run is basically random. Doesn't really matter for your question about setting the preset, but I just thought I'd mention it.
Anyway, to ensure BlehSetSamplerPreset runs first, you can do something like this:

any_input just passes through. We know KSampler needs a latent as input, so it can't run before it runs Empty Latent Image and BlehSetSamplerPreset - and at that point the preset will have been set and is available.
There's one other thing to be aware of if you're using the same preset in multiple places ( see https://github.com/blepping/ComfyUI-bleh#blehsetsamplerpreset ). Note this isn't the case with what you're trying to do right now. Anyway, just for future reference:

dummy_opt doesn't actually get used, but since nodes have to run again if their inputs change, you can use that connection with something like the Primitive integer node to force the node to run again whenever you want. i.e. if you run the workflow, then change the Primitive integer to 2 then ComfyUI will have to run BlehSetSamplerPreset again as well as any downstream nodes that depended on its input (and then anything that depended on those nodes and so on).
Hopefully this makes sense! I have a bad habit of making my explanations too complicated, so if it's not clear please let me know.

can you give me an example of how to use your BlehSetSamplerPreset

Sure. The main thing to keep in mind is the BlehSetSamplerPreset node has to run before anything uses that preset. ComfyUI runs nodes based on how their results are required. Just for example, in the little example image above, the first SamplerCustomAdvanced has to run before the second one, because the second one is using its output. Nodes also have to get run again when their inputs change.
Of course, you can't always know the order for sure. As another example, for the first SamplerCustomAdvanced, will the RandomNoise node run first or will the KSamplerSelect node run first? We don't know - the other those nodes run is basically random. Doesn't really matter for your question about setting the preset, but I just thought I'd mention it.
Anyway, to ensure BlehSetSamplerPreset runs first, you can do something like this:
dummy_opt doesn't actually get used, but since nodes have to run again if their inputs change, you can use that connection with something like the Primitive integer node to force the node to run again whenever you want. i.e. if you run the workflow, then change the Primitive integer to 2 then ComfyUI will have to run BlehSetSamplerPreset again as well as any downstream nodes that depended on its input (and then anything that depended on those nodes and so on).
Hopefully this makes sense! I have a bad habit of making my explanations too complicated, so if it's not clear please let me know.

I understand a bit now. I don't have PC at the moment but as far as I understand: First we need to run workflow process to create a preset first. For example as your workflow show:
1 - We put an empty latent size in to your BlehSetSamplerPreset then connect to a Ksampler to make it run. So if I want to create a 1920x1080 size do I need to change the empty latent size to 1920x1080, and what happen if I use face detailer which empty lantent size will change randomly base on the image face size.
2- After we get the preset should I do something like your guide in Daemon Detailer to make Ksampler work. I wonder if I use unet loader not checkpoint loader, does it work ?

Please explain this to me. Thank you so much

Sorry if I wasn't clear, I wasn't saying that you necessarily would use an Empty Latent node or a KSampler - just an example to show the general structure.
To put it a different way, if you have a workflow set up like Node A -> Node B where you're feeding the output of Node A into Node B then you know Node A has to run first: Node B needs its result, which is only available after it runs.
Using that logic and assuming Node B uses a bleh preset, you can ensure the BlehSetSamplerPreset node runs first by changing that to Node A -> BlehSetSamplerPreset -> Node B. It doesn't matter what types of nodes Node A and Node B are or what you're passing through the BlehSetSamplerPreset node. It doesn't care about the contents of the any_input input - it only acts as a bridge and returns exactly the same thing you connected to it.
Does that make sense?

Sorry if I wasn't clear, I wasn't saying that you necessarily would use an Empty Latent node or a KSampler - just an example to show the general structure.
To put it a different way, if you have a workflow set up like Node A -> Node B where you're feeding the output of Node A into Node B then you know Node A has to run first: Node B needs its result, which is only available after it runs.
Using that logic and assuming Node B uses a bleh preset, you can ensure the BlehSetSamplerPreset node runs first by changing that to Node A -> BlehSetSamplerPreset -> Node B. It doesn't matter what types of nodes Node A and Node B are or what you're passing through the BlehSetSamplerPreset node. It doesn't care about the contents of the any_input input - it only acts as a bridge and returns exactly the same thing you connected to it.
Does that make sense?

Yes I understand but does Node A -> BlehSetSamplerPreset -> Node B have to be output an result to set the preset.
As your reference image show: Empty Latent -> BlehSetSamplerPreset -> Ksampler.
Do I need to connect Ksampler to Vae Decode and Preview Image to make creating preset work.
Empty Latent -> BlehSetSamplerPreset -> Ksampler -> Vae Decode -> Preview Image
And after Preset was set. I just need to use Ksampler and select sample name: Bleh_preset_0 and I don't need to use BlehSetSamplerPreset or Sample Custom Advance again right ?

Yes I understand but does Node A -> BlehSetSamplerPreset -> Node B have to be output an result to set the preset.

If I understand the question, you mean something like if you just have a stray Empty Latent Image node that's not connected to anything and then you make that Empty Latent Image -> BlehSetSamplerPreset (still not connecting the output to anything) will that set the preset? The answer is no: none of those nodes will run in either case because no other nodes depend on their input.
ComfyUI has a distinction between normal nodes and "output" nodes. Output nodes would be something like Save Image, Preview Image - those nodes run because they produce an output even though a Save Image node won't have its output connected to anything (and in fact does not actually have an output at all).

Do I need to connect Ksampler to Vae Decode and Preview Image to make creating preset work.

The simplest approach is just use BlehSetSamplerPreset as a bridge for some input the sampler you're using (doesn't matter whether it's a KSampler, FaceDetailer, SamplerCustomAdvanced, whatever) will require. That way you know the BlehSetSamplerPreset will run before the sampler that uses the preset.

And after Preset was set. I just need to use Ksampler and select sample name: Bleh_preset_0 and I don't need to use BlehSetSamplerPreset or Sample Custom Advance again right ?

Short answer: Correct.
Longer answer: The presets get saved into the running ComfyUI server. So if for example you set up your workflow so the preset got set, then you delete the BlehSetSamplerPreset node and restart the ComfyUI server - at that point the preset doesn't exist anymore. This is not a problem you'll run into if you set up your workflow the way I recommended though.
Also, setting a preset basically has no performance cost and overwriting the preset with the same settings is completely fine.

Yes I understand but does Node A -> BlehSetSamplerPreset -> Node B have to be output an result to set the preset.

If I understand the question, you mean something like if you just have a stray Empty Latent Image node that's not connected to anything and then you make that Empty Latent Image -> BlehSetSamplerPreset (still not connecting the output to anything) will that set the preset? The answer is no: none of those nodes will run in either case because no other nodes depend on their input.
ComfyUI has a distinction between normal nodes and "output" nodes. Output nodes would be something like Save Image, Preview Image - those nodes run because they produce an output even though a Save Image node won't have its output connected to anything (and in fact does not actually have an output at all).

Do I need to connect Ksampler to Vae Decode and Preview Image to make creating preset work.

The simplest approach is just use BlehSetSamplerPreset as a bridge for some input the sampler you're using (doesn't matter whether it's a KSampler, FaceDetailer, SamplerCustomAdvanced, whatever) will require. That way you know the BlehSetSamplerPreset will run before the sampler that uses the preset.

And after Preset was set. I just need to use Ksampler and select sample name: Bleh_preset_0 and I don't need to use BlehSetSamplerPreset or Sample Custom Advance again right ?

Short answer: Correct.
Longer answer: The presets get saved into the running ComfyUI server. So if for example you set up your workflow so the preset got set, then you delete the BlehSetSamplerPreset node and restart the ComfyUI server - at that point the preset doesn't exist anymore. This is not a problem you'll run into if you set up your workflow the way I recommended though.
Also, setting a preset basically has no performance cost and overwriting the preset with the same settings is completely fine.

Ok I understand now. So basically, I just need to connect BlehSetSamplerPreset to anynode that require an input and run it, since I dont connect an output to the node connect with BlehSetSamplerPreset node so it won't run so no performance cost but still recognize sample and create a sample name Bleh_preset_0.
After this, I just use normal Ksampler and choose sampler name Bleh_preset_0 then run like normal workflow.

Correct. Just use BlehSetSamplerPreset as a bridge for any input that will be required before the first place that preset actually is used, and after that you're free to use that preset in whatever nodes you want.

I just need to connect BlehSetSamplerPreset to anynode that require an input

The only thing I'd add here is it's not just "any node", but any node that has to run before the place you actually use the preset.

Another thing I want to ask. This is not related to your BlehSetSamplerPreset
It's about dynamic CFG like Adaptive Guidance I talked above.
Due to Flux model is quite heavy if I set CFG >1 the inference speed reduce 50%.
Adaptive Guidance using some formula to run CFG >1 for a few step and calculate the likeliness of the image about 90% to 99%  of full run CFG>1 then change to CFG=1 to increase the speed 100%. This method is helping us increase only 30% inference time but get 90% or more of the image with full run CFG>1 which negative prompt worked. And also it does not depend on step, it depend on the % of likeliness we set.
Can split sigma node do the same or we need to set a fixed amout of step run with CFG>1 then another fixed step with CFG=1

Can split sigma node do the same or we need to set a fixed amout of step run with CFG>1 then another fixed step with CFG=1

Sorry, I'm not sure I understand your question. Splitting sigmas just lets you split the steps so you can use a different sampler with different parameters (different guider, different sampler, etc). So I guess the answer would be no?
I'm also not 100% sure I understand the problem you're trying to solve (or if it's exactly the same as the person who opened the issue - I am assuming you're a different person). Maybe something like this?

Adaptive Guidance will dynamically decide whether to run just for a couple steps or maybe it will decide to run for all steps.
Detail Daemon does not work well when it's enabled at the same time as Adaptive Guidance.
You want to avoid the case where they are both running concurrently.

Note that I phrased that as a set of statements but I am just guessing what your problem could be and I also don't know that Adaptive Guidance actually works like that. If that actually is your problem, the conversation so far won't really help you because you'd still want to use the Adaptive Guidance guider for both of them (since it might want to run for the remaining steps as well). If that is the case, then really the only solution would be some kind of integration between Adaptive Guidance and DD (or some kind of feature in Adaptive Guidance that would let it switch samplers depending on whether it is enabled or not).

Can split sigma node do the same or we need to set a fixed amout of step run with CFG>1 then another fixed step with CFG=1

Sorry, I'm not sure I understand your question. Splitting sigmas just lets you split the steps so you can use a different sampler with different parameters (different guider, different sampler, etc). So I guess the answer would be no?
I'm also not 100% sure I understand the problem you're trying to solve (or if it's exactly the same as the person who opened the issue - I am assuming you're a different person). Maybe something like this?

Adaptive Guidance will dynamically decide whether to run just for a couple steps or maybe it will decide to run for all steps.
Detail Daemon does not work well when it's enabled at the same time as Adaptive Guidance.
You want to avoid the case where they are both running concurrently.

Note that I phrased that as a set of statements but I am just guessing what your problem could be and I also don't know that Adaptive Guidance actually works like that. If that actually is your problem, the conversation so far won't really help you because you'd still want to use the Adaptive Guidance guider for both of them (since it might want to run for the remaining steps as well). If that is the case, then really the only solution would be some kind of integration between Adaptive Guidance and DD (or some kind of feature in Adaptive Guidance that would let it switch samplers depending on whether it is enabled or not).

Yeah my question is a bit too much. Now I try to figure out that make Detail Daemon working with Adaptive Guidance together.
Both of them work very well when we don't combine them together.
For example I use Detail Daemon, I get very detail with more content in my generate image however I lost the ability to control mutated limb, hand, feet, plastic skin due to negative prompt is not work with CFG=1.
If I use Adaptive Guidance I get the control ability but lose content in detail.
I try to combine both and get a overnoise image. First I try to make each of them work in a fixed step but now I find out that each of them need to work first few beginning steps to make it's effective. So today I will try to mess with Detail Daemon settings or Adaptive Guidance to make them work well together.
At least I solved the problem face detailer and ksampler does not work with Detail Daemon thank to you

For example I use Detail Daemon, I get very detail with more content in my generate image however I lost the ability to control mutated limb, hand, feet, plastic skin due to negative prompt is not work with CFG=1.

Are you positive using Adaptive Guider is actually doing something when used with Flux? From what I understand, Flux just doesn't use CFG so setting CFG to higher values does nothing except make everything twice as slow.
I looked at the code and it doesn't seem to have any special handling for Flux, so in essence it's just using CFG=1 for part of sampling and CFG=different_value for a different part. Since it's Flux, those both should produce the same result.

For example I use Detail Daemon, I get very detail with more content in my generate image however I lost the ability to control mutated limb, hand, feet, plastic skin due to negative prompt is not work with CFG=1.

Are you positive using Adaptive Guider is actually doing something when used with Flux? From what I understand, Flux just doesn't use CFG so setting CFG to higher values does nothing except make everything twice as slow.
I looked at the code and it doesn't seem to have any special handling for Flux, so in essence it's just using CFG=1 for part of sampling and CFG=different_value for a different part. Since it's Flux, those both should produce the same result.

No I already experience with high CFG, it fixed mutated hand for me, I will show workflow later for you today when I get back home with my PC.

Yes I already experienced with high CFG, it fixed mutated hand for me, I will show workflow later for you today when I get back home with my PC.

You're 100% sure you used absolutely identical settings in both cases, including seeds? ComfyUI defaults to randomizing the seeds so you specifically have to set the seed type to "fixed" to get reproducible results.

Actually Lying Sigma Sampler of Detail Daemon work with my workflow, only Detail Daemon Sampler get over noise with my image.

This sounds more like an issue of parameters rather than a conflict between DD and the Adaptive Guider. You should be able to configure DD to produce the same results as LSS. One thing to try is setting force CFG to 1 in the DD node - by default it tries to scale the effect based on CFG while LSS just uses a static value. Another thing to try is making sure DD and LSS are enabled for the same time ranges in sampling.

Yes I already experienced with high CFG, it fixed mutated hand for me, I will show workflow later for you today when I get back home with my PC.

You're 100% sure you used absolutely identical settings in both cases, including seeds? ComfyUI defaults to randomizing the seeds so you specifically have to set the seed type to "fixed" to get reproducible results.

Actually Lying Sigma Sampler of Detail Daemon work with my workflow, only Detail Daemon Sampler get over noise with my image.

This sounds more like an issue of parameters rather than a conflict between DD and the Adaptive Guider. You should be able to configure DD to produce the same results as LSS. One thing to try is setting force CFG to 1 in the DD node - by default it tries to scale the effect based on CFG while LSS just uses a static value. Another thing to try is making sure DD and LSS are enabled for the same time ranges in sampling.

Yes I know to fix seed. Actually you can download the workflow from civitai and experiment yourself in this link:
https://civitai.com/models/625042/flux-negative-prompting-workflows
I use this guide to implement negative prompt into my workflow

Well @blepping
I try as you said and still failed to make your node work:

Here is my workflow:
I try to run Group Test 1 first to create a preset then run Group Test to check whether preset work:

Can you take a look ?
Thank you

I try to run Group Test 1 first to create a preset then run Group Test to check whether preset work:

The KSampler in Test 1 isn't connected to anything. Since nothing uses its output, it will never run. Since it doesn't run, the stuff connected to it won't run either.
Use the same approach in the Test group and it should work. For example, use the BlehSet SamplerPreset node as a bridge for Empty Latent and KSampler in the Test group.
The rule is: Nodes run when something needs their output. Otherwise they don't. For BlehSetSamplerPreset, you need to put it somewhere where it will have to run before the place you actually use the preset. So, just for example you could add a VAE decode and preview node in the Test 1 group and that would make the KSampler there run (and the preset get set) BUT that won't necessarily fix your issue because the KSampler that actually uses the preset has no dependency on that group or its nodes so there's no guarantee that group will be the one that runs first.

I try to run Group Test 1 first to create a preset then run Group Test to check whether preset work:

The KSampler in Test 1 isn't connected to anything. Since nothing uses its output, it will never run. Since it doesn't run, the stuff connected to it won't run either.
Use the same approach in the Test group and it should work. For example, use the BlehSet SamplerPreset node as a bridge for Empty Latent and KSampler in the Test group.
The rule is: Nodes run when something needs their output. Otherwise they don't. For BlehSetSamplerPreset, you need to put it somewhere where it will have to run before the place you actually use the preset. So, just for example you could add a VAE decode and preview node in the Test 1 group and that would make the KSampler there run (and the preset get set) BUT that won't necessarily fix your issue because the KSampler that actually uses the preset has no dependency on that group or its nodes so there's no guarantee that group will be the one that runs first.

I see. I thought run before mean run another workflow before. But as you explain I need to attach your BlehSetSamplerPreset along with my running workflow to generate image, something like "hello please let me tag along, don't worry, I don't cost anything =))). I will try again later today.

hello please let me tag along, don't worry, I don't cost anything =))).

Haha, yes. Something like that. :) The only requirement is that the workflow is set up in a way that the node will definitely be run before that preset is actually used.
It's probably intuitive to think about workflows like the one you posted from left to right - and that's also how the nodes run while we watch it. However, it is the output nodes (Save Image, Preview, etc - pretty much anything that does an output action rather than having an output connection) that actually drive the graph.
When you queue a workflow in ComfyUI, it'll find the output nodes and work back from there, figuring out what's needed to satisfy their dependencies.

Still failed miserably. Here my workflow. Can you check again:

I only use bleh_0 in facedetailer node

Thank you

Still failed miserably.

I can't see how it wouldn't work, it doesn't look like you did anything wrong. Something weird must be going on. I might not get a chance to look into it in-depth today, but tomorrow definitely!

@wardensc2 I did some testing and couldn't reproduce your issue. The only thing I can think of is maybe your bleh or ComfyUI versions are too old? This workflow image also contains metadata so you can load it into ComfyUI directly. You'll probably have to change the checkpoint and model loaded in UltralyticsDetectorProvider.

It shouldn't matter what models/samplers/whatever you use. If you use that workflow (changing the checkpoint/ultralytics model is fine) do you still experience the problem?

@wardensc2 I did some testing and couldn't reproduce your issue. The only thing I can think of is maybe your bleh or ComfyUI versions are too old? This workflow image also contains metadata so you can load it into ComfyUI directly. You'll probably have to change the checkpoint and model loaded in UltralyticsDetectorProvider.

It shouldn't matter what models/samplers/whatever you use. If you use that workflow (changing the checkpoint/ultralytics model is fine) do you still experience the problem?

Thank, I'm updated ComfyUI yesterday, let me check all the node related to the workflow up to date or not and try again later, today.

@wardensc2 I did some testing and couldn't reproduce your issue. The only thing I can think of is maybe your bleh or ComfyUI versions are too old? This workflow image also contains metadata so you can load it into ComfyUI directly. You'll probably have to change the checkpoint and model loaded in UltralyticsDetectorProvider.

It shouldn't matter what models/samplers/whatever you use. If you use that workflow (changing the checkpoint/ultralytics model is fine) do you still experience the problem?

Hi @blepping
I already test, even download your workflow to run. Your workflow only run with SD 1.5, when I change to SDXL or Flux, it's error again. Maybe it's the node problem, maybe you should check your node.

I already test, even download your workflow to run. Your workflow only run 1 time with SD 1.5, when I change the prompt, it's error. You can try change a cute dog into a cute girl or a girl, you will see

Unfortunately, I can't reproduce your issue. I can change the prompt as many times as I want without any issue. Is there anything unusual with your Comfy installation like using it through a different front end or that kind of thing?
I'm struggling to think of something that could cause this issue. What other extensions do you have enabled?

I already test, even download your workflow to run. Your workflow only run 1 time with SD 1.5, when I change the prompt, it's error. You can try change a cute dog into a cute girl or a girl, you will see

Unfortunately, I can't reproduce your issue. I can change the prompt as many times as I want without any issue. Is there anything unusual with your Comfy installation like using it through a different front end or that kind of thing?
I'm struggling to think of something that could cause this issue. What other extensions do you have enabled?

Hi @blepping
This is the log when I get errors

I will try to reinstall comfyUI again and test tomorrow

I already test, even download your workflow to run. Your workflow only run 1 time with SD 1.5, when I change the prompt, it's error. You can try change a cute dog into a cute girl or a girl, you will see

Unfortunately, I can't reproduce your issue. I can change the prompt as many times as I want without any issue. Is there anything unusual with your Comfy installation like using it through a different front end or that kind of thing?
I'm struggling to think of something that could cause this issue. What other extensions do you have enabled?

 Thank you for your really passionate support. Thank you so much

Hi @blepping
At last, I found the culprit why I can't run your Bleh node.
It's your Hidiffusion node: https://github.com/blepping/comfyui_jankhidiffusion
I try to navigate down which extension got me in trouble since I reinstall comfyUI. First when comfy clean, I only install node to run your workflow. It run flawless but after I install all the workflows I get to trouble again. After isolate many extension I found your Hidiffusion is the main reason BlehPreset wont work. Please check because I also need Bleh to run high res SDXL.
Thank you

Thank you very much for following up on that! I think I might have an idea of what's going on, it's probably related to how jankhidiffusion can integrate with bleh (to add blend modes and such) which involves it loading bleh as a Python module.
I'm not quite sure how things are going wrong with that so I can't really give you an ETA on how long it will take to fix but I will definitely be looking into it.
By the way, personally I feel like DiffuseHigh is the best option for high res generation at the moment. It's what I switched to from HiDiffusion: https://github.com/blepping/comfyui_jankdiffusehigh (though MSW-MSA attention is still good, especially with SD15.) I think it probably will avoid the issue you're having as well since it takes a different approach to integrating with other node packs. Not completely sure though.

Thank you very much for following up on that! I think I might have an idea of what's going on, it's probably related to how jankhidiffusion can integrate with bleh (to add blend modes and such) which involves it loading bleh as a Python module.
I'm not quite sure how things are going wrong with that so I can't really give you an ETA on how long it will take to fix but I will definitely be looking into it.
By the way, personally I feel like DiffuseHigh is the best option for high res generation at the moment. It's what I switched to from HiDiffusion: https://github.com/blepping/comfyui_jankdiffusehigh (though MSW-MSA attention is still good, especially with SD15.) I think it probably will avoid the issue you're having as well since it takes a different approach to integrating with other node packs. Not completely sure though.

Sound promising, I almost mistake DiffuseHigh with HiDiffusion = ))) At the first glance, it's looked a bit complited but I will try. If it's good we don't need to use HiDiffusion for SDXL anymore.

@wardensc2 I think I have a fix for jankdiffusehigh. Please try this branch: https://github.com/blepping/comfyui_jankhidiffusion/pull/28
Let's continue discussion about the issue here: https://github.com/blepping/comfyui_jankhidiffusion/issues/27
I also pushed a small change for bleh to prevent it from adding presets if they already exist. That might be enough to fix the problem for you on its own (I suspect if you'd tried the correct bleh_preset_0 it would have worked) but I'd appreciate it if you could test the fix jankhidiffusion.
Just a note: My ComfyUI-sonar nodes are also using the old approach to integration currently so they'll probably trigger this issue as well if you're using them. That's the next one on my list.

@wardensc2 I think I have a fix for jankdiffusehigh. Please try this branch: blepping/comfyui_jankhidiffusion#28
Let's continue discussion about the issue here: blepping/comfyui_jankhidiffusion#27
I also pushed a small change for bleh to prevent it from adding presets if they already exist. That might be enough to fix the problem for you on its own (I suspect if you'd tried the correct bleh_preset_0 it would have worked) but I'd appreciate it if you could test the fix for jankhidiffusion.
Just a note: My ComfyUI-sonar nodes are also using the old approach to integration currently so they'll probably trigger this issue as well if you're using them. That's the next one on my list.

Hi I already test, BlehPreset work fine. About DiffuseHi vs HiDiffusion. The image of DiffuseHi look more blurry than HiDiffusion
DiffuseHi:

HiDiffusion:

We will discuss more in DiffuseHi github

Hi I already test, BlehPreset work fine.

Great, thanks for testing!

We will discuss more in DiffuseHi github

Sounds good. Please feel free to create a discussion and @ me. The defaults for DiffuseHigh might not be optimal - I just tried to keep them pretty close to the official implementation but there are a lot of parameters that can be tuned. One thing to try is setting the guidance mode to latent (will also be much faster).


Release 1.2.1

Codecov Report

Merging #140 (24d5dca) into master (76c4d26) will increase coverage by 0.15%.
The diff coverage is 53.77%.

@@             Coverage Diff              @@
##             master     #140      +/-   ##
============================================
+ Coverage     53.80%   53.95%   +0.15%
- Complexity      385      416      +31
============================================
  Files            42       44       +2
  Lines          3390     3675     +285
  Branches        263      298      +35
============================================
+ Hits           1824     1983     +159
- Misses         1380     1479      +99
- Partials        186      213      +27

Impacted Files
Coverage Δ

...ain/java/com/webank/wecross/common/FabricType.java
6.25% <0.00%> (-0.42%)
:arrow_down:

.../wecross/stub/fabric/ChaincodeResourceManager.java
77.00% <ø> (ø)

...ebank/wecross/account/ExtendedIdentityFactory.java
28.57% <28.57%> (ø)

...ain/java/com/webank/wecross/utils/FabricUtils.java
53.52% <32.00%> (-11.70%)
:arrow_down:

...m/webank/wecross/account/FabricAccountFactory.java
45.31% <33.33%> (+7.21%)
:arrow_up:

...a/com/webank/wecross/stub/fabric/FabricDriver.java
57.43% <37.50%> (-0.94%)
:arrow_down:

...va/com/webank/wecross/stub/fabric/FabricBlock.java
61.61% <56.03%> (-17.46%)
:arrow_down:

...k/wecross/account/X509ExtendedSigningIdentity.java
78.57% <78.57%> (ø)

...m/webank/wecross/stub/fabric/FabricConnection.java
58.80% <90.00%> (+0.58%)
:arrow_up:

...java/com/webank/wecross/account/FabricAccount.java
70.00% <100.00%> (+3.33%)
:arrow_up:

... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d93629...24d5dca. Read the comment docs.


Update to Play 2.4 and Silhouette 3.0

This template depends on play-yeoman which isn't updated to Play 2.4 at this time. There exists an open pull request to solve this issue.

play-yeoman is now updated for play 2.4
It would be great to have an update to this project

If I find the time then at the end of the week. Otherwise at the end of the next week.

+1

Please see #15

Fixed. But note there is an open issue which stops the Twitter support.


Unsupported image type error

Running Howdy 2.6.1, on elementary OS 7.1 (based on Ubuntu 22.04.3), IR camera is built-in on XPS 15 9500 (ID 0bda:5510 Realtek Semiconductor Corp. Integrated_Webcam_HD).
Howdy worked very well on this exact setup less than a week ago, which is part of the reason I'm so stumped here. The error I keep getting is as follows:
Traceback (most recent call last):
  File "/usr/local/bin/howdy", line 95, in <module>
    import cli.add
  File "/usr/lib/security/howdy/cli/add.py", line 157, in <module>
    face_locations = face_detector(gsframe, 1)
RuntimeError: Unsupported image type, must be 8bit gray or RGB image.

The exact same message when testing or adding a new face. Running the snapshot test, the jpg that I get is clearly taken in IR, and is in 8bit greyscale as it should be. Snapshot test is attached if that helps. The device path is configured properly and the IR emitters light up when I test or add a new face, it just is giving a weird file to the program for some reason.

I've uninstalled and reinstalled howdy probably 10 times now, tried every camera device path I have, changed the /lib/security/howdy/compare.py file as this solution suggested, all of them give me the exact same error every time.
Like I said, this version of howdy ran beautifully out of the box on this exact OS, with this exact camera, I can't think for the life of me what I might be doing different this time around.
Any help or suggestions would be appreciated! I really liked this project when I got it running last time, I wouldn't be trying this hard otherwise :)

I experience the same problem.

Same problem

Linux ThinkPad-X1-Yoga-4th 6.5.0-35-generic #35~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue May  7 09:00:52 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Output of v4l2-ctl command
Driver Info:
        Driver name      : uvcvideo
        Card type        : Integrated Camera: Integrated I
        Bus info         : usb-0000:00:14.0-8
        Driver version   : 6.5.13
        Capabilities     : 0x84a00001
                Video Capture
                Metadata Capture
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x04200001
                Video Capture
                Streaming
                Extended Pix Format
Media Driver Info:
        Driver name      : uvcvideo
        Model            : Integrated Camera: Integrated I
        Serial           : 6926
        Bus info         : usb-0000:00:14.0-8
        Media version    : 6.5.13
        Hardware revision: 0x00006926 (26918)
        Driver version   : 6.5.13
Interface Info:
        ID               : 0x03000002
        Type             : V4L Video
Entity Info:
        ID               : 0x00000001 (1)
        Name             : Integrated Camera: Integrated I
        Function         : V4L2 I/O
        Flags            : default
        Pad 0x01000007   : 0: Sink
          Link 0x02000019: from remote pad 0x100000a of entity 'Extension 14' (Video Pixel
Formatter): Data, Enabled, Immutable
Priority: 2
Video input : 0 (Camera 8: ok)
Format Video Capture:
        Width/Height      : 640/360
        Pixel Format      : 'GREY' (8-bit Greyscale)
        Field             : None
        Bytes per Line    : 640
        Size Image        : 230400
        Colorspace        : sRGB
        Transfer Function : Rec. 709
        YCbCr/HSV Encoding: ITU-R 601
        Quantization      : Default (maps to Full Range)
        Flags             :
Crop Capability Video Capture:
        Bounds      : Left 0, Top 0, Width 640, Height 360
        Default     : Left 0, Top 0, Width 640, Height 360
        Pixel Aspect: 1/1
Selection Video Capture: crop_default, Left 0, Top 0, Width 640, Height 360, Flags:
Selection Video Capture: crop_bounds, Left 0, Top 0, Width 640, Height 360, Flags:
Streaming Parameters Video Capture:
        Capabilities     : timeperframe
        Frames per second: 30.000 (30/1)
        Read buffers     : 0

I got the same issue in Ubuntu 22.04 with "balance" configuration.
However, after I remove the howdy and autoremove, then install with "fast" configuration, the problem solved!

Having the same issue with Ubuntu 22.04 LTS:
_

Adding face model for the user peter
Enter a label for this new model [Initial model] (max 24 characters): me
Please look straight into the camera
Traceback (most recent call last):
File "/usr/local/bin/howdy", line 95, in
import cli.add
File "/usr/lib/security/howdy/cli/add.py", line 157, in
face_locations = face_detector(gsframe, 1)
RuntimeError: Unsupported image type, must be 8bit gray or RGB image.

Using the Logi Brio Ultra Webcam, CUDA Tools 12.5, the camera is working fine for 'sudo howdy snapshot' - saves the image and it is a clear image. Both 'add' and 'test' generate the error message.
Hope we get an answer soon!

Same issue for me on Ubuntu 22.04.4

Same issue!

I am having the same issue in Ubuntu 22.04

Yes, me too.
I've tried alot of linux distro but all seems to have this bug...
But in the past i've had successfully installed howdy on this machine (dell latitude 5290 2in1) with ubuntu and it was working perfectly. Hope the developer fix this problem!

Same here, with Logitech BRIO 4k

Same issue on Linux Mint 21.3

Same error here, Ubuntu 22.04 LTS.
I've tried also cloning the git repo and building from source, but I've got the exact same error (maybe related to dlib?)

Same here, Ubuntu 22.04 LTS on 3th Gen Lenovo T14 notebook

Same issue on Arch, and it only happened after a recent update.

thank you @simonporter007
That fixed it for me

Looks like an issue with numpy being upgraded to v2: https://stackoverflow.com/a/78638053
Workaround which worked for me to add a model was to downgrade it to 1.26.4 again: sudo pip install --upgrade numpy==1.26.4. However, model still not recognized after that (timeout) so not sure if that's a complete workaround.

That workaround fixed it for me! At least for now.
Tks, @simonporter007

Thanks a lot, that worked! Hopefully the devs fix this to work with numpy 2.0 soon!

Worked for me as well, thank you so much for figuring that out!

I don't think this should be closed

Downgrading numpy to 1.26.4 also worked for me. Thank you for posting!

understood, thank you!

For anyone who can't wait, compile from the git repo itself to get latest changes.
Currently using latest python-dlib-git on aur to use the latest commit version.

Looks like an issue with numpy being upgraded to v2: https://stackoverflow.com/a/78638053
Workaround which worked for me to add a model was to downgrade it to 1.26.4 again: sudo pip install --upgrade numpy==1.26.4. However, model still not recognized after that (timeout) so not sure if that's a complete workaround.

its worked for me in ubuntu 24.04 and linuxmint 21.3 very well.
slimbook face recognition also worked after this workaround

Thanks, the workaround works.
But there are other problems.

Login after restart. Recognition works, but then you don't get to the start screen and jump back to the login screen.
Login after standby mode does not work at all.

Linux Mint 21.3 on HP ProBook
Still a great project.
I'm out for now, sorry

Downgrading numpy to 1.26.4 also worked for me on Linux Mint 21.3

Looks like an issue with numpy being upgraded to v2: https://stackoverflow.com/a/78638053
Workaround which worked for me to add a model was to downgrade it to 1.26.4 again: sudo pip install --upgrade numpy==1.26.4. However, model still not recognized after that (timeout) so not sure if that's a complete workaround.

It works for me in Zorin 17.1 Thank you.

Downgrading numpy to 1.26.4 worked for me on Pop!_OS 22.04 LTS.

Downgrading numpy to 1.26.4 worked for me on Pop!_OS 22.04 LTS.

Same here on the same OS! Thank you for the fix

Downgrading numpy to 1.26.4 worked for me on Pop!_OS 22.04 LTS.

Did you break another program that required a more recent version, or is there no risk?

I don't want to break something while downgrading numpy. Does someone succeed to run howdy in a python virtual environment ?

For anyone who can't wait, compile from the git repo itself to get latest changes.
Currently using latest python-dlib-git on aur to use the latest commit version.

This worked

A version of dlib has been released that supports numpy 2, users on rolling-release distros like Arch should be able to update, unsure about Debian, Ubuntu and their derivatives.

still not closed?
downgrade solves the issue. but often i get timeout error,
please fix it xD


plugin woocomerce problem

Hello, I have this plugins in woocomerce and I like the convert plus method but for a couple of days when my clients try to make a payment the plugins instead of sending it to the 2checkout page it sends it to my wordpress administrator page.
The inline plugins always give me an error and the person has to go through the purchase process twice and usually that causes disagreement.
I would appreciate a help.

Same here!!

Thanks for reporting the issues. I am unable to replicate anything like this, it could be 2Checkout account related. Can you provide a screen shot of the error you are getting?

This error shows me within the 2checkout platform.
When I activate the convertPlus plugins it does not redirect me to the payment page, but it sends me to the wordpress administrator
I would appreciate your prompt help since I need the convertplus, since I sell to several countries and people feel more comfortable making the payment with their local currency.

Here is a video of the error when I try to enable ConvertPlus
https://www.loom.com/share/c4d640aa5e2e43498c14fbecdbbf28ec

Yes, that's exactly the same issue I'm facing! I now tried ConvertPlus on another website and is showing the same error.


Possibility to submit docker build

Hey there.
Is there any way to achieve something like:
https://cloud.google.com/build/docs/building/build-containers?hl=en#use-dockerfile
So to build via dockerfile?
Couldn't find anything like that in the docs.
Thanks,
Sebi

If I try to deploy an uploaded gzipped tar (***.tar.gz) from cloud storage via:
const cb = new CloudBuildClient();
 await cb.createBuild({
   build: {
   id: `k8s-build-${this.commonConfig.tag}`,
      source: {
          storageSource: {
             bucket: this.serviceConfig.bucketName,
             object: this.imageTar
           }
        },
        tags: [this.commonConfig.tag],
        name: `eu.gcr.io/${this.commonConfig.projectId}/${this.commonConfig.repoName}`
    },
    projectId: this.commonConfig.projectId
});

I get following error: Cannot read property 'decode' of null
Any help here?

Hi Sebastian,
I don't think you can create a build with the client library without defining the steps.  An easy solution is to create one step that does a Docker build.  Here is an example in YAML:  https://github.com/GoogleCloudPlatform/cloud-build-samples/blob/main/run-example-builddeploy/cloudbuild.yaml#L18
I hope that helps!
-Dina

Does this also work if we want to reference local files similar to the example found here, such that we're able to do something like
const cb = new CloudBuildClient();
await cb.createBuild({
   build: {
   id: `some-build`,
   projectId: 'YOUR_PROJECT_ID,
   source: { "what do I put in here?" }
    "steps": [
      {
          "name": "gcr.io/cloud-builders/docker",
          "args": [
            "build",
            "-t",
            "gcr.io/PROJECT_ID/IMAGE_NAME", "."
           ]
       }
     ]
});


SSL_CTX_set_ssl_version() preserves TLS 1.3 ciphersuites

SSL_CTX_set_ssl_version() is intended to adapt an SSL_CTX to a new (presumably, fixed-version) SSL_METHOD, filtering down the cipher list to ciphers that are supported by the new method.  However, there is no TLS 1.3-specific method, so for any fixed-version method, no TLS 1.3 ciphers should apply.  The current implementation of this function just blindly preserves the TLS 1.3 ciphers in the resulting (combined) cipher list, which does not seem like the correct behavior.
This is an issue and not a pull request because I'm not sure if we need to handle the case of
SSL_CTX_set_ssl_version(., TLS_method()).

It looks like the TLS 1.3 specific cipher list is attempted to be used first if possible when ssl_create_cipher_list() is called.  However if that is not possible the result cipherstack values are used instead.
// ssl/ssl_ciph.c
STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(...) {

    ...

    /*
     * Allocate new "cipherstack" for the result, return with error
     * if we cannot get one.
     */
    if ((cipherstack = sk_SSL_CIPHER_new_null()) == NULL) {
        OPENSSL_free(co_list);
        return NULL;
    }

    /* Add TLSv1.3 ciphers first - we always prefer those if possible */
    for (i = 0; i < sk_SSL_CIPHER_num(tls13_ciphersuites); i++) {
        if (!sk_SSL_CIPHER_push(cipherstack,
                                sk_SSL_CIPHER_value(tls13_ciphersuites, i))) {
            sk_SSL_CIPHER_free(cipherstack);
            return NULL;
        }
    }

    /*
     * The cipher selection for the list is done. The ciphers are added
     * to the resulting precedence to the STACK_OF(SSL_CIPHER).
     */
    for (curr = head; curr != NULL; curr = curr->next) {
        if (curr->active) {
            if (!sk_SSL_CIPHER_push(cipherstack, curr->cipher)) {
                OPENSSL_free(co_list);
                sk_SSL_CIPHER_free(cipherstack);
                return NULL;
            }
#ifdef CIPHER_DEBUG
            fprintf(stderr, "<%s>\n", curr->cipher->name);
#endif
        }
    }
    OPENSSL_free(co_list);      /* Not needed any longer */

    ...
}

It's not "instead"; the other ciphers are added after the TLS 1.3 ciphers.
But that's not really the question I was trying to ask about, which is more about whether setting the SSL_METHOD back to the TLS_method() is supposed to be a supported operation.  If not, then we can just put an empty stack of TLS 1.3 ciphers in place and not worry about it.
I was hoping that @mattcaswell or maybe @t-j-h would have some additional perspective here.

SSL_CTX_set_ssl_version(., TLS_method());
should be a perfectly normal operation to be able to use.
And SSL_CTX_set_ssl_version preserves the currently set tls13 ciphersuites so I would expect it to continue to operate correctly.
I don't think I'm following what you are trying to achieve here specifically.

I would expect the ciphersuite list to remain the same before and after the SSL_CTX_set_ssl_version call as conceptually the cipher suite settings are against the SSL_CTX not against the SSL_METHOD.

And checking the actual behaviour - it is precisely that - changing the method does not alter the output from SSL_get_ciphers after each method change; and you do see a difference depending on the version-specific method for SSL_get1_supported_ciphers exactly as I would have expected.

Would this be an appropriate case for a unit test, @t-j-h?

I'm trying to figure out the point @kaduk is trying to make that I'm not following before I get a sense of whether or not there is some unexpected behaviour or anything that would suggest another test case would be useful.

The historical behavior of SSL_CTX_set_ssl_version() is to restore the default cipher list for that version, e.g.:
bkaduk:~/git/openssl$ cat ../ciph2.c
#include "openssl/ssl.h"
#include "openssl/safestack.h"

int main(void)
{
    SSL_CTX *ctx;
    STACK_OF(SSL_CIPHER) *ciph;

    ctx = SSL_CTX_new(TLSv1_2_method());
    ciph = SSL_CTX_get_ciphers(ctx);
    printf("stock TLS_method() first (non-TLS1.3) cipher is: %s\n",
        SSL_CIPHER_get_name(sk_SSL_CIPHER_value(ciph, 3)));
    SSL_CTX_set_cipher_list(ctx, "ECDHE-RSA-AES128-SHA256");
    ciph = SSL_CTX_get_ciphers(ctx);
    printf("after SSL_CTX_set_cipher_list() first (non-TLS1.3) cipher is: %s\n",
        SSL_CIPHER_get_name(sk_SSL_CIPHER_value(ciph, 3)));
    SSL_CTX_set_ssl_version(ctx, TLSv1_2_method());
    ciph = SSL_CTX_get_ciphers(ctx);
    printf("after SSL_CTX_set_ssl_version() first (non-TLS1.3) cipher is: %s\n",
        SSL_CIPHER_get_name(sk_SSL_CIPHER_value(ciph, 3)));
    return 0;
}
bkaduk:~/git/openssl$ LD_LIBRARY_PATH=. ./ciph2stock TLS_method() first (non-TLS1.3) cipher is: ECDHE-ECDSA-AES256-GCM-SHA384
after SSL_CTX_set_cipher_list() first (non-TLS1.3) cipher is: ECDHE-RSA-AES128-SHA256
after SSL_CTX_set_ssl_version() first (non-TLS1.3) cipher is: ECDHE-ECDSA-AES256-GCM-SHA384

But the behavior for TLS 1.3 cipher suites is different from this historical pattern; they are not reset by SSL_CTX_set_ssl_version():
bkaduk:~/git/openssl$ cat ../ciph.c
#include "openssl/ssl.h"
#include "openssl/safestack.h"

int main(void)
{
    SSL_CTX *ctx;
    STACK_OF(SSL_CIPHER) *ciph;

    ctx = SSL_CTX_new(TLS_method());
    ciph = SSL_CTX_get_ciphers(ctx);
    printf("stock TLS_method() first cipher is: %s\n",
        SSL_CIPHER_get_name(sk_SSL_CIPHER_value(ciph, 0)));
    SSL_CTX_set_ciphersuites(ctx, "TLS_CHACHA20_POLY1305_SHA256");
    ciph = SSL_CTX_get_ciphers(ctx);
    printf("after SSL_CTX_set_ciphersuites() first cipher is: %s\n",
        SSL_CIPHER_get_name(sk_SSL_CIPHER_value(ciph, 0)));
    SSL_CTX_set_ssl_version(ctx, TLS_method());
    ciph = SSL_CTX_get_ciphers(ctx);
    printf("after SSL_CTX_set_ssl_version() first cipher is: %s\n",
        SSL_CIPHER_get_name(sk_SSL_CIPHER_value(ciph, 0)));
    return 0;
}
bkaduk:~/git/openssl$ LD_LIBRARY_PATH=. ./ciph
stock TLS_method() first cipher is: TLS_AES_256_GCM_SHA384
after SSL_CTX_set_ciphersuites() first cipher is: TLS_CHACHA20_POLY1305_SHA256
after SSL_CTX_set_ssl_version() first cipher is: TLS_CHACHA20_POLY1305_SHA256

Whether or not this behavior of SSL_CTX_set_ssl_version() (or even its existence) is a good idea, I think we are supposed to keep the behavior "consistent", and as far as I can tell from pattern analysis, the "rule of behavior" that we are keeping consistent is that this function restores the configured ciphers to the default values for the library.

The concept of changing the method associated with an SSL_CTX after it has been created is just horrible. I'm also not sure why anyone would want to do that. Really once an SSL_CTX has been created an used to create SSL objects we should not be modifying it. So this limits the usefulness of this function to the period between when the SSL_CTX is first created, and when it is first used to create an SSL.
This only really makes sense if using a fixed version method. All of those are now deprecated - so we really ought to deprecate this function too.
That all being said, and on the assumption that we have to live with it, the fix proposed by @kaduk looks ok.

A better way to look at it is to see the differences in handling between ciphersuites and the cipher list. They are different - they don't do the same thing.
So if you print out the full list of ciphers and not just cherry pick a single item you can see the difference in behaviour much more clearly.

After SSL_CTX_set_cipher_list the list is small and the defaults are gone.
After SSL_CTX_set_ciphersuites the list is large as the cipher suites are prepended to the list.

I would argue that "large" vs. "small" is not quite the best way to look at things, either, as there are different reference points to compare to.  There are some 300-odd pre-TLS 1.3 ciphersuites currently defined, and only 5 TLS 1.3 ciphersuites defined (we only have three of them in our default list).  Internally, our code is being forced (for some reasonably good reasons) to squash the TLS 1.2 and TLS 1.3 ciphersuites into a single list, but they are conceptually different and non-interchangable.  So it's not that the ciphersuites are "prepended to" the list; they are just the first part of the list, when we squash different things into a single container.  So I would think of this as "after SSL_CTX_set_cipher_list, we have 3/3 TLS 1.3 and (small/large) TLS 1.2", and "after SSL_CTX_set_ciphersuites, we have 1/3 TLS 1.3 and (large/large) TLS 1.2".  The 1/3 and small/large seem commensurate to me, and more meaningful than the squashed-together (3+small)/(large) and (1+large)/large.

Now I would argue that resetting the cipher list or the cipher suites is actually the wrong thing to be doing in this function - but that is a separate topic - but having a function which has an entirely undocumented side effect (resetting things) should be fixed. Right now you are noting that a non-documented behaviour isn't consistent with a new concept and I think that is because it wasn't documented.

I would also argue that it's the wrong thing to be doing, but not very hard, as I'd be arguing that the function shouldn't exist in the first place :)
I'm happy to prepare some documentation (which will note that this function should be considered deprecated, at least in the first cut).