Treatment of labels and messages

We have had a couple of discussions about labels and messages in various contexts, mainly sending and receiving. There were questions as to what is visible to whom. The goal of this issue is to summarize the current state and to discuss how we want the Bitcoin Core App to work in the future.
Overview
If they are included in an invoice/payment request, both the label and the message are included in the URI and commonly stored by the spender’s wallet software. But they are never added to the actual transaction, so other Bitcoin users cannot see them.
Receiving:

If included in the payment request, the label is added to the URI and the label is visible to the sender.
If added to an incoming payment is not visible to the sender and can be used to organize payments/addresses internally. However, it seems that only one label can be added currently, which somewhat limits their usefulness as an organizational feature.
Messages can only be added as part of a payment request. They cannot be added to a payment later on.

Sending
Labels that are added to outgoing transactions, they are only visible to the sender.
Wallet support
I tested with a couple of wallets that I had available (BlueWallet,Sparrow, Blockstream Green, Muun, Bitkit, Phoenix and Bitkey). I did not have access to other apps like Ledger Live, Trezor Suite or the BitBox app.
Most wallets support labels, while only few of them support messages. Apart from Bitcoin Core, only Phoenix and Muun display messages. Bitkey and Bitkit displayed neither the label, nor the message.
Implications
Based on the above, there are a couple of implications that we should consider for the Bitcoin Core App:

It should be indicated to users that if they add labels and/or messages to a payment request, both pieces of information are visible to the sender, if they use a wallet that support them.
If users want to use labels for organizational/accounting reasons, the should add them after the fact.
Users need to be aware that the sender might not see the label and/or message, because they might use a wallat that does not support them.

Possible future scope: tags and notes
Thinking beyond current functionality, it could be helpful to create a clear distinction between information that is shared with the sender as part of a payment request and information that is used internally to organize payments, e.g. for accounting reasons or to support coin control.
This could be achieved by introducing tags and notes. Tags are keywords that are defined by the user to group addresses and payments based on criteria that are relevant to them. Tags can also be used as filters on the activity screen. Notes are free form text that can be used to provide more context to an address, payment or contact.
Some wallets already allow creating tags and internal notes (sometimes also called “note to self”). Many banking applications also support such featues.
Tags and notes would need to be stored locally. Ideally, they are auto-saved and backed up each time the wallet detects a new entry or a change of existing ones.

Does anybody know what the rationale is for adding the address label to the URI, as is currently the case for QT? Why does the sender need to see the label that I give to my address?
Seems to me that, as a receiver, I want to use labels for myself to organize my receiving addresses. If I want to convey some information to the sender, I would add a message to the transaction.

That is what we need to clarify. With the current logic, I think there are two ways users can use labels, neither of which are great.

I use the label to let the sender know my name. The sender can then correctly connect my name with my address in their contacts. Problem is that this label is also connected to my receive address in my app. Result is that all my addresses are labelled with my own name in my app.
I use the label to identify the sender. That way I can see which address I used for my transaction partners. But then the sender has their own name connected to my address and the transaction in their app.

The message is very simple, it explains what the transaction is about. That is helpful for both parties. If the label is shared, then it also has to serve both parties, and cannot be used by one party for their own purposes. That leaves us with three options:

Change the label info so that it serves both parties well, like including both names "Alice - Bob"
Do not share the label, so that the requester can use it for their own purposes. The sender can then add their own label
Create another label that is not shared. Rename label 1 to "From" and label 2 to "To". Then share "From" so the sender knows who is making the request, and keep the "To" attached to the receive address/transaction, so the requester knows who they transacted with. This would not need any under-the-hood technical changes.

Does that make sense?

I would go for options 2 or 3.
What I like about option 3 is that it reinforces the fact that it is a transaction between two people. It also implicitly guides users what they should enter in these fields. The term "label" is quite vague in that regard.
"From" and "To" might be a bit tricky, though. I would use them in the opposite way :-). As a user filling out a payment request I would associate the word "From" with the person who will pay me, and "to" with myself, as I'm the recipient of the payment. What about "Payer" and "Requester" (or something like that)? Would make the roles more clear, I think. So, the basic structure of the form would be: Amount, From (Payer), To (Payee), Message.

Is message the field to say what the payment is for? Then a nice default label in my wallet after they pay my payment request is Payer - Message or From Payer, for Message.
People are familiar with Cash App adding “For” before a transaction label.
On Fri, Jul 19, 2024 at 00:29, Michael Haase @.***(mailto:On Fri, Jul 19, 2024 at 00:29, Michael Haase < wrote:

I would go for options 2 or 3.
What I like about option 3 is that it reinforces the fact that it is a transaction between two people. It also implicitly guides users what they should enter in these fields. The term "label" is quite vague in that regard.
"From" and "To" might be a bit tricky, though. I would use them in the opposite way :-). As a user filling out a payment request I would associate the word "From" with the person who will pay me, and "to" with myself, as I'm the recipient of the payment. What about "Payer" and "Requester" (or something like that)? Would make the roles more clear, I think. So, the basic structure of the form would be: Amount, From (Payer), To (Payee), Message.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: @.***>

"From" and "To" might be a bit tricky

Yeah, that does not work too well... it's helpful to see it mocked up.
How about something like this?

First, I changed the title. The tab is already called "Receive", so having another title that also says "Receive" is not super helpful. "New payment request" is a better description of what the user is doing here.
The main part though are the two fields "Your name" and "Private note".

"Your name" is what is appended to the BIP21 URI as the "label". The app can remember that across payment requests to the user does not have to type their name every time.
"Private note" is what is attached to the address (and payment request) that is being generated.

"Your name" then is pre-filled in the senders transaction as the label description for the receive address. So it helps them have nicely labelled addresses.
How does that work for you? Is the usage intent clear?

If users want to use labels for organizational/accounting reasons, the should add them after the fact.

@rabbitholiness I don't get how did you end up with that conclusion? Why after the fact?
Option 3, treating "label" as a place to put your name seems strange to me. Mostly because I assume other wallets do not treat it like this, so when interacting with other wallets, there may be some confusion and very different usage of this field between different wallet users (confusion can arise then on both ends, Core user and other wallets users). I think no other wallet treats labels/messages like this.
Also, bitcoin wiki advices not to use person identifiers but payment identifiers:

Payment identifiers, not person identifiers
Current best practices are that a unique address should be used for every transaction. Therefore, a URI scheme should not represent an exchange of personal information, but a one-time payment.

Although at the same time they use Luke Jr name as an example of Label.
For me, option 2 is the best and most simple and rational:

Do not share the label, so that the requester can use it for their own purposes. The sender can then add their own label this option

So we'd keep quite general description of the field "label", which is also what lots of current bitcoin users are used to. I think most users, and this is also confirmed partially in our user research, use it to manage UTXOs and thus identify the source of funds. Label then is not shared, making sure it's a private message for the owner of the wallet.
Only worry for me is for users who might want to have both Label and Message shared with the sender. Eg. Labeling it "Monthly Payment" and then putting invoice number in "Message".
Also I wonder If this won't break a bit how BIP21 is used and eg unified bitcoin QRs

On further thought, I wouldn't hide label, as I think it can break some UXes around transacting. If we want private note, then that could be additional thing as @GBKS pointed out. After some reading and playing with Core and the fact that Label is then used for "address book" seems like it really works well as "name", although I wonder If we can also accommodate different usage (naming rather transaction, than identifying ourselves):

Otherwise I like Christoph idea to just prefill that with last used Name

If users want to use labels for organizational/accounting reasons, the should add them after the fact.

@rabbitholiness I don't get how did you end up with that conclusion? Why after the fact?

Because, currently in Core, the label will be shared with the sender, if you include it in the payment request. So if you want to use a label privately for your own reference, then you need to add it after you have shared the payment request. That means if we keep the current behavior, users need to be aware of this. But I don't think this is very intuitive or convenient. That's why I don't think Option 1 is the one to go with.
Option 2 is fine for me as well and will work as expected for users that use, for example, Sparrow or other software. I think we can always fall back on this, if we can't make Option 3 work.

Only worry for me is for users who might want to have both Label and Message shared with the sender. Eg. Labeling it "Monthly Payment" and then putting invoice number in "Message".

This is why I think exploring option 3 is a worthwile exercise. It gives a bit more flexibility to the user.  But I see your point that the wording should be consistent with other wallet applications. So keeping "label" or "address label" but not sharing it with the payer makes sense to me.
Something like in the screenshot, maybe? The term "subject" could be interesting, it can be my name or some other identifying piece of information. "Subject" plays well with "message" (like emails have a subject and a message). It also makes it clear that those two pieces of information will be shared with the payer, while the address label will stay private, as a tool for me to manage receiving addresses. The subject could be prefilled on the payer's side as @GBKS describes.

I don't think we have to maintain the wording used elsewhere if it is confusing. We could always have a small tooltip to indicate how other wallets might use it.

I updated the design docs for receiving with a note about how we are looking to clarify the use of labels. I think the design intent makes sense and our logic is sound, even if we tweak the actual label text a bit further.

Here's a table of current proposals by @rabbitholiness

Just stumbled across Labelbase, a service to manage and sync labels. Have not dug deep, but maybe there's something relevant there to this conversation.


use vscode-jsonrpc library for JSON-RPC implementation

The vscode-jsonrpc library is from the VS Code team and is used by many VS Code extensions and language servers to implement the JSON-RPC 2.0 protocol. It is well tested, heavily used, actively maintained, and supports both Node and Web platforms.
Using this library means we can remove our own implementation of the protocol, which is (a) less code to maintain, (b) does not support the browser, and (c) is not spec-compliant (for example, it supports string values as request parameters, which are disallowed by the JSON-RPC 2.0 spec).
Test plan

CI
Manually ran BFG (cody-engine) tests and confirm that embeddings/set-token works in the output channel
Manually tested with the CLI
Test with JetBrains
TODO before merging: compare protocol traffic with JetBrains before and after to ensure no change (or no meaningful change)

I'm looking into the git conflict. I didn't intend my websocket PR to conflict with this PR. I don't mind if the websocket functionality breaks

@sqs I rebased and fixed the websocket implementation to use the new vscode-jsonrpc API, which is btw much nicer to use than the old custom implementation.
I didn't want to force push to your branch. Feel free to rebase on top of main and delete changes from my websocket PR that are causing conflicts/errors. I can send a separate PR to fix the websocket demo
https://github.com/sourcegraph/cody/compare/olafurpg/jsonrpc-rebased?expand=1

@olafurpg Great, thank you!
Instead of fallbackHandler in the old impl, I think you can use the no-method-name overload of onRequest and onNotification.

Yes, i got it working with that API. It’s much cleaner than the old implementation https://github.com/sourcegraph/cody/compare/olafurpg/jsonrpc-rebased?expand=1#diff-19e9fdb0f04bd6f4c5559271fea95b9e91ed400c1c9d246df0e58a964367fe6fR27
I’ll send a separate PR to fix web sockets once this PR lands.

I will merge this as the QA report came back clean (no add'l flaws introduced by this PR, confirmed in https://sourcegraph.slack.com/archives/C06GFD8GFCN/p1713820552328229 and https://github.com/sourcegraph/jetbrains/pull/1368#issuecomment-2071995482).


Truncation one more time

Overhauled the truncation logic to be much more specific to when it is triggered. Each row will have a resize sensor added that triggers when the height changes instead of when new row values are present. This causes the logic to trigger less often and be more performant.
Made the "update" event trigger on reference-modified as well (instead of when rowValues changes) for the same reasons as above.
To handle the specific case with aggregates that we saw most of the problems with, after aggregates data is loaded, an event is emitted that adds a resize sensor to the specific cell in the table.

You should also document the class name that you've used in the markdown documentation to warn data-modelers where this class name is used. You should also mention that we're looking for the string, so it shouldn't even be part of their values. And I think because of that we should change this class and at least add a $ before it.


Registering functions and types to the engine at runtime

Motivation
Suppose I want my users to be able to load native Rust functions into the engine at runtime (I'm writing a REPL), by using something like libloading to resolve a symbol from a C dynamic library (.so) compiled by the user:
User code:
// I want the user to be able to load this at runtime.
fn user_func(a : i64) -> i64 { a };

// The user would write something like:
#[no_mangle]
extern "C" fn register_user_func(engine : &mut rhai::Engine) {
	engine.register_fn("user_func", |a : i64| -> i64 { user_func(a) });
}

This assumes the user crate depends on the same rhai version and is compiled with the same compiler version as the host REPL is compiled with, to guarantee the types will have the same memory layout.
The code to load the function at the REPL is something like:
type RegisterSymbol = libloading::Symbol<unsafe extern fn(*mut rhai::Engine)>;

let mut engine = Engine::new();
let lib = libloading::Library::new("path/to/user/crate/target/release/libuser.so").unwrap();
let register_func : RegisterSymbol = lib.get(b"register_user_func").unwrap();
unsafe { register_func(&mut engine as *mut _) };
println!("{}", engine.eval::<i64>("user_func(1)"));

The advantage here is that the user does not need to worry about details of how the engine is being managed. The "host" REPL is compiled once (an might be distributed, for example, with a cargo install) and the user just needs to worry about exposing his implementation of a standard registration API.
Currrently, this won't work because the ahash::AHasher will generate different function hashing strategies at the version compiled by the user, and the version compiled by the host. Any functions registered from a dynamic library loaded at runtime will not be found by the Engine (from a quick investigation, it seems there is an RNG seed generated at compile time that is used to determine the function hash, so this is expected).
Proposed solution
By using constant keys when generating the hasher at src/func/hashing.rs, the hashing at the client and the host REPL will be the same, and the strategy I'm trying above works.
/// Create an instance of the default hasher.
#[inline(always)]
#[must_use]
pub fn get_hasher() -> ahash::AHasher {
    //Default::default() < -- Replace this
    ahash::AHasher::new_with_keys(1234, 5678) // < -- With this (or any other fixed key)
}

I'm proposing to allow this user case by, for example, adding a custom feature that uses a fixed hash, making the function hashes compatible across compilations:
/// Create an instance of the default hasher.
#[inline(always)]
#[must_use]
pub fn get_hasher() -> ahash::AHasher {

    #[cfg(feature="constant_hasher")
    return ahash::AHasher::new_with_keys(1234, 5678);

    Default::default()
}

There might be security issues involved with using the strategy above I'm not aware of, which is why I think this use
case could be solved via a custom feature not enabled by default.

There doesn't seem to be such a limitation right now.  Rhai can already load functions during runtime.  Hashes are recalculated during the Engine::register_fn call, so the external function does not have to worry about hashing.
To support externally-loaded functions with variable parameter types, you may use Engine::register_raw_fn which lets you specify the types of parameters.  However, that is where the real complication arises.
Rhai relies on std::any::TypeId to generate a unique hash ID for each type during compile time.  When you dynamically load an external library, it is quite impossible to generate TypeId's for parameter types, unless they are pre-generated inside that library.
It is understood that TypeId is not stable between compiler versions and possibly different environments. This is the main issue with getting what you are trying to do to work -- mismatch between the external library's TypeId's and Rhai's.

What you need to register a function with Rhai are several things:

The name of the function, which should be easy
A list of parameter types, in TypeId format -- that's the potential incompatibility, especially when the external function is in a library not written in Rust (e.g. written in C)
A pointer to that function, as a trait object like dyn FnAny -- since you're loading the library as an external C library, the function is in C calling convention, not Rust.  Therefore, you may not be able to make it a function trait object to pass to Rhai.

In other words, the main killer would be the incompatible calling convention between a Rust function and a C function, and you won't be able to avoid that if you load the external library as a C lib.

OK, the first Google result came back with this: https://rust-lang.github.io/unsafe-code-guidelines/layout/function-pointers.html
It seems that the C and Rust function ABI's are compatible.  In this case you can make this work.

Thanks for replying in such short notice.
Yes, the TypeId is one issue with this solution. But I'm assuming for now that the external library I want to call will always be compiled from Rust (supporting C libraries is not a goal), using the same compiler version used to compile the executable of the REPL I'm writing. This can be enforced by requiring the user API to implement a call to rustc_version::version(). This can be done, e.g. by requiring the user to implement a trait, which calls this function from a provided method, so the user does not have to worry about it: Just throw a runtime error for any version mismatches before loading the lib. The actual function pointers passed via engine.register_fn are always valid while the REPL is running, since the libraries I'm loading always outlive the rhai Engine.
My objective is not register into the engine the loaded symbols per se - But to just recover a single function symbol from each user library, that does all the registration. The user will have rhai as a dependency, and call register_fn from his own function implementation. When that happens, the function will receive a hash. The Default implementation for AHasher that rhai uses relies in the following static variable located at aHash/src/random_state.rs (Line 49):
#[cfg(not(all(target_arch = "arm", target_os = "none")))]
static RAND_SOURCE: OnceBox<Box<dyn RandomSource + Send + Sync>> = OnceBox::new();

Here is where the issue happens: The user compiled his lib with one static copy of the variable, but my REPL (that instantiated the engine) uses another copy of the same static variable, so the hashes will be different, and rhai will be unable to find the function, returning an EvalAltResult::ErrorFunctionNotFound. If instead of using the Default implementation for AHasher, we call AHasher::new_with_keys (aHash/src/aes_hash.rs(Line 53) as in the snippet I provided above, there is no dependency on any static variables when creating theAHasher, and rhai` is able to load the user function successfully.

Ah.  Now I get it.  You'd need a hasher with predictable output, such as from a static seed.
I believe AHasher supports such a thing... it may be easy to add to Rhai, as all hashing boils down to one single function to get a hasher.

Exactly. If you agree to implement this as a crate feature I can submit a PR with the implementation I described above. There are other ways to do that as well (for example, with an Engine::new_with_hasher method), but implementing as an optional crate feature wouldn't require any changes in the API, or require the engine to have one extra field to store a custom hasher.

Yes, I suppose this usage is rare enough that making it into the API doesn't make sense.  Better have a feature gate.
Would appreciate a PR!  Hopefully you can demonstrate that it works in real life...

Come to think of it...  I am not sure why AHasher would need to be called both in the library and in the Engine... the only place where hashing is performed is all done in the Engine...
The only place where such hashes are calculated outside of the Engine is when you create a module (which requires hashing the functions).
So if you define modules in the library, then yes, both hashers need to be the same.  If you use register_raw_fn exclusively, then you won't need to have two hashers.

I'm not worrying about modules yet. My objective, for now, is just to load types and functions using register_type and register_fn to the global namespace, but called from possibly many differently dynamic libraries compiled from different user crates (I'd like to avoid the raw function API, since register_fn is much simpler to use, since the user can just forward plain Rust functions to it without worrying about determining TypeIds manually). I fail to see how the issue can solved with the raw function API, however. Suppose the user calls register_raw_fn instead of register_fn. This retrieves the global namespace module and eventually calculates a hash at set_fn (src/module/mod.rs:705):
let hash_fn = calc_native_fn_hash(empty(), name.as_ref(), &param_types);

Which is stored at the functions field of the module.
Now when the program evaluates any expression, it will hit another call requiring the hasher at the parsing stage, such as in parse_stmt (src/parser.rs:2815):
let hash = calc_fn_hash(&func.name, func.params.len());

So the problem still persists even if the user uses uses the register_raw_fn API from its library, that is, if I'm not missing some detail about the implementation. FnCallExpr always uses functions hashes, right?

register_fn internally calls register_raw_fn so I believe it is simply syntactical sugar.
Yes, function hashes are pre-calculated during parsing, but usually you parse the script with the same Engine, right?  No?
If you parse the script with the same Engine which you did the register_fn, then it'll use the exact same hasher.
Of course, you can compile the script with a different Engine from a different library (why would you want to?), in that case yes, you'd need stability of the hash.
I suggest you simply do it and see if it works as it stands.  You can debug-print the AST to check out the hashes.  If you want, I can include the function hashes in the JSON metadata. Then it would be easy for you to debug.

Yes, the engine to which functions are registered is the same engine that does the parsing. I think that storing an empty ahash::AHasher as a field of the Engine (cloned each time it is required) is another way to solve the issue, since it would behave according to your description. But in the current implementation, it seems parse_fn_call makes calls to calc_fn_hash and calc_qualified_fn_hash, which in turn creates a new instance of ahash::AHasher via its Default implementation every time a rhai expression containing function calls is parsed. You can assume this hasher behaves in the same way when all functions are registered and used from the same compiled crate, but you run into the duplicate definition of the static random key I mentioned above if they are registered from crates compiled separately.
I am currently able to register functions from a dynamically loaded library by depending on a local version of rhai where I made the change to a fixed hasher I described initially (although only for primitive types such as i64 and f64, I'm still running into some TypeId mismatches for String functions).

In that case, maybe the solution is, as you said, to create a field that holds a AHasher with an (possibly internals) API Engine::new_with_hasher and/or Engine::new_raw_with_hasher.

After some experimentation, it would not work to add a hasher field to Engine unless I add one to the entire API for Module as well, since it is entirely possible that modules are built outside of any Engine.
How about going the other way and providing Engine::get_hasher which returns an instance of the AHasher used by the Engine?

The way I see it the feature gate is the least disruptive way of allowing functions to be registered across compilation units. There might problems of doing that in a security-sensitive context (which I don't have any experience with), so perhaps the feature, if implemented, could be named untrusted_hasher or something like that.
Today I came up with a different approach to solve the problem that does not require the client to register the functions, therefore not requiring any changes to the hashing logic as it is implemented. Instead of doing the registration on the user-compiled dynamic library, I do it on the host, by requiring the user to just hand me the information required to register all his functions (packed into a boxed type for FFI compatibility):
// User will implement something like this...
pub struct FuncInfo {
    name : String,
    args : Vec<String>,
    func : CallableFunction
}

#[no_mangle]
pub extern "C" fn get_funcs() -> Box<Vec<FuncInfo>> {
    // User passes his functions here
}

// And the host registers them with
let mut module = rhai::Module::new();
for func in *get_funcs() {
  // Keep TypeIds of host in a parallel registry  (a HashMap<String, TypeId>,
  // mapping namespace-qualified types to ids)
  let param_types : Vec<TypeId> = func.args.iter().map(|ty| { host_types[&ty[..]] }).collect();
  module.set_fn(
      func.name,
      rhai::FnNamespace::Global,
      rhai::FnAccess::Public,
      Some(&func.types.iter().map(|v| v.as_ref()).collect::<Vec<_>>()),
      &param_types[..],
      func.func,
  );
}
engine.register_global_module(module.into());

I can confirm this works to register functions that work with basic rhai types. I've managed to write a generic wrapper similar to what set_native_fn does, but to generate this intermediary structure instead of registering the function directly. I haven't tested custom types yet, but since registering them seems to just add their name to a catalog kept by the engine, it can be done on the host side as well. I'm still studying how to register getters, setters and iterators using this method.
It seems the library had the tools that I required to solve this problem (CallableFunction and RegisterNativeFunction::into_callable_function), but this solution wasn't clear to me at first, registering the types at the client library seem to be the most natural solution. If support for dynamically-loaded modules is not planned for rhai, perhaps this topic is a good candidate for at least a new documentation section.

Instead of doing the registration on the user-compiled dynamic library, I do it on the host, by requiring the user to just hand me the information required to register all his functions (packed into a boxed type for FFI compatibility):

I think this is the best way to approach it.

If support for dynamically-loaded modules is not planned for rhai, perhaps this topic is a good candidate for at least a new documentation section.

In my personal project (that uses Rhai), I actually load dynamic libraries, but I didn't do it like you did by wrapping a CallableFunction across FFI.  I recreated the Rust API on the host side.  Your solution seems much easier and versatile except for the fact that we must use the same version of Rust to compile both (TypeId stability).  This may not work out for my requirements, but some applications may be able to live within this restriction.
I strongly suggest you have a working example up, and I can write up a new "patterns" topic for the Book.

Relying on TypeId might be an issue even using the same compiler version. One issue I ran across had to to with ImmutableString. I could get the TypeId of i64, f64 bool (all from standard library) to match between host and client. But the TypeId of ImmutableString did not match (even with the same compiler version, and depending on the same local rhai version). I suspect that it is because ImmutableString relies on a third-party crate for its implementation (SmartString), and the compiler might be doing something different to resolve the TypeId across builds. Since rhai converts the TypeId of any &str and String arguments to ImmutableString internally, any functions relying on those also didn't work. That was the motivation of passing the type names into my FuncInfo structure: I just match the qualified type names recovered at the client with RegisterNativeFunction::param_names() to the TypeId at the host. Since rhai relies on just a few types and I'm not anticipating any collision issues.
I do plan on getting my solution published (it will most likely be a REPL offered with a cargo install), but I have to figure out some things first. For the host to have feature-parity with the generic functions that rhai offer (register_type_with_name, set_getter_fn, set_iterator, etc), I would need non-generic versions of those (register_raw_type_with_name, set_raw_getter_fn, set_raw_iterator, etc) that took the std::any::type_name() and std::any::TypeId::of of the user types. It is not hard to duplicate the implementations for how, since they all call Modue::set_fn internally, but having them implemented at rhai itself (perhaps behind the internals feature) would minimize the risk of incompatibilities at future updates. I'll share the implementation once I have it working.

If you're only facing a mismatch for ImmutableString, maybe we can track down the core problem?  For example, display the TypeId both in Rhai as well as in the external library.
Try with other structs to see if it is a struct issue, or at the core a SmartString issue.
To be honest, I can't think of a reason why TypeId would be portable for some types and not others... so there must be a deeper reason at work here.

I checked on my machine and Rust seems to generate the same TypeId for ImmutableString every time...
One of the potential reasons is perhaps two independent copies of SmartString being compiled in by cargo with the Rhai host and the library both using different versions of the same SmartString?
I'd definitely suggest trying the TypeId for, say, Vec and some other structs to see if you get a consistent behavior here.

OK.  Gotcha.  I think I know why.
TypeId seems to include the entire path of the type in its hash.
That's why primitive types like u64 are the same.  They are called the same in all places.
For ImmutableString, the Rhai Engine and  your library see differently:
Engine:  I see crate::ImmutableString
Your library: I see rhai::ImmutableString
So the names of non-primitive types are different because their roots are different.

I think you are right about the namespace difference. Relying on the std::any::type_name() seems to be a safe bet for now, since fully qualified namespaces won't change between client and user. I'm now using the client type names as indices to a HashMap (built at the host) which hold the host TypeId for primitive and rhai types. Any time I register a new user type, I have to register the client TypeId as well, since the host has no way of knowing what types exist or not. Since the hashes seems to be based on the type namespace, I don't think duplicates will be a problem.
I've now managed to replicate everything that rhai does with generic registering functions, but to generate the intermediate FuncInfo that is handed to the host. Methods, indexers, iterators, all looking good. The only change I had to make to rhai was to add a non-generic way to register types:
impl Engine {

// ...

/// Registers a type to the engine that is not actually represented in the current
/// program (e.g. a type that exists only behind a dynamic library loaded at runtime).
/// Qualified is the output of std::any::type_name(). short is the actual type name
/// (used to build the function metadata)
pub fn register_raw_type_with_name(&mut self, qualified : &str, short: &str) -> &mut Self {
    self.type_names.insert(qualified.into(), Box::new(short.into()));
    self
}

// ...

}

Any chance of adding this to the library? This is the only missing piece I need.

One extremely simple solution is to simply replace TypeId::of::<ImmutableString>() with a pre-generated constant, thus both sides can use it.

Come to think about it, even simpler would be to have a new internals only API that simply returns the TypeId of ImmutableString as a value so outside crates can use it.

Any chance of adding this to the library? This is the only missing piece I need.

That would be no problem at all.  I'll add that under the internals gate.

Great! Give me a few days to check that everything is working, and if so I'll close this issue.

I can confirm my solution is working with the 1.3 version of the crate, since it is now possible to register types to the engine without knowing what the types are at compile time (via Engine::register_type_with_name_raw). For future reference, the best way to solve the issue was to send the rhai::plugin::CallableFunction over the FFI boundary (easily obtained via RegisterNativeFunction::into_callable_function). At the host engine, the user function can then be registered via Module::set_fn. Doing that avoids both the hashing issue, and the possible difference between the TypeId of ImmutableString at the host and client REPL. Thanks for the help, hopefully I'll have something up on crates.io soon.


Free-D protocol frame

Hello,
I allow myself to contact you because I would like to know if you have an example frame, information expected by your plugin. I find very little information about the Free-D protocol. I would like to be able to send it in generic because we are developing our camera system with javascript.
Thank you in advance for your reply.
Cordially,
vincent
PS: sorry we are French and we made the translation with google

if you need Free-D packet composing code, please refer to:

https://github.com/max-verem/LensRotaryDecoder/blob/master/src/common/FreeD.h
https://github.com/max-verem/LensRotaryDecoder/blob/master/src/common/FreeD.c

example of using:

https://github.com/max-verem/LensRotaryDecoder/blob/master/src/ez-usb/freed_sender.c
https://github.com/max-verem/VRPN-FreeD-OpenVR/blob/77207206d34043c0e26dacab9cd07e45e83632e7/VRPN-OpenVR/vrpn_Tracker_Camera.cpp#L202

also some docs available at https://github.com/max-verem/VRPN-Free-d/blob/master/free-d.pdf

Thank you for the information, we have successfully sent the data.
Do you know the baud rate enough to get a few things smooth?
Do you know if it is simple to convert pelco-d protocol to free-d?
Thank you very much for your work


ESP8266EX Support?

I can successfully flash my ESP32 but not my two NodeMCU (v2 and v3), which use the ESP8266EX Chip.
error: "failed_initialize", details: Error: Unknown Chip
esptool flash_id returns:
Connecting....
Detecting chip type... ESP8266
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: c4:5b:be:60:dc:da
Uploading stub...
Running stub...
Stub running...
Manufacturer: d8
Device: 4016
Detected flash size: 4MB

I just tested it myself on a node that's not possible to flash using the web flasher:

esptool.py v3.0
Serial port COM3
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: 2c:3a:e8:39:13:7c

Flash Chip ID:
Vendor: 0x20 Device: 0x4016

Flash Chip Real Size:
4096 kB

Flash IDE Size:
4096 kB

Flash IDE Speed:
40 MHz

Flash IDE Mode:
DOUT

I added a breakpoint to where the exception was caught:
domexception a framing error has been detected

I can successfully flash my ESP32 but not my two NodeMCU (v2 and v3), which use the ESP8266EX Chip.
error: "failed_initialize", details: Error: Unknown Chip
esptool flash_id returns:
Connecting....
Detecting chip type... ESP8266
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: c4:5b:be:60:dc:da
Uploading stub...
Running stub...
Stub running...
Manufacturer: d8
Device: 4016
Detected flash size: 4MB

Can you post what you see on Chrome's Developer console?

I just tested it myself on a node that's not possible to flash using the web flasher:

esptool.py v3.0
Serial port COM3
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: 2c:3a:e8:39:13:7c

Flash Chip ID:	Vendor: 0x20 Device: 0x4016
Flash Chip Real Size:	4096 kB
Flash IDE Size:	4096 kB
Flash IDE Speed:	40 MHz
Flash IDE Mode:	DOUT

Is that all the console shows? No log messages?

Yep that's all.

Yep that's all.

Can you click on the login levels and enable Verbose?

Not much of a difference:

Can you post what you see on Chrome's Developer console?

It's empty, even with Verbose enabled. Do i need to do anything to have more debug output like @TD-er above?

By default it doesn't log to the console. I've just updated the demo to do so e6a198fefab3d16193bf797a51345716c96c82ab

By default it doesn't log to the console. I've just updated the demo to do so e6a198f. So now open the chrome dev tools prior to flashing.

Thanks, now i see the following in the console (with the demo)

This attempt I held GPIO-0 down and pressed reset.
The attempt before was the same, without reset + GPIO-0 held.

The error has been updated by @conradopoole. It should now include the magic value of your chip so we can add it to the detected list. The demo has been updated.
@ghtomcat can you try it again and report the updated error message.

The error has been updated by @conradopoole. It should now include the magic value of your chip so we can add it to the detected list. The demo has been updated.

start-flash-542f194f.js?module:1 Error: Unknown Chip: 0x-F3EFF

Me too, on one module (with CH340) Unknown Chip: 0x-F3EFF
The unresponsive unit with CP2102 USB to serial:

@ghtomcat this should be fixed in 3.4.2, live now in the docs. Can you try it?

Does work with the node that gave the unknown chip error before.
The "timeout" unit still remains unresponsive, but had not expected it to be fixed by these changes.

@ghtomcat this should be fixed in 3.4.2, live now in the docs. Can you try it?

Works now, thank you!

Me too, on one module (with CH340) Unknown Chip: 0x-F3EFF
The unresponsive unit with CP2102 USB to serial:

Did you get the CP2102 working? if have the same issue with ESP8266EX modules

@degraafm76 yes, after the update it works for cp2102 and ch340

@degraafm76 yes, after the update it works for cp2102 and ch340

I see, but not for ESP8266EX modules.


Update go versions

Version 1.19 is not supported anymore.

I'll remove support for 1.20 in a follow-up PR. For now, I just wanted #74 to go through.


Slack pings

The key has already been added as a repository secret. The original file with the Slack nicks will be added to the centos-bot repository so it can be updated easily.
You can test this locally by replacing the slack_notify statements with prints.

I think you also need to pass the secret in the github job: https://docs.github.com/en/actions/security-guides/encrypted-secrets#using-encrypted-secrets-in-a-workflow

Great catch - thanks for looking closely! (fixed)


Distinguish the key with same scancodes but different logical key.

Use case
This issue is extracted a common issue from #97582 and #88646.
In some cases, keys with same scancodes (both is zero) will be pressed at the same time with different logical key.
This is caused by emulated shortcut key on Windows.

On Windows (both win32 apps and web pages on Chromium), pasting from Win+V will emulate Ctrl+V but both scancode is 0.
On web on Chromium on Windows, the emoji item in the context menu on textarea will emulate Win+period but both scancode is 0.

In #97582 and #88646:
This assertion https://github.com/flutter/flutter/blob/ab89ce285f76a3fd3a9328ec863f5623cc5ac8bc/packages/flutter/lib/src/services/hardware_keyboard.dart#L444-L447
is fails because PhysicalKeyboardKey uses the pyhsical key to remain pressed keys: https://github.com/flutter/flutter/blob/ab89ce285f76a3fd3a9328ec863f5623cc5ac8bc/packages/flutter/lib/src/services/hardware_keyboard.dart#L392
Proposal

The PhysicalKeyboardKey should distinguish these keys.

/cc @dkwingsmt @gspencergoog

Thanks for investigating this! Can I assume that this only happens when the scancode is 0?

On web

On Chromium it seems empty (I forgot to write, sorry), but it is reported as bug (https://bugs.chromium.org/p/chromium/issues/detail?id=834189).
On Firefox, code can be Unidentified (KeyboardEvent: code values | MDN), but in Win+V, Firefox seems to complete code from virtual key.

On Windows, it seems 0.

I saw these behavior, but I couldn't find any docs to describe this.

Thank you! I'll take a look when I have time.


How i can get IV

Hello dear
if I received the key_material , I need IV code  to continue the steps and get the private key.
How do I get this code for the following step
key_material: cff3213b83e3d3246335960c6db1f7e127dfba5e7086a77fc6285595b110bcc7
For ckey or mkey: b801713ef9e6e80d3033335623503f018eb953faffa4908cff7cdb5479bf68ac5f6d7b16f3f9ac028e781201dfedea87

but i need iv for
aesdecrypt IV Key (Mkey or Ciphertext)
where is  iv   ???

The second question ??
When I use the following code
`keyderivation nDerivations salt passphrase

keyderivation 43401 ea6a06bbdd04a660 S5315926
crackBTC > keyderivation 43401 ea6a06bbdd04a660 S5315926
derivation_n: 43401
mkey_salt: ea6a06bbdd04a660
passphrase: S5315926
sha512:
cff3213b83e3d3246335960c6db1f7e127dfba5e7086a77fc6285595b110bcc7de6d96008e7bf97c50cbe7f3bc1ec3a6148ff9402196345206a2f54fc5d479a3`

cff32.....  Is this code sha512?
Isn't there a way to get the code sha512 without having a password, that is, because the password is inside the wallet.dat file, there is no way to get sha512 without a password ???

Hello dear
if I received the key_material , I need IV code to continue the steps and get the private key.
How do I get this code for the following step
key_material: cff3213b83e3d3246335960c6db1f7e127dfba5e7086a77fc6285595b110bcc7
For ckey or mkey: b801713ef9e6e80d3033335623503f018eb953faffa4908cff7cdb5479bf68ac5f6d7b16f3f9ac028e781201dfedea87

but i need iv for
aesdecrypt IV Key (Mkey or Ciphertext)
where is iv ???

@azalia22
But first how to ckey and mkey?
Where can i store my wallet.dat file?

cff32..... Is this code sha512?

Yes

Isn't there a way to get the code sha512 without having a password, that is, because the password is inside the wallet.dat file, there is no way to get sha512 without a password ???

No there is no other way, well the only other way is with brute force but is an infeasible method.

where is iv ???

The first IV is calculated with the Keyderivation of the password it is part of the final sha512 so... again without the password, only with brute force

btw, for your example your IV is de6d96008e7bf97c50cbe7f3bc1ec3a6 the command for decrypt is:
aesdecrypt de6d96008e7bf97c50cbe7f3bc1ec3a6 cff3213b83e3d3246335960c6db1f7e127dfba5e7086a77fc6285595b110bcc7 b801713ef9e6e80d3033335623503f018eb953faffa4908cff7cdb5479bf68ac5f6d7b16f3f9ac028e781201dfedea87
Result:
91a51203a850aa01686ac61449da6f85794d01fdff4628e9cdeaeb77f7e5eba510101010101010101010101010101010
without the<PHONE_NUMBER>1010101010101010101010
91a51203a850aa01686ac61449da6f85794d01fdff4628e9cdeaeb77f7e5eba5

Hey there
How you get material key ?
Which key I have to use


Implement domain deprecate CLI command

What changed?
Implement domain deprecate CLI command

Why?

How did you test it?

Potential risks

Coverage decreased (-0.09%) to 64.487% when pulling 5745415bb2c0bf280535437294a4ad573a74bfae on Shaddoll:cli-domain into 174ca09326c2fb483092dc8266a194f51cff170c on uber:master.


Slime organs metabolizing slime restores blood level

About the PR
Slimes metabolizing slime now restores blood level.
Removing 15us of slime from a slime person reduces their blood level to 95%, so injecting (or otherwise metabolizing) 15us of slime will lead to their blood level being increased by 5%.
Why / Balance
Currently when a slime (or anyone, really) is injected with slime it only satiates their hunger. Slimes have slime in them, so it makes sense for the amount of slime in them to be restored when it's put back in them.
Note, this is nowhere near as effective as saline, it's a 1 to 1.
When you drink slime off of the floor, you drink 1u after the do-after, so... you're getting there in terms of your blood level but it's not crazy.
Although it would be pretty funny to see the police barge in on a nuclear operative slime person drinking their slime off the floor because the agent died.
Technical details
YAML editing lmao.
Makes it so that the slime reagent has a drink metabolism that restores blood level by an amount of 0.5 if the metabolizing organ is a slime organ.
Media

Slime has variables changed so they no longer restore blood on their own accord. 30u of slime is removed from their body. Blood level falls to 90%.

15u of slime is injected into them. Their blood level increases to 95%.

Requirements

[x] I have read and am following the Pull Request and Changelog Guidelines.
[x] I have added media to this PR or it does not require an ingame showcase.

Changelog
:cl:

add: Slimes can now metabolize their own slime to restore their blood level.

Infinite food glitch? :trollface:
Blue blood restoring spider blood level for infninite gauze soon? :godmode:

Infinite food glitch?
If that's a big concern (this was already achievable before this PR, this change just makes it 5% more feasible) we can just have that happen when it's not a slime organ.

Blue blood restoring spider blood level for infninite gauze soon?
Be real.

Infinite food glitch? :trollface:

Wonder if it's possible to make slime not satiate slime people unless they're full of blood

New additions (I'm done with changes unless maintainers have any other ideas).
The effectiveness of hunger satiation is halved (when metabolized by a slime organ) to discourage people from drawing and injecting their own slime over and over for free satiation.

CL update soon.


Fix pydantic ValidationError due to wrong default type

I am currently implementing the same in ekss, as it is testing the Vault config a type problem with the default None has occurred. Should we implement this as suggested in this PR or with Optional[str] and Optional[SecretStr] and default is None?

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 81.871%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
280

Relevant Lines:
342

💛  - Coveralls

Looks good now, but the openapi.yaml still needs to be updated.


Considering using output format xml to decode notarization output? #question

Hi,
Maybe for compatibility issue with old 4d version or for performance issue you use some regex trick.
But I am wondering if it will be much cleaner to use '--output-format xml' for actool command.
Then decode plist format XML as c_object (using dom or command line plutil -convert json)
For instance it easier to decode other key like productErrors with useful information about process

that definitely sounds smarter!

Parsing productError could help to bypass some issue
Like this one managed by chromium if you request info just after uploading
https://github.com/stanleywxc/chromium-noupdator/blob/637f32e9bf9079f31430c9aa9c64a75247993a71/src/chrome/installer/mac/signing/notarize.py#L86


Dockerfile rework

Negative approach to .dockerignore
Reintruduction of turbo
Removed unnecessary apt-get calls

closed in favor of https://github.com/w3f/1k-validators-be/pull/2939, will address all of the comments there


Negative max field in tables filters incorrectly on production

Was found here https://github.com/ral-facilities/inventory-management-system/pull/1087#discussion_r1819194053. Currently traced to https://github.com/ral-facilities/inventory-management-system/pull/922 having fixed it for catalogue items and items tables. Not sure why though. It is currently present in production and may effect other tables as well.

Traced it to the preserved state changes - still not sure why, but passing columnFilterFns: {} as the table state fixes the issue.

Its also present on their MRTs website: https://www.material-react-table.com/docs/guides/column-filtering. Only when I go to open it in code sandbox the issue disappears. I tried recreating my lockfile and deleting MRT but it still did not fix.

https://github.com/ral-facilities/inventory-management-system/pull/1046 breaks it just for the items table for some reason.

I think this occurs as MRT assume the that the default minimum value is 0  so if you have a max range to -687 it would be invalid as it from min: 0 to max: -687.
There are really two valid fixes for this.

We add extra error handling to the range columns filters in MRT (for invalid ranges)
We change the minimum value default value from 0 to negative infinity

#634


orca: update the orca validation annotation

By the way, I think we may also need to add a validation to the request_cost to ensure every cost value is larger than or equal to 0?

@adisuissa


Model local scope receives different amount of parameters

Laravel Version: 5.8.14
PHP Version: 7.1.28
Database Driver & Version: MySQL

Description:
Model specific scope sometimes receives 2 instead of 1 parameter.
This makes it almost impossible to pass and unpredictable to pass in random parameters,
because at one point you get TooFewArgumentsPassed exception.
Model relations
Model1 => hasMany Model3
Model3 => hasMany Model4
Model4 => belongsTo Model5
Model2 => ManyToMany Model5
Model5 => ManyToMany Model6
Scenario 1:
Model1->load(Model3.Model4.Model5 => function($query) {
    $query->with([Model6 => function($query) {
        $query->whereStuff($parameter);
    }]);
});

Scenario 2:
Model2->load(Model5.Model6 => function($query) {
    $query->whereStuff($parameter);
});

Steps To Reproduce:
create local scope in Model6:
public function scopeWhereStuff() {
  dd(count(func_get_args));
}

Scenario 1:
dd(count(func_get_args)); => 2
Scenario 2:
dd(count(func_get_args)); => 3
In case of scenario 2 a BelongsToMany relation is also passed in.

Can you maybe recreate this problem on a sample app and load it up to a public repo on Github so we can reproduce? There seems to be some really specific code involved here and it would help if we got all of that. Try to commit the faulty code in a separate commit if possible.

Please provide a minimal example with default models like User, Post and Comment.

I will look more into it. I did create fresh project with schema I pointed out in original post and the issue didn't replicate.
I then cleaned our code to be more representable to schema and the issue persisted.
I have solved it currently with just getting param count with func_get_args, once I have a bit more time I will try go more deeply into the problem.
Thank you for helping out

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel free to reply if you're still experiencing this issue.


fix: new folder with contextMenu after rename file cause error

Types

[x] 🐛 Bug Fixes

Background or solution
fixed #458
Changelog
new folder with contextMenu after rename file cause error

/publish

LGTM


Using nbclean to hide lesson code in output

Look into using nbclean to hide specific parts of the lesson code. @lwasser

this also should be closed


Flutter TextFormField space between  input text and the bottom line.

I am an anroid developer and I have just started developing for flutter. I am creating a login screen in which TextFormField is used with label but the space between the input text and the bottom line is larger as from android Edittext, so am I missing some property or it behaves likes this?

I also have created a form and getting this space between text and bottom line

@HansMuller Hey thanks. The link you shared has the perfect solution for my problem. It is solved now.

But the error here is that now the labelText and hintText are getting squeezed(i.e are just starting directly no padding or margin inside the border) if the border is enabled

But the error here is that now the labelText and hintText are getting squeezed(i.e are just starting directly no padding or margin inside the border) if the border is enabled

use contentpadding: const Edge.........
in the decoration

As described above the issue can be solved by using  contentpadding property in textField and here is the code.
TextField(                         //maxLength: 10,                         maxLines: 1,                         minLines: 1,                         keyboardType: TextInputType.number,                         decoration: InputDecoration(                             //labelText: "Phone number",                             hintText: "Phone number",                             contentPadding: EdgeInsets.all(5),                             isDense: true,                             enabledBorder: UnderlineInputBorder(                                 borderSide: BorderSide(color: Colors.grey)),                             focusedBorder: UnderlineInputBorder(                                 borderSide: BorderSide(color: Colors.red[400]))                             //border: InputBorder.none                             ),                       ),


elements-headings, settings-global, generic-reset, generic-shared: build and docs

With the exception of a couple places in the Markdown Docs referencing the wrong section of your Sass the particular partial would need to get included (a few examples point out) this looks great. Thanks @mikemai2awesome !


support CRON expression

I would be interesting to create a job from a cron expression.
For exemple, I want to schedule a job every day a 7 I would use something like
schedule.cronline("0 7 * * *").do(job)
Here the same example for weekday at 8:30
schedule.cronline("30 8 * * 1-5").do(job)
It is something implemented in python-crontab lib. See job.setall(string)

Hi @Sispheor :wave:
Thanks for raising the question but supporting Cron's syntax is out of scope for Schedule. The goal for Schedule is to provide an alternative / "more readable" (I know this is subjective) syntax for scheduling recurring jobs. Thanks for pointing out Python-crontab. I'm sure it'll be helpful for some folks!
Closing this for now to keep the tracker clean. Please feel free to re-open or create a follow up issue if you have any other questions. Thanks!


fix(package): ensure log.config is packed in releases

This file is needed at runtime.

Codecov Report

Merging #16 into master will decrease coverage by 1.23%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
- Coverage    75.1%   73.87%   -1.24%
==========================================
  Files          12       11       -1
  Lines         237      222      -15
  Branches       34       34
==========================================
- Hits          178      164      -14
+ Misses         46       45       -1
  Partials       13       13

Impacted Files
Coverage Δ

src/line-parsers/index.ts
100% <0%> (ø)
:arrow_up:

src/index.ts

src/GameState.ts
83.33% <0%> (+4.76%)
:arrow_up:

Continue to review full report at Codecov.

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


Invoke-WebRequest claims it is "writing request stream (number of bytes written X)" when downloading

Steps to reproduce
Invoke-WebRequest https://dist.nuget.org/win-x86-commandline/latest/nuget.exe -outfile $env:temp\nuget.exe

Expected behavior
The PowerShell progress dialog suggests we are reading bytes from the response stream.
Actual behavior
The PowerShell progress dialog claims it is "Writing request stream (number of bytes written X)"
Environment data
 $psversiontable

Name                           Value
----                           -----
PSVersion                      5.1.15063.168
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.15063.168
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           <IP_ADDRESS>

@AArnott Thanks for your report!
I believe the message should depend on the Get/Post method.

GET verbs have no request stream so response stream messages would always be appropriate.
But POST verbs have both a request and a response stream. So ideally the message shown would be appropriate for the stream that is currently being written or read from.

@SteveL-MSFT

Can we fix this by changing Resx strings to
Web request status
    Number of bytes processed: 99999

@iSazonov Are you working on this one? if not, this one was next on my list.

@markekraus Please grab it - thanks!

@iSazonov or @SteveL-MSFT I see that is is possible to test the progress output through a job. If I'm just changing the .resx strings, is there any need to write pester tests for it?

@markekraus assigned it to me for now.  If you are only changing the resx strings, no tests are typically needed as those strings are expected to be localized at some time.  We have some meta tests for resx files.

PR #5078


Added to 4 categories.

Summary of your changes
Description
Added:
'Vsauce2' to when you get bored of CS stuff.
'Network Chuck' to youtube.
'Automate the boring stuff with Python' to computer books. 'picoCTF' to competitive programming
Checklist

[ - ] My change follows the Contributing Guidelines
[ ] I have added only one new link to the list.
[ - ] I have checked that the link that I added does NOT exist in the project already.
[ - ] I have sorted the link alphabetically under the related section.

Sorry, I'm stupid, the checklist options that I messed up are the ones I meant to select.


[Improvement] Separate the evaluation of Plugins

Evaluation of plugins within the SaaS integration manager should be individual, so that if one plugin fails, it won't make the other plugins fail.

@mohanarpit We've noticed that form rendering (i.e., form evaluation) was failing for Airtable because the Twilio JSON was malformed. This only happened in the organization that we were testing Twilio in, where both the plugins were present in the client side redux state at the same time. In our current flow, mallformed JSONs would not be available for GA, but this is still a scary place to be. :)
Modifying labels, because the issue is with form evaluation and not the SaaS manager app. @ayushpahwa will look into this with the next plugin that we test, since we do not want to stop publishing Twilio to investigate this.


Linux system-wide install guidance

I tried to do a system-wide install on Linux and it's a bit of a mess. I ended up with this:
/var/lib/polaris
/var/lib/polaris/.local
/var/lib/polaris/.local/share
/var/lib/polaris/.local/share/polaris
/var/lib/polaris/.local/share/polaris/polaris.log
/var/lib/polaris/.local/share/polaris/polaris.pid
/var/lib/polaris/.local/share/polaris/thumbnails
/var/lib/polaris/.local/share/polaris/thumbnails/12698004523255186174.jpg
/var/lib/polaris/.local/share/polaris/thumbnails/11339598049783905663.jpg
/var/lib/polaris/.local/share/polaris/thumbnails/17518483773397318633.jpg
/var/lib/polaris/.local/share/polaris/db.sqlite

The .local/share part shouldn't be there, the log file should be under /var/log, and the PID file should be under /var/run.
This one isn't great, but it would be nice to include a systemd unit in the package. This can be used as a starting point.
[Unit]
Description=Polaris
After=network.target

[Service]
User=polaris
ExecStart=/usr/bin/polaris -f -w /usr/share/polaris/web -s /usr/share/polaris/swagger
WorkingDirectory=/var/lib/polaris

[Install]
WantedBy=default.target

How did you end up with things under /var/lib? The Makefile tries to put thing under your home directory, following a structure which I found recommendations for when I looked it up (example: https://unix.stackexchange.com/questions/316765/which-distributions-have-home-local-bin-in-path). If you have a suggestion for a solution that is more standard across distributions, changes are welcome. I am more of a Windows developer so it's a bit hard for me to figure out what the best practices really are.
Does most software come with a systemd unit file? I thought that was something which would be added when packaging for a specific distro / package manager.

I wanted to run Polaris as a daemon, under its own user. ~/.local and ~/.config are perfectly fine for software that's run by multiple users; your desktop environment or text editor might keep their settings there. But if you have a system-wide daemon, the conventions are to:

keep logs under /var/log or /var/log/app
keep mutable state under /var/lib/app
keep immutable files under /usr/share/app
keep transient files under /run/app

See https://www.freedesktop.org/software/systemd/man/file-hierarchy.html for more information.

Does most software come with a systemd unit file? I thought that was something which would be added when packaging for a specific distro / package manager.

A lot of software does. Before systemd, you needed shell scripts that were often distro-specific, so it didn't really make sense for the maintainer to provide an upstream init script. Now systemd is used by most distributions (Ubuntu, Debian, CentOS/RHEL/Fedora, OpenMandriva and Arch Linux, at least), so its unit files are pretty portable. Even if not everyone is using systemd, it doesn't hurt to have a sample unit in the repository for packagers or users from other distros.

I'm trying to package polaris for NixOS, but I'm having some problems with web and swagger. I'd like to include those in the package, but a package can't modify the user's home directory, and the default search path for them is under ~, so I have to either patch the source to make it search in a system directory first, or wrap the program with a script that hardcodes --web and --swagger. Both solutions have their problems though, so it'd be great to have a way to set the default web/swagger path at install time.

Thanks for reviving this issue, I did let it linger a lot longer than I should have. I am very interested in having Polaris available through a package manager too. Please let me know if/when you succeed and we can add it as a possible installation method in the README.
I just opened a PR, guided by @lnicola's suggestions and by https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard. Testing and feedback would be very welcome on this as I don't have a very convenient setup to validate this work myself. The testing process would look like:

Clone the directories branch of polaris
Download (from Github) and unzip a build of polaris-web. It should be a 'web' directory within the cloned polaris repo.
From the polaris repo top-level directory, run ./res/unix/release_script.sh
You should end up with an archive under ./release, which is what we would publish for end users
Follow the install process from the README (aka. make install)
Check that Polaris works and files end up in the expected locations (see updated README in PR #91)
Run the uninstall command and make sure everything is gone

@fgaz  Specifically for NixOS, I am not sure this will get us all the way through. I just read that NixOS doesn't follow the Linux Filesystem Hierarchy Standard linked above, although it is unclear to me if the differences will be relevant here.
I am happy to consider adding options to the install script (which is currently very trivial), but it looks like moving the goalpost to the next step. How would the Polaris executable know where to find the locations that were used at install time to store the web and swagger static files? Polluting user systems with an environment variable for this seems a little inelegant.

@agersant the usual way to do this is to use variables such as PREFIX and BINDIR in the makefile, so that distributions can override them with their preferred locations: https://www.gnu.org/prep/standards/html_node/Directory-Variables.html
Usually build systems offer a way to encode that variable in the executable (for example in C you can do something like $(CC) -DDEFAULT_WEB_PATH=$(DATADIR)/web)

From a quick search it looks like it can be done with compile time environment variables! https://stackoverflow.com/questions/51620782/is-it-possible-to-initialize-a-variable-from-an-environment-variable-at-compilat/51620853#51620853

Ah yes, this approach makes perfect sense. I think a good Rust equivalent of -D compiler flags would be option_env which can read environment variables at compile time. This solves the implementation problems nicely.
On the configuration front, I am trying to map these GNU installation variables to what Polaris needs:

POLARIS_BIN_DIR would use bindir
POLARIS_STATIC_DIR (where web and swagger files go) would use datadir
POLARIS_DB_DIR would use localstatedir - although this would mean /usr/local/var/polaris by default instead of /var/lib/polaris which was suggested by the filesystem hierarchy standard.
POLARIS_PID_DIR would use runstatedir - the documention here is confusing as where this would end up by default (/var/run/polaris or /usr/local/var/run/polaris?). Neither of them is /run/polaris as suggested by the filesystem hierarchy standard.
I don't see applicable standard variables for POLARIS_LOG_DIR and POLARIS_CACHE_DIR

Also, are these variables guaranteed to exist on all Linux systems or should I provide fallback locations?

I'm not sure about localstatedir. While the / - /usr/local/ split is widely accepted for read-only files, I've actually never seen /usr/local/var in the wild. I guess you could set /var as default localstatedir (I see lots of packages doing this) and then put web and swagger in $(localstatedir)/lib/polaris/. Usually logs are under /var/log/ while caches are under /var/cache/, so you can use localstatedir for those too.
Yes, you should provide fallbacks

I updated the PR following your advice, could you take another look at the Makefile (and the rest if you're curious)?

As side note, for user installation I think that must fulfill the XDG standard.
About this, IMHO:

Database must be in: $XDG_RUNTIME_DIR (default $HOME/.local/share)
Thumbnails must be in: $XDG_CACHE_HOME (default $HOME/.cache) since can be regenerated if deleted, logs must be in $XDG_CACHE_HOME too since this directory is for "user specific non-essential data files"
PID must be in: $XDG_RUNTIME_DIR this has not default, can fallback to /tmp or similar directory the important is that this directory MUST have 700 permissions and MUST be deleted after a reboot.

About $XDG_RUNTIME_DIR, systemd defines /run/user/$UID as it, so you can define $XDG_RUNTIME_DIR/polaris and fallback to /tmp/polaris-$UID if $XDG_RUNTIME_DIR is not defined.
If you follow the specifications the default must be:
/var/lib/polaris/.local/share/polaris/db.sqlite -> $XDG_RUNTIME_DIR/polaris for database
/var/lib/polaris/.cache/polaris/thumbnails/12698004523255186174.jpg -> $XDG_CACHE_HOME/polaris
/var/lib/polaris/.cache/polaris/polaris.log -> $XDG_CACHE_HOME/polaris
/run/user/$UID/polaris/polaris.pid -> $XDG_RUNTIME_DIR/polaris or fallback to /tmp/polaris-$UID

The XDG standard and the GNU standard are in complete contradiction so it's going to be difficult to accommodate both.
After a lot of reading, this is the pros and cons I see in each of these standards:

XDG is a much simpler and cleaner standard than the GNU installation directories
The GNU installation process is more customizable, which is nicer for package maintainers (like @fgaz)
Usage of the GNU installation directories is more widespread (eg. XDG directories are not set on stock Arch Linux)
The XDG standard does not specify where essential but non-user-specific files (polaris-web and swagger files) should go
The XDG standard does not specify where the program executable should go
The XDG standard does not specify a fallback for $XDG_RUNTIME_DIR
The XDG standard is ambiguous when it specifies where log files should go (citation)

Overall, this comparison makes me want to use the GNU standard over XDG.
However, you do bring up a good point in that it should be easy for users to figure out how to install Polaris within their home directory. I suggest solving this with the addition of an alternative install command, make install-xdg, which would use the following directories:

$XDG_DATA_HOME/polaris/… (w/ standard fallback) for the executable, static files (polaris-web and swagger) and log file
$XDG_CACHE_HOME/polaris/thumbnails (w/ standard fallback) for thumbnails
$XDG_RUNTIME_DIR/polaris/polaris.pid for the pid file or the /tmp fallback you suggested

I think this solution gets us the best of both worlds in that:

XDG compliant installation is available as a zero-config one-liner, which we can recommend in the docs for users without admin rights or trying to avoid a system-wide install.
Customizable GNU-standard installation is available for package maintainers and users who want a system-wide install.

Would this work for you?

Sorry for the late reply!

The XDG standard and the GNU standard are in complete contradiction so it's going to be difficult to accommodate both

As far as I know, the gnu standard is for system, while xdg is for user stuff, so it makes sense that they are in conflict

The XDG standard does not specify where essential but non-user-specific files (polaris-web and swagger files) should go

For user installations, it'd probably be in .local/share, like you suggested. There is no real distinction between static and dynamic stuff there, unlike with /usr and /var. Not sure about $XDG_RUNTIME_DIR, but what you proposed is probably ok

The XDG standard does not specify where the program executable should go

User-level installations usually put executables in custom locations or -better- ~/.local/bin

make install-xdg

sounds good! (if it doesn't have a high complexity/maintenance cost on the makefile of course. not sure how easy it is to set variables in a target while allowing overrides)
But really, as maintainer and user the most essential things would only be:

The location of static files must have a default set at compile time

possibly with standard presets (basically $PREFIX/share/..)

There must be a way to set all the locations of dynamic files (db, logs, cache, pid) at runtime, so that they can be set to custom / user-writable locations

The first point is extensively solved in the pr, but for the second it's currently only possible to set the db path as far as I can see

https://github.com/agersant/polaris/pull/91 has been merged 🥳


Network creating is broken in openstack hypervisor

https://github.com/puppetlabs/beaker/pull/564

Closed, merged.


Limit '--upstream' to only work with cosmo?

this flag make sense only for cosmo, if we set it for the others like int2lm it will just take the upstream and not cmpile anything. Perhaps we could make it only compatible with cosmo command ?
Originally posted by @cosunae in https://github.com/C2SM/spack-c2sm/pull/309#r751978088

@cosunae Why does it make sense for cosmo then?

"[...] it will just take the upstream and not cmpile anything." - that's exactly the idea of upstream
Also, '--upstream' works for everything.


temp file for validation and sequence/linked residue summary

User description
put temp file for validation and sequence/linked residue summary in system temp directory and delete after
adds missing doc strings,
rename a variable in vaidate_schema.py

PR Type
Documentation, Enhancement

Description
Major improvements to code organization and documentation:

Enhanced system temp file handling:

Changed temp directory to use system temp instead of home directory
Added proper cleanup of temporary files after validation
Improved file operations safety

Documentation improvements:

Added comprehensive docstrings to most modules and functions
Improved error handling documentation
Added type hints and parameter descriptions

Code cleanup:

Removed unused NumpyEncoder and GUID implementations
Fixed file handling operations

Minor enhancements:

Added proper error handling for file operations
Improved code organization and readability

Changes walkthrough 📝
Relevant filesDocumentation

config_parser.pyAdd docstrings to database config parser

config/config_parser.py
Added module and function docstrings for database configuration parsing

+9/-0

Enhancement

MzIdParser.pyEnhance MzIdParser documentation and regex

parser/MzIdParser.py
Added module and function docstrings  Improved regex pattern for MSI parsing  Added error handling documentation

+19/-2

process_dataset.pyImprove temp file handling and cleanup

parser/process_dataset.py
Changed temp directory to use system temp instead of home directory  Added proper file cleanup after validation  Improved error handling and file operations

+17/-21

NumpyEncoder.pyRemove unused NumpyEncoder class

parser/NumpyEncoder.py

Removed unused NumpyEncoder class

+0/-17

guid.pyRemove unused GUID database type

parser/database/guid.py

Removed unused GUID type implementation

+0/-39

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

doesn't find logging.ini even though i have it in my 'config' folder

think thats because I'm not running it properly as a module


Update app-parts.md

Changed misspelling of the word "behavior"

@behm Thank you for taking the time to correct this problem. Your change will go live later today.


Fix Hound Sass checking

[x] We probably need to disable scss since we are using stylelint now
[ ] Hound doesn't warn on some cases; an example is

 .navbar {
   display: flex;
   position: relative;
   flex-wrap: wrap; // allow us to do the line break for collapsing content
   align-items: center;
   justify-content: space-between; // space out brand from logo
   padding: $navbar-padding-y $navbar-padding-x;
}

I already contacted the Hound people; we might be doing something wrong or their stylelint implementation doesn't work for Scss files. If it's the later we'll need to move away from Hound (at least) for scss files.

@mdo @bardiharborow: OK so from what I see so far, Hound doesn't work with our config for scss files.
I have disabled scss checking so that we don't get any false reports. If we run css-lint and css-lint-docs in Travis we should be covered, otherwise we should add that there until we sort Hound.


Add social graph tags

Figure out what best practices / modules are available in Svelte for social graph tags. Typically one targets Twitter + Facebook with slightly different variants.

image: we need a character here at some point, can literally just be an ancient crab for now
title
description

@bgins marking this as closed, we've got our crab for now.


Enhance the API Logging to include the request action.

Purpose or Intent

updated request adapter's to_hash to include action
updated action to be defined for all HTTP methods
updated rspec

@imtayadeway this was the logging enhancement that pulled out of PR# 10240.


Fix nested relations

Please use #4116 to test this. That should be merged here for this to properly work.
It's separated so we can discuss separate issues in separate PR.
WHAT DONT WORK 4 SURE:

nested repeatables (so all relations that could use repeatable don't work, they work as selects/direct fields).

You know what we can do? After your refactor, let's have a meeting and re-do this PR, following the rules I've outlined above. I think it'll be a good way to test the rules, practice the rules, and see the difference they would make.
Let's do a pair-programming session, where we redo this PR in a way I would be happy to merge it.

I believe https://github.com/Laravel-Backpack/CRUD/pull/4127 is a complete replacement for this. So I've slapped the "WON'T" badge on this. Am I correct @pxpm ? If so, let's please close this PR and delete the branch. We can always undelete it later.


chore: Enhance the README with information about Cluster Profile API

The SIG-MC community has decided on the Cluster Profile renaming. This PR is to highlight the relationship between the Cluster Inventory and the Cluster Profile API.
It's assume that the KEP rename PR https://github.com/kubernetes/enhancements/pull/4533 will be merged shortly.

/assign @qiujian16
/assign @ryanzhang-oss

/assign @ArangoGutierrez


Offer a general purpose ReplicatedResource for clusterset

Describe the enhancement
Currently there are case-by-case resources to replicate Resources in the Hub cluster to spoke cluster namespaces. Examples are:

ManifestWorkResplicaSet
ClusterAddonManager
Potentially one for ManagedSeviceAccount https://github.com/open-cluster-management-io/managed-serviceaccount/issues/90
For future addons that will also have a similar CRD like ManagedSeviceAccount

Will it make sense to support one general purpose ManagedResourceSet that will take a Placement and resourceTemplate and replicate that into the cluster namespaces according to placement logic. This can handle all the above cases as far as I can understand ( i have not thought through it deeply).
I am curious if this something like this was considered? If not, is this an option?
Thanks.

Hi @tamalsaha
I will let other maintainers chime in on your suggestion.
But just FYI I had a similar requirement and @JustinKuli was able provide me with a clever solution here: https://gist.github.com/JustinKuli/b66d1096e1262a0bc51f8ff8f29169f8 using ocm policy addon.

The idea is to use a Placement and a Policy to distribute a resource to managed cluster namespaces on a hub cluster, not to the actual managed clusters.

Let me know if you have any questions.

ManifestWorkResplicaSet and ClusterAddonManager has slightly different purposes.
ManifestWorkResplicaSet is for the purpose of user can "replicate manifests to multiple clusters".
ClusterAddonManager is for the addon developer to develop an addon, then user can choose to enable the addon cluster by cluster, or by a certain strategy.
ManagedSeviceAccount seems to me also different, it meant to ask each spoke cluster provide some credential to access them.
The ManagedResourceSet seems to be quite similar as what ManifestWorkResplicaSet did?

I was referring to the part where the controller for ManifestWorkResplicaSet,  ClusterAddonManager, higher-level-of-ManagedSeviceAccount are all performing one general operation: create the individual ManifestWork, ManagedClusterAddon, ManagedServiceAccount object by copying the the parent object into the cluster namespaces. This general part can be offered a single resource. The template for the parent object can have the full underlying object spec (like PodTemplate).
apiVersion: cluster.open-cluster-management.io/v1beta2
kind: ManagedClusterSetResource
metadata:
  name: <name>
  namespace: <ns>
spec:
  placementRefs:
    - name: placement-byname # Name of a created Placement
  template:
    apiVersion:
    kind:
    metadata:
      ...
   spec:
      ...

spec.template.apiVersion and spec.template.kind can dictate whether the underlying object is a ManfestWork / ManagedClusterAddon / ManagedServiceAccount .


CORS error resolving DID

What is the status of this app template? Is it supposed to work? Can the app proceed beyond the "Initializing file system" screen hanging for indefinite time?
Tried registering at:
http://localhost:5173/
https://odd-template.fission.app/
Result:
"Initializing file system" screen
Browser console:
Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://dns.google/resolve?name=_did.runfission.com&type=txt. (Reason: CORS request did not succeed). Status code: (null).
Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://cloudflare-dns.com/dns-query?name=_did.runfission.com&type=txt. (Reason: CORS request did not succeed). Status code: (null).

Hi @youk 👋 . Thanks for opening the issue.
This is an issue for the ODD SDK. Would you mind opening an issue over here: https://github.com/oddsdk/ts-odd.
As a quick note, I think I've seen this error, but only at one specific coffee shop in a town nearby me. My theory is they have some router configuration that prevents these DNS lookups. It might be worth trying at another location to narrow down possible causes.

As for the issue itself, the browser clearly complains about CORS violation.

Browsers sometimes complain about CORS issues when root cause is something else.

I don't quite understand how changing DNS configuration should help. Unless those queries are only made as fallback option.

This lookup is an essential part of the ODD SDK. We use it to look up the DID of the Fission server. It can be seen in action here: https://dnsrecords.io/_did.runfission.com.
We are making the query from JavaScript using DNS over HTTPS, instead of the browser resolving a DNS record.
This approach is newer, and that along with only experiencing this error in certain locations makes me suspicious of a router configuration. Could be something else though! I haven't investigated it enough to say.

Thanks for looking into this further! ❤

Couldn't the error handling in the app be improved? I mean not the indefinitely spinning "Initializing file system" screen, but a meaningful error message after DoH query fails? It seems the responsibility of an app, not the SDK.

Yes, indeed. We could have a better error message here. I'll re-open this issue and change the title.
Would you be interested in adding a better error message? I think I can test it out at that local coffee shop, assuming they haven't changed their router config.

Were you thinking about making this address resolution procedure more fail-proof? I don't have hands-on programming experience with DoH, but when I try with "Use CORS Proxy" option on https://dohjs.org/, the resolution works even in my configuration. I am not saying that my router's behavior is correct (it is the effect of ad-blocking enabled), but this is OOTB feature and when enabled it will break things for other people too.

It's something we can investigate. If you are still up for it, an issue in https://github.com/oddsdk/ts-odd would be appreciated. Our current focus is IPVM, so we may not get to it right away.


Problems in reproducibility of mediation analysis script

Hi,
While attempting to reproduce the mediation analyses, I was not able to do so. The warning pointed to object 'TM7' not being found. I was only able to retrieve 64 results for the Biofuel   ->   Microbial feature  ->   FEV1/FVC before the code aborted.
I am attaching a scrrenshot of the error.

The problem described in your request is because the taxa TM7-3 has a special character '-' which cannot be recognized by the code. We have uploaded a new script mediationAnalysis_v2.r to the same folder that should fix this problem.


xWebSite: adding support for Site Id in xWebSite

Pull Request (PR) description
Adding SiteId to the xWebSite resource allowing setting the desired IIS site Id.
As per comments on the original issue #396 SiteId is being used instead of Id to avoid potential future conflicts or confusions.
This Pull Request (PR) fixes the following issues
- Fixes #396

Task list

[x] Added an entry under the Unreleased section of the change log in the README.md.
Entry should say what was changed, and how that affects users (if applicable).
[x] Resource documentation added/updated in README.md.
[x] Resource parameter descriptions added/updated in README.md, schema.mof
and comment-based help.
[ ] Comment-based help added/updated.
[ ] Localization strings added/updated in all localization files as appropriate.
[x] Examples appropriately added/updated.
[ ] Unit tests added/updated. See DSC Resource Testing Guidelines.
[ ] Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
[x] New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.:x: csharmath sign nowYou have signed the CLA already but the status is still pending? Let us recheck it.

I've asked my employer about the CLA, once I get a go ahead will sign.

I didn't have a chance yet to check how the unit and integration tests could work and what would be needed to cover this new field. Happy to work on that as well.
the comment based help is already incomplete, so I skipped adding this new property. Perhaps another new issue could be added for that?
haven't seen any localization

@csharmath Thanks for opening a PR for this issue, this will be a great enhancement to the resource.
This PR will need to include tests to cover the changes before we start reviewing.  However, I did glance over it and noticed Get-TargetResource wasn't updated to return the new SiteId property.  Get-TargetResource should return all properties defined in the schema.mof.
You're right about the resource lacking localization, that should be a separate PR.  For the comment-based help, I think we should update accordingly as we add new parameters, but not necessarily go back and add prior existing parameter help.

Codecov Report

Merging #415 into dev will decrease coverage by 0.17%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##              dev     #415      +/-   ##
==========================================
- Coverage   90.67%   90.49%   -0.18%
==========================================
  Files          17       17
  Lines        2412     2420       +8
==========================================
+ Hits         2187     2190       +3
- Misses        225      230       +5

Impacted Files
Coverage Δ

DSCResources/MSFT_xWebsite/MSFT_xWebsite.psm1
96.15% <50%> (-0.88%)
:arrow_down:

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 85ff094...d65ada6. Read the comment docs.

However, I did glance over it and noticed Get-TargetResource wasn't updated to return the new SiteId property. Get-TargetResource should return all properties defined in the schema.mof.

Fixed

You're right about the resource lacking localization, that should be a separate PR. For the comment-based help, I think we should update accordingly as we add new parameters, but not necessarily go back and add prior existing parameter help.

Fixed

This PR will need to include tests to cover the changes before we start reviewing

Just added the unit and integration test changes. Writing the unit test helped me to discover a bug, so
that's a good thing.

@regedit32  let me know if there is anything else.


Windows tool_integration_tests_2_6 flakes on deferred_components_test

(part of https://github.com/flutter/flutter/issues/97036)
Two flakes in recent 100 runs.
https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/845/overview
https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/756/overview
14:25 +37 -1: test\integration.shard\deferred_components_test.dart: simple build appbundle mismatched golden no-validate-deferred-components succeeds [E]
  Expected: <true>
    Actual: <false>

  package:test_api                                            expect
  test\integration.shard\deferred_components_test.dart 165:5  main.<fn>

@GaryQian could you please triage this following https://github.com/flutter/flutter/wiki/Issue-hygiene#flaky-tests.

@GaryQian have you had a chance to take a look at this one?

Not yet, ill look at it today.

Not yet, ill look at it today.

Thanks!

Another instance today: https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/955/overview

Another instance today: https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/955/overview

@GaryQian looks like this is stlil happening

@GaryQian were you able to take a look at this one?

Looking now

Thanks!

https://github.com/flutter/flutter/pull/103161 Adds temporary logging. More info is needed to deflake this.

Thanks for working on this!
Another two same instances:
https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/1272/overview
https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/1276/overview
There is another flake on deferred_components_test: https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/1252/overview
12:15 +35 -1: test\integration.shard\deferred_components_test.dart: simple build appbundle all targets succeeds [E]
  Expected: contains 'app-release.aab'
    Actual: 'Deferred components prebuild validation passed.\n'
              '\n'
              ' Building with sound null safety \n'
              '\n'
              'Running Gradle task \'bundleRelease\'...                          \n'
              'Shrinking has been disabled for this build due to deferred components. Shrinking is not available for multi-apk applications. This limitation is expected to be removed when Gradle plugin 4.2+ is available in Flutter.\n'
              'Running Gradle task \'bundleRelease\'...                             10.6s\n'
              ''

  package:test_api                                           expect
  test\integration.shard\deferred_components_test.dart 75:5  main.<fn>

Here's a build that is I believe post Gary's logging change, although I don't see more logging: https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/1276/overview

Here's a build that is I believe post Gary's logging change, although I don't see more logging: https://ci.chromium.org/ui/p/flutter/builders/prod/Windows tool_integration_tests_2_6/1276/overview

@GaryQian any update on this one?

Even so, the previous failures seem to happen across multiple variants of the test and are quite varied in cause.

In the last 400 commits, I count 1 flakes and 2 failures for this builder, which would be under the threshold.
@keyonghan am I reading this correctly?

I'm going to close this one since we aren't seeing it very much any more, but feel free to reopen if you have any idea what's causing it or if it starts happening more often...


[6.x] Add extra JSON assertion methods for expected value at given key

This PR adds two assertions on the TestResponse class: assertJsonContains and its inverse assertJsonNotContains.
These methods make it more convenient to test the value for a single key somewhere in the JSON response using dot-notation.
Previously this required two steps, but can now be rewritten as one method call directly on the TestResponse:
// before:
$response = $this->getJson('/api/orders')->decodeResponseJson();
$this->assertEquals('expected value', $response['payment']['provider']);

// after:
$this->getJson('/api/orders')->assertJsonContains('expected value', 'payment.provider');

The method names assertJsonContains and assertJsonNotContains are based on assertStringContains, assertStringNotContains and the contains collection helper. Any thoughts on more coherent/evident names are welcome.

I feel like it should throw an error when the key does not exist. For example there is no separation between null and missing key if you assert value is null.

It seems like most of the other JSON assertions are (key, value)... but this one is (value, key). Could that be a consistency problem?

Well this is awkward: I just stumbled uppon assertJsonPath in TestResponse which does exactly the same as assertJsonContains.
I'm gonna close this for now (unless someone really wants to see the inverse method added).
@donnysim the assertJsonPath has the same issue where a non-existing key would not trigger an error when compared to null. Maybe a nice addition there.