Improve way how it handles the SVG files

Currently, there is only one way to import SVGs, which converts to a React Component. We've faced some issues while trying to import as path, like a regular file. So the idea here is to improve how we handle and make sure that all kind of assets is working fine:
For SVGS files: https://www.npmjs.com/package/@svgr/webpack
import starUrl, { ReactComponent as Star } from './star.svg'
const App = () => (

    <Star />

)

For images:
https://www.npmjs.com/package/next-images#usage

done!


feature: Allow for fixed offset positioning of rider

I am attempting to use isometric maps and tokens.  When doing this, the positioning of ridable puts them in really weird spots.  It could work, good enough, if I could specify a fixed offset for riders.  I would just do this for each individuals mount.  In this instance, it's simply 1 horse = 1 rider.  An X/Y offset would be great. A button for draggable movement would be stellar.

I will look into an offset setting. With "button for draggable movement", do you mean being able to set the X/Y offset by positioning a token and saving its position offset as the new offset (similar to the positioning setting supported by e.g. Monks active tile triggers)? If so i will see what i can do in that regard

Yeah exactly like that
On Mon, Feb 19, 2024, 3:24 AM Saibot393 @.***> wrote:

I will look into an offset setting. With "button for draggable movement",
do you mean being able to set the X/Y offset by positioning a token and
saving its position offset as the new offset (similar to the positioning
setting supported by e.g. Monks active tile triggers)? If so i will see
what i can do in that regard
—
Reply to this email directly, view it on GitHub
https://github.com/Saibot393/Rideable/issues/66#issuecomment-1952027273,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAFMKV7IE42E2OSE2442GVTYUMK57AVCNFSM6AAAAABDPCZMVGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJSGAZDOMRXGM
.
You are receiving this because you authored the thread.Message ID:
@.***>

I have added a token/tile setting for the riders offset with v2.20.0


Support tracking additional SDK versions and use 'sdks' array

There's been some new updates for older SDK versions. It would be good to keep tracking these to ensure older release channels are kept up to date.
The releases.json also now has a 'sdks' array that may contain more recent versions of SDKs too.

The packages have been making full use of the "sdks" array for nearly a year now.


Update api-alerts-close-false-positive.md

portal.cloudappsecurity.com is the correct domain. Contoso.com is not, however it is mentioned in a few docs.

@batamig

Can you review this PR?
IMPORTANT: When this content is ready to merge, you must add #sign-off in a comment or the approval may get overlooked.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

Author confirmed via email.


[Feature Request]: All workflows completed successfully.but graph embedding is null

Do you need to file an issue?

[x] I have searched the existing issues and this feature is not already filed.
[ ] My model is hosted on OpenAI or Azure. If not, please look at the "model providers" issue and don't file a new one here.
[x] I believe this is a legitimate feature request, not just a question. If this is a question, please use the Discussions area.

Is your feature request related to a problem? Please describe.
No response
Describe the solution you'd like
No response
Additional context
No response

Graph embeddings are only used if you want to perform visualization, in which case we turn on embed_graph.enabled and umap.enabled in settings.yml. Doing this will result in "real" x/y positions in your create_final_nodes.parquet, otherwise they will be set to 0,0.
Is this what you are looking for?

thanks


RFC: Explicit VL API as The Final Vector Intrinsic API Proposal

TL;DR
We want to remove implicit VL API and made some API changes and intent to change the high-level semantic of vector intrinsic, so that we can doing more aggressive optimization.
Abstract
The goal of this RFC is improving the intrinsic for vector programming, and chose the explicit VL API as the only one intrinsic API, this RFC is consistent with two part, first part is explanation why we pick explicit VL API as final proposal, and second part is removing the concept of low level register status from the intrinsic programming model.
Background
Last year we've announce the intrinsic spec for vector programming in C. We got lots of useful feedback from several different parties including BSC, SiPearl, Andes, OpenCV China, PLCT Lab, Alibaba/T-Head.
Today, we have open source implementation on both GCC and LLVM*, which is implemented in a different approach, implicit VL and explicit VL, we were expect compiler could be using simple C wrapper to implement each other API, e.g. using implicit VL API with C wrapper to implement explicit VL, however it turns out become a barrier of optimization.
The issue is because the concepts of both API are kind of incompatible, after long discussion and exploration, we think it’s time to pick one as the final proposal of intrinsics spec, in order to reduce the compiler maintenance effort and reduce the learning curve of  intrinsic function.
Keep only one intrinsic API also having an advantage on the compiler optimization side, we found several optimization opportunities, but we can’t do that because we need to make sure both API have correct semantic and behavior.
So which one is better is the question, back to the reason why we have two different style API is because we don’t have a conclusion on which API is better before, but this time is different, we have enough exploration, experience and feedback to make the right decision.

LLVM parts are upstream in progress.

Explicit VL API As The Final Vector Intrinsic
After implementing the intrinsic API on both GCC and LLVM, we found several good reasons for the using explicit VL API from the compiler aspect, explicit VL define-use relationship is more natural to the compiler, it made the analysis and optimization more easier.
We also get feedback from users for both API, implicit VL API is less verbose, but is hard to track the status of VL register, that’s also made debug harder, having an explicit VL argument makes programming easier.
So based on both sides - user feedback and consideration of compiler implementation, we believe explicit is the right way to go, and based on the decision we propose following changes for the vector intrinsic API.
Abstract Low-Level Register Modeling in High-Level Language Layer
During the implementation phase, we found a fact is the status of VL register becomes an optimization barrier, we must maintain the correct order between vreadvl and vsetvli and all other explicit VL API, because explicit VL API has the semantic of writing VL register.
For example, we can’t reorder the operations across vreadvl for the following program.
n = 10000;
avl = vsetvl_e32m4 (n) // Assume VLEN=128, so avl = 16
vl1 = vreadvl(); // 16
vint32m4_t const_one = vmv_v_x_i32m4_vl(1, 4);
vl2 = vreadvl(); // 4
vint32m4_t tmp = vadd_vx_i32m4_vl(const_one, 4, vl / 2);
vl3 = vreadvl(); // 8
vse32_v_i32m4_vl(a, tmp, vl);

Furthermore, we can't move explicit VL vector intrinsic across any other explicit VL vector intrinsic, because the define-use relationship is modeled as coarse-grain, we only model the intrinsic write some global status, but it’s hard to detailly describe and track which status is changed, that’s kind of compiler implementation limitation on current mainstream open-source compiler.
So abstracting the low-level register status from the high-level language layer is a straightforward option here, abstracting the VL register, making the vector length just as an argument, that releases us from implementation limitation, and we also found that’s also comes with several advantages on optimization view - we can model almost all intrinsic function as pure function except load/store and few special instructions, which is no side effect, that’s the fanatic property in the compiler optimization land.
Using an example to demonstrate the power if we treat most intrinsic functions as pure function, here is a function with a loop, and having a loop invariant there.
void foo(int *a, int n) {
  while (vl = vsetvl_e32m4 (n)) {
    vlmax = vsetvlmax_i32m4 ();
    vint32m4_t const_one = vmv_v_x_i32m4_vl(1, 4, vlmax);
    vl = vsetvl_e32m4 (n);
    vint32m4_t tmp = vadd_vx_i32m4_vl(const_one, vl);
    vse32_v_i32m4_vl(a, tmp, vl);
    n -= vl;
    a += vl;
  }
}

Since vsetvl and vsetvlmax is pure function now, so vlmax = vsetvlmax_i32m4 (); can be safely hoist outside the loop
void foo(int *a, int n) {
  vlmax = vsetvlmax_i32m4 ();
  while (vl = vsetvl_e32m4 (n)) {
    vint32m4_t const_one = vmv_v_x_i32m4_vl(1, 4, vlmax);
    vl = vsetvl_e32m4 (n);
    vint32m4_t tmp = vadd_vx_i32m4_vl(const_one, vl);
    vse32_v_i32m4_vl(a, tmp);
    n -= vl;
    a += vl;
  }
}

And then all arguments of  vmv_v_x_i32m4_vl are loop invariant, so we can hoist that too.
void foo(int *a, int n) {
  vlmax = vsetvlmax_i32m4 ();
  vint32m4_t const_one = vmv_v_x_i32m4_vl(1, 4, vlmax);
  while (vl = vsetvl_e32m4 (n)) {
    vl = vsetvl_e32m4 (n);
    vint32m4_t tmp = vadd_vx_i32m4_vl(const_one, vl);
    vse32_v_i32m4_vl(a, tmp);
    n -= vl;
    a += vl;
  }
}

We also found the vsetvl has been called twice with the same input, because it’s pure function, so the CSE pass can easily optimize that!
void foo(int *a, int n) {
  vlmax = vsetvlmax_i32m4 ();
  vint32m4_t const_one = vmv_v_x_i32m4_vl(1, 4, vlmax);
  while (vl = vsetvl_e32m4 (n)) {
    vl = vsetvl_e32m4 (n);
    vint32m4_t tmp = vadd_vx_i32m4_vl(const_one, vl);
    vse32_v_i32m4_vl(a, tmp);
    n -= vl;
    a += vl;
  }
}

According to the above demonstration, the advantage of abstracting VL register status is very obvious, and that’s also hard to do for implicit VL API, considering following example:
void foo(int *a, int n) {
  while (vl = vsetvl_e32m4 (n)) {
    vsetvlmax_i32m4 ();
    vint32m4_t const_one = vmv_v_x_i32m4(1);
    vsetvl_e32m4 (n);
    vint32m4_t tmp = vadd_vx_i32m4(const_one, 4);
    vse32_v_i32m4 (a, tmp);
    n -= vl;
    a += vl;
  }
}

Since there is hidden dependency between all vector intrinsic functions, it’s impossible to reorder between any vector intrinsic function, so that means no optimization can be done at all.
Additionally, we also found this could fix other potential issues for code gen of GNU vector extension type, since all other generic compiler infrastructure won’t aware the VL register status, and that will cause poor performance.
So what is GNU vector extension type? GCC and Clang/LLVM both provide vector type extension for easier write SIMD program, for example, we can declare a type with vector size attribute, and then you can operate variables like normal scalar type via ordinary operator.
typedef int int32x4_t __attribute__ ((vector_size (16)));

int32x4_t a, b, c;
a = b + c;          // NO explicit VL reg use or def in middle-end
                    // but it will expand to vsetvli_e32m1(4) and vadd

We can code gen that with vector instruction, however this code gen path might require changing VL to fit the semantic of operation, in above example, the VL should set to 4 before doing operation, and that would be an issue if we have model VL in the compiler middle-end.
LLVM VPlan IR has similar situation on the compiler middle-end for the GNU vector code gen,
According to the above reasons, we believe abstract low-level register modeling in high-level language layer is right way to go.
API Changing
However the several API must be changed due to removing the concept of VL from the C language layer.
The first one is the vreadvl API, which exposes the status of VL register, so we must remove that to prevent leak of the low-level info to high-level programming languages.
The second, And here is an instructions in RISC-V vector ISA will change the VL other than vsetvl[i] instruction, which is vle*ff.v instructions, the instruction will update VL register if it got exception before load VL-elements, we’ll introduce an extra argument to get the content of the VL register:
vint16m1_t vle16ff_v_i16m1 (const int16_t *base);  // Current API
vint16m1_t vle16ff_v_i16m1 (const int16_t *base, size_t *vl); // New API

And the last change is the API name, the _vl suffix will become redundant and verbose once we use explicit VL API as final vector intrinsic API.
Conclusion
In this RFC we proposed chose the explicit VL API as final vector intrinsic API, to reduce the complexity of compiler implementation, second, propose abstract low-level register modeling in high-level language layer to enable the opportunity of further optimization for the vector intrinsic program, and last, we have to change part of intrinsic API due to above changes.

Note: Corresponding PR will sent in next few days.

Note: Corresponding PR will sent in next few days.

@kito-cheng
Sounds good to me. This seems to match the direction taken by llvm-vp (adding VL and mask to IR ops). We have also seen some bugs caused by (manually) reordering intrinsics, which would not have happened if they were pure functions.
Just to be sure: a function that wants to "set VL" and then restore the previous value would just vsetvl, pass the result to *_vl() ops, and the compiler takes care of restoring/setting other VL after returning?
Also, is vse32_v_i32m4_vl(a, tmp); missing the last argument?

@kito-cheng
Sounds good to me. This seems to match the direction taken by llvm-vp (adding VL and mask to IR ops). We have also seen some bugs caused by (manually) reordering intrinsics, which would not have happened if they were pure functions.
Just to be sure: a function that wants to "set VL" and then restore the previous value would just vsetvl, pass the result to *_vl() ops, and the compiler takes care of restoring/setting other VL after returning?
Also, is vse32_v_i32m4_vl(a, tmp); missing the last argument?

Works for me (given previous discussion in e.g. #8 no surprise there ;-) ).

Works for me (given previous discussion in e.g. #8 no surprise there ;-) ).

Just to be sure: a function that wants to "set VL" and then restore the previous value would just vsetvl, pass the result to *_vl() ops, and the compiler takes care of restoring/setting other VL after returning?

In theory, you don't need to care about the VL register anymore if we only using explicit VL API, so yes, the compiler will handle that VL register restoring/setting, it's all compiler's responsibility now.

Also, is vse32_v_i32m4_vl(a, tmp); missing the last argument?

Updated, Thanks!

Just to be sure: a function that wants to "set VL" and then restore the previous value would just vsetvl, pass the result to *_vl() ops, and the compiler takes care of restoring/setting other VL after returning?

In theory, you don't need to care about the VL register anymore if we only using explicit VL API, so yes, the compiler will handle that VL register restoring/setting, it's all compiler's responsibility now.

Also, is vse32_v_i32m4_vl(a, tmp); missing the last argument?

Updated, Thanks!

Works for me (given previous discussion in e.g. #8 no surprise there ;-) ).

@rdolbeau Thanks your take a stand for this RFC!

Works for me (given previous discussion in e.g. #8 no surprise there ;-) ).

@rdolbeau Thanks your take a stand for this RFC!

I was formerly a strong proponent of the implicit-VL API, and a mild opponent of the explicit-VL API. I am now convinced that explicit-VL can provide better code generation, due to capabilities/limitations of current compilers. This is a practical argument, not a mathematical one, but that distinction is irrelevant to the performance programmer.
I am on board.

I was formerly a strong proponent of the implicit-VL API, and a mild opponent of the explicit-VL API. I am now convinced that explicit-VL can provide better code generation, due to capabilities/limitations of current compilers. This is a practical argument, not a mathematical one, but that distinction is irrelevant to the performance programmer.
I am on board.

@kito-cheng, thanks for your sharing.
Same with @knightsifive, I was formerly a strong proponent of the implicit-VL API. However, this proposal convinces us. If possible, let this proposal being landing down asap.

@kito-cheng, thanks for your sharing.
Same with @knightsifive, I was formerly a strong proponent of the implicit-VL API. However, this proposal convinces us. If possible, let this proposal being landing down asap.

Here is the PR https://github.com/riscv/rvv-intrinsic-doc/pull/64 for this RFC :)

Here is the PR https://github.com/riscv/rvv-intrinsic-doc/pull/64 for this RFC :)

Corresponding changes are merged.


Addressing issue #59 magnitude and phase function for Matlab

Magnitude and phase function developed in Matlab with identical function to the one available in Python.
The following code can be used to test 2D function in Matlab
magnitude = 9;
y = sqrt(1/2*magnitude^2);, x=y;
phase = atan2(y,x);
[mag, theta] = magnitude_phase({x; y});
assert(all(magnitude == mag))
assert(all(phase == theta))
xx = [x,x]; yy = [y,y];
[mag, theta] = magnitude_phase({xx; yy});
assert(all(magnitude == mag))
assert(all(phase == theta))
The following code can be used to test 3D function in Matlab
magnitude = 9;
y = sqrt(1/3*magnitude^2);, x=y;, z=y;
phase1 = atan2(y,x);
phase2 = atan2(sqrt(x.^2 + y.^2),z);
[mag, theta, phi] = magnitude_phase({x; y; z});
assert(all(magnitude == mag))
assert(all(phase1 == theta))
assert(all(phase2 == phi))
xx = [x,x]; yy = [y,y]; zz = [z,z];
[mag, theta, phi] = magnitude_phase({xx; yy; zz});
assert(all(magnitude == mag))
assert(all(phase1 == theta))
assert(all(phase2 == phi))

@Alex-McVey similarly to your other PR, this will need test(s) written.


[Discussion] The attribute operators : and = need to have a unified, simple and clear form that is easier for users to understand and use.

Discussion
Background
In the configuration scenario, there are often problems of configuration data inheritance, overwriting, and merging, such as the classic three-way merge problem. Some parameters in different environments in multiple environments expect to inherit the configuration of the base, and some expect to override the configuration of the base.
Goals

Eliminate users' doubts about using styles for different attribute operators (Should I use ':' or '=' to gracefully solve the configuration merge problem in my scenario
Use the most common symbols to meet the most basic functions (configure merge override scenarios). For more scenarios, use more prominent extension symbols.
Each operator is only responsible for the function of a single atom and can be combined.

Problems
Different attribute operators are supported in KCL to express configuration coverage and merging. At the language level, two attribute operators, : and =, are provided to express merge and coverage respectively. Although this provides the flexibility of configuration merging, there are some problems:

There is no limit to the number of configuration merges and the configured k-v-o entry consisting of key, value and the attribute operator is a runtime value, which makes users unclear about the path of their own configuration merges (improved by enhancing error messages and displaying merge tracing #193).
The writing style of attribute operators may be inconsistent and Inconspicuous  (whether to use a more conspicuous symbol).

Example 1
KCL code (main.k)
config1 =  {image = "xx", env = []} | {image = "override_image", env += []}
config2 =  {image: "xx", env: []} | {image = "override_image", env += []}

The output YAML (kcl main.k)
config1:
  image: override_image
  env:
  - ENV
config2:
  image: override_image
  env:
  - ENV

The writing methods of config1 and config2 are equivalent, and their results are the same. Users can write both ':' and '='
Analysis
A direct cause of the current situation is that the user cannot immediately determine that one is an idempotent merge function and the other is overwrite function through the symbols ':' and '='. In many languages, the setting of ':' and '=' is':=', which represents the semantics of assignment, such as the following go code (= used in normal assign statements and : used in struct initialize expressions):
package main

import "fmt"

type Person struct {
	name string
}

func main() {
	name := "Alice"
	name = "Bob"
	p := Person{name: name}
	fmt.Println(p)
}

Thus, if merging is regarded as an advanced use of overriding attributes, perhaps the forms +: and : like jsonnet are more acceptable. However, it does not solve the problem of unlimited merging and coverage. Of course, providing the traceback of merging or limiting the number of parameters can alleviate this problem to some extent, but it does not solve this problem from the language level.
Design
Design I
It is specified that the best operator for KCL is : operator, | When combining two configurations, the Python dict Merge specification is used by default). However, the type of the original attribute cannot be changed during the merge process, so we will modify this process slightly:
|: indicates the dict merge patch and list merge patch like origin attr op :
+: indicates the list append op like origin the attribute op attr op +=
!: Indicates that the configuration will perform a merge conflict check.
Because KCL is not a jsonlike language, we use += and |=for top level variables.
# Literal

a = {a: 1} | {a: 2} # {"a": 2}
b = {a: 1} | {b: 2}  # {"a": 1, "b": 2}

# Dict

c = {a: {a: 1, b: 2}} | {a: {c: 3}} # {"a": {"c": 1}} The same key will be overwritten directly. If you want to merge, you can use "|:"
d = {a: {a: 1, b: 2}} | {a |: {c: 3}} # {"a": {"a": 1, "b": 2, "c": 3}}

# List

e = {a: [{a: 1}]} | {a: [{b: 2}]} # {"a": [{"b": 2}]}
f = {a: [{a: 1}]} | {a+: [{b: 2}]} # {"a": [{"a": 1}, {"b": 2}]}
g = {a: [{a: 1}]} | {a|: [{b: 2}]} # {"a": [{"a": 1, 'b": 2}]}

Design II
Expand immutable semantics for attribute operators in KCL:

Properties declared with the=operator are immutable.
Attributes declared using the: and+=operators are mutable.

config1 = {a = 1} | {a = 2} # Error: `=` denotes that `a` is immutable.
config2 = {a: 1} | {a = 2}  # Ok: `:` denotes that `a` is mutable

Reference

KCL Config Operation: https://kusionstack.io/zh-CN/docs/reference/lang/lang/tour/#配置操作
Jsonnet Nested Field Inheritance: https://jsonnet.org/ref/language.html +: and :
The Logic of CUE: https://cuelang.org/docs/concepts/logic/
Ksonnet: https://github.com/ksonnet/ksonnet
StackOverflow Question where to place cross-environment Terraform configs?
JSON Merge Patch RFC: https://www.rfc-editor.org/rfc/rfc7386
JSON Patch: https://jsonpatch.com/
JSON Merger:https://pythonhosted.org/json-merger/_modules/json_merger/merger.html
Acorn CUE Merge Function: https://docs.acorn.io/reference/functions#merge
Python. PEP 584: https://peps.python.org/pep-0584/

Base {image, env} prod image + 加一个环境

{image = "xx", env = []} | builtin.Object {
    set: {
        image = "override_image"
    },
    append: {
        env: []
    }
}

A = {
  image = "xx"
  env = []
}
B = {
  @overwrite
  image = "override_image"
  @append
  env = []
}
A|B

In addition, CUE can complete up to two layers of data inheritance through *default_value, and can write multi-environment configurations to a certain extent, but it seems that it cannot cover the problems and scenarios that KCL needs to solve. Ref: https://cuetorials.com/deep-dives/attributes/#cues-attributes

显式写出 keyword action.

{
config1 =  {
image : "xx",
env : []
} |
{
image : "override_image",
env : [ ]
}
}

A = {
image = "xx"
env = []
}
B = {
image = "override_image"
env = []
}
B.override(A)
or
B override A


[microk8s] Testing local deployment of cc charts

Closes https://github.com/CoffeaTeam/coffea-casa/issues/233
I tried to build on 3 different distributions: K3s, minikube, microk8s
On microk8s, I still see an issue with generation of hub-extra-config-d configmap, recreating manually helps:
kubectl delete configmap hub-extra-config-d -n coffea-casa
kubectl create configmap hub-extra-config-d -n coffea-casa --from-file=coffea-casa/files/hub-extra/

On minikube takes a bit of time to pull image from OSG hub, also iterative development doesn't work because of enabled next line in secret_creation_hook.py but building from scratch helps:
spawner.volume_mounts.extend([{"name": "cmsaf-secrets", "mountPath": "/etc/cmsaf-secrets"}])

K3s is covered in GH CI, but not tested.
Todo list:

[x] Add basic unit tests

Failure in building images is because of lack of space:
System.IO.IOException: No space left on device : '/home/runner/runners/2.301.1/_diag/Worker_20230203-140102-utc.log'

we will follow up in a separate issue.


Fix ViewRenderer missing dependency

Fixes https://github.com/trinodb/trino-gateway/issues/213

Thanks for the fix @andythsu and @ebyhr  ... will see how that affects the dependency analysis


remove old versions

Hey, I was mistaken with my previous update. My existing build was able to run, but could not complete dependancy resolution from scratch. I have removed the issues.

Thanks, @dallman2.  I must admit, I haven't really touched Meteor in years, but won't this then break the package for everyone on an earlier version?
If that's the only way get it working with v3, I can do a major version bump to keep semver compatibility.  But it would be great if there's someway to have one version of the package that works for all meteor versions?

@gadicc you raise a good point, a change like that could break very old projects. I think a major version bump would be a good solution for now. I am not totally sure if there is a solution that keeps everything working on one version, as Meteor's dependancy resolution system is somewhat mysterious to me.
As a matter of context, the version of Node that comes bundled with Meteor versions 2.14 and below will stop receiving security updates this April. Hopefully, many projects on Meteor 2.x and even 1.x will migrate to avoid the security headache:)
In the meantime, I think this change with a major version bump would be a good for now.

Sounds good, thanks!  Going to go ahead with this :pray:

Oops, ref'd #29 in the commit message of the v2 publish, so here it is for reference:
https://github.com/gadicc/meteor-blaze-react-component/commit/430a652616888d7f8d174b5e5df8a146857e4245


SecurityError: Permission denied to access property "pageXOffset" on cross-origin object

Environment:

Node.js version: 14.17.0
NPM version: 6.14.13 (but using yarn 1.22.10)
Browser name and version: Firefox 103.0
Platform name and version: Linux
WebdriverIO version: 7.19.5
wdio-image-comparison-service version: 3.1.1

Config of WebdriverIO + wdio-image-comparison-service
An example of how you configured the wdio-image-comparison-service
Firefox running on Selenium grid in Docker:
Hub: https://hub.docker.com/layers/hub/selenium/hub/4.3.0/images/sha256-f009530d6cbe2e6399bca61e09d907e34471dc98ec5fc2f4ecff84e79c54487b?context=explore
Firefox image: https://hub.docker.com/layers/node-firefox/selenium/node-firefox/103.0/images/sha256-e50a53879dcd59cc1e22c0705f11b2b139cc8189e8ba5a558e56098d4752607a?context=explore

        'maxInstances': 1,
        'browserName': 'firefox',
        'moz:firefoxOptions': {
            'args':[
                `-width=1920`,
                `-height=1080`
            ],
            'prefs':{
                'browser.download.dir': /tmp/downloads,
                'browser.download.folderList': 2,
                'browser.download.manager.showWhenStarting': false,
                'browser.helperApps.neverAsk.saveToDisk': 'text/plain, application/epub+zip, audio/mpeg, text/html, application/rtf'
            }

services: [
        ['image-comparison', {
            baselineFolder: join(process.cwd(), './ui/baseline/'),
            screenshotPath: join(process.cwd(), './output/'),
            formatImageName: '{tag}-{browserName}-{width}x{height}',
            returnAllCompareData: true,
            ignoreTransparentPixel: true,
            ignoreAntialiasing: true
        }],
    ]

Describe the bug
On newer versions of Firefox I recieving SecurityError: Permission denied to access property "pageXOffset" on cross-origin object when using browser.checkElement() function on cross-origin iFrame, cannot provide our app, but can be simulated by test on example below.
It's happening only on Firefox, works fine on newer versions Chrome.
To Reproduce
Steps to reproduce the behavior:
Run
describe('cross', () => {
    it('should visual check iframe', () => {
        browser.url('https://olweus.sites.clemson.edu/js/html2canvas/tests/reftests/crossorigin-iframe.html')
        browser.$('//iframe').waitForDisplayed()
        browser.switchToFrame($('//iframe'))
        browser.checkElement($('//iframe'), './baseline')
    })
})

Expected behavior
Comparison will work normally like on Chrome browser.
Log
[firefox 103.0 LINUX #1-0] SecurityError: Permission denied to access property "pageXOffset" on cross-origin object
[firefox 103.0 LINUX #1-0] unknown error: SecurityError: Permission denied to access property "pageXOffset" on cross-origin object
[firefox 103.0 LINUX #1-0]     at /Users/user/work/test/node_modules/webdriver-image-comparison/lib/methods/screenshots.ts:370:10
[firefox 103.0 LINUX #1-0] at Object.next (/Users/user/work/test/node_modules/webdriver-image-comparison/build/methods/screenshots.js:25:53)

Hi @jkostrhun
this module never officially supported iframes so never looked into this. I know that there can be cross-origin security issues with iframes. Are you able to interact with an element? Like clicking on it and so on?

Hi @wswebcreation ,
yes, this issue is only with browser.checkElement() function while taking screenshot.

Closing this as it's a third party dependency issue, see also https://github.com/mozilla/geckodriver/issues/2068


Just a head's up

This is a brilliant piece of software!
I was going to submit this issue to ask for a few features such as padding and adding a window frame.
However, I just came across this tool which is very similar and has those features: https://crates.io/crates/term-transcript-cli
Life is short, figured I'd give you a head's up!

Hey! thanks

I was going to submit this issue to ask for a few features such as padding and adding a window frame.

@patrislav1 started looking into margin/border support here https://github.com/wader/ansisvg/pull/43

However, I just came across this tool which is very similar and has those features: https://crates.io/crates/term-transcript-cli
Life is short, figured I'd give you a head's up!

No worries, i think there is room for all of them :) we should probably add a "similar tools" section also.
Had a quick loo. Interestingly similar approach to rendering via template and usage of switch (for fallback) and animate (animate the viewbox) elements.

Closing. I guess the feature request about border is covered by the PR i linked


🎨 Add a New Theme to LisBook

LisBook allows users to change the theme, but we want to add more options! Your task is to create and add a new theme (color scheme) for the player.
Steps:
Fork the repository.
Create a new branch: add-new-theme.
Add a new CSS file with the theme's color scheme (you can modify the existing one or create a completely new look).
Update the theme selection dropdown to include your new theme.
Submit a pull request.
Example:
Dark mode, light mode, or even a funky neon theme—the choice is yours!

Hey, I would love to implement this, kindly assign it to me.

Sure @Vedantjn go ahead. Happy Coding :)

@Ctoic , kindly update the topic:

kindly check the labels and lemme know

Great, it is correct now.


jnp.linalg.norm() underflows

Simple example:
jnp.linalg.norm(jnp.array([1e-20], jnp.float32))

produces 0.
Denormals are flushed to zero, and 1e-20**2 is smaller than the smallest normal float32.
It's possible to define a non-underflowing Euclidean norm using variadic reductions:
def norm2(xs):
  def reducer(xs, ys):
    (ssq_x, scale_x) = xs
    (ssq_y, scale_y) = ys
    scale = lax.max(scale_x, scale_y)
    scale_is_zero = lax.eq(scale, jnp.float32(0))
    ssq = ssq_x * (scale_x/scale)** 2 + ssq_y * (scale_y / scale)**2
    return (lax.select(scale_is_zero, jnp.ones_like(ssq), ssq), scale)

  ssq_y, scale_y = lax.reduce(
    (np.ones_like(x), jnp.abs(x)),
    (jnp.float32(1), jnp.float32(0)),
    reducer, dimensions=(0,))
  return scale_y * jnp.sqrt(ssq_y)

We should determine whether we want to do this; if so it probably needs to be implemented as a primitive.
Interestingly numpy.linalg.norm also produces 0 if the CPU ftz flag is set (e.g., via the daz package), but scipy.linalg.norm does not.

Rasmus pointed me to https://hal.archives-ouvertes.fr/hal-01511120/document which looks interesting.

Another possibility is to rescale the norm by the element with the largest absolute value. This would require a second reduction, which perhaps rules it out for performance reasons.

@tlu7

Hi @hawkinsp
I tested the provided reproducible code on Google colab with JAX version 0.4.26. Now jnp.linalg.norm does not produce 0 for 1e-20.
import jax.numpy as jnp
import numpy as np
import scipy

print("JAX  :", jnp.linalg.norm(jnp.array([1e-20], jnp.float32)))
print("NumPy:", np.linalg.norm(np.array([1e-20], np.float32)))
print("SciPy:", scipy.linalg.norm(np.array([1e-20], np.float32)))

output:
JAX  : 1e-20
NumPy: 9.999973e-21
SciPy: 9.999999682655225e-21

For 1e-20**2:
JAX  : 1e-40
NumPy: 0.0
SciPy: 9.99994610111476e-41

Please find the gist for reference.
Thank you.

Thanks for the followup!


Delegate Admin epic design

https://apps.nrs.gov.bc.ca/int/confluence/display/FSAST1/Delegated+Access+Administration+Design


New release not published on packagist

Hi,
The new release 4.4.0 is not published on https://packagist.org/packages/twbs/bootstrap
thanks ;)

@mdo please upload v4.4.0 and v4.4.1 to the other package managers you have access yourself only

Assets don't appear to have been uploaded to StackPath either. I'm still getting a HTTP 403 at https://stackpath.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css

Yeah, I know, see https://github.com/MaxCDN/bootstrapcdn/pull/1420.
I'll take measures so that this doesn't happen in the future.

BTW v4.4.0 is on the CDN

Gotcha, thanks!

I’ve never uploaded anything to Packagist as far as I know. I also don’t have a laptop on me because it’s Thanksgiving today in the US.

The CDN is updated with the new versions

I'm able to access 4.4.1 now. Thanks!

@mdo unless we control packagist, we should remove any links. Ultimately we should control it, though.

Than who is https://packagist.org/users/twbs/? 🤔
Shall we contact packagist about this?

I think the twbs user and this commit have been created at the same time : https://github.com/twbs/bootstrap/commit/be0974802d9ff19d6b4c12c288b3a4fd5c625415

It would be great if this could get fixed soon so I can access the latest releases on the CDN we use.

On our fork, we have a webhook configured to publish on packagist, maybe somebody disable or remove it from bootstrap repository?

Confirming the webhook was the culprit. We were using the old legacy one, so I’ve just done two things:

Manually triggered an update on Packagist, so we’ve picked up the latest releases.
Updated the webhook to the newest version for future releases.

Sorry about that, y’all!


timing_info in archive database not updated

In the archive database, the timing_info table is populated with the genesis ledger accounts, but is never updated. It should be updated when a new account is created.
There are Rosetta queries that rely on this table, so it should be kept up-to-date.

Ok, thanks @mrmr1993 - moving this out to revisit when creating time locked accounts


Extension dependencies are not installed when installing extension from vsix

Have a starter extension, and add the following to it's package.json
"extensionDependencies": [ "github.copilot"]
Package your extension into a vsix
Drag and drop your VSIX into a VS Code Insiders
The extension dependencies are not installed 🐛

If I install the extension from the Marketplace the dependencies will get installed.

@isidorn It did install dependencies for me. Can you please share the vsix?

Worked for me fine now in latest insiders. Closing


Read screens from Tailwind config

I refactored the code to be more readable and easier to understand.
Goal: resolve #1 and #4 (and #6)
Before we can merge this, we have to find out why the client sometimes reads an old version of the tailwind.config.js. I added a demo custom config and if I console.log this outside of the client-only onMount() function, I always see the correct values. If I do this inside of onMount(), I get the default config without my changes. I'm pretty sure that at some point I saw an older custom config there - so maybe it's some kind of caching? Maybe caused by the changes to vite.config?

I spent a couple of hours playing around with it, but I can't make sense of why the tailwind config file won't update at all.  It looks to be properly implemented per the docs.
Probably best to post the config on the vite github and see what tiny thing we're missing.

@jwatts777 I think I found the culprit: We need to forcce-reload the tailwind.config.js using
export default defineConfig({
	optimizeDeps: {
		include: ['tailwind-config'],
		force: true // force to refresh the config file on change in DEV mode
	},
	// ...
});

After changes to the config, you also need to restart the dev server. I added these infos the to README and demo page.
Could you double-check if it's working for you, too?

@jwatts777 I think I found the culprit: We need to forcce-reload the tailwind.config.js using
export default defineConfig({
	optimizeDeps: {
		include: ['tailwind-config'],
		force: true // force to refresh the config file on change in DEV mode
	},
	// ...
});

After changes to the config, you also need to restart the dev server. I added these infos the to README and demo page.
Could you double-check if it's working for you, too?

Give me a sec and I will spin one up. That's a interesting vite config option.
Wonder why it's not default. Will be making a note to add to all future and current projects.

Looks good to me, and functions as expected.


fix build.

checked on ubuntu 14.04.

Thanks.  I'll merge the PR, but you should know that the canonical version of this project is living on Google Code: https://code.google.com/p/lua-checker/

Yep i knew. But project seems inactive. Thx for the review. :)
Le 9 juil. 2015 6:29 PM, "Russell Power"<EMAIL_ADDRESS>a
écrit :

Thanks. I'll merge the PR, but you should know that the canonical version
of this project is living on Google Code:
https://code.google.com/p/lua-checker/
—
Reply to this email directly or view it on GitHub
https://github.com/rjpower/lua-checker/pull/3#issuecomment-120059486.


Combinators to assert performance metrics

Super cool library!
I could imagine there being combinators to assert performance metrics for individual test cases. Like "fail this test if it the runtime differs by x% off some baseline".

allow annotation of tests/benches with expected runtime + allowed deviation
maybe even save metrics to a file and have a command line flag to accept changes

Imagine CI checked performance regressions, very similar to what GHC does in it's CI infrastructure.

@alexbiehl-gc could you possibly give a try to a01a3ff667b9fe55a456e4434403ec4b3d147069? I've added a basic support of baselines. It works as follows:

Dump benchmark results in CSV format:

$ cabal bench --benchmark-options '--csv fibo.csv'
All
  fibonacci numbers
    fifth:     OK (1.32s)
      155 ns ±  13 ns
    tenth:     OK (1.89s)
      1.8 μs ± 104 ns
    twentieth: OK (1.94s)
      235 μs ±  20 μs
    from file: OK (2.00s)
      1.9 μs ± 133 ns

Change implementation as desired.
Rerun benchmarks, providing previous report as a baseline:

$ cabal bench --benchmark-options '--baseline fibo.csv'
All
  fibonacci numbers
    fifth:     OK (1.46s)
      172 ns ±  15 ns
      11.3% slower than baseline
    tenth:     OK (2.08s)
      2.0 μs ± 174 ns
      11.8% slower than baseline
    twentieth: OK (4.23s)
      258 μs ±  11 μs
      9.6% slower than baseline
    from file: OK (2.18s)
      2.1 μs ± 119 ns
      9.8% slower than baseline

How does it look?

Sweet! I think this is a great first step! I am unsure how an assertion over these would look like. As for the assertion one possible idea could be to redefine Benchmarkable as
data Benchmarkable = Benchmarkable {
  benchmark            :: Int64 -> IO (),
  baselineDeviation :: Maybe Double
}

and have another combinator to annotate Benchmarkables. Now, if a baseline deviation and --baseline is present, the results are being compared and the test succeeds or fails accordingly.

b0448b43297da71885e02356b8b02e868ba77d2d adds --fail-if-slower PERCENT command-line option and a corresponding tasty Option for fine-grained control:
$ cabal bench --benchmark-options '--baseline fibo.csv --fail-if-slower 10'
All
  fibonacci numbers
    fifth:      FAIL (1.32s)
      156 ns ±  13 ns, 24% slower than baseline
    ten.th:     OK (1.92s)
      1.8 μs ± 128 ns
    twen,tieth: OK (3.84s)
      236 μs ± 9.6 μs
    from file:  OK (2.02s)
      1.9 μs ± 177 ns

1 out of 4 tests failed (9.15s)

Passing --fail-if-slower 30 still reports about 24% slowdown, but does not fail the benchmark.
If it looks good, I'll proceed with documentation.

This is good stuff. Wanted to have this for some time at work but never got to it. Thank you for your efforts!
One last nit: Personally I’d prefer a dedicated combinator instead of raw localOption.

One thing: It might be good to have another, stricter version of the combinator that fails if too slow or too fast. It would help always be on top and aware of changes that cause baseline adjustment.

One last nit: Personally I’d prefer a dedicated combinator instead of plain localOption.

Probably not at this stage: I'd like to see this feature getting significant traction first.

It might be good to have another, stricter version of the combinator that fails if too slow or too fast.

Good idea. Added --fail-if-faster.
I'd appreciate if you give this functionality a try, before I release it officially.

https://hackage.haskell.org/package/tasty-bench-0.2.1 has been released, including this feature.


typo: adminstrative -> administrative

@nschonni - This one is saying there is a conflict. One thing to check is make sure your fork is up to date. You can do a 'reverse pull request'. You go to the main repo, and click the Pull Request button.. then click the Across Forks link and then you can merge latest changes from Master in this main repo to your branch in your fork. Let me know if you have any questions or get stuck on it. I am not able to resolve these conflicts for some reason, the button is grayed out. Thanks as usual for your continual help and dilligence on these fixes! Much appreciated!

If you re-open the PR I can rebase this branch later

I think it re-opened when you reply. Let me know if there is something else I can do. :)

Looks like one of the files was deleted, and the other change was made in 11f7351eab47ce3f138c9658e782b05f10dd047e


fixed navbar at the top with some hover effects

This pull request proposes improvements to the UI of the navigation bar using CSS.
Changes Made:
Fixed navbar at the top
Added hover effects to enhance user interaction
Fixes issue #7
Testing: Manually tested the changes on various screen sizes.
Please review the implemented changes and provide feedback. I welcome your suggestions for further improvement.

@CSEdgeOff please review and merge it, it's been done since 2 days.

@adarsh-fxz we'll give you an update soon.


chore: update dependencies

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (4e7086c) 97.53% compared to head (53f9b63) 97.53%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@           Coverage Diff           @@
##           master      #85   +/-   ##
=======================================
  Coverage   97.53%   97.53%
=======================================
  Files           9        9
  Lines         162      162
  Branches       41       41
=======================================
  Hits          158      158
  Misses          4        4

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


Implement System.Quantity and System.Date

System.Quantity was not implemented in favor of FHIR.Quantity. Per the FHIRPath spec tests, System.Quantity should be used for quantity literals that occur in an expression.
System.Date was also not implemented in favor of System.DateTime.

Fixed in #446


Adding support for Spaces

what?

Ability to create a dedicated space for any stack via it's Spacelift settings (although it's primarily meant for admin stacks)
Ability to override space id for attachments (policies, contexts, etc.) for when we need to put them in a space separate from the admin stack (e.g. root)
Ability to override space id for stacks created by an admin stack (stacks_space_id)
Ability to specify space inheritance
Should default to legacy space everywhere so as to not break existing users who haven't migrated to spaces

notes

This was tested successfully w/ a CP client
Also tested a legacy setup by running a plan on cplive and verifying nothing is broken

/test test/bats

/test test/readme

alright @aknysh @nitrocode I made the changes and re-tested/verified everything. Can I get one more final review?

/test test/bats

/test test/readme


Ankidroid integration via custom URL scheme

Related: #18 #19 #20
References:

https://github.com/ankidroid/Anki-Android/issues/3479
https://stackoverflow.com/questions/33786651/open-android-app-with-custom-url-scheme

Since there are Anki addons that add J-J definitions, and copying text is useful for (among other things) creating Ankidroid cards, and because switching apps closes the results window, cutting out the middle man and adding Ankidroid integration via a custom url scheme would be extremely helpful.
Desired features:

Sending entire text to Ankidroid as the "Sentence"
Sending a screenshot of the text without the result window, possible croppable, to Ankidroid. This can actually just be done by the user before OCR'in, and then they can add most recent screenshot when the card is created, so it's not necessary.
Sending the 原形/citation/dictionary form of the clicked-on/currently-highlighted word to Anki as the "Expression"
A definition (using the current dictionary, which is probably at this stage going to be English)
An option in the settings to specify which fields are actually exported, since different people will want different things.

Linking #26 to keep it in mind.

The AnkiMobile url sceme is really helpful in creating new notes from browser.
I have implemented this cloze generation addon (kind of) for AnkiMobile.
https://github.com/krmanik/AnkiMobile-Addons/tree/main/ocloze
So, I think AnkiDroid should also have this features. But the AnkiDroid only follow Anki upstream so to have this feature in AnkiDroid it needs to have in Anki Desktop first.


Failed to read the duration of webm file

Here are the errors in backend while my frontend looks fine

Nevermind, I fixed it!!!

Actually, I still got the problem with ASGI application, which make the web not stable for me!

Can you share your logs again?
In your previous log, you have an extra space at the end of the OpenAI key.

It works fine for images but the record. I fix sk-key, so it run well but
the record.
[image: image.png][image: image.png]
Vào CN, 7 thg 4, 2024 vào lúc 19:10 Abi Raja @.***>
đã viết:

Can you share your logs again?
In your previous log, you have an extra space at the end of the OpenAI key.
—
Reply to this email directly, view it on GitHub
https://github.com/abi/screenshot-to-code/issues/297#issuecomment-2041740177,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ASAK5LLY5CK6FHZXLRJU46TY4H37RAVCNFSM6AAAAABF3YYNYCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRG42DAMJXG4
.
You are receiving this because you modified the open/close state.Message
ID: @.***>

Your images aren't showing up here. I'll need more information (browser console, browser version and backend logs) to help you debug the record issues.

Thanks for sharing the logs. I'll have to dig into the code to solve this.
Instead of the record, can you try recording a video using your computer and then, dragging in the video? I think that should work. The problem here is with the video format that the browser is recording.
Which browser are you using BTW?

I will try to record a video. I am using Microsoft Edge btw.

Cool, let me know how that goes. I'll spend more time debugging issues with the screen recording over the next week.


Interactsh Server doesn't push DNS interactions to the client despite everything is working fine with http

Interactsh version:
1.2.0
Current Behavior:
It seems that the interactsh-server which I run locally doesn't push dns queries to the client. I know that because running the client without parameters creates a subdomain  of oast.fun and it is pulling the dns records just fine from your interactsh-server, but it doesn't show the dns queries that come to my hosted server, I configured my server to be name server of my own domain and it seems to be working fine, but the only problem dns queries are not be pushed to the interactsh-client.
Expected Behavior:
interactsh-client pulls dns queries from the local hosted interactsh-server
Steps To Reproduce:

run the server like this:
./interactsh-server -http-directory /root/interactsh -dsp /root/interactsh-storage -ds -wildcard -d interact.myowndomain.com -http-index /root/interactsh/banner.html -server-header "nginx/1.19.2" -disable-version -cidn 1 -cidl 1
take the client token from above and feed it to this command:
./interactsh-client -server http://interact.rt.dnsabr.com -token your_token_goes_here -v -json -cidn 1 -cidl 1
3.check if it works:
curl https://dededded.interact.myowndomain.com
4.you will see that it works and the client shows all http interactions but not the dns ones.

I hope this helps
Regards

Yes I get the same issue. curl mydomain.com works but nslookup mydomain.com doesn't work.

hello @DOCTORBALISE ,
I just found out that setting -cidn and -cidl to small values somehow is the problem (I might be wrong),  I removed them altogether and I get everything to work fine even the http index page that I configured the server to use (and it wasn't using it for some reason) now is being used.
I tried to figure out the minimal values for dns to work and I got this:
-cidn 5 -cidl 4
I hope this helps
Regards


Minification failed with create react app since last version

Thank you for the handy library. Unfortunately since last the version it fails the build of create react app. More context is provided at the folling link:
https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/template/README.md#npm-run-build-fails-to-minify
The issue also affects get-value.

None of the libraries you mention, nor their dependencies are using es6 features. I suggest opening an issue on create-react-app. If you do find an error in this library, please provide more information so I can try to reproduce the error.


feat(engine): no_std support

Description of change
no_std support for stronghold engine part
Type of change
Choose a type of change, and delete any options that are not relevant.

[ ] Bug fix (a non-breaking change which fixes an issue)
[x] Enhancement (a non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation Fix

How the change has been tested

engine and runtime can be built on no_std
whole stronghold tests are passing on std

Change checklist
Add an x to the boxes that are relevant to your changes, and delete any items that are not.

[x] I have followed the contribution guidelines for this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes

Hello @wusyong, thank you for your contribution and sorry for the late reply. A lot has changed since your PR, so i will close it down. We also changed direction concerning no_std. The goal would be to have everything no_std ready, but the parts that require a standard library environment eg client, would require to explicitly opt-in for std as feature.


Support drag-and-drop of audio file (using Shift) into Markdown file

Version 1.78 supports drag-and-drop of a video file into a Markdown file. So the idea is to do the same for audio files, using an audio HTML tag.

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.
Happy Coding!

I would finish this feature if I could

Hey @anthonyvdotbe or anyone who's reading this, can you tell me in which file the code is present for this.

Fixed by #181653


dash-nlp TypeError in lda_analysis

App name
dash-nlp
Description of bug
Followed the instructions to git clone the app and while I'm able to run it normally, the LDA graph is not working. There is a TypeError in ldacomplaints.py.
This is the error message:
File "C:\Users\dash-sample-apps\apps\dash-nlp\app.py", line 705, in update_lda_table complaints_text, list(STOPWORDS) File "C:\Users\apps\dash-nlp\ldacomplaints.py", line 51, in lda_analysis df = df[pd.notnull(df["Consumer complaint narrative"])] TypeError: list indices must be integers or slices, not str
I'm not able to find a solution to this. Can anyone please help?

@dataxienxe I am not able to reproduce this error with the latest version of master. Are you selecting some specific bank? Did you manage to install all the requirements?


Use Java 11 for Docker Images

Is your enhancement request related to a problem? Please describe.
The current docker images are built with Java 8. According to the 2.3.0 changelog Pulsar supports Java 11 since early 2019.
Java 8 and Docker doesn't work that well together. Some things got backported to Java 8 but only Java 10+ supports Memory and CPU limits inside Docker Containers correctly. More on that here: https://www.royvanrijn.com/blog/2018/05/java-and-docker-memory-limits/
Describe the solution you'd like
I'd like to see Docker Images that use Java 11 & use the correct start parameters by default. The current images rely on openjdk:8
https://github.com/apache/pulsar/blob/b410274554508684f4a15cc1f5a98a71d85fbe2e/docker/pulsar/Dockerfile#L41
https://github.com/apache/pulsar/blob/b410274554508684f4a15cc1f5a98a71d85fbe2e/docker/pulsar-standalone/Dockerfile#L27
Describe alternatives you've considered
Alternatively there could be a java11 version of the image, parallel to the current images. Like "2.7.0" and "2.7.0-java11" https://hub.docker.com/r/apachepulsar/pulsar/tags

Java 8 and Docker doesn't work that well together. Some things got backported to Java 8 but only Java 10+ supports Memory and CPU limits inside Docker Containers correctly.

This has changed in the last 2 years. The Java 8 version used in Pulsar images already supports container memory & cpu limits. This blog post explains it.
One way to verify that it's supported is to run this docker command:
docker run --memory 100m --cpus 1 --rm --entrypoint '/bin/bash' apachepulsar/pulsar:2.6.1 -c 'java -XX:+UnlockDiagnosticVMOptions -XX:+PrintContainerInfo -version'

it will printout diagnostics about the limits that the JVM detects:
OSContainer::init: Initializing Container Support
Path to /memory.limit_in_bytes is /sys/fs/cgroup/memory/memory.limit_in_bytes
Memory Limit is: 104857600
Path to /cpu.cfs_quota_us is /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us
CPU Quota is: 100000
Path to /cpu.cfs_period_us is /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us
CPU Period is: 100000
Path to /cpu.shares is /sys/fs/cgroup/cpu,cpuacct/cpu.shares
CPU Shares is: 1024
CPU Quota count based on quota/period: 1
OSContainer::active_processor_count: 1

It's already possible to use the JVM container support in Pulsar containers by specifying PULSAR_MEM="-XX:InitialRAMPercentage=50.0 -XX:MaxRAMPercentage=50.0 -XX:MinRAMPercentage=50.0".
For example:
docker run --name pulsar-standalone --memory 512m --cpus 1 -e PULSAR_MEM="-XX:InitialRAMPercentage=50.0 -XX:MaxRAMPercentage=50.0 -XX:MinRAMPercentage=50.0" -e PULSAR_GC="-XX:+UseG1GC" apachepulsar/pulsar:2.6.1 bin/pulsar standalone

Verifying in another terminal after startup:
$ docker exec pulsar-standalone jcmd 1 GC.heap_info
1:
 garbage-first heap   total 262144K, used 146496K [0x00000000f0000000, 0x00000000f0100800, 0x0000000100000000)
  region size 1024K, 75 young (76800K), 7 survivors (7168K)
 Metaspace       used 78433K, capacity 82292K, committed 82460K, reserved 1122304K
  class space    used 9461K, capacity 10287K, committed 10316K, reserved 1048576K

Heap size is 256MB, which is expected. Since Pulsar uses also native memory, it's probably not worth increasing the heap limit too much over 50% so that the container doesn't get killed. The default MaxDirectMemorySize is the maximum heap size. The total memory consumption of a Java process is explained here.
It's also worth noticing that PULSAR_GC should be set for docker containers so that the not-so-good defaults don't get used. There's a PR already for addressing this issue. One issue with the defaults is that there's a huge number of GC threads configured.

:+1: to using JDK 11
We have run into some issues due to Pulsar using JDK 8 (quite old these days) and we are now considering creating our own Docker image for Pulsar instead.
We have some Pulsar functions being compiled with JDK 11 but they need to run on JDK 8 and there are a lot of incompatibilities.

The images now use JDK11. I believe this issue can be closed.


mobile: Enable setting the Platform Cert Validator thread priority

If the Envoy Mobile thread has a thread priority set, the Platform Cert Validator's validation thread also has the thread priority set to the same value.

/assign @fredyw

The CI failure looks legit: https://github.com/envoyproxy/envoy/actions/runs/10837324343/job/30073099078


Create multi collections implementations

Reading about format of data returned by models served by mlflow we would need the the MultiMap and MultiSet collections I think (as we can see at https://www.mlflow.org/docs/latest/models.html#local-model-deployment). There is a need of our own implementation as for scala 2.12 I don't see any usable one.

Just not to forget - unit tests of introduced should be included at least partially in this PR :wink:


ascii: allow build on darwin

Isn't meta.platforms = all the default anyway? I don't think it's necessary to specify that attribute explicitly.

oh so I should delete platforms attribute when unnecessary ?
I'll try that, thx

No, it's then not built by hydra.
On Sun, 1 Nov 2015, 13:48 Stéphane Jourdois<EMAIL_ADDRESS>wrote:

oh so I should delete platforms attribute when unnecessary ?
I'll try that, thx
—
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/pull/10754#issuecomment-152823488.

Ah, okay. So it is necessary.


EOBot needs retries

CI tests in etheos fail periodically due to things that could easily be wrapped in a retry loop. Retry policy should be implemented as a command-line parameter input.
Any operations that fail should be retried according to the specified policy. This includes but is not limited to - establishing connection with the server, and executing a script.

CI tests in etheos now pass more consistently due to use of a self-hosted build agent. However, introducing a retry policy would still be nice to have in EOBot.


Task: Add Core Competencies Section

Description: Highlight your core competencies and skills.
Subtasks:

[ ] Create a section listing key skills and competencies.
[ ] Use bullet points or a table to present this information clearly.

Highlight your core competencies and skills.


[nova,cinder] Fix paths for default config

By default config files were checked in /etc/service/etc/service/...

Changes Unknown when pulling bade1d3434ef8ef4dbcea7a44595f4b4ff1cff0a on viraptor:proper_paths into ** on HewlettPackard:master**.

Coverage remained the same at 37.376% when pulling bade1d3434ef8ef4dbcea7a44595f4b4ff1cff0a on viraptor:proper_paths into 7f4af697b941e1acab7a2fff12bf1c5a70ca3c06 on HewlettPackard:master.


Allow customization of the graph "id" field

Per https://github.com/movio/bramble/issues/109, it was a bit restrictive that Bramble mandated the name id as the only allowed graph key. This adds configuration to customize the package's IdFieldName, at which time a graph may specify its own universal boundary key name. Also updates related docs.
Resolves https://github.com/movio/bramble/issues/109.

Codecov Report

Merging #116 (6378386) into main (6d3f91c) will decrease coverage by 0.09%.
The diff coverage is 65.00%.

@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
- Coverage   68.75%   68.66%   -0.10%
==========================================
  Files          24       24
  Lines        2612     2614       +2
==========================================
- Hits         1796     1795       -1
- Misses        681      684       +3
  Partials      135      135

Impacted Files
Coverage Δ

config.go
6.97% <0.00%> (-0.11%)
:arrow_down:

plugins/meta.go
10.07% <0.00%> (ø)

schema.go
100.00% <ø> (ø)

validate.go
81.41% <60.00%> (ø)

merge.go
93.54% <100.00%> (ø)

plan.go
83.78% <100.00%> (ø)

auth.go
86.79% <0.00%> (-0.63%)
: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 6d3f91c...6378386. Read the comment docs.

Committed all suggestions, @pkqk!


Blazor WASM PWA support

Fixes #17198
Internal Review Topic
Not edited for style+grammar+UE. 🏃😅 Will return to do so asap.

Approved offline.


NodeError: Must use import to load ES Module

So the code doesn't work, since the it build into "ESNEXT" according to https://github.com/117/alpaca/blob/master/tsconfig.json
You're using the export from syntax which is not supported by the latest version of node yet.
Try it here: https://runkit.com/ehab180hb/5f9f225f041987001aafe129

I have this fixed in the latest version v2.1.3. Let me know if this resolves the issue. 🙂

Works like a charm, thanks!


Fixed transpose/reshape to th/tf dim_ordering

As per comment on Line 153, converting a PIL image to a numpy array creates an image with dim_ordering (H, W, C) (if RGB - otherwise (H, W) if grayscale).
Thus to create Theano and TensorFlow dim_ordering (as per the main image doc, i.e. (C, W, H) and (W, H, C), we need to transpose/expand x as per this pull request.

I am worried that this would create issues in any user code that relies on the current implementation. For instance, within the Keras codebase alone, this would affect several example scripts (e.g. style transfer).

It is a risk - I came across the error having taken small crops within images loaded via load_img() and img_to_array(). Saving them back to disk (via array_to_img()) showed incorrect crops.
Running grep on all files within the most recent pull of the keras repo:
$ grep -r "img_to_array" *
docs/templates/applications.md:x = image.img_to_array(img)
docs/templates/applications.md:x = image.img_to_array(img)
docs/templates/applications.md:x = image.img_to_array(img)
examples/neural_doodle.py:from keras.preprocessing.image import load_img, img_to_array
examples/neural_doodle.py:    img = img_to_array(img)
examples/neural_doodle.py:    target_mask_img = img_to_array(target_mask_img)
examples/neural_doodle.py:    style_mask_img = img_to_array(style_mask_img)
examples/neural_style_transfer.py:from keras.preprocessing.image import load_img, img_to_array
examples/neural_style_transfer.py:    img = img_to_array(img)
keras/preprocessing/image.py:def img_to_array(img, dim_ordering='default'):
keras/preprocessing/image.py:            x = img_to_array(img, dim_ordering=self.dim_ordering)
tests/keras/preprocessing/test_image.py:                img_list.append(img_to_array(im)[None, ...])

$ grep -r "array_to_img" *
Binary file keras/preprocessing/image.pyc matches
keras/preprocessing/image.py:def array_to_img(x, dim_ordering='default', scale=True):
keras/preprocessing/image.py:                img = array_to_img(batch_x[i], self.dim_ordering, scale=True)
keras/preprocessing/image.py:                img = array_to_img(batch_x[i], self.dim_ordering, scale=True)

We could go through each such file and try to ensure this change doesn't break anything (of course any independent projects could break) - alternatively, should this fix be applied within a future (major) release?

I suggest that we modify all places that are affected in the Keras codebase and examples (as part of the present PR), and merge the PR.

I've resolved these issues and added a unit test for it.


Add Basic UI

In this milestone, I did the following;

Add a basic-UI
Create an app.rb file that will serve as a console app entry point
Create a Main.rb file which acts as an entry point for the app

HI @erickma1, it would be nice to remove the live demo section from your readme as you are not going to deploy this project 👍🏿

Removed demo section

HI @erickma1, it would be nice to remove the live demo section from your readme as you are not going to deploy this project 👍🏿
@prolajumokeoni Demo section removed

HI @erickma1, it would be nice to remove the live demo section from your readme as you are not going to deploy this project 👍🏿
@prolajumokeoni Demo section removed

Kindly await a review 👍🏿

Moved the list of options presented to the user to the main.rb file


Weird numbers when using a VPS based on xen technologies

Bug description
When I cat /sys/devices/virtual/powercap/intel-rapl/intel-rapl:0/energy_uj every seconds on my VPS I get the following result
194127997354
45766381128
45831036419
45895297790
194961270917
46025818025
195088717917
195154518652
195217628171
195283094556
195347616974
46422262201
46488823065
46554152305
195605807719
46686185292
195732606528
46819468154
195866360824
195930510501
195995971759
47080028827
47148009244
47213175459
47280270685
47345139416
47409635711
196448336263
196513124183
47604736713
196643367563
196707819058

We can see values that are no consistent.
To Reproduce

Take a vps based on xen technologies
cat a energy_uj file every 1s seconds

Expected behavior
Having a way to detect when the context switches and ignore the previous value.
Environment

Linux distribution version: Debian buster
Kernel version: 4.19.0-14-amd64 (might explain why as well...)

Hi @jdrouet,
This seems like a powercap-specific topic, but could be avery valuable insight on the Xen hypervisor behavior.
Could you share what provider you use ?
I have honestly no idea why you seem to get values about two different sources. Maybe discussing about this with the provider could help ?

It's a VPS from ikoula

Moving this to discussion.


Create RFC around Github and PR etiquette

How do authors and reviewers interact with PRs? Document the standard process that we typically have used here at CDS.
(Usually authors merge, review required before merging, whether we prefer squash and merge, whether or when we want design approval or when just dev is enough, etc etc). I have my own opinions about a lot of these, based on how I've operated since being at CDS, but I am sure these would be better if discussed and agreed upon by other folks.
Also put in guidelines for how reviewers and reviewees should act towards each other in a PR review. I know paul had some documentation around this that was excellent. I will try to find it.

This is a great idea. When you've got some time can you put some of your opinions here so we can start to collate them? Some of mine would be:

Authors merge

Author is in best position to know if there are any slight changes or followups before merge

Use squash and merge

The history of the development is captured in the PR. Working commits usually have less useful commit messages which aren't useful in the main repo

Code review is required. (Do we want some kind of carve out for a to-be-reviewed for emergencies?)
The purpose of code review is to get to a good state. The code doesn't have to be perfect you're aiming for a LGTM (looks good to me). Note the good, not perfect, not excellent, but good.
Escalate if the review is blocked
Even if you can't review, put a comment in the PR if it's going be longer then 24hrs before you do review to let the author either switch reviewers, or have an idea of when they'll hear back.
When reviewing, try to stick to questions as opposed to giving orders. You don't know what the author has thought of or planned for. It's better to ask if they considered something, as they may have rejected it already, then to tell them to use it and have then try to explain why not.

I agree with much of your opinions. Here's mine more specifically:

Authors merge
Squash and merge preferred
I like having github auto delete branches after they've been merged, as I don't like merged branches cluttering up the repo
Code review required, with at least one reviewer. Always at least 1 dev reviewer, and ideally have at least one design reviewer on anything front-facing or content.
Author should link the PR to any tracked issues or trello cards. Author should also include any context around whether the issue has been broken down into several PRs and justification for where they've chosen to break it up. Next steps are great to include in this situation as well (as well as being tracked in their appropriate issue).
Author should include any steps to test, caveats, or gotchas that the reviewer will need to understand the PR.
I am not a huge fan of commenting your own PR through github - if you need to include comments to explain something in the PR, these should more often than not be code comments.
Reviewers should point out the good as well as the bad - if the author has done something particularly well, reviewers should point it out as a well done.
+1 to the questions vs orders for reviewers, always seek clarification on why something was done a certain way as opposed to assuming.

I'm still trying to dig up that PR etiquette documentation. Will look more today.

Found it!
https://github.com/cds-snc/cra-claim-tax-benefits/blob/main/docs/CODE-REVIEW-GUIDELINES.md


ci: bump travis, appveyor and module versions

Please take a look.

@sbinet Can we drop appveyor? They are always late and the documentation is pretty poor.

Sure.
As long as we keep a Windows build+test, the ci platform doesn't matter much.
(We could try the GitHub one if/when the Travis one is getting overcrowded)

It's working now though, so I'll file an issue to deal with that.

ping


xml_request_operation_processing_queue() is Mac-only.

Wrap xml_request_operation_processing_queue() in a test for
__MAC_OS_X_VERSION_MIN_REQUIRED because it is only used by a
Mac-specific code block down below.
This addresses an unused symbol warning when compiling with strict
settings.

Good call. Thanks!


Implement the chibicu self command

The command to create self signed certificates is not implemented yet.

Closed with 5d8c98b


Support BedrockDB

https://bedrockdb.com/

Bedrock is a simple, modular, WAN-replicated, Blockchain-based data foundation for global-scale applications.
...
Bedrock was built by Expensify, and is a networking and distributed transaction layer built atop SQLite, the fastest, most reliable, and most widely distributed database in the world.
...
Bedrock supports the MySQL protocol, meaning you can continue using whatever MySQL client you prefer

Might just work out of the box.

Doesn't seem to be that easy unfortunately:
$ /home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/.bin/prisma migrate save --create-db --experimental
2020-07-01T18:56:56.713Z prisma Environment variables not loaded
Error: Error: undefined: Error querying the database: Error querying the database: Unknown authentication plugin `@BhfHt/-4^Z,'.

    at Object.ensureDatabaseExists (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/@prisma/cli/build/index.js:2:2818789)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async MigrateSave.parse (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/@prisma/cli/build/index.js:2:2089873)
    at async main (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/@prisma/cli/build/index.js:2:1561778)

I also found this yesterday—you can't make schema changes on a running instance (so Migrate won't work): https://github.com/Expensify/Bedrock/issues/218
There's a rationale for it in the comment just before mine. Here's the docs for concurrent transactions: https://www.sqlite.org/cgi/src/doc/begin-concurrent/doc/begin_concurrent.md
There's a workaround in the first comment (disable concurrent transactions) which may not be that much of a bottleneck...apparently all COMMITs are serialized anyway, regardless of the transaction type.

Seems our DB client does not like that there is no authentication at all required.

I haven't tried this yet, but maybe you can just put anything you want into username/password and Bedrock will ignore it and connect? Same for the database name—when I connect with my SQL GUI it sees a single database called main, but SQLite doesn't really have named databases, so that may just be there to satisfy MySQL clients.

Thanks for this comment @cannikin, I migrated a SQLite database locally, committed it and used that to start Bedrock - and we have passing tests: https://github.com/prisma/e2e-tests/pull/509 🚀
So Prisma Client works perfectly fine with BedrockDB, it is only Migrate that has a slight problem right now.

Holy cow did you see this @janpio ?? 😮 https://github.com/Expensify/Bedrock/issues/218#issuecomment-660715844

A new release is out, but no change for the use of migrate: https://github.com/prisma/e2e-tests/pull/610
Still the same error as https://github.com/Expensify/Bedrock/issues/818 though - so there is possible another blocker.

Good news:
It is fixed, but not released yet: https://github.com/Expensify/Bedrock/issues/818#issuecomment-673173943
The PR shows a follow up error though - I think we will look into this when the release happened.

And now it is released, and our CI run confirms the same problem: https://github.com/prisma/e2e-tests/pull/610/checks?check_run_id=984394627
+ yarn prisma migrate save --create-db --experimental
yarn run v1.22.4
warning ../../package.json: No license field
$ /home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/.bin/prisma migrate save --create-db --experimental
Environment variables loaded from prisma/.env
Error: Error: P1001: Can't reach database server at `<IP_ADDRESS>`:`3306`

Please make sure your database server is running at `<IP_ADDRESS>`:`3306`.
    at Object.ensureDatabaseExists (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/@prisma/cli/build/index.js:2:2818789)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async MigrateSave.parse (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/@prisma/cli/build/index.js:2:2089873)
    at async main (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/@prisma/cli/build/index.js:2:1561778)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Will investigate later.

@janpio We just found this...https://dqlite.io/

Very different beast @cannikin - this has its own "language" you talk to it, so on the Prisma side this would need a new connector (similar to MySQL, PostgreSQL, SQLite and so on).

@janpio We've got another contender!  https://github.com/rqlite/rqlite

@janpio We've got another contender!  https://github.com/rqlite/rqlite

In my understanding - and that might be wrong, I only skimmed - rqlite also only offers its own data API (and not a standard MySQL API as Bedrock) to talk to it and send queries. That would be a totally different connector then. Here is its node client for example: https://github.com/rqlite/rqlite-js

In my understanding - and that might be wrong, I only skimmed - rqlite also only offers its own data API (and not a standard MySQL API as Bedrock) to talk to it and send queries. That would be a totally different connector then. Here is its node client for example: https://github.com/rqlite/rqlite-js

Huh...does Prisma have any plans to provide an interface, like via the SDK, that would let you create your own adapter to a DB? Then I could just integrate all of these databases myself and stop bugging you guys. 😉

Huh...does Prisma have any plans to provide an interface, like via the SDK, that would let you create your own adapter to a DB? Then I could just integrate all of these databases myself and stop bugging you guys. 😉

@janpio I was reminded of this issue again today! Any idea if the new Migrate stuff can work with Bedrock?
Doesn't look like there's been any movement on this issue, though: https://github.com/Expensify/Bedrock/issues/846

No difference with the new Migrate unfortunately:
$ /home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/.bin/prisma migrate dev --preview-feature
2021-02-12T20:27:56.285Z tryLoadEnv Environment variables not found at null
2021-02-12T20:27:56.286Z tryLoadEnv Environment variables not found at undefined
2021-02-12T20:27:56.286Z tryLoadEnv No Environment variables loaded
Prisma schema loaded from prisma/schema.prisma
Datasource "db": MySQL database "bedrock" at "<IP_ADDRESS>:3306"

Error: Error: P1001: Can't reach database server at `<IP_ADDRESS>`:`3306`

Please make sure your database server is running at `<IP_ADDRESS>`:`3306`.
    at Object.ensureDatabaseExists (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/prisma/build/index.js:60473:13)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async MigrateDev2.parse (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/prisma/build/index.js:62432:28)
    at async main (/home/runner/work/e2e-tests/e2e-tests/databases/bedrock/node_modules/prisma/build/index.js:105570:18)

Error message is indeed still the same as in https://github.com/Expensify/Bedrock/issues/846

Someone just posted this screenshot and I saw a mention of "Bedrock"... is something actually working??

No interest from users, so we will not prioritize this - so closing this issue to reflect that.


? Use system property as an alternative way to specify diktat config path

Now we sometimes cannot use custom config because ktlint doesn't have such functionality and doesn't allow extending CLI flags. But, e.g., for gradle plugin there is no other way to pass custom config, and that now leads to duplication of config files in multi-module project.

Or, since it's needed only in gradle plugin, we could recommend configuring diktat for root project only and then filter file path patterns before passing them to ktlint.


KVv2 secrets are always entirely removed from Vault when RandomSecret is deleted from Kubernetes since v0.8.17

Release v0.8.17 changes the logic to delete the KVv2 secret entirely from Vault whenever the RandomSecret Kubernetes resource is deleted, instead of removing only the last version of the Vault secret (releases v0.8.16 and below).
As mentioned in https://github.com/redhat-cop/vault-config-operator/issues/134#issuecomment-1587520678 , there are situations where deleting only the last version of a KVv2 secret is still desirable.
A very common example  is to use a RandomSecret to generate a root password or database admin password.
In such a case, you would like to protect this KVv2 secret at any cost - loosing it for any reason (think about an erroneous gitops operation removing the RandomSecret resource, or even a straight kubectl deletion) would be catastrophic as the secret can only be recovered from a Vault backup.
So I'd suggest to make this KVv2 deletion logic configurable.
The deletion logic (either delete-last-version or delete-all-versions) could be set using:

an environment variable
a new RandomSecret spec field (requires changing the CRD here)

Consider .2 first, then if not set, then use 1. , and if not set either, use the current behavior.
This looks to maximize the use case coverage and flexibility.
@trevorbox @raffaelespazzoli does this make sense?
I can provide a PR along this lines.

I find this very disturbing as well. As for me, I'd completely block the secret removal instead of even the version removal. If the secret has only one version and then this version gets removed, the entire secret is removed as well, if I get it right?

I find this very disturbing as well. As for me, I'd completely block the secret removal instead of even the version removal. If the secret has only one version and then this version gets removed, the entire secret is removed as well, if I get it right?

With v0.8.17+, the entire secret is removed from Vault, irrespective of the number of versions.
Previous behavior (for KVv2 secrets) was to mark the last version as deleted, so when attempting to access this secret, you would get an error stating that this secret does not exist. However, you could go to Vault and undelete this latest version, making it available again (no data loss here).
In some cases, the entire removal could be useful as well, such as a testing system with a lot of automation around it: not removing these secrets from Vault would leave a lot of useless mess behind, even potentially creating unexpected behaviour if reusing the same secrets across testing sessions.
So what I suggest is that the deletion logic should be made configurable to suit the use case at hand.
Some users may decide that no secret should be deleted for production systems, because too sensitive if secret is lost.
Some others could choose to delete the last version only, so that there is an error but without data loss (requiring manual undelete if this was a mistake).
Some could decide that the full removal better suits their need because their deployment process is stable and robust enough to avoid the situation entirely...
My point is that there is no one-suits-all deletion logic and should be made configurable...

I find this very disturbing as well. As for me, I'd completely block the secret removal instead of even the version removal. If the secret has only one version and then this version gets removed, the entire secret is removed as well, if I get it right?

With v0.8.17+, the entire secret is removed from Vault, irrespective of the number of versions. Previous behavior (for KVv2 secrets) was to mark the last version as deleted, so when attempting to access this secret, you would get an error stating that this secret does not exist. However, you could go to Vault and undelete this latest version, making it available again (no data loss here).
In some cases, the entire removal could be useful as well, such as a testing system with a lot of automation around it: not removing these secrets from Vault would leave a lot of useless mess behind, even potentially creating unexpected behaviour if reusing the same secrets across testing sessions.
So what I suggest is that the deletion logic should be made configurable to suit the use case at hand. Some users may decide that no secret should be deleted for production systems, because too sensitive if secret is lost. Some others could choose to delete the last version only, so that there is an error but without data loss (requiring manual undelete if this was a mistake). Some could decide that the full removal better suits their need because their deployment process is stable and robust enough to avoid the situation entirely...
My point is that there is no one-suits-all deletion logic and should be made configurable...

I agree it would be safer to keep it configurable. The conditional should probably apply to both KVv1 and KVv2 RandomSecret types - completely remove the secret in vault or not upon RandomSecret deletion.
I don't see the point in worrying about deleting specific versions in a KVv2 secret, are there use cases anyone cares to share? I think it would be good to keep the deletion logic consistent/simple... completely delete or do nothing to the secret in vault.

@trevorbox thanks for the feedback.

I agree it would be safer to keep it configurable. The conditional should probably apply to both KVv1 and KVv2
RandomSecret types - completely remove the secret in vault or not upon RandomSecret deletion.

Ok, that makes sense.
Before I get into the coding phase, let's agree on details.
The global environment variable to drive this behavior could be named KEEP_VAULT_KV_ON_REMOVAL (when true, then KV secrets are NOT removed from Vault when the resource is deleted from k8s).
Thoughts, any other proposal?
Second, is there a need to override this settings at RandomSecret resource level? If yes, what could be the (optional) field's name?

I don't see the point in worrying about deleting specific versions in a KVv2 secret, are these use cases anyone cares to share? I think it would be good to keep the deletion logic consistent/simple... completely delete or do nothing to the secret in vault.

I can imagine some as written above (an additional one was to maintain previous versions behavior), but I personally have no such need.

Probably it makes sense to reuse the k8s naming convention that they use for PersistentVolume that covers same issues.
https://kubernetes.io/docs/concepts/storage/persistent-volumes/#reclaiming

@smnbbrv you suggest to use the word "retain" instead of "keep" so that the environment variable is named something like RETAIN_VAULT_KV_ON_REMOVAL?
I like that idea. 👍

@davoustp might be better to add another option to the the RandomSecretSpec for more granular control of deletion behavior? The retainVaulltKVSecretOnRemoval option should default to false to match existing behavior. Perhaps a RETAIN_VAULT_KV_SECRET_ON_REMOVAL_OVERRIDE env variable could override any individual option to make things simpler.

@trevorbox agreed, more granular control is better.
Last one: what's the best precedence? Use the envvar settings (defaulting to current behaviour) that can be overriden at resource level?
Or the other way around?

@smnbbrv you suggest to use the word "retain" instead of "keep" so that the environment variable is named something like RETAIN_VAULT_KV_ON_REMOVAL?

Not really, I would just call it policy. KV_SECRET_RETAIN_POLICY or similar, that can further accept values like retain, delete or even deleteLastVersion in case it is implemented

@trevorbox agreed, more granular control is better. Last one: what's the best precedence? Use the envvar settings (defaulting to current behaviour) that can be overriden at resource level? Or the other way around?

We need to decide if it would be useful to override any and all settings, if not then I don't see the point in an env var. I could see a use case to be able to override for testing purposes. Thoughts @davoustp @smnbbrv ?

@smnbbrv you suggest to use the word "retain" instead of "keep" so that the environment variable is named something like RETAIN_VAULT_KV_ON_REMOVAL?

Not really, I would just call it policy. KV_SECRET_RETAIN_POLICY or similar, that can further accept values like retain, delete or even deleteLastVersion in case it is implemented

I think that's a good idea, the field in the spec could be kvSecretRetainPolicy to default to delete with support for retain for now.

We need to decide if it would be useful to override any and all settings, if not then I don't see the point in an env var. I could see a use case to be able to override for testing purposes. Thoughts @davoustp @smnbbrv ?

@trevorbox when running in production, I would shoot for the safest policy - that is, retaining the Vault KV secret on deletion. If there is no way to set this globally as the default value, then it would mean that we need to explicitly set the flag on each and every RandomSecret resource, which is a pain.
Hence the proposal to use of an env var that would allow setting this globally (but any other global mechanism to set this flag would fit as well).

I think that's a good idea, the field in the spec could be kvSecretRetainPolicy to default to delete with support for retain for now.

Agreed.

guys I have been following the discussion.
I have two questions:

if we control this with an environment variable the behavior will be applied to all the random secrets. Should we (also) control this behavior via a field in the CR?
if I chose to retain a secret and the RandomSecret CR is deleted and then recreated, what behavior do you expect?

@raffaelespazzoli thanks for the (very relevant) questions! :-)

if we control this with an environment variable the behavior will be applied to all the random secrets. Should we (also) control this behavior via a field in the CR?

I think so - the envvar would be here to control the default policy, and the field to override this policy at resource level.

if I chose to retain a secret and the RandomSecret CR is deleted and then recreated, what behavior do you expect?

Unless the RandomSecret have periodic rotation defined, if it exists, leave it alone, as the sole goal of the rotation-less RandomSecret is to generate a Vault secret, which already exists in this case.

To be totally honest, the downside that I see with the environment variable defining the default policy is that the behavior of the RandomSecret resources is not self-defined anymore: you need to know the value of the envvar (if set) in addition to the resource spec to understand how it behaves globally.
And I dislike this situation - a lot.
It could prove useful, but it will also create headaches: each time a new issue is created with RandomSecrets will raise the question about the envvar... or deploying the same resources in two similar environments may lead to different behavior just because this envvar is not set to the same value.
So now, I'm backtracking on this envvar proposal of mine - I think that we should drop it and keep only the resource field.
This is more consistent in the long run.
Suggestions?

To be totally honest, the downside that I see with the environment variable defining the default policy is that the behavior of the RandomSecret resources is not self-defined anymore: you need to know the value of the envvar (if set) in addition to the resource spec to understand how it behaves globally.
And I dislike this situation - a lot.
It could prove useful, but it will also create headaches: each time a new issue is created with RandomSecrets will raise the question about the envvar... or deploying the same resources in two similar environments may lead to different behavior just because this envvar is not set to the same value.
So now, I'm backtracking on this envvar proposal of mine - I think that we should drop it and keep only the resource field. This is more consistent in the long run.
Suggestions?

I think it most reasonable to simply add the retention policy to the CR and default to delete with an option to retain. The status section in the RandomSecret CR could explain an override env variable but its probably not intuitive so lets exclude that idea for now.

Ok. Let me summarize, this means:

adding of a new optional RandomSecret spec field named kvSecretRetainPolicy, accepting delete (default) and retain
updating the documentation
adding the related tests

@trevorbox @raffaelespazzoli @smnbbrv agreed?
If yes, I'll work along those lines and propose a PR.

@davoustp LGTM, thank you! The default delete would also be aligned with https://kubernetes.io/docs/concepts/storage/storage-classes/#reclaim-policy

@davoustp I like it.

Hi @trevorbox , @smnbbrv and @raffaelespazzoli
Sorry, been busy :-( - and now I finally got a few hours to get back to this.
I just implemented what was discussed above a while ago (you can can have a look at fork above, code changes are quite small):

added the additional optional retention policy attribute to RandomSecret
added existing Vault secret overwrite protection upon one-off RandomSecret creation (no refresh period defined)
add a documentation section for this, describing the behaviour and use case
manually tested all cases I could think of (incl. v1 and v2)

However, and before I create the PR, I failed to find any existing RandomSecret integration tests that I could enrich with these use cases.
Are there any?
If not, I'm not that confident to scaffold everything from scratch, to be honest... What would be the best course of actions?

Hi @raffaelespazzoli @trevorbox ,
Would it be possible to have a look at that PR and provide feedback, please?
We've been bitten once again by RandomSecrets removal leading to the deletion of the corresponding Vault secret...
Thanks a lot!