[Feature] Gym compatibility: Terminal and truncated

from torchrl.envs import check_env_specs
from torchrl.envs.libs.gym import set_gym_backend, GymEnv

with set_gym_backend("gym"):
    env = GymEnv("CartPole-v1")
    print(env.rollout(10))
    check_env_specs(env)

Prints
TensorDict(
    fields={
        action: Tensor(shape=torch.Size([10, 2]), device=cpu, dtype=torch.int64, is_shared=False),
        done: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.bool, is_shared=False),
        next: TensorDict(
            fields={
                done: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.bool, is_shared=False),
                observation: Tensor(shape=torch.Size([10, 4]), device=cpu, dtype=torch.float32, is_shared=False),
                reward: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.float32, is_shared=False)},
            batch_size=torch.Size([10]),
            device=cpu,
            is_shared=False),
        observation: Tensor(shape=torch.Size([10, 4]), device=cpu, dtype=torch.float32, is_shared=False)},
    batch_size=torch.Size([10]),
    device=cpu,
    is_shared=False)

with set_gym_backend("gymnasium"):
    env = GymEnv("CartPole-v1")
    print(env.rollout(10))
    check_env_specs(env)

Prints
TensorDict(
    fields={
        action: Tensor(shape=torch.Size([10, 2]), device=cpu, dtype=torch.int64, is_shared=False),
        done: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.bool, is_shared=False),
        next: TensorDict(
            fields={
                done: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.bool, is_shared=False),
                observation: Tensor(shape=torch.Size([10, 4]), device=cpu, dtype=torch.float32, is_shared=False),
                reward: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.float32, is_shared=False),
                truncated: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.bool, is_shared=False)},
            batch_size=torch.Size([10]),
            device=cpu,
            is_shared=False),
        observation: Tensor(shape=torch.Size([10, 4]), device=cpu, dtype=torch.float32, is_shared=False),
        truncated: Tensor(shape=torch.Size([10, 1]), device=cpu, dtype=torch.bool, is_shared=False)},
    batch_size=torch.Size([10]),
    device=cpu,
    is_shared=False)

Also fixes a bunch of bugs with set_gym_backend and implement_for
cc @skandermoalla

@skandermoalla The current project is to keep done as it was and add truncated.
Most of our code base interprets done as termination, ie: in gym, you have done = termination | truncation but in torchrl you'd have end_of_traj = done | truncation.
This naming is a bit different than gym but the alternative would be to rename done in termination everywhere in the code.
With @matteobettini we thought that proceeding as we do now is less disruptive than renaming every single occurence of done. Happy to read your thoughts about this!

Thanks a lot for starting this and for pinging me!
IMO naming differences are fine as long as done and truncation consistently mean the Gymnasium termination and truncation everywhere else in TorchRL and that they're correctly used.

Most of our code base interprets done as termination

So here "most" should be very carefully assessed. It would be terrible to have done mean terminated at some point and end_of_traj at some other point. I can see exceptions coming from the fact that TorchRL supports multiple environment libraries and some of them only return end_of_traj,  but I think there is no way to handle this underspecified signal and from the TorchRL perspective should be treated as terminated.
I think I sometimes confused the done in TorchRL for an end_of_traj signal. I don't recall exactly where, but that's to emphasize that it would be good to check this, especially in value estimators and transforms. (An end_of_traj signal would not allow to bootstrap correctly without further considering the truncated bit).
So maybe also making the docs more explicit about this definition would help a lot of people arriving from Gymnasium.
I'm also thinking it would be useful to allow the use StepCounter(max_steps=N) to mark a native time limit (with the done key) to adjust the finite horizon or an env, vs. to mark an external time limit (truncation). I think this is already possible by just changing the truncation key to done right? Will this be safe?
I don't know how Gymnasium differentiates between internal and external time limits to correctly set the truncation key.

@skandermoalla
To come back at your original point, I actually agree: naming done what is named terminated in gym isn't a great idea. It will only bring confusion, even for us.
I think the only way forward is this:
For now, any env that return termination, truncation gets a done spec made of done, terminated, truncated, ie, we add the done state. This has a low memory footprint and ensures that most things hold.
We adapt the collectors (as in this PR), the transforms, the value functions and the losses to read the proper end-of-traj signal: either truncated and terminated are provided, or only done. If only done, we assume that truncated = terminated = done.
@matteobettini wdyt? Feasible?

Good so replying to myself.
After a thorough discussion with @matteobettini we concluded that the best thing (least BC breaking) would be:

Keep done in the lib as the default (not to break everything)
If an env just returns a done, the most natural thing to us is to assume that this means terminated, ie we don't assume that this means truncated. This avoids ditching data in value functions for instance (a problem @BY571 just encoutered).
If the env returns termination, truncation we interpret that as done, truncated in tochrl nomenclature. This isn't great from a UX perspective as done means 2 different things in different libs (gym doc says that done = truncation | termination but we say done = termination). Unfortunately it's that or do hugely bc-breaking changes...
We add a "stop" entry along with truncated and done that means "end of trajectory (for whatever reason) => you should call reset"

We think that this is the least bc-breaking and the clearest we can do. Obviously this would be documented in bold characters in the doc.
A big thanks to @matteobettini for your patience during this discussion! I can be quite stubborn and opinionated :p
Do you have anything to add?
@btx0424 for context

I actually think that done should be named panic.
Joking, looks good to me!

Keep done in the lib as the default (not to break everything)

Okay. And this done has the exact use of terminated in Gymnasium? I.e. it is when the MDP reaches a terminal state and should bypass bootstrapping.

If an env just returns a done, the most natural thing to us is to assume that this means terminated, ie we don't assume that this means truncated.

Totally agree.

This avoids ditching data in value functions for instance (a problem @BY571 just encountered).

This isn't relevant anymore, but can you elaborate on how truncated would ditch data in the value functions? My understanding is the opposite, truncated keeps the bootstrap, and terminated bypasses it. Truncation could instead lead to ditching episode returns.

If the env returns termination, truncation we interpret that as done, truncated in tochrl nomenclature.

Good.

This isn't great from a UX perspective as done means 2 different things in different libs.

Yes, but it's functional and correct. As long as it's documented it's fine. Logic should be settled first. Some major refactoring could be done at some point if needed to change names but without changing any logic.

We add a "stop" entry along with truncated and done that means "end of trajectory (for whatever reason) => you should call reset"

This is a very good thing. I would allow components that only need that bit to free themselves from the logic of termination and truncation. Making development on them also easier.
Thanks a lot for this guys!

Okay. And this done has the exact use of terminated in Gymnasium? I.e. it is when the MDP reaches a terminal state and should bypass bootstrapping.

Not now but I started a branch implementing these changes!
Value estimators will accept all the done/truncated/stop triad and make the best educated guess they can in case only a partial info is provided. Wish me luck because it's gonna be a nightmare with the vectorised versions!

This isn't relevant anymore, but can you elaborate on how truncated would ditch data in the value functions? My understanding is the opposite, truncated keeps the bootstrap, and terminated bypasses it. Truncation could instead lead to ditching episode returns.

Yes you're right. I realise I missed a step when writing my thought process. Anyhow: the point is that assuming unspecified eot is truncated is the least reasable option, and for value estimation we need to decide if it's terminated U truncated U terminated and truncated.

Yes, but it's functional and correct. As long as it's documented it's fine.

Glad you agree

This is a very good thing. I would allow components that only need that bit to free themselves from the logic of termination and truncation. Making development on them also easier.

Good! Do you think we should always have a stop when we have a truncated and a done (eg, in StepCounter)? Right now I disabled it by default there but enabled it in gymnasium => the lack of consistency isn't great...

Anyhow: the point is that assuming unspecified eot is truncated is the least reasable option

I agree and I don't think we should do anything more than assuming that an underdetermined done means termination.

Value estimators will accept all the done/truncated/stop triad and make the best educated guess they can in case only a partial info is provided.

So here I'm not sure we want to do that educated guess.
I think the API unification should be solved at the environment level so that vectorized envs all carry the same semantics.
Then value estimators (as far as how we're currently doing RL) should only care about done. (termination in gymnasium).
They could optionally read the others if doing things that I don't see yet.

Do you think we should always have a stop when we have a truncated and a done (eg, in StepCounter)? Right now I disabled it by default there but enabled it in gymnasium => the lack of consistency isn't great...

When is the stop present at the moment?
I would say it's fine (maybe better) to always have it. It would be a redundant bit in some cases, but if it allows for more consistency and for some components to just read that signal (instead of reading both done and truncated) then it'd be good.

So here I'm not sure we want to do that educated guess.
I think the API unification should be solved at the environment level so that vectorized envs all carry the same semantics.
Then value estimators (as far as how we're currently doing RL) should only care about done. (termination in gymnasium).
They could optionally read the others if doing things that I don't see yet.

I think in estimators that use a trajectory like GAE and TDLambda, we do need to know also about truncations (if any) in order to avoid contamination when there are more consecutive not-done trajectories.
this is currently an issue with MultiSyncCollectors where the environment has no batch size. this class will cat the frames of all workers on the time dimension without any mark (done or truncated) and thus this data will be leading to wrong outputs from the trajectory based estimators

I would say it's fine (maybe better) to always have it. It would be a redundant bit in some cases, but if it allows for more consistency and for some components to just read that signal (instead of reading both done and truncated) then it'd be good.

I think this choice requires us to be careful. I would prefer  to avoid  writing an additional stop key at every step. Imagine i am a non-gym user and i only use only the torchrl done (like vmas, smac, ...) then i would copy paste the only done key i have all the time into the stop key.
Where is a stop key actually used? The only use i can think is for resetting, but that should only happen max once per step and it could be an or among all the done keys in the same td. Are there other usages for the stop key?

value functions should be happy with done and truncated
environments should be happy with just done or the done-truncated pair

Value estimators will accept all the done/truncated/stop triad and make the best educated guess they can in case only a partial info is provided. Wish me luck because it's gonna be a nightmare with the vectorised versions!

To expand a bit on this it seems to me that the value estimators need 2 inputs regarding these flags and not 3:

one input is to indicate that trajectory ends and no bootstrapping
one input is to indicate that trajectorty ends but yes bootstrapping

now let's see how we use them:

if i have only done this is input 1
if i have done and truncated they are input 1 and 2
if ihave done and truncated and stop then like above and stop is ignored
if i have only stop (should not happen but who knows) this is input 1 cause that is the best i can do
and so on..

so the user can just pass to input 1 the key that she prefers to indicate no bootstrapping and to input 2 the key she prefers to say traj is over but bootstrap.
and we do not have to juggle internally with these cases i enumerated

To expand a bit on this it seems to me that the value estimators need 2 inputs regarding these flags and not 3:
1.one input is to indicate that trajectory ends and no bootstrapping
2.one input is to indicate that trajectorty ends but yes bootstrapping

Yes, my bad. I forgot about consecutive trajectories from the same environment in the same sequence. The estimator would indeed need two bits (or to be exact, 1.5 bits). This is exactly the information you wrote above.
To me, the most intuitive way to encode this is with a stop key (indicating the end of a trajectory, to stop summing) and a done key (indicating to bypass or not bootstrapping).
This can always be present no matter whether the environment returns a truncation signal or not.
# only done
done, stop = done, done
# terminated, truncated
done, stop = terminated, terminated | truncated
# only truncated?
done, stop = False, truncated

However, this is not how things have historically been done, and as you said, the information can also be expressed with a done and a truncated (at the expense of the value estimator computing its own stop).
Since, done and truncated are likely to stay, if not writing a stop saves some significant space and compute, then yes, I agree that the trouble of adding a stop is not worth it.
Now for (non-consecutive) trajectories of different environments in the same sequence. We fall into the problem you mention here:

this is currently an issue with MultiSyncCollectors where the environment has no batch size. this class will cat the frames of all workers on the time dimension without any mark (done or truncated) and thus this data will be leading to wrong outputs from the trajectory based estimators

This is quite bad indeed (I haven't used this collector before). We don't want to write an artificial truncated to split the trajectories as this will overload its semantics and potentially cause unexpected issues. We can't write a stop either for the same reason.
I don't see an obvious solution tbh, other than splitting the trajectories in the collector or adding another key to be read by the value estimator to effectively act like truncated but not be the truncated key.

This is quite bad indeed (I haven't used this collector before). We don't want to write an artificial truncated to split the trajectories as this will overload its semantics and potentially cause unexpected issues. We can't write a stop either for the same reason.

I thought about this. What I would suggest as a refactoring of the multi-sync collector is to have it collect trajectories from the various sub-collectors and always stack along the time dimension. The reason is that we have the capability of imposing a preemptive_threshold that will stop some collectors from collecting once a certain amount of data is collected.
At this point: either we return only the valid data (which we can't stack because the output of the collectors will have different shapes) or we add a mask key and we can return padded data which, in this case can be stacked.
If we concatenate along the time dimension, we definitely need a truncated signal (maybe named otherwise) for the obvious reason we talked about previously.
I agree that we don't want for this truncated to interfere with the one coming from the env: overriding it would be loosing/corrupting info which is never appropriate.
We already have a "collector" entry in the data so we could add a ("collector", "truncated") entry there too.
After that, we can adapt loss modules / value modules to check for that key and if present do a truncated = data["next", "truncated"] | data["collector", "truncated"].

I like the idea of a ("collector", "truncated") key. It seems to be the least disruptive.
But I also think that the masking feature would be nice to have as it would support other cases, e.g. episodic losses like REINFORCE: One can use the
single SyncCollector with parallel environments to collect trajectories containing multiple episodes of different lengths, and stacked along the env batch dim, then the loss can get a mask to discard the trailing observations from unfinished episodes.
This is not a feature for long environments and modern estimators/losses, but for controlled experiments with old school REINFORCE loss.
I'm doing something like this myself for my current project.

@matteobettini @skandermoalla
This PR is almost ready.
One doubt I have is whether or not we should decorate the old gym envs with done, truncated and stop.
Pros: interchangeablility: we need to be able to write code that is somewhat oblivious to the gym version. For instance, if we do final_reward = episode["next", "reward"][episode["next", "done"]] this can result in an empty tensor after this PR for gymnasium ("done" must be swapped with "stop")
Cons: an env could be truncated, but not signaled as such, and our "truncated" won't track that. That could be misleading. We can always get out of this by saying it isn't our fault but it isn't idea.
An intermediate thing could be to always have "stop", but avoid "truncated" in order gym versions.

What's the current state of things regarding each key (done, truncated, stop)? Maybe a recap can be beneficial.

What's the current state of things regarding each key (done, truncated, stop)? Maybe a recap can be beneficial.

I was thinking that following this PR:

"done" would always mean completed
"truncated" would always mean early interruption
"stop" would be the intersection

If someone defines an env with a "done" key only, we automatically add a "stop" key in EnvBase step and reset methods to have this info accessible.
I'm just hesitant to add a "truncated" too

@skandermoalla
Sorry to bother you again with this
I'm struggling a bit with "stop"
In the code, we seem to be assuming that done is terminated | truncated more often than terminated (for instance to look at the final reward or obs of a trajectory etc).
So My current thinking is:

have "completed", "truncated" and "done".
Now "done" is the stop signal (yeah I know I change my mind often). The advantage is that it is minimally disruptive. Is is also aligned with gym nomenclature.
If you have termination (completed) and truncation (terminated) signal we also register them, along with done.
Otherwise, you just have done.
When completed and truncated are needed (eg in losses / value) but not available, we set completed = done; truncated=False which is the best assumption we can have.

If this is the road we are taking for me it should be "terminated", "truncated" and "done" to align with gym.
Introducing a new keyword seems a lot.
also will all the envs that just have a done in the codebase have that renamed to "completed" (or "terminated")?

If this is the road we are taking for me it should be "terminated", "truncated" and "done" to align with gym.

Gym uses "done", "termination" and "truncation"
But you can say "an env is done" but not "an env is termination" so I don't think we should use these terms.
Open to terminated but while we're at it, why not use "completed"? For instance: if my job is terminated, it could mean that I'm fired (but haven't completed my work). If my work is completed, I have finished the task.
But I will fold to whatever the people say obviously :)

also will all the envs that just have a done in the codebase have that renamed to "completed" (or "terminated")?

No by default it will be done because we can't tell what you mean.
In older versions of gym it could mean both. Even in MARL codebase we use done as the intersection of both. So this is the minimally disruptive decision.

if we do not do this and consider those dones as done then they will be modified by an external truncation (e.g., a transform)

Yes I added this to the doc.
(1) this is already the case so I would not worry too much about that.
(2) You can always rename your done via a RenameTransform if you want to keep the original value.
(3) I think it is what we want when we do this

Gym uses "done", "termination" and "truncation"
But you can say "an env is done" but not "an env is termination" so I don't think we should use these terms.
Open to terminated but while we're at it, why not use "completed"? For instance: if my job is terminated, it could mean that I'm fired (but haven't completed my work). If my work is completed, I have finished the task.

you can say an env is terminated and an env is truncated.
i can male the counter argument to yours that for example a video game can be game over when the task is not completed. to me when the env is terminated it just means that the mdp reached a terminal state and does not correlate with task completion. since the state is called "terminal", the think that makes most sense to me is terminated, which also aligns with gym

No by default it will be done because we can't tell what you mean.

mmh this is not a conservative choice to me.
if we cannot tell what you mean it is safer to not overwrite it with an external truncated (instead of overwriting it by default) and assume it as the terminal state of the mdp.

if we cannot tell what you mean it is safer to not overwrite it with an external truncated (instead of overwriting it by default) and assume it as the terminal state of the mdp.

I think the assumption here is that we cannot assume that we know so we should not override. But the library is very clear (and actually already behaves in this way) that we say that done is anything that ends a trajectory. To me it's even more surprising that a trajectory is truncated but not done. That means that the code snippet I linked above will fail whenever you truncate your trajectory.
This is (1) minimally disruptive (not bc-breaking), (2) coherent with the rest of the library and (3) reversible and documented so I don't see why not adopting that. What other option do we have?

It seems like the thread would benefit more from a live discussion at this moment. Would you guys want to meet @vmoens @matteobettini?
We can summarise the result in a comment here afterward.

Nomenclature

Gym uses "done", "termination" and "truncation"

terminated or termination, truncation or truncated, are not relevant. These are used interchangeably as adjectives or nouns. The variables in the code are usually the adjectives terminated and truncated (see code example on the gymnasium lading page https://gymnasium.farama.org/).
I think terminated would be better than completed. As @matteobettini argues, it refers to the MDP reaching a terminal state and is what people would probably appreciate more as it's the same as gymnasium.
In what follows I'll use the terminated, truncated, done := terminated | truncated nomenclature.
What's necessary to have
Let's make a recap of what components need what:

Losses and value estimators need a done (to split trajectories) and a terminated (to know if to bypass bootstrapping or not). In particular, they don't necessarily need a truncated. (Value estimators will have to be fixed btw as done is not enough).
Done can always be provided (as it's a superset). Terminated is subject to assumptions.
Collectors for resetting etc. only need done.
Computing episodic stats needs terminated (done is missing information). Computing trajectory stats can be settled with done. In particular, this should be using terminated instead, and is already making the assumption that done in those envs means terminated.
Maybe there are other components I'm missing.

What you can see from this is that  truncated doesn't seem to be necessary anywhere, as long as it's taken into account in done.
Assumptions
I think there are two conflicting facts that we can't rally with a unifying assumption. We need to treat different cases of single-done signals differently.

In old gym, this meant terminated | truncated.
In other envs this can mean terminated as @matteobettini is saying. I forgot about those in my previous arguments.

I think we can make:
# As soon as you get the output from the native env do:

# terminated, truncated in gymnasium
terminated, truncated, done = terminated, truncated, terminated | truncated

# done only, in gym
terminated, truncated, done = done and 'TimeLimit.truncated' not in info, info.get("TimeLimit.truncated", False), done # https://gymnasium.farama.org/tutorials/gymnasium_basics/handling_time_limits/#solution

# done only, in other envs if not specified
terminated, done = done, done

# Otherwise handle the thing differently per library.

The two first snippets don't need justification. The last one is where the assumption is made.

@skandermoalla
me and @vmoens were thinking about a thing.
If the episode reward statistic should use terminated, but an environment is reset with just a truncation (and not a termination) the statistics will be invalid as it would cross over 2 episodes.
I think as long as we reset our environments using done we have to use that to compute episode statistics.

If the episode reward statistic should use terminated, but an environment is reset with just a truncation (and not a termination) the statistics will be invalid as it would cross over 2 episodes.

Can you give more context on how those statistics are computed?
The way I think of it is that episode statistics are initialized whenever a done signal is received, keep being recorded, and are logged only when a terminated is received (i.e. at the end of the episode).
As opposed to trajectory statistics which are logged whenever a done is received (i.e. at the end of the trajectory).
In this sense, episode statistics are reset at every done (so they don't span 2 episodes) but only logged if a termination is reached.
trajectory statistics will always be logged.

@skandermoalla I added this to read the truncated state in older versions of gym (it's a mess, the flag is only there when truncated id True, sight!)
Do you think it's robust enough?

LGTM. I'm just not aware of what the _info_spec of a BaseDictInfoReader is, so skipped this line.
With this, will the truncated key always be written in the output tensordict of gym/gymnasium environments?

After reading this truncated key, it should be combined with the done key of the older gym version to get the terminated key, where terminated := done and not truncated.

Btw, I don't know where this fits in the conversation at the moment, but time limits made to model the environment as a finite-horizon MDP should be written to the terminated key and not the truncated key.
This is not necessary for this PR, I'm just mentioning it in case we make design choices that make it harder to include later, but it would be a nice feature of the StepCounter wrapper. One would then need a signal representing the remaining time in the observation to maintain the Markov property.

terminated := done and not truncated

Not sure about that one: what if the env is truly terminated but incidentally also truncated? Seems a bit dangerous to me

Not sure about that one: what if the env is truly terminated but incidentally also truncated? Seems a bit dangerous to me

Ah, that's a good point. Somehow that's what Gymnasium  says to do
terminated = done and 'TimeLimit.truncated' not in info

what if the env is truly terminated but incidentally also truncated?

With the TorchRL step counter this can happen indeed, but can this happen with the TimeLimitWrapper of the old gym?
This is the code is v24.1, the last version prior to the API change if I'm correct:
https://github.com/openai/gym/blob/66c431d4b3072a1db44d564dab812b9d23c06e14/gym/wrappers/time_limit.py#L13C4-L14C96

The done-signal originates from the time limit (i.e. it signifies a truncation) if and only if
the key "TimeLimit.truncated" exists in info and the corresponding value is True.

And in the code they do
https://github.com/openai/gym/blob/66c431d4b3072a1db44d564dab812b9d23c06e14/gym/wrappers/time_limit.py#L52
if self._elapsed_steps >= self._max_episode_steps:
            info["TimeLimit.truncated"] = not done
            done = True

So it seems like a correct implementation of a reader reading the done and info directly from the env would be:
terminated = done and not info.get('TimeLimit.truncated', False)

What do you think?

Now surprise,
info.get('TimeLimit.truncated', False)

can be a bool but also a list (with vec envs)
so it should be
        truncated = info.get('TimeLimit.truncated', False)
        if not isinstance(truncated, bool):
            truncated = np.array(truncated)
        terminated = done and not truncated

but it's a detail...
I think my latest commit should do the trick

Haha yes yes, this
terminated = done and not info.get('TimeLimit.truncated', False)

was only meant as pseudo-code. The details and getting around silent __bool__(self) will be harder.

I think we're reaching a good state for this PR.
I will do some cleanup tomorrow and maybe add a couple of tests. I will also test that on a separate branch with the SOTA implementations we have to check that everything's running smoothly!

Final list of changes part of this PR:

Add full_done_spec, full_observation_spec, full_action_spec, full_state_spec, full_reward_spec properties.
Make all envs have "done" and "terminated".
Add a _complete_done private method within envs to complete output data if done_keys are missing. Make this compatible with transforms to avoid duplicated calls to the method.
Full conversion to done_keys wherever done_key was used.
versioned reading helpers in GymWrapper to account for discrepancies between versions
terminated_or_truncated, a function that reads the data, writes the "_reset" keys and return a boolean indicating if anything is done.
Fix @implement_for, set_gym_backend and related + add tests
Fix envs tutorial and add info wrt new "done" attributes
Compatibility with batched environments
Fix transforms: VecGymEnv, Reward2GoTransform, RenameTransform, InitTracker, TimeMaxPool, SelectTransform, ExcludeTransform, StepCounter
Fix ModelBased envs, RoboHive, D4RL, RLHF
Adapt collectors to new logic
Better handling of workers refusing to close in collectors

I ll try to make a full review before tonight if thats ok

pettingzoo has truncated and terminated in each agent group so we can adapt that

I don't get this comment, what do you mean? Is there some extra work needed? I think we cover multiple truncated and terminated within the data structure

vmas, smac and similar simulators should output terminated instead of done so that we keep consistency

As discussed, they now output both without you (the user) needing to do anything:

As discussed, they now output both without you (the user) needing to do anything:

yep got that but if we keep writing done explicitly instead of terminated in the wrappers then the users will be deceived on the meaning of what comes from the simulator.
I think terminated should be the one provided and done created through a clone

I don't get this comment, what do you mean? Is there some extra work needed? I think we cover multiple truncated and terminated within the data structure

right now in pettingzoo there is an or between the 2 to create a done, we should just adapt that

As discussed, they now output both without you (the user) needing to do anything:

yep got that but if we keep writing done explicitly instead of terminated in the wrappers then the users will be deceived on the meaning of what comes from the simulator.

ah got it, so your point is that as part of this PR all simulators that were writing done should write terminated instead?

I think terminated should be the one provided and done created through a clone
Per se both are always provided
I think it's ok but it's not user-facing, mostly stuff that lives in private methods. We can check that the doc is accurate.

EDIT: if you look at the latest commit, what I did is just placing done in terminated and done in each env. I think it's clearer than placing just one and let the magic happen

so placing both directly in the env? we can do this but let s make sure to have it uniformly everywhere


Remove disable_compat_19 from consul configs

Description
Removing the disable_compat_19 field from consul config which has been deprecated.
Motivation and Context
Consul won't start up anymore with this field set.
https://www.consul.io/docs/upgrading/upgrade-specific#1-9-telemetry-compatibility
How Has This Been Tested?

Types of changes

[X] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Enhancement (improves on existing behavior)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[X] My code follows the code style of this project. (Did you install and run the pre-commit hooks?)
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.

:+1:


Dockerfile needs Java 11

using https://github.com/flutter/flutter/blob/stable/dev/ci/docker_linux/Dockerfile and running flutter build appbundle gives the following error
root@6f44ef8f29e6:~/app_2i2i# flutter build appbundle --flavor production -t lib/main.dart

💪 Building with sound null safety 💪

FAILURE: Build failed with an exception.

* Where:
Build file '/root/app_2i2i/android/app/build.gradle' line: 24

* What went wrong:
A problem occurred evaluating project ':app'.
> Failed to apply plugin 'com.android.internal.application'.
   > Android Gradle plugin requires Java 11 to run. You are currently using Java 1.8.
     Your current JDK is located in  /usr/lib/jvm/adoptopenjdk-8-hotspot-amd64/jre
     You can try some of the following options:
       - changing the IDE settings.
       - changing the JAVA_HOME environment variable.
       - changing `org.gradle.java.home` in `gradle.properties`.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 37s
Running Gradle task 'bundleProductionRelease'...                   38.4s

┌─ Flutter Fix ─────────────────────────────────────────────────────────────────┐
│ [!] You need Java 11 or higher to build your app with this version of Gradle. │
│                                                                               │
│ To get Java 11, update to the latest version of Android Studio on             │
│ https://developer.android.com/studio/install.                                 │
│                                                                               │
│ To check the Java version used by Flutter, run `flutter doctor -v`.           │
└───────────────────────────────────────────────────────────────────────────────┘
Gradle task bundleProductionRelease failed with exit code 1

also, why not add flutter installation to the Dockerfile?

Could you link to the PR that requires this change?

Which JDK version are you using?
@2i2i

Which JDK version are you using? @2i2i

the Dockerfile has
RUN apt-get update && apt-get install -y --no-install-recommends \
  adoptopenjdk-8-hotspot \
  default-jdk-headless

and the log has
Android Gradle plugin requires Java 11 to run. You are currently using Java 1.8.
     Your current JDK is located in  /usr/lib/jvm/adoptopenjdk-8-hotspot-amd64/jre

so, its java 8, i think.
I think there should be Dockerfiles that work with the newest android sdk and flutter to build bundles.
Same for iOS, web, desktop. Have a Dockerfile that is maintained which is able to build a flutter app for each platform.
It's a convenience for devs to use ci

It's a convenience for devs to use ci

This Dockerfile isn't intended as a general-purpose convenience service, it's for our own CI (as noted in the README). Anyone in the community is welcome to maintain a Dockerfile for general use if they are interested in doing so.
Closing as based on the above this doesn't appear to be driven by a need in Flutter CI.

ah...does that mean that you are using a lower gradle version?

I think you have to download jdk 11 from the package manager if it's available. @2i2i

Also, the Dockerfile itself needs to be functional, correct?
running docker build -t 2i2i_ci_flutter .
gives an error: #11 11.51 /usr/bin/gcloud: 181: exec: python: not found
should that be a separate issue?
1m1@1m1s-MacBook-Air flutter % docker build -t 2i2i_ci_flutter .
[+] Building 12.0s (11/32)
 => [internal] load build definition from Dockerfile                                                                                0.0s
 => => transferring dockerfile: 4.55kB                                                                                0.0s
 => [internal] load .dockerignore                                                                                0.0s
 => => transferring context: 2B                                                                                0.0s
 => [internal] load metadata for docker.io/library/debian@sha256:f6957458017ec31c4e325a76f39d6323c4c21b0e31572efa006baa927a160891                                        0.0s
 => [ 1/29] FROM docker.io/library/debian@sha256:f6957458017ec31c4e325a76f39d6323c4c21b0e31572efa006baa927a160891                                                        0.0s
 => CACHED [ 2/29] RUN apt-get update -y &&   apt-get upgrade -y                                                                                0.0s
 => CACHED [ 3/29] RUN apt-get install -y --no-install-recommends   git   wget   curl   zip   unzip   apt-transport-https   ca-certificates   gnupg                      0.0s
 => CACHED [ 4/29] RUN wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add -                                                                0.0s
 => CACHED [ 5/29] RUN echo 'deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main' |     tee /etc/apt/sources.list.d/google-chrome.list                   0.0s
 => CACHED [ 6/29] RUN echo "deb [signed-by=/usr/share/keyrings/cloud.google.gpg] https://packages.cloud.google.com/apt cloud-sdk main" |     tee -a /etc/apt/sources.l  0.0s
 => CACHED [ 7/29] RUN curl https://packages.cloud.google.com/apt/doc/apt-key.gpg |     apt-key --keyring /usr/share/keyrings/cloud.google.gpg add -                     0.0s
 => ERROR [ 8/29] RUN apt-get update && apt-get install -y google-cloud-sdk &&     gcloud config set core/disable_usage_reporting true &&     gcloud config set compon  11.9s
------
 > [ 8/29] RUN apt-get update && apt-get install -y google-cloud-sdk &&     gcloud config set core/disable_usage_reporting true &&     gcloud config set component_manager/disable_update_check true:
#11 0.288 Get:1 http://dl.google.com/linux/chrome/deb stable InRelease [1811 B]
#11 0.298 Hit:2 http://deb.debian.org/debian bullseye InRelease
#11 0.306 Hit:3 http://security.debian.org/debian-security bullseye-security InRelease
#11 0.335 Hit:4 http://deb.debian.org/debian bullseye-updates InRelease
#11 0.397 Get:5 http://dl.google.com/linux/chrome/deb stable/main amd64 Packages [1097 B]
#11 0.430 Get:6 https://packages.cloud.google.com/apt cloud-sdk InRelease [6751 B]
#11 0.652 Get:7 https://packages.cloud.google.com/apt cloud-sdk/main arm64 Packages [143 kB]
#11 0.757 Fetched 152 kB in 1s (265 kB/s)
#11 0.757 Reading package lists...
#11 1.031 Reading package lists...
#11 1.287 Building dependency tree...
#11 1.351 Reading state information...
#11 1.416 Suggested packages:
#11 1.416   google-cloud-sdk-app-engine-java google-cloud-sdk-app-engine-python
#11 1.416   google-cloud-sdk-pubsub-emulator google-cloud-sdk-bigtable-emulator
#11 1.416   google-cloud-sdk-datastore-emulator kubectl
#11 1.425 The following NEW packages will be installed:
#11 1.425   google-cloud-sdk
#11 1.622 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
#11 1.622 Need to get 128 MB of archives.
#11 1.622 After this operation, 705 MB of additional disk space will be used.
#11 1.622 Get:1 https://packages.cloud.google.com/apt cloud-sdk/main arm64 google-cloud-sdk all 401.0.0-0 [128 MB]
#11 3.444 debconf: delaying package configuration, since apt-utils is not installed
#11 3.461 Fetched 128 MB in 2s (67.0 MB/s)
#11 3.490 Selecting previously unselected package google-cloud-sdk.
(Reading database ... 10466 files and directories currently installed.)
#11 3.496 Preparing to unpack .../google-cloud-sdk_401.0.0-0_all.deb ...
#11 3.498 Unpacking google-cloud-sdk (401.0.0-0) ...
#11 11.50 Setting up google-cloud-sdk (401.0.0-0) ...
#11 11.51 /usr/bin/gcloud: 181: exec: python: not found
#11 11.51 dpkg: error processing package google-cloud-sdk (--configure):
#11 11.51  installed google-cloud-sdk package post-installation script subprocess returned error exit status 127
#11 11.51 Errors were encountered while processing:
#11 11.51  google-cloud-sdk
#11 11.54 E: Sub-process /usr/bin/dpkg returned an error code (1)
------
executor failed running [/bin/sh -c apt-get update && apt-get install -y google-cloud-sdk &&     gcloud config set core/disable_usage_reporting true &&     gcloud config set component_manager/disable_update_check true]: exit code: 100

Yeah it must be a separate issue.
@2i2i


feat: circulating supply helper function

I was accidentally working on my master branch in the other pr for #94. So I closed it and opened this one inside with a new branch. Linking the closed pr for reference:
https://github.com/interlay/interbtc-squid/pull/108

Assumptions I made:

The total issuance is changed I am directly adding the delta to the circulatingSupply.
Code here:
https://github.com/interlay/interbtc-squid/blob/561197505bcd7833ed9733eaa6291ead57595eee/src/mappings/event/circulating-supply.ts#L32
I am doing it based off this math but I am not sure if an increase in total supply will result in an injection into the circulatingSupply
Math: total_issuance - total_locked - total_reserved - system_account_supply
https://github.com/interlay/interbtc-squid/pull/108#issuecomment-1503638864

Hi @gregdhill and @nud3l,
I understand how we're using the variables total_issuance, total_locked, total_reserved, and system_account_supply to calculate the circulating supply. This makes sense given the logic we've discussed.
However, I'm not clear on how we should calculate totalVestingTokens and totalEscrowTokens fields that @nud3l would  like to see in the cumulativeSupply object. Could you clarify how we should derive these variables? Can the total_locked, total_reserved, and system_account_supply be used directly here?

However, I'm not clear on how we should calculate totalVestingTokens and totalEscrowTokens fields that @nud3l would like to see in the cumulativeSupply object. Could you clarify how we should derive these variables? Can the total_locked, total_reserved, and system_account_supply be used directly here?

Do I need to track all four of these for the total_locked, The events seems super redundant. Could I get away with only tracking 2 of them?

I think you just need to use Locked and Unlocked to calculate total_locked.

I have been having trouble trying to listen to this event. Is it possible that we didn't use this event to set the initial supply?

It is possible, for the total supply we can just initialize using some constant value.

However, I'm not clear on how we should calculate totalVestingTokens and totalEscrowTokens fields that @nud3l would like to see in the cumulativeSupply object. Could you clarify how we should derive these variables?

Take a look at the orml-vesting events to update totalVestingTokens and the escrow pallet events to update totalEscrowTokens - note that would only give the KINT/INTR amount not in vKINT/vINTR.

Do I need to track all four of these for the total_locked, The events seems super redundant. Could I get away with only tracking 2 of them?

I think you just need to use Locked and Unlocked to calculate total_locked.

I have been having trouble trying to listen to this event. Is it possible that we didn't use this event to set the initial supply?

It is possible, for the total supply we can just initialize using some constant value.

I think you just need to use Locked and Unlocked to calculate total_locked.

@gregdhill When I try to generate types for these events. I get an error that these events are not included in the chain metadata. So I decided to tracked the lockSet and lockRemoved instead, but the lockRemoved event does not include the amount removed.
I am currently blocked here.

Closing PR as it was replaced with #124


18.2.10 - How to Compile

Question
I need to use DevEx 18.2.10, but the NuGet for eXpand only goes through 18.2.7. I used to build the source locally, but it appears that process has changed. What is the quickest way to get eXpand built against 18.2.10? Thanks!!

downlload the source
Run 18.2.10 project converter
Modify the Build.ps1 both version and sources

I pulled master and ran the project converter. In Xpand\Xpand.Xpo\DB\SchemaColumnSizeUpdater.cs line 25 AsyncLockHelper does not exist. That must be new after 18.2.
When I switch to the 18.2 branch and run the project converter, the build.ps1 gives two errors.

Get-Feed : The term 'Get-Feed' is not recognized as the name of a cmdlet, function, script file, or operable program.
The term 'D:\NewGithub\eXpand\support\build\go.ps1' is not recognized as the name of a cmdlet, function, script..... that file is not in the source.

I pulled master

checkout the 18.2.7 tag and run the 18.2.10 projectconverter there

replace the Get-Feed -DX call with
C:\Program Files (x86)\DevExpress 18.2\Components\System\Components\packages\

The support folder is empty so the new error is The term 'D:\eXpand-18.2.704.0\support\build\go.ps1' is not recognized as the name of a cmdlet

staring from 19.1 Support is not a git SubModule anymore and will be download with the Sources as a normal folder. For past versions the submodule is loacated at https://github.com/eXpandFramework/Support download and put it in the Support folder in the root

u should run ProjectConverter there as well

I'm just giving up for now. Sorry to waste your time. Here is the latest error.
Errors: Package 'devexpress.document.processor 18.2.7' is not found in the following primary
source(s): 'C:\Users\rober.nuget\packages,https://api.nuget.org/v3/index.json,https://xpandnugetserv
er.azurewebsites.net/nuget,C:\Program Files (x86)\DevExpress
18.2\Components\System\Components\Packages'. Please verify all your online package sources are
available (OR) package id, version are specified correctly.

havinf 18.2.7 in the GAC i remove all DX nuget from the build.ps1 located in Support/Build folder and was able to build. If 18.2.10 has no XAF breaking changes u should be able to build elese u have to open the breaking project and fix it manually
Build.zip

unfortunately we have no instrastructure to support builds like 18.2.10 which is released outdated after a newer XAF version is out.

if there are breaking changes u can look how it was fixed in a newer version of the file using git File history


Document decision on testing strategy

@jgaehring as discussed in our meeting today, I will leave it to you to update here with a couple of sentences what was decided for the testing strategy of the Type script connector and close the issue. Many thanks!


Schema API

Adding public schema API.
interface BaseRealm : Versioned {
    fun schema(): RealmSchema
}

interface RealmSchema {
    val classes: Collection<RealmClass>
    operator fun get(key: String): RealmClass?
}

interface RealmClass { // Matches realm_class_info_t except that attributes are also attached here
    val name: String
    val properties: Collection<RealmProperty>
    operator fun get(key: String): RealmProperty?
    // If this is never to change we could make it a val, but if using same API for mutable schema
    // it could actually change
    fun primaryKey(): RealmProperty? // equivalent to:  properties.any { it.primaryKey }
}

interface RealmProperty { // Matches realm_property_info_t
    val name: String
    val type: RealmPropertyType
    // Referring to actual Kotlin KType property nullability and not Core's nullability flag
    // (though being the same for singular types and always false for Non-singular types)
    val isNullable: Boolean
}

sealed interface RealmPropertyType {
    val storageType: RealmStorageType
    val isNullable: Boolean
}

class SingularPropertyType(
    override val storageType: RealmStorageType,
    override val isNullable: Boolean,
    val isPrimaryKey: Boolean,
    val isIndexed: Boolean
) : RealmPropertyType

class ListPropertyType(
    override val storageType: RealmStorageType,
    override val isNullable: Boolean = false
) : RealmPropertyType

Closes #85
Closes #238
Closes #201 (Added final test of embedded indicator in schema retrieved from Core)

@nhachicha @cmelchior I have tried to update according to discussion, but there are some annoying things around initializing the schema details from the compiler plugin.
I wrapped it as an implementation detail of RealmClass in https://github.com/realm/realm-kotlin/pull/539/files#diff-af15d86f1d0cfbfd742b652cd227e5a386558f19cf2b968714d4915812e9dffdR27. But it still feels annoying to initialize the details according to the C-interops instead of using the abstraction of flags, etc. as in https://github.com/realm/realm-kotlin/pull/539/files#diff-66ad0c5e3b9031ba0b834a0d7e8c081238e8ce69ff0ccc5a2dd0a1d9a14a4ed0R26
Besides of the current code not working on Darwin (🤦 ... I am looking into that...),  my gut feeling would be that we would be better off using the clean abstraction of RealmClassImpl with the penalty of having to initialize the Table/Property on open. At least until we know that it is an issue or we get another enlightenment from caching the keys/updating schema on realm_schema_changed-callbacks.
It would maybe even be safer if this was going through a factory method in the SDK, so that it would be more flexible to update if we were changing conventions. Has there been any considerations/issues with that in realm-java?

I tried to rework the property type information after our discussions. Please have another look.

@cmelchior Updated naming, added Dokka (though I am not sure I got the vocabulary right on all the meta description, schema, realm object, realm object model, etc. 🤯) and adjusted and commented the test a bit.
Regarding the exhaustiveness of tests: I think it makes sense to just focus on the codepaths of the schema API and leave the exhaustiveness tests to the various modeling feature tests. Besides that, I think it will require too much restructure of migrating the various TypeDescriptor members into the various schema API classes and would probably fit better in a separate PR. I get that this exercise might uncover some details/feedback to the API, but the API seems relatively simple so I envision that it is more additive than breaking.


[nx-cloudflare] Getting multiple warnings when I run my project with serve command

Current Behavior
When I run my project through the serve command I'm getting this warning in my terminal:
▲ [WARNING] kj/filesystem-disk-unix.c++:1709: warning: PWD environment variable doesn't match current directory; pwd = /mnlbox

Expected Behavior
It's a bit annoying as it prints two warning each time the server reload so it's a bit noisy in the terminal.
Should I ignore it or it's a bug on the serve command? Or maybe my project structure is not correct? 🤔
GitHub Repo
No response
Steps to Reproduce

Create my project with this command:

pnpm nx generate @naxodev/nx-cloudflare:application --name api-test--template hono --directory apis/test --port 4001 --projectNameAndRootFormat=as-provided

Then run my project with this command:

pnpm nx serve api-test

My project is run successfully and I can see the Hello Hono! message in the browser when I visit: http://localhost:4001
But I'm getting two warning like this:
 ⛅ wrangler 3.48.0
-------------------
▲ [WARNING] kj/filesystem-disk-unix.c++:1709: warning: PWD environment variable doesn't match current directory; pwd = /mnlbox

[wrangler:inf] Ready on http://localhost:4001
⎔ Starting local server...
▲ [WARNING] kj/filesystem-disk-unix.c++:1709: warning: PWD environment variable doesn't match current directory; pwd = /mnlbox

Nx Report
NX   Report complete - copy this into the issue template

Node   : 20.11.0
OS     : linux-arm64
pnpm   : 8.15.5

nx                 : 18.2.4
@nx/js             : 18.2.4
@nx/jest           : 18.2.4
@nx/linter         : 18.2.4
@nx/eslint         : 18.2.4
@nx/workspace      : 18.2.4
@nx/devkit         : 18.2.4
@nx/esbuild        : 18.0.6
@nx/eslint-plugin  : 18.2.4
@nx/nest           : 18.2.4
@nx/node           : 18.2.4
@nx/playwright     : 18.2.4
@nx/react          : 18.2.4
@nrwl/tao          : 18.2.4
@nx/vite           : 18.2.4
@nx/web            : 18.2.4
@nx/webpack        : 18.2.4
typescript         : 5.4.4
---------------------------------------
Registered Plugins:
@nx/webpack/plugin
@nx/eslint/plugin
@nx/vite/plugin
@nx/playwright/plugin
---------------------------------------
Community plugins:
@naxodev/nx-cloudflare : 2.0.1
---------------------------------------
The following packages should match the installed version of nx
  -<EMAIL_ADDRESS>  -<EMAIL_ADDRESS>
To fix this, run `nx migrate<EMAIL_ADDRESS>
Failure Logs
No response
Operating System

[x] macOS
[X] Linux
[ ] Windows
[ ] Other (Please specify)

Additional Information
I'm using vscode devContainer with a Debian based linux in a MacOs host machine.

And BTW as I checked the NX report now I think the @nx/esbuild package is installed by @naxodev/nx-cloudflare package to build the cloudflare function but it seems the version is not matched based on this section of NX report:
The following packages should match the installed version of nx
  -<EMAIL_ADDRESS>  -<EMAIL_ADDRESS>
Maybe it's better to have these packages as as peerDependencies or maybe just update them in the generator.

Hey @mnlbox, thanks for the thorough issue!
Regarding the warning, this is very likely coming from Wrangler. You can verify that by running wrangler dev directly on the application folder. This serve executor wraps over it.
Regarding the version mismatch, that's a good catch, so I'll have it fixed.

And BTW, one more small suggestion: I guess it's useful to add the project website url to the NPM page here as well:
https://www.npmjs.com/package/@naxodev/nx-cloudflare
I had to search and find this repo from your GitHub profile to report this issue as there was no website available on the NPM page of the nx-cloudflare package.
And thanks for this awesome and useful project. I like it ❤
It's a good idea to organize all of the tiny Cloudflare Worker projects in an NX monorepo. 😉

I was able to reproduce your issue; it should be easier to address now.

I know what the problem is. We need to set the cwd on the forked thread to aim for the project folder, but that's not changing the matching of the pwd, which is the project's root.

@NachoVazquez Thanks for your support. I'll fix it whenever you release it and update you here 😉

I'll release this changes with the fixes to the deploy/publish command


End of line cursor position differences when app is run in Windows Terminal

This isn't so much a bug report as I'm hoping for clarification on some behavior we're seeing in Windows Terminal related to a bug over at HttpRepl. Rich Turner suggested I file here to get more details.
I'm trying to understand the different behavior I'm seeing around the cursor position when our app (a dotnet global tool, though it doesn't appear specific to that type of app) is run from a normal cmd.exe prompt vs when it is run from within Windows Terminal, in a profile that launches cmd.exe. Specifically, this is about how the cursor position is updated when it is at the end of a line.
When our app does a Console.Write(char), in most cases we see the Console.CursorLeft (checked before and after the Write call) increase to the next slot after the character that was just written. Where we see the difference is when we Console.WriteLine(char) to the last slot on a line.
Running our app in vanilla cmd.exe, the character is written and the cursor visibly moves down to the beginning of the next line. Checking Console.CursorLeft and Console.CursorTop, we can see that the former reset to 0 and the latter increased by 1.
Running our app in cmd.exe launched in a Windows Terminal profile, the character is written, but the cursor disappears. Checking Console.CursorLeft and Console.CursorTop, we can see that neither of them changed - they now refer to the slot to which the character was just written.
Steps to reproduce
I set up a fairly minimal repro that demonstrates the difference. Run the app, start typing characters, and watch the upper-left of the console where we are printing the current Console.CursorLeft/Console.CursorTop position. Try it in both vanilla cmd.exe and cmd.exe from within Windows Terminal. Note the different behavior when you get to the last character on the line (and subsequent characters, actually, but I assume that's a derivative problem).
Note that the behavior is also different (in the same way) between a vanilla powershell prompt and a powershell prompt from within Windows Terminal.
Environment
It doesn't seem tied to a particular version of Windows or Terminal, but I've confirmed it on:
Windows build number: 10.0.20257.0
Windows Terminal version (if applicable): 1.5.3142.0

Expected behavior
I'm not asserting that this is unexpected behavior, necessarily. Instead I'm looking for clarity - maybe a little bit of "why" but also if it is possible for us to detect the different behavior so we can account for it.
Actual behavior
The behavior of the cursor position within a console app differs when it is run in Windows Terminal vs not.

So, this is likely an effect of the console output modes ENABLE_WRAP_AT_EOL_OUTPUT and DISABLE_NEWLINE_AUTO_RETURN. They dictate whether the cursor wraps to the next line or "hangs" off the end of the line (technically) and report coordinates that look like they're at the end of the line.
Generally, we recommend that applications enforce the modes that work best for them :smile: since there's no guarantee that the shell that spawns them is going to set the same modes forever.

If you interrogate the mode with GetConsoleMode on the output handle, are you getting different values when run from CMD in conhost (the traditional console window) versus Terminal?

@DHowett Thanks for the quick reply!
It does look like there is a difference. Specifically, the DISABLE_NEWLINE_AUTO_RETURN was set for Windows Terminal, but not on conhost. Also ENABLE_LVB_GRID_WORLDWIDE was set for conhost but not for Windows Terminal, though that seems less relevant here.
However, it doesn't appear that changing that makes any difference in the behavior. I can modify it with SetConsoleMode, confirm it is set, and the subsequent behavior is still the same, both in HttpRepl and in the repro app I posted.
I'm not positive I did it correctly - used GetStdHandle(STD_INPUT_HANDLE) for the console window handle for the GetConsoleMode/SetConsoleMode calls and I'm not sure that's right. I thought this seemed like the exact thing I was looking for, so I was surprised it didn't make a difference.
Any thoughts?

Hmmmm.. If you try it against the STD_OUTPUT_HANDLE instead, do you get a different result? The console mode is unfortunately split across those two handles (with the acceptable enum values coming from those two tables on the docs page.)

Ah, darnit. I even looked at the two different lists and realized I needed to be using OUTPUT but didn't notice I had only tried STD_INPUT_HANDLE.
OK, so using STD_OUTPUT_HANDLE, I find that the results are all the same between the two ways of running it (conhost/terminal). ENABLE_WRAP_AT_EOL_OUTPUT is set on both and DISABLE_NEWLINE_AUTO_RETURN is not set on either.
But the one thing I noticed is that the difference appears to be the ENABLE_VIRTUAL_TERMINAL_PROCESSING  setting. That's different between the working and non-working scenarios. And it turns out if I remove that flag, then things work as expected in Windows Terminal.
The descriptions for those two original flags describe what is happening very clearly. In HttpRepl, the combination of both being set would be exactly what we see - because it does move to the next line eventually on the subsequent character.
So its weird that the ENABLE_VIRTUAL_TERMINAL_PROCESSING flag is what makes a difference. Any idea why that would be? I recognize this might be straying from Windows Terminal-specific info, so feel free to redirect me elsewhere if needed.

Oh, oh, okay. This makes sense.
First, though:

straying from Windows Terminal-specific info

I've got really good news: this repo is also home to conhost (the window CMD lives in!) (which also hosts all other Win32 console applications)
So, ENABLE_VIRTUAL_TERMINAL_PROCESSING is a fun one. There's a few key differences between traditional terminal emulators and the Windows Console. One of them is support for control sequences (cursor movement, color) in-band with text. Another one is cursor position handling. VT processing enables, implicitly, the aforementioned carriage handling behavior because that's what terminal emulators typically do.
We made a change back in https://github.com/microsoft/terminal/pull/2824 to fix https://github.com/microsoft/terminal/issues/1965. The salient comments, if you want to follow along, are https://github.com/microsoft/terminal/issues/1965#issuecomment-533290250 and https://github.com/microsoft/terminal/issues/1965#issuecomment-533590497. In short, we made Terminal (and any application like Terminal (VSCode, ConEmu in PTY mode, WSL in interop mode ...)) automatically enable ENABLE_VIRTUAL_TERMINAL_PROCESSING because they'll want their applications to act more like they would under terminals. There was a huge downside to us not enabling VT processing in those applications: sometimes they would emit text that tricked connected terminals into thinking that they were speaking in control codes when they actually weren't. We were aghast when we figured that one out.
Anyway, this is roughly somewhere between "by design" and "something the application can explicitly turn off", but I do accept that a small set of applications will find the observable oddity now and again. :grin:
I'm going to mark this one as question/answered/closed, but don't let that deter you from asking further questions. I love talking about this stuff!

Yes, this is all starting to make sense now. The timing of it all too - when this chunk of our code was written, that issue you linked to, and the PR didn't even exist yet. So... assumptions were made :) Thanks for all the detail!

but don't let that deter you from asking further questions. I love talking about this stuff!

Well, since you asked!
The question I posed in the original post was for one of two issues we ran into that impacts both conhost and wt (in slightly different ways). But since you offered, I could probably use a bit more clarification to make sure I understand it so we can properly work around it.
Running our app in WT, we see that the user is typing on the last line of the window (say, line 30), and the cursor reaches the end (say, position 120), the cursor position wraps back around to slot 0 (not on the next character, but on the following one, because of the above-discussed issues). But the line number doesn't change - its still on line 30. Checking Console.BufferHeight while running in WT shows the buffer height is the same as the number of lines on the screen. So that sort of makes sense - everything gets shifted up one since you're at the end.
We initially thought this problem didn't exist in plain cmd.exe. But it turned out that the difference was that the Console.BufferHeight was returning ~9000. If we filled that whole buffer, then ran our app, we still hit the same problem. So its not WT-specific.
It's clearly a bug on our side in how we handle that (the assumption that the row number would keep increasing in perpetuity) and I'm working on changing that code to handle it more appropriately. But I figured I could clarify if my understanding was right to make sure our fix is right.
I'm assuming that this difference in behavior is related to the "virtual" terminal idea. The "buffer" is only as big as it needs to be to render to the screen, even though the actually history is longer than that. So plain cmd.exe/conhost without VT processing enabled has a large buffer tied to the actual history, and then when it is in WT, with VT processing enabled, it is virtualized. Is that all there is to it? Or is there something more going on?

Ah yeah, so that one's gonna be a big gotcha no matter where you're running.
The Terminal and the Console are actually the same beast, through-and-through[1]. The interface presented to the application--the console API--is actually serviced by an embedded instance of conhost (which when compiled out of this repository is called OpenConsole). So, behind every pane in Terminal there's a traditional console host. It's really cool!
That console host is run in a specific mode that (sometimes) catches applications out. I'll get to that in a sec. That mode is called "ConPTY".
That console host also handles translation from Win32 Console APIs to the standard control sequences that normal terminals everywhere except Windows understand. Terminal's built on top of that support: it doesn't need to know anything about the Win32 Console APIs, it just needs to know about VT.
That mode, though. This is where it gets "fun" for applications.
We made a critical mistake approximately forty years ago (:grin:) and made the entire buffer (120x9001 or whatever the user requests) available to a console application. There's the viewport (120x30) and the buffer (120x9001), both of which the application can control. It can draw outside of its bounds, and it can know exactly where in the buffer it's printing. Pretty cool!
Applications that have some amount of buffer awareness will eventually reach a point where they're on line 9001 and oops the line under 9001 is also line 9001 (because the old line 0 got destroyed and shifted off the top of the buffer.) This bit is the crux of your issue :smile:. We call it "circling."
Terminals have never supported that viewport/buffer split. They've only ever offered viewport control, and the buffer was an artifact owned by the terminal itself (because, like, it might have been a line printer or a serial device or a modem . . . instead of a terminal emulator application). That puts the terminal in charge of scrollback, and scrollback is stored in a place where the client application can't see (or change) it.
To account for this difference, ConPTY mode restricts the buffer size to match the viewport size. This restriction applies regardless of ENABLE_VIRTUAL_TERMINAL_PROCESSING (which is a feature of conhost, even without Terminal involved.)
This means that that circling behavior, where the line numbers get recycled, happens pretty much instantly. Like, after the first 25 lines are printed, the bottom line stops incrementing. It'll always be 25, or 30, or whatever the viewport height is.

1. footnote about terminal/console being the same
It's kinda funny actually, they also share code. It's kinda like this:

Terminal shares modules with conhost, and also runs a separate copy of conhost. The shared modules deal with text buffers, parsing VT, etc. because both of them have got to do it.

If you really want to go crazy with doc reading, @miniksa wrote up our ecosystem roadmap which covers a good bit of our history and where we're taking this starship. :)


feat: download clients integration and widget

(Screenshot only include test files and linux ISO, therefore safe to use)
Usenet and Torrent downloads in 1 widget.
sabNZBd, NzbGet, Deluge, qBitTorrent, and transmission support.
Columns can be reordered in Edit mode.
Sorting enabled.
Time uses Dayjs with auto translation.
Can pause/resume single items, clients, or all.
Can delete items (With option to delete assossiated files).
Clients list and details.
Include all filtering and processing for ratio from oldmarr torrent widget.
Invalidation of old data (older than 30 seconds) to show an integration is not responding anymore.
Misc:
Fixed validation error with multiText.
Fixed translation application for multiSelect to behave the same as select.
Added background to gitignore (I needed to add a background to visually test opacity, probably will in the future too)
Added setOptions to frontend components so potential updates made from the Dashboard can be saved.
Extracted background and border color to use in widgets.
humanFileSize function based on the si format (powers of 1024, not 1000).
Improved integrationCreatorByKind by @Meierschlumpf.
Added getIntegrationKindsByCategory to get a list of integrations sharing categories.
Added IntegrationKindByCategory type to get the types possible for a category (Great to cast on integration.kind that isn't already properly limited/typed but for which we know the limitation)
Added a common AtLeastOneOf type. Applied to TKind and IntegrationSecretKind[] where it was already being used and Added to the getIntegrationKindsByCategory's output to be more freely used.
ToDo: (Might actually need help on these)

Disable controls if user doesn't have permissions (Router already prevents it, but no point showing them then), and adapt columns visibility for it.

Forgot to put it as draft.
There is still a todo list.

Very nicely documented & made PR. @Meierschlumpf is there anything missing or can you re-review?

There's still a little ToDo that I would like to clear quick first. It's a really small list though.

If you want me to help you with the todo, just let me know 👍🏼

I would suggest to merge this pull request (not squash) what do you think?

I converted it back to a draft as there are pending todos, just that we don't merge it accidentally

Would be beneficial for the extras that would be independent components. But keep in mind there's at least 30 commits and a lot of them are just corrections or merging things.


Add a wiki: Find Everything Wiki

Link to origin wiki: https://find-everything-rbx.fandom.com/
Link to destination (independent) wiki: https://findeverything.miraheze.org/

Added in commit https://github.com/KevinPayravi/indie-wiki-buddy/commit/a6c9e8071556d5089cc0daf791aa0dbbe45501f8

Now available in version 2.4.4 (may take a few days to appear in the Chrome store)


[boost-iostreams] Should not depend on copy-left licensed code by default

Is your feature request related to a problem? Please describe.
Boost has a very specific license - The Boost License. The "boost-iostreams" package, however, consumes ports with very different licenses by default. "boost-iostreams" (as per the CONTROL file) depends on non-Boost ports that have other licenses. Specifically, liblzma and zstd have a mix of licenses that includes copy-left licenses. I don't think that consumers of the Boost library would expect copy-left dependencies included by default.

liblzma - The COPYING file shows the multiple licenses that are applied. ClearlyDefined.io declares the copyleft licenses.
zstd - This project contains two licenses, for BSD-3 and GPL-2. ClearlyDefined.io shows both.

Proposed solution
boost-iostreams should not depend on copy-left licensed code by default. Consumers should be able to opt-in.
Describe alternatives you've considered
Forking the ports is an option.

@mschofie, it seems the license https://github.com/boostorg/iostreams/blob/master/LICENSE is same with https://www.boost.org/LICENSE_1_0.txt, and it says 'boostorg/iostreams is licensed under the Boost Software License 1.0'.
@BillyONeal, could you help take a look?

You would need to take this up with the boost-iostreams developers; boost-iostreams requires those external libraries. Boost makes claims about their own code, but clearly document that they need those external dependencies here: https://www.boost.org/doc/libs/1_74_0/libs/iostreams/doc/index.html
They do have some options to disable these compression features, however, they appear to default to enabling them, given the name of the supplied option being NO_Xxx rather than USE_Xxx or similar.
As a result I believe we should respect the upstream project's decision, and they turn those on by default.
We would certainly accept a patch to the port enabling those features to be disabled so that you could disable the problematic parts by saying something like boost-iostreams[core].

clearly document that they need those external dependencies here

Well that link didn't work because they use frames :/
https://www.boost.org/doc/libs/1_74_0/libs/iostreams/doc/installation.html#ide

Thanks, @BillyONeal, I appreciate the perspective. I agree, making a change to support opting out seems to be the right thing to do given the boost-iostream decisions.


lint: import eslint from v2.x

god damnit
repeating:
@stevepeak hey how do i switch this over to codecov? i can't figure out why coveralls is still a status check and not codecov

I cannot answer for Coveralls, but Codecov did post a status for the commit prior and this one :+1:

oh now it's there!!!

It will set the status upon the first upload. In the near future it will post immediately upon syncing with GH. Thanks!


Update Onboarding

Unregistered Professional Address Warning (feat): A new feature alerts users if the professional address they enter is not registered, enhancing the UX by providing immediate feedback without blocking form submission.

Onboarding and User Selection Removal (chore): Removed outdated onboarding and user selection code to declutter the codebase.

Generic Onboarding Component (feat): Implemented a new generic onboarding component that efficiently captures and stores basic user data within the contracts, simplifying the user's initial setup experience.

These updates collectively refine the user journey, reduce unnecessary complexity.
Closes #189
Closes #182

what is this huge checkbox???

about the storage of tax document, we should not store the plain data, but a hashed version of it (with proper salta)

there is a bug with all the checkbox styles in the application, I have already reported it to Mika and he will apply the default style, more info on #194


Property with link to value of enumerated dataType

In v1.1, a property with link to value of enumerated dataType looks like:
{
  "class": {"id":"PC-Priority"},
  "values": ["V-Prio-1"]
}

where "V-Prio-1"is the id of an enumerated value of the referenced dataType.
In order to be consistent with other internal links, it is proposed:
{
  "class": {"id":"PC-Priority"},
  "values": [{"id":"V-Prio-1"}]
}

Towards this end, the schema element SpecifValue needs an additional option.

Implemented with schema v1.2.


AFJSONObjectByRemovingKeysWithNullValues use less memory

There's no need to create a mutable dictionary if it is already.
This saves a lot of memory especially with big responses.

Thanks for the PR! However, due to its age it will not be merged. If the issue is still occurring feel free to open a new PR.


0.1.1 is completely broken

lzip-rs isn't finished yet, only lzip-sys is.
Unfortunately, I don't have the time to work on lzip-rs.

@firasuke It's a little bit confusing, since this comes up first when searching for a rust lzip library. Would it be possible to either remove the crate or somehow mark it as broken?

Well lzip-sys is finished, and lzip-rs is currently broken. IMO removing it will reduce the attention it's receiving, given that lzip is more of a niche algorithm, thus reducing the hopes of having someone try to actually fix it.
If that is considered bad practice, then I'll remove lzip-rs but there are no guarantees a working crate will ever show up.

I am not quite sure what the best practices are for publishing crates.
I think a bold warning at the top of the readme would help those who are looking for a working library. You could also link to lzip-sys as an alternative. :)

I appreciate your concern, lzip-sys is the low level bindings crate, and it is complete; while lzip-rs is the high level wrapper, and it's broken currently.


Docs update - JWKS example

JWKS example updated to use key as a function

Sure. I will take care of that too


aspy.refactor_imports.classify - _get_module_info fails for zipped third party libraries

There is a special case logic gate in classify.py:
# special case pypy3 bug(?)
elif not os.path.exists(spec.origin):
    return True, '(builtin)', True

If you are using a zipped third-party package, then this will falsely identify it as part of the standard library.
Here's a code snippet showing the value of spec.origin in such a case:
Python 3.7.5 (default, Oct 31 2019, 15:18:51) [MSC v.1916 64 bit (AMD64)]
Type 'copyright', 'credits' or 'license' for more information
IPython 7.9.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import importlib.util

In [2]: spec = importlib.util.find_spec('py4j')

In [3]: spec
Out[3]: ModuleSpec(name='py4j', loader=<zipimporter object "C:\DevApps\spark\spark-2.4.4-bin\python\lib\py4j-0.10.7-src.zip">, origin='C:\\DevApps\\spark\\spark-2.4.4-bin\\python\\lib\\py4j-0.10.7-src.zip\\py4j\\__init__.py', submodule_search_locations=['C:\\DevApps\\spark\\spark-2.4.4-bin\\python\\lib\\py4j-0.10.7-src.zip\\py4j'])

In [4]: spec.origin
Out[4]: 'C:\\DevApps\\spark\\spark-2.4.4-bin\\python\\lib\\py4j-0.10.7-src.zip\\py4j\\__init__.py'

That spec.origin doesn't exist on the filesystem, so os.path.exists(spec.origin) is False.
I realize we're using %PythonPath% which isn't really supported, but I do believe this would also be an issue in other scenarios.  (Sourcing py4j from a zip included with a Spark distribution is actually a fairly common example I believe.)
Would it be reasonable to get another logic gate in there to identify the zipped library scenario?  I'd be happy to try and make the addition if you like.
Thanks for the great tool.  We're fine with mixing our third-party and first-party imports due to using %PythonPath%.  We really appreciate the black compatibility (and the focus on clean diffs).

PYTHONPATH and eggs are intentionally not supported but if you can submit a small tested patch I'll consider it


Event Request Table 37 "Sales Line" OnBeforeGetReservationQty

Hello, could you please change the code of the GetReservationQty procedure in the table 37 like this?
procedure GetReservationQty(var QtyReserved: Decimal; var QtyReservedBase: Decimal; var QtyToReserve: Decimal; var QtyToReserveBase: Decimal): Decimal
var
IsHandled: Boolean;
begin
    IsHandled := false;
    OnBeforeGetReservationQty(IsHandled, Rec, QtyReserved, QtyReservedBase, QtyToReserve, QtyToReserveBase);
    if not IsHandled then begin
        CalcFields("Reserved Quantity", "Reserved Qty. (Base)");
        if "Document Type" = "Document Type"::"Return Order" then begin
            "Reserved Quantity" := -"Reserved Quantity";
            "Reserved Qty. (Base)" := -"Reserved Qty. (Base)";
        end;
        QtyReserved := "Reserved Quantity";
        QtyReservedBase := "Reserved Qty. (Base)";
        QtyToReserve := "Outstanding Quantity";
        QtyToReserveBase := "Outstanding Qty. (Base)";
        exit("Qty. per Unit of Measure");
   end else
      exit("Qty. per Unit of Measure");
end;

[IntegrationEvent(false, false)]
local procedure OnBeforeGetReservationQty(var IsHandled: Boolean; var SalesLine: Record "Sales Line"; var QtyReserved: Decimal; var QtyReservedBase: Decimal; var QtyToReserve: Decimal; var QtyToReserveBase: Decimal)
begin

end;

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.


[2.9] [Backport] Rancher chart support for additional tolerations

Per title, this is to backport this PR: https://github.com/rancher/rancher/pull/47038/files
Backport PR here: https://github.com/rancher/rancher/pull/48534

This is merged and can be tested when a 2.9.6 chart RC/alpha has been produced. This may need to be requested from the release team by the QA team.

Backported from card https://github.com/rancher/rancher/issues/47584


full-elastic-stack-example vector use datastream

Updating the vector config to generate a datastream rather than an index - this works better for production. Also sets the data_stream fields accordingly
Just double-checked that this works twice (both with the namespace and elasticsearch rename from the last PR just to be double sure that it works)
For now this is my last PR :)

For this one, I think it's better not to include this in the example as often the ES datastreams are set up via the elastic cluster rather than the api clients https://www.elastic.co/guide/en/elasticsearch/reference/current/data-streams.html and I'm trying to keep this example simple for folks to see what's going on rather than have something production ready. At that point we should probably be adding lifecycle management etc as well 😅
Thank you though

That's alright :) This is why I made three separate PR's :)
Maybe it could be included in another example or in a comment or something?

Makes sense, I'm happy to link to an "Elastic best practises" document from this example's README which says use datastreams/retentions/don't have excessive fields/etc if you have a recommendation?


Add onlyListed to CacheRouter.getSafeAppsCacheDir argument list

Changes

Adds onlyListed parameter to the CacheDir field when storing Safe Apps list in the cache.

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

0 of 0   changed or added relevant lines in 0 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-0.01%) to 91.925%

Files with Coverage Reduction
New Missed Lines
%

src/routes/transactions/entities/tests/human-description.builder.ts
1
80.0%

Totals

Change from base Build<PHONE_NUMBER>:
-0.01%

Covered Lines:
6882

Relevant Lines:
7225

💛  - Coveralls


Automated Package Upgrade

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

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


ci: Publish to NPM automatically

:tada: This PR is included in version 1.2.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


Introduce reverse colors and show legend options

Features 🌟

Reverse Colors - reverse the order of colors in the traffic lights.
Show Legend - render the legend / query name separate to the value and trend.

Tests 🧪

Replace duplicated test id strings with TEST_IDS const.
Update to plugin-e2e 1.x.x.
Add tests for rendering TrafficLightValue component.

fixes: #3

Thanks, @jackw , for implementing this feature! Highly appreciated.


why is uninstal.txt spelled with a single "l"?

nevermind, saw this
https://github.com/vim/vim/pull/1695


objdiff-gui: Implement keyboard shortcuts

Keyboard-only navigation demonstration:
https://github.com/user-attachments/assets/6abf4824-ca4d-490a-b259-650a9a096e3e
This PR implements the following keyboard shortcuts:
Symbol list view:

Enter or Space or Mouse5: Enter the diff view for the symbol that is currently highlighted.
Up/Down or W/S: Select the symbol above/below the currently selected symbol. If no symbol is highlighted, select the topmost symbol.
PageUp/PageDown: Scroll up/down by the window height.
Home/End: Scroll to the top/bottom.
Ctrl+F: Give focus to the object filter input. (Note: egui's builtin Tab/Shift+Tab/Enter shortcuts already work fine for selecting an object from the filtered list, so I didn't need to add new ones for that.)
Ctrl+S: Give focus to the symbol filter input.

Function/data/extab diff view:

Backspace or Escape or Mouse4: Go back to the symbol list.
Up/Down or W/S: Scroll up/down by 10% of the window height.
PageUp/PageDown: Scroll up/down by the window height.
Home/End: Scroll to the top/bottom.

Function diff view only:

Ctrl+T: Change target function mapped to this base function.
Ctrl+B: Change base function mapped to this target function.

I also wanted to add a shortcut to open the context menus via the keyboard, but I don't think it's possible in egui :/
Most of these are pretty self-explanatory, but the shortcuts to select the symbol above/below the current one in the listing required a few changes to how objdiff handles the highlighted symbols.
The short version is that the highlighted symbols pair is now treated more like a permanent cursor that you can use to navigate around without the mouse.
Long version:

The highlighted symbol is no longer cleared when backing out of a diff view. This is to make it easier to re-enter the same symbol you were just diffing, or to go to the one above it, etc.
When the highlighted symbol is changed via the keyboard, the newly selected symbols will both be scrolled into view one time. When using the mouse, this is not done.
Using the mouse to highlight a symbol that has no counterpart on the opposite side now gives it a blue (permanent) highlight like other symbols, instead of just a grey (temporary) highlight. This is so you can start by selecting a symbol with the mouse, and then move the cursor around with the keyboard to continue selecting other symbols.

Also, the "split view" where half the window is the function view and the other half is the symbol list view is supported. The symbol list's shortcuts work normally here, while the function diff half has the up/down shortcuts disabled so it doesn't scroll accidentally while navigating the symbol list, but the PageUp/PageDown/Home/End shortcuts are still enabled in case you do want to scroll the function diff in the split view.
Let me know if anything could be improved!

Thanks for this! I'll likely keep poking at the Alt shortcuts in the alt-keys branch, but no reason not to merge this as-is.


The SSL connection could not be established, see inner exception.

Hi, running the program everything goes fine, but on "Online" the extension store doesn't show plugins for add.

Here is the log file:
[Macro Deck] [Error] >> Failed to automatically check for updates
System.Net.Http.HttpRequestException: The SSL connection could not be established, see inner exception.
 ---> System.Security.Authentication.AuthenticationException: The remote certificate is invalid because of errors in the certificate chain: UntrustedRoot
   at System.Net.Security.SslStream.CompleteHandshake(SslAuthenticationOptions sslAuthenticationOptions)
   at System.Net.Security.SslStream.ForceAuthenticationAsync[TIOAdapter](Boolean receiveFirst, Byte[] reAuthenticationData, CancellationToken cancellationToken)
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsync(SslClientAuthenticationOptions sslOptions, HttpRequestMessage request, Boolean async, Stream stream, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Net.Http.ConnectHelper.EstablishSslConnectionAsync(SslClientAuthenticationOptions sslOptions, HttpRequestMessage request, Boolean async, Stream stream, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.AddHttp11ConnectionAsync(QueueItem queueItem)
   at System.Threading.Tasks.TaskCompletionSourceWithCancellation`1.WaitWithCancellationAsync(CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at System.Net.Http.Json.HttpClientJsonExtensions.<FromJsonAsyncCore>g__Core|12_0[TValue,TJsonOptions](HttpClient client, Task`1 responseTask, Boolean usingResponseHeadersRead, CancellationTokenSource linkedCTS, Func`4 deserializeMethod, TJsonOptions jsonOptions, CancellationToken cancellationToken)
   at SuchByte.MacroDeck.Services.UpdateService.CheckForUpdatesAsync(CancellationToken cancellationToken) in D:\a\Macro-Deck\Macro-Deck\MacroDeck\Services\UpdateService.cs:line 62
   at SuchByte.MacroDeck.Services.UpdateService.DoWork(CancellationToken cancellationToken) in D:\a\Macro-Deck\Macro-Deck\MacroDeck\Services\UpdateService.cs:line 126
[Macro Deck] [Error] >> Error while loading extensions from the extension store:
The SSL connection could not be established, see inner exception.

Cheers.

I cannot reproduce this issue. The certificate for the Extension Store is valid.
Are you able to access the following url in your browser?
https://extensionstore.api.macro-deck.app/rest/v2/extensions

Are you able to access the following url in your browser? https://extensionstore.api.macro-deck.app/rest/v2/extensions

On FireFox yes, its a json file, but on IE it showed_"There is a problem with this website's security certificate errors"_, which I solved installing this, and then double click on it –> Install Certificate –> Next –> select the second option and go to examine –> select “Entities Trusted Root Certification” –> Accept –> Next –> Finish.
Now it works! Thank you very much.

Are you able to access the following url in your browser? https://extensionstore.api.macro-deck.app/rest/v2/extensions

Solved installing this, and then double click on it –> Install Certificate –> Next –> select the second option and go to examine –> select “Entities Trusted Root Certification” –> Accept –> Next –> Finish.
Now it works! Thank you very much.

Are you able to access the following url in your browser? https://extensionstore.api.macro-deck.app/rest/v2/extensions

Solved installing this, and then double click on it –> Install Certificate –> Next –> select the second option and go to examine –> select “Entities Trusted Root Certification” –> Accept –> Next –> Finish.
Now it works! Thank you very much.


bluebird 3.x

[x] fix Promise.delay arguments order - was (value, ms) is (ms, value) as of 3.x
[ ] fix test/debug.coffee - fails at bravo_charlie

thanks a lot. version 1.0.0-rc.7 now works with bluebird 3.1.5 : )


Fix tests on release/8.0.0 branch

Having failing tests is making it a pain to develop and review code for the 8.0.0 release branch, failing tests makes it impossible for me to keep the branch up to date with develop. See https://travis-ci.org/concrete5/concrete5/branches

This is not complete, the tests complain of php7 LSP errors

That's gonna have to be someone else, I'm not running PHP 7.

Last time I checked, the release/8.0.0 branch failed to even install on php7...

991130a fixes install @mlocati. @aembler again comments for these methods would've made these changes a lot easier.


Removes duplicated --spa flag on nuxt-generate help

Codecov Report

Merging #2467 into dev will not change coverage.
The diff coverage is n/a.

@@         Coverage Diff          @@
##            dev   #2467   +/-   ##
====================================
  Coverage   100%    100%
====================================
  Files        20      20
  Lines      1145    1145
====================================
  Hits       1145    1145

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 447b314...50c8c05. Read the comment docs.


add signature and qualified assembly name support to json

Changes:

Created a test-integrations.json file and added support for loading multiple files (so we can have integrations for testing that aren't deployed to customers)
Put the DATADOG_INTEGRATIONS env variable back
Added signature support to the json file. Signatures are required and are arrays of integers, which is a bit clumsy but hopefully we can build tools to make generating this file easier.
Updated the assembly name in the json file to be the fully qualified name so that we can load the assemblies based on the definitions in the integerations.json file, instead of hardcoding the managed DLL
Added the AssemblyReference struct and all the necessary parsing code.
Added an example console app that calls the example library I'm using in unit tests.
Turned on instrumentation for all modules when no caller_method is defined.
Replaced the EmitAssemblyRef function with one that takes an AssemblyReference instead.
Fixed a few places where we were doing &v[0] when we should've been doing v.data() (which handles the empty array case)
Added a string splitting utility function
Fixed a bug in the metadata builder that caused unsigned assemblies to fail to load

@lucaspimentel when you get a chance, I think this PR might be ready. It worked locally with the console app. I'll try getting a VM setup tomorrow so I can test with IIS going forward.
The major change here is fixing the JSON file so it loads integrations properly and also supports different assemblies. (which should make it possible to write unit tests for code related to instrumenting assemblies)

@willgittoes-dd mind taking a look at the string parsing code to make sure I didn't do anything dumb?

Thanks for the review. I will fix the style problems.
I have some tests for the string parsing and json deserialization, but could use some for the file loading, iterators and instrumentation. I'll probably do it in a separate PR though because it will likely require some refactoring.
I'm trying to merge often at this stage so the changes don't become unwieldy but it means they're not totally complete... we will need to button down the process when we go to beta.


Encode getTag URI

Fixed getTag() for tags that contained a "?" or "\"

Fixed getTag() for tags that contained a "?" or "\"

LGTM, thanks! 👍


Remove newline \n in href

The newline character in href is invalid and will cause warnings when build/publish.
We created this pull request to fix above issue and unblock docfx v3 migration.

Docs Build status updates of commit d8c6a9a:
:warning: Validation status: warnings

File
Status
Preview URL
Details

docs/sdn-api/properties-element-qualityupdate-element-sdn-api-schema-a.md
:warning:Warning

Details

docs/sdn-api/properties-element-qualityupdate-element-sdn-api-schema-a.md

Line 0: [Warning] Invalid file link:(~/sdn-api/spoiledtilepercentaverage-element-sdn-api-schema-a).

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

Reassigning to office account as I am not the author for this content.


ci: remove dependabot (use renovate only)

Removing dependabot and making Renovate do all its tasks (updating golang and GH actions dependencies). This will reduce some complexity by only have one dependency bot in our repository.
I enabled automerge for patch updates. This will not behave exactly as the dependabot-auto-merge workflow, since that used the auto-merge strategy built into GitHub, while Renovate automerge will merge the PR "manually" after all checks and approvals are in place.

Nice, thanks!


RTMP Problem with streaming stop after start

Guys, I've been working with this module for some time and noticed a strange problem in the live application. When I am transmitting normally, I then open 2 player and beauty are synchronized, but when I open a third it opens later in the video and after a few seconds it hangs and waits for the other players to arrive at or near the time of the video. This happens because the server does not have chunk files of the video so the last one to connect has to wait a few seconds to load. But I think the server should treat this and not the client just like Wowza. I do not know how to implement but it should have a server side buffer to keep the live streaming always a little behind so clients do not watch for a few seconds and then crash until the server loads new chunks files.
my configuration
`user  nginx;
worker_processes  auto;
worker_rlimit_nofile 66536;
#error_log  logs/error.log;
#error_log  logs/error.log  notice;
#error_log  logs/error.log  info;
pid        /var/run/nginx.pid;
events {
worker_connections 66536;
use epoll;
multi_accept on;
accept_mutex on;
accept_mutex_delay 500ms;
}
CONFIG RTMP
rtmp {
# Logs
access_log  /etc/nginx/rtmp/access.log;
    # Server
    server {
            listen 1935;
            chunk_size 4096;
            notify_method get;
            #on_connect http://localhost/auth;

            # Load applications
            include /etc/nginx/rtmp/applications/*.conf;
    }

}
`
APPLICATION
`application live {
live on;
hls on;
hls_path /tmp/live;
#hls_playlist_length 30m;
    # record first 1K of stream
    #recorder all {
    #        record all manual;
    #	record_path /home/records;
    #        record_max_size 500M;
    #        record_suffix _%y-%m-%d--%T.mp4;
    #	record_unique on;
    #        record_notify on;
    #}

}
`
Exit
       location /live {             # Serve HLS fragments             types {                 application/vnd.apple.mpegurl m3u8;                 video/mp2t ts;             }             root /tmp;             add_header Cache-Control no-cache;             add_header Access-Control-Allow-Origin '*';         }
please help

@janiel14 what happened?