implement-batch-norm-layer

Proposed changes
Description
This pull request introduces implementation of Batch Normalization, following the specifications outlined in the paper Batch Normalization: Accelerating Deep Network Training by Reducing Internal Covariate Shift.
Changes Made

Added a new class BatchNorm1d that extends the Module class.
The implementation includes the forward pass logic for Batch Normalization.
Options for configurable parameters such as eps (numerical stability constant), momentum (for running mean and variance updates), and affine (whether to include learnable affine parameters).
Provided examples in the documentation to demonstrate how to use the BatchNorm1d module with and without learnable parameters.

Usage
import mlx.core as mx
import mlx.nn as nn

# With Learnable Parameters
m = nn.BatchNorm1d(100)
# Without Learnable Parameters
m = nn.BatchNorm1d(4, affine=False)
input = mx.random.normal(20, 4)
output = m(input)

Notes

The implementation ensures compatibility with mlx conventions and practices.

Please review and provide feedback.
Checklist
Put an x in the boxes that apply.

[x] I have read the CONTRIBUTING document
[x] I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
[x] I have added tests that prove my fix is effective or that my feature works
[x] I have updated the necessary documentation (if needed)

Hey @awni, I've been thinking about how we should structure the batch normalization module. Do you think it's a good idea to have one class that covers all batch normalization types (like BatchNorm1d, BatchNorm2d, BatchNorm3d), or do we go down the road of having separate classes for each type? I'd love to know what you think!

Adds #204 #216

From an implementation standpoint, I think having a single BatchNorm with the ability to specify a tuple of axes is a good idea, I would also go with that from an API standpoint for now since it's pretty clean and general (compared to adding ND options).
@gboduljak Has an implementation in #216

I fully agree with the idea of a unified BatchNorm. It's a clean and versatile approach for both implementation and the API.

@awni Here is an updated version of BN, that is general:

from typing import Tuple

import mlx.core as mx
from mlx.nn.layers.base import Module

class BatchNorm(Module):
    def __init__(
        self,
        num_features: int,
        num_dims: int,
        eps: float = 1e-5,
        momentum: float = 0.1,
        affine: bool = True,
        track_running_stats: bool = True,
    ):
        super().__init__()

        dims_dict = {
            2: ((1, num_features), (0,)),
            3: ((1, num_features, 1), (0, 2)),
            4: ((1, num_features, 1, 1), (0, 2, 3)),
        }

        if num_dims not in dims_dict:
            raise ValueError(f"expected num_dims to be 2, 3, or 4 (got {num_dims})")

        shape, self.reduction_axes = dims_dict[num_dims]
        self.num_features = num_features
        self.eps = eps
        self.momentum = momentum
        self.affine = affine
        self.track_running_stats = track_running_stats

        if self.affine:
            self.weight = mx.ones(shape)
            self.bias = mx.zeros(shape)

        if self.track_running_stats:
            self.running_mean = mx.zeros(shape)
            self.running_var = mx.ones(shape)

    def _extra_repr(self):
        return f"{self.num_features}, eps={self.eps}, momentum={self.momentum}, affine={'weight' in self}, track_running_stats={self.track_running_stats}"

    def _calc_stats(self, x: mx.array) -> Tuple[mx.array, mx.array]:
        """
        Calculate the mean and variance of the input tensor.

        Args:
            x (mx.array): Input tensor.

        Returns:
            tuple: Tuple containing mean and variance.
        """

        means = mx.mean(x, axis=self.reduction_axes, keepdims=True)
        var = mx.var(x, axis=self.reduction_axes, keepdims=True)

        if self.track_running_stats and self.training:
            self.running_mean = (
                1 - self.momentum
            ) * self.running_mean + self.momentum * means
            self.running_var = (
                1 - self.momentum
            ) * self.running_var + self.momentum * var
        return means, var

    def __call__(self, x: mx.array):
        """
        Forward pass of BatchNorm1d.

        Args:
            x (mx.array): Input tensor.

        Returns:
            mx.array: Output tensor.
        """

        if self.training or not self.track_running_stats:
            means, var = self._calc_stats(x)
        else:
            means, var = self.running_mean, self.running_var
        x = (x - means) * mx.rsqrt(var + self.eps)
        return (self.weight * x + self.bias) if "weight" in self else x
        # return x

We can remove the num_dims parameter by updating the implementation like so.
class BatchNorm1d(Module):
    def __init__(
        self,
        num_features: int,
        eps: float = 1e-5,
        momentum: float = 0.1,
        affine: bool = True,
        track_running_stats: bool = True,
    ):
        super().__init__()

        self.num_features = num_features
        self.eps = eps
        self.momentum = momentum
        self.affine = affine
        self.track_running_stats = track_running_stats

        if self.affine:
            self.weight = mx.ones((num_features,))
            self.bias = mx.zeros((num_features,))

        if self.track_running_stats:
            self.running_mean = mx.zeros((num_features,))
            self.running_var = mx.ones((num_features,))

    def _extra_repr(self):
        return f"{self.num_features}, eps={self.eps}, momentum={self.momentum}, affine={'weight' in self}, track_running_stats={self.track_running_stats}"

    def _check_and_expand_dims(self, x: mx.array):
        """
        Check if the input is a 2D or 3D tensor and expand the weight, bias, running mean, and running variance accordingly.

        Args:
            x (mx.array): Input tensor.
        """

        num_dims = len(x.shape)
        dims_dict = {
            2: ((1, self.num_features), (0,)),
            3: ((1, self.num_features, 1), (0, 2)),
            4: ((1, self.num_features, 1, 1), (0, 2, 3)),
        }

        if num_dims not in dims_dict:
            raise ValueError(f"expected num_dims to be 2, 3, or 4 (got {num_dims})")

        shape, self.reduction_axes = dims_dict[num_dims]

        if self.affine and self.weight.ndim != num_dims:
            self.weight = mx.expand_dims(self.weight, self.reduction_axes)
            self.bias = mx.expand_dims(self.bias, self.reduction_axes)

        if self.track_running_stats and self.running_mean.ndim != num_dims:
            self.running_mean = mx.expand_dims(self.running_mean, self.reduction_axes)
            self.running_var = mx.expand_dims(self.running_var, self.reduction_axes)

    def _calc_stats(self, x: mx.array) -> Tuple[mx.array, mx.array]:
        """
        Calculate the mean and variance of the input tensor.

        Args:
            x (mx.array): Input tensor.

        Returns:
            tuple: Tuple containing mean and variance.
        """

        means = mx.mean(x, axis=self.reduction_axes, keepdims=True)
        var = mx.var(x, axis=self.reduction_axes, keepdims=True)

        if self.track_running_stats and self.training:
            self.running_mean = (
                1 - self.momentum
            ) * self.running_mean + self.momentum * means
            self.running_var = (
                1 - self.momentum
            ) * self.running_var + self.momentum * var
        return means, var

    def __call__(self, x: mx.array):
        """
        Forward pass of BatchNorm1d.

        Args:
            x (mx.array): Input tensor.

        Returns:
            mx.array: Output tensor.
        """

        self._check_and_expand_dims(x)

        if self.training or not self.track_running_stats:
            means, var = self._calc_stats(x)
        else:
            means, var = self.running_mean, self.running_var
        x = (x - means) * mx.rsqrt(var + self.eps)
        return (self.weight * x + self.bias) if "weight" in self else x

We can remove the num_dims parameter by updating the implementation like so.
class BatchNorm(Module):
    def __init__(
        self,
        num_features: int,
        eps: float = 1e-5,
        momentum: float = 0.1,
        affine: bool = True,
        track_running_stats: bool = True,
    ):
        super().__init__()

        self.num_features = num_features
        self.eps = eps
        self.momentum = momentum
        self.affine = affine
        self.track_running_stats = track_running_stats

        if self.affine:
            self.weight = mx.ones((num_features,))
            self.bias = mx.zeros((num_features,))

        if self.track_running_stats:
            self.running_mean = mx.zeros((num_features,))
            self.running_var = mx.ones((num_features,))

    def _extra_repr(self):
        return f"{self.num_features}, eps={self.eps}, momentum={self.momentum}, affine={'weight' in self}, track_running_stats={self.track_running_stats}"

    def _check_and_expand_dims(self, x: mx.array):
        """
        Check if the input is a 2D or 3D tensor and expand the weight, bias, running mean, and running variance accordingly.

        Args:
            x (mx.array): Input tensor.
        """

        num_dims = len(x.shape)
        dims_dict = {
            2: ((1, self.num_features), (0,)),
            3: ((1, self.num_features, 1), (0, 2)),
            4: ((1, self.num_features, 1, 1), (0, 2, 3)),
        }

        if num_dims not in dims_dict:
            raise ValueError(f"expected num_dims to be 2, 3, or 4 (got {num_dims})")

        shape, self.reduction_axes = dims_dict[num_dims]

        if self.affine and self.weight.ndim != num_dims:
            self.weight = mx.expand_dims(self.weight, self.reduction_axes)
            self.bias = mx.expand_dims(self.bias, self.reduction_axes)

        if self.track_running_stats and self.running_mean.ndim != num_dims:
            self.running_mean = mx.expand_dims(self.running_mean, self.reduction_axes)
            self.running_var = mx.expand_dims(self.running_var, self.reduction_axes)

    def _calc_stats(self, x: mx.array) -> Tuple[mx.array, mx.array]:
        """
        Calculate the mean and variance of the input tensor.

        Args:
            x (mx.array): Input tensor.

        Returns:
            tuple: Tuple containing mean and variance.
        """

        means = mx.mean(x, axis=self.reduction_axes, keepdims=True)
        var = mx.var(x, axis=self.reduction_axes, keepdims=True)

        if self.track_running_stats and self.training:
            self.running_mean = (
                1 - self.momentum
            ) * self.running_mean + self.momentum * means
            self.running_var = (
                1 - self.momentum
            ) * self.running_var + self.momentum * var
        return means, var

    def __call__(self, x: mx.array):
        """
        Forward pass of BatchNorm1d.

        Args:
            x (mx.array): Input tensor.

        Returns:
            mx.array: Output tensor.
        """

        self._check_and_expand_dims(x)

        if self.training or not self.track_running_stats:
            means, var = self._calc_stats(x)
        else:
            means, var = self.running_mean, self.running_var
        x = (x - means) * mx.rsqrt(var + self.eps)
        return (self.weight * x + self.bias) if "weight" in self else x

@m0saan your final suggestion looks great. Your dims_dict captures commonly used scenarios well. However, I think that more flexibility in selection of reduction axes and feature axes is beneficial. Maybe we cannot always infer these from the shape of the array. To this end, we can maybe keep reduction_axes, feature_axes and num_features  arguments to the constructor of BatchNorm. However, we can extract your dims_dict  outside of the BatchNorm class and use it in an enum.
Then we can use BatchNorm as follows,
bn = mx.layers.BatchNorm(
   num_features=16,
   reduction_axes=BatchNormReductionAxes.2D,
   feature_axes=BatchNormFeatureAxes.2D
)

We can implement some validations, e.g. axis cannot be both a reduction axis and a feature axis.
However, I am not sure that this generality is necessary. I am also not sure what are the performance implications of using expand_dims. Thus, a 'static' implementation as above may be faster.

Hey @gboduljak, thanks a lot for your input! I really value your ideas on making the BatchNorm class more flexible. We would like to maintain the simplicity and user-friendliness of the framework. While your proposed changes provide additional options, they may also introduce complexity that might be unnecessary for many use cases.  Maybe @awni has some thoughts on this too?
Regarding your point on the performance impact of using expand_dims, I get your concern. We're doing it just once to ensure all created arrays have shapes suitable for broadcasting. In my opinion, it's not a big issue, but I'd love to hear @awni's thoughts to make sure!

In this discussion post, I included some ideas to test batch norm layers. I think it is important to verify we match PyTorch and/or Jax implementations. Maybe you can use these tests. I can also add them. Your tests look good as well, but we may want to test whether we are doing moving stats tracking correctly. It would be also beneficial to test that BatchNorm is behaving correctly in train/eval mode.

I will incorporate these ideas into the testing of BN. If you have additional tests to add, feel free to include them, and we can collaborate to ensure comprehensive testing.

Could someone  explain to me what is the axes in num_dict referring to?  I know that having this dict we can generalise the BatchNorm Class to higher-dimension but I do not understand what is the meaning of (1, self.num_features), (1, self.num_features, 1), and (1, self.num_features, 1,1)
dims_dict = {
            2: ((1, self.num_features), (0,)),
            3: ((1, self.num_features, 1), (0, 2)),
            4: ((1, self.num_features, 1, 1), (0, 2, 3)),
        }

Could someone explain to me what is the axes in num_dict referring to? I know that having this dict we can generalise the BatchNorm Class to higher-dimension but I do not understand what is the meaning of (1, self.num_features), (1, self.num_features, 1), and (1, self.num_features, 1,1)
dims_dict = {
            2: ((1, self.num_features), (0,)),
            3: ((1, self.num_features, 1), (0, 2)),
            4: ((1, self.num_features, 1, 1), (0, 2, 3)),
        }

Depending on the BatchNorm you want to implement (e.g. 1D, 2D), you want to normalize over different axes.
For example, in BatchNorm1D, your input is of shape [batch_dim, num_features] and you normalize over the batch axis (0). This means you also have num_features scale and shift parameters. To broadcast correctly when normalizing in _calc_stats, your parameter shape is [1, num_features]. If you use BatchNorm2D, your input is of shape [batch_dim, num_features, height, width] want to normalize over all axes except the channel axis (axis 1). Thus, you normalize over axes (0, 2, 3). You also have num_features parameters, but you need to keep them in a tensor of shape [1, num_features, 1, 1] to broadcast correctly when normalizing in _calc_stats. Hope this helps :)

We can remove the num_dims parameter by updating the implementation like so.
class BatchNorm(Module):
    def __init__(
        self,
        num_features: int,
        eps: float = 1e-5,
        momentum: float = 0.1,
        affine: bool = True,
        track_running_stats: bool = True,
    ):
        super().__init__()

        self.num_features = num_features
        self.eps = eps
        self.momentum = momentum
        self.affine = affine
        self.track_running_stats = track_running_stats

        if self.affine:
            self.weight = mx.ones((num_features,))
            self.bias = mx.zeros((num_features,))

        if self.track_running_stats:
            self.running_mean = mx.zeros((num_features,))
            self.running_var = mx.ones((num_features,))

    def _extra_repr(self):
        return f"{self.num_features}, eps={self.eps}, momentum={self.momentum}, affine={'weight' in self}, track_running_stats={self.track_running_stats}"

    def _check_and_expand_dims(self, x: mx.array):
        """
        Check if the input is a 2D or 3D tensor and expand the weight, bias, running mean, and running variance accordingly.

        Args:
            x (mx.array): Input tensor.
        """

        num_dims = len(x.shape)
        dims_dict = {
            2: ((1, self.num_features), (0,)),
            3: ((1, self.num_features, 1), (0, 2)),
            4: ((1, self.num_features, 1, 1), (0, 2, 3)),
        }

        if num_dims not in dims_dict:
            raise ValueError(f"expected num_dims to be 2, 3, or 4 (got {num_dims})")

        shape, self.reduction_axes = dims_dict[num_dims]

        if self.affine and self.weight.ndim != num_dims:
            self.weight = mx.expand_dims(self.weight, self.reduction_axes)
            self.bias = mx.expand_dims(self.bias, self.reduction_axes)

        if self.track_running_stats and self.running_mean.ndim != num_dims:
            self.running_mean = mx.expand_dims(self.running_mean, self.reduction_axes)
            self.running_var = mx.expand_dims(self.running_var, self.reduction_axes)

    def _calc_stats(self, x: mx.array) -> Tuple[mx.array, mx.array]:
        """
        Calculate the mean and variance of the input tensor.

        Args:
            x (mx.array): Input tensor.

        Returns:
            tuple: Tuple containing mean and variance.
        """

        means = mx.mean(x, axis=self.reduction_axes, keepdims=True)
        var = mx.var(x, axis=self.reduction_axes, keepdims=True)

        if self.track_running_stats and self.training:
            self.running_mean = (
                1 - self.momentum
            ) * self.running_mean + self.momentum * means
            self.running_var = (
                1 - self.momentum
            ) * self.running_var + self.momentum * var
        return means, var

    def __call__(self, x: mx.array):
        """
        Forward pass of BatchNorm1d.

        Args:
            x (mx.array): Input tensor.

        Returns:
            mx.array: Output tensor.
        """

        self._check_and_expand_dims(x)

        if self.training or not self.track_running_stats:
            means, var = self._calc_stats(x)
        else:
            means, var = self.running_mean, self.running_var
        x = (x - means) * mx.rsqrt(var + self.eps)
        return (self.weight * x + self.bias) if "weight" in self else x

I just came up with a new idea to avoid repeatedly calling _check_and_expand_dims. In (very) large models, this may cause some overhead, due to the cost of setting up a new call frame in Python. We can use the condition self.weight.ndim != num_dims within __call__ to determine whether it is necessary to expand dims. If it is, we can call _setup_parameter_shape. Alternatively, we may just include all the logic for expansion in the __call__.

Hello @gboduljak, I apologize for the delayed response. Using self.weight.ndim != num_dims may not be effective in all scenarios. Consider a situation where self.affine is set to False; in this case, there is no weight parameter in the BN Module, rendering the condition self.weight.ndim != num_dims inappropriate. To address this, we could introduce an additional check, such as if self.affine and self.weight.ndim != num_dims. However, implementing this check may pose a challenge because it prevents the reshaping of moving_mean and moving_var.

@awni can you please review?

Sorry for the delay in reviewing this, we were busy getting v0.0.6 out yesterday w/ quantization etc. I will get on this asap.

Sorry for the delay in reviewing this, we were busy getting v0.0.6 out yesterday w/ quantization etc. I will get on this asap.

sure, thanks. I have a question, while trying to make sure that the stats calculated by BatchNorm layer is correct I noticed this, the variance is somehow is a bit not equal to the one calculated by PyTorch:

@m0saan
torch.var uses a bias correction by default whereas MLX and NumPy do not, that is why you see the slight difference. I think it is the right call for now to use the uncorrected variance in our BN as I believe PyTorch also uses an uncorrected variance in their normalization layers.

PS @gboduljak, @dc-dc-dc, @robertmccraith  thanks for the extra reviews / discussion!
PS @robertmccraith I'm following MIMM eagerly, keep us posted on how it's going and what else you need to get it fully operational!

thanks @awni for the inputs

thanks @awni for your inputs

thanks @awni for your inputs!


Retrieve messing up layouts due to spaces and url encoding

Here's something weird:

Got a custom metadata type called Healthcare.gov Endpoint, now for standard Salesforce API names, this is fine, as every weird thing gets moved to an underscore. But for layouts, for some inconceivable reason, Salesforce decides to keep the spaces, and looks like encoding the .. This makes it so when passing the metada name from telescope to the command, the spaces break up the metadata name parameter, and the % in the escaped dot is expanded by nvim into the full buffer path :rofl:
Maybe we could enclose the whole thing in quotes to avoid the spaces? Not sure how to avoid the expansion of %.
Tbh, I'm kinda swamped with things right now, but this may not be urgent, so if it's still around in a few weeks to a month, I might even try and take a look at solving it.

All commands to be processed in the integrated terminal are sent to Term.run(c), which uses expandcmd() to expand the entire command string c. Check :h expand to see what special characters are expanded.
Either we split c into chunks and expand only the parts we want to, or you fix it in telescope :).  I don't use telescope anymore, why does it alter your file name into %:p %:h ?

Actually, it looks like telescope is passing the correct cmd. This is a print before sending it to T.run:

Of course, it's missing the quotes to handle spaces, but that's easy enough now that I look at it. The expand though, not sure what the best approach might be

is your code aligned with my latest? I do already have quoted \'%s:*\' in my latest version. https://github.com/xixiaofinland/sf.nvim/blob/844c9f01dd08036b9b1cafc9d037152b83ac167b/lua/sf/md.lua#L205

Hey, you're right, I was out of sync with upstream. Love some of the new changes, especially making hotkeys optional and separate from commands! :grin:
I do have the quotes there now, but those are only for pulling all components of a single type. Pulling a single component still has no quotes (but I can make a PR for that, no worries).
As for the issue, here's some steps to reproduce:

Create a custom object (custom metadata type is way easier) that has a . in the label
Add Layout to the md types to retrieve
Attempt to retrieve the layout for the created object.

The . in the label should get encoded to %2E in the layout's name, which when passed to Term, will get expanded to <current buffer>2E in the middle of the layout name

Thank you for the PR, merged :)!


Adds the latency plugin. Closes #321

Adds the latency plugin. Closes #321

Let's merge after releasing v0.11


Confirm Unvote transaction before Voting

This is designed for the purpose of voting for a new Delegate, while still voting for someone else.
It's the same process when already voted, just with a dialog:

Add second delegate
Untick currently voted delegate
Vote
Sign unvote transaction
Send
Wait for Unvote to confirm
Sign new vote transaction
Send

Resolves #86

It works like a charm, but I'll not merge it since the next protocol update will fix this to the same transaction. Good job!

No worries! I thought that would be the case after AIP 11 was announced 😁 do you think it's worth implementing the wait screen for transactions to be confirmed anyway, possibly as an option when making the transaction?


HDDS-1653. Add option to "ozone scmcli printTopology" to order the ou…

…tput acccording to topology layer. Contributed by Xiaoyu Yao.

Attach a sample output:
bash-4.2$ ozone scmcli printTopology
State = HEALTHY
<IP_ADDRESS>(datanode_4.ozone-topology_net)    /rack2
<IP_ADDRESS>(datanode_2.ozone-topology_net)    /rack1
<IP_ADDRESS>(datanode_3.ozone-topology_net)    /rack1
<IP_ADDRESS>(datanode_1.ozone-topology_net)    /rack1
bash-4.2$ ozone scmcli printTopology -o
State = HEALTHY
Location: /rack1
<IP_ADDRESS>(datanode_1.ozone-topology_net)
<IP_ADDRESS>(datanode_2.ozone-topology_net)
<IP_ADDRESS>(datanode_3.ozone-topology_net)
Location: /rack2
<IP_ADDRESS>(datanode_4.ozone-topology_net)

:broken_heart: -1 overall

Vote
Subsystem
Runtime
Comment

0
reexec
31
Docker mode activated.

_ Prechecks _

+1
dupname
0
No case conflicting files found.

+1
@author
0
The patch does not contain any @author tags.

-1
test4tests
0
The patch doesn't appear to include any new or modified tests.  Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.

_ trunk Compile Tests _

+1
mvninstall
479
trunk passed

+1
compile
256
trunk passed

+1
checkstyle
73
trunk passed

+1
mvnsite
0
trunk passed

+1
shadedclient
852
branch has no errors when building and testing our client artifacts.

+1
javadoc
159
trunk passed

0
spotbugs
320
Used deprecated FindBugs config; considering switching to SpotBugs.

+1
findbugs
508
trunk passed

_ Patch Compile Tests _

+1
mvninstall
450
the patch passed

+1
compile
250
the patch passed

+1
javac
250
the patch passed

+1
checkstyle
81
the patch passed

+1
mvnsite
0
the patch passed

+1
whitespace
0
The patch has no whitespace issues.

+1
shadedclient
679
patch has no errors when building and testing our client artifacts.

+1
javadoc
161
the patch passed

+1
findbugs
522
the patch passed

_ Other Tests _

+1
unit
286
hadoop-hdds in the patch passed.

-1
unit
1751
hadoop-ozone in the patch failed.

+1
asflicense
37
The patch does not generate ASF License warnings.

6725

Reason
Tests

Failed junit tests
hadoop.ozone.om.TestOzoneManagerHA

hadoop.ozone.client.rpc.TestOzoneRpcClientWithRatis

hadoop.ozone.client.rpc.TestOzoneAtRestEncryption

hadoop.ozone.client.rpc.TestFailureHandlingByClient

hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestory

hadoop.ozone.client.rpc.TestCommitWatcher

hadoop.ozone.client.rpc.TestOzoneRpcClient

hadoop.ozone.client.rpc.TestWatchForCommit

hadoop.ozone.client.rpc.TestSecureOzoneRpcClient

hadoop.ozone.TestMiniChaosOzoneCluster

Subsystem
Report/Notes

Docker
Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/1/artifact/out/Dockerfile

GITHUB PR
https://github.com/apache/hadoop/pull/1067

Optional Tests
dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle

uname
Linux 940da1eaf7a1 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

Build tool
maven

Personality
personality/hadoop.sh

git revision
trunk / 993dc87

Default Java
1.8.0_212

unit
https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/1/artifact/out/patch-unit-hadoop-ozone.txt

Test Results
https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/1/testReport/

Max. process+thread count
5231 (vs. ulimit of 5500)

modules
C: hadoop-hdds/tools U: hadoop-hdds/tools

Console output
https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/1/console

versions
git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1

Powered by
Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@xiaoyuyao , with uuid as network name, here is output,
ozone scmcli printTopology
State = HEALTHY
<IP_ADDRESS>(ozone-topology_datanode_2_1.ozone-topology_net)    /rack1
<IP_ADDRESS>(ozone-topology_datanode_3_1.ozone-topology_net)    /rack1
<IP_ADDRESS>(ozone-topology_datanode_1_1.ozone-topology_net)    /rack1
ozone scmcli printTopology -o
State = HEALTHY
Location: /rack1
2a2e4f1a-47a8-4c9f-8368-d719aab3057b
2c6478ae-dbba-4349-8759-7c803e2e6617
a803fa42-291c-42b3-8577-719f5746980b
I would suggest add Ipaddress & hostname in "-o" options.

:broken_heart: -1 overall

Vote
Subsystem
Runtime
Comment

0
reexec
76
Docker mode activated.

_ Prechecks _

+1
dupname
0
No case conflicting files found.

0
shelldocs
1
Shelldocs was not available.

+1
@author
0
The patch does not contain any @author tags.

-1
test4tests
0
The patch doesn't appear to include any new or modified tests.  Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.

_ trunk Compile Tests _

0
mvndep
71
Maven dependency ordering for branch

+1
mvninstall
518
trunk passed

+1
compile
315
trunk passed

+1
checkstyle
88
trunk passed

+1
mvnsite
0
trunk passed

+1
shadedclient
871
branch has no errors when building and testing our client artifacts.

+1
javadoc
183
trunk passed

0
spotbugs
342
Used deprecated FindBugs config; considering switching to SpotBugs.

+1
findbugs
558
trunk passed

_ Patch Compile Tests _

0
mvndep
41
Maven dependency ordering for patch

+1
mvninstall
497
the patch passed

+1
compile
321
the patch passed

+1
javac
321
the patch passed

+1
checkstyle
93
the patch passed

+1
mvnsite
0
the patch passed

+1
shellcheck
1
There were no new shellcheck issues.

+1
whitespace
0
The patch has no whitespace issues.

+1
shadedclient
814
patch has no errors when building and testing our client artifacts.

+1
javadoc
218
the patch passed

+1
findbugs
653
the patch passed

_ Other Tests _

-1
unit
125
hadoop-hdds in the patch failed.

-1
unit
2095
hadoop-ozone in the patch failed.

+1
asflicense
51
The patch does not generate ASF License warnings.

7870

Reason
Tests

Failed junit tests
hadoop.ozone.lock.TestLockManager

hadoop.ozone.client.rpc.TestMultiBlockWritesWithDnFailures

hadoop.ozone.client.rpc.TestOzoneClientRetriesOnException

hadoop.ozone.client.rpc.TestSecureOzoneRpcClient

hadoop.ozone.client.rpc.TestFailureHandlingByClient

hadoop.ozone.client.rpc.TestOzoneRpcClient

hadoop.ozone.client.rpc.TestBlockOutputStreamWithFailures

hadoop.ozone.client.rpc.TestOzoneAtRestEncryption

hadoop.ozone.client.rpc.TestCloseContainerHandlingByClient

hadoop.ozone.container.server.TestSecureContainerServer

hadoop.ozone.client.rpc.TestWatchForCommit

hadoop.ozone.container.ozoneimpl.TestSecureOzoneContainer

hadoop.ozone.client.rpc.TestOzoneRpcClientWithRatis

hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestory

Subsystem
Report/Notes

Docker
Client=18.09.7 Server=18.09.7 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/2/artifact/out/Dockerfile

GITHUB PR
https://github.com/apache/hadoop/pull/1067

Optional Tests
dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle shellcheck shelldocs

uname
Linux 7fc322282b06 4.15.0-52-generic #56-Ubuntu SMP Tue Jun 4 22:49:08 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

Build tool
maven

Personality
personality/hadoop.sh

git revision
trunk / 9838a47

Default Java
1.8.0_212

unit
https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/2/artifact/out/patch-unit-hadoop-hdds.txt

unit
https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/2/artifact/out/patch-unit-hadoop-ozone.txt

Test Results
https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/2/testReport/

Max. process+thread count
4862 (vs. ulimit of 5500)

modules
C: hadoop-hdds/tools hadoop-ozone/dist U: .

Console output
https://builds.apache.org/job/hadoop-multibranch/job/PR-1067/2/console

versions
git=2.7.4 maven=3.3.9 shellcheck=0.4.6 findbugs=3.1.0-RC1

Powered by
Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

+1, will commit shortly.


Fix ffmpeg sha sums

Hello!

So here's the commit: https://github.com/kesha-antonov/ffmpeg-android-java/commit/70e2c41aa13099ab274115c1991887d0aa394feb

P.S. Sorry for the mess with commits. I don't know how to merge them properly.
If you know - tell me. I will do it.
What I did:

git pull https://github.com/shahen94/react-native-video-processing into my local master
fixed conflicts & commited
git checkout -b my_new_branch_for_this_fix

Thanks @kesha-antonov !.
That's fine, i'll squash and merge this PR.


Decide which benchmarking framework should be used for the entity system

Decide which benchmarking framework should be used to test the performance of the new entity system.

hayai
https://bruun.co/2012/02/07/easy-cpp-benchmarking

From my current research I come to the conclusion that hayai is a good point to start. It looks very simple and it's similar to gtest.


CI: Skip uploading artifacts and crash reports to paid services if API key/secret env vars aren't set

Requirements for Adding, Changing, or Removing a Feature (from template)
Requirements for Adding, Changing, or Removing a Feature

Fill out the template below. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
The pull request must contribute a change that has been endorsed by the maintainer team. See details in the template below.
The pull request must update the test suite to exercise the updated functionality. For guidance, please see https://flight-manual.atom.io/hacking-atom/sections/writing-specs/.
After you create the pull request, all status checks must be pass before a maintainer reviews your contribution. For more details, please see https://github.com/atom/.github/tree/master/CONTRIBUTING.md#pull-requests.

Issue or RFC Endorsed by Atom's Maintainers
https://github.com/atom-ide-community/atom/issues/1#issuecomment-663877340

Description of the Change

Skip uploading build artifacts to Amazon S3 if no credentials are set, in script/vsts/upload-artifacts.js

Similarly, skip uploading crash reports to Amazon S3 if credentials aren't set, in script/vsts/upload-crash-reports.js

Skip uploading Linux packages (.deb, .rpm) to PackageCloud.io if API key isn't set, in script/vsts/upload-artifacts.js

This is to stop these uploads from being attempted with misconfigured/blank credentials or upload destinations. Such misconfigured uploads error out and cause our CI to fail at the last step, after all tests have passed.

Alternate Designs
None.

Possible Drawbacks
None for us. Upstream may decide they don't want it, I guess.

Verification Process
CI is showing that these changes are working. See this comment for details: https://github.com/atom-ide-community/atom/pull/66#issuecomment-673524996

Release Notes

N/A

@aminya please take a look.
If you like this, or have any suggestions for improvement, this PR can give us green CI on the "Release Branch Build" and "Nightly Release" pipelines.
Since this is one of those things that benefits forks in general, we might consider how best to format and code this for posting as a PR to upstream.

The reason I made the slightly funky "skip" option for S3 is that if upstream merges this, minus the commits that turn it off, it won't disrupt their release flow.
If we can convince them to change their CI config, then we have more options for how to format this.
Particularly, you can't check if a secret variable in Azure Pipelines is empty very easily, as the variable tends to expand to its full name, literally a string containing $(VAR_NAME) rather than a truly empty string. If we check for a secret variable being empty, such as S3 authentication keys, it requires us and any forks to explicitly set that var empty rather than being able to leave it unset.
That's why I made this on by default in the JS, but you can disable it by setting a flag in the pipeline .yml files.

Updated.

It occurs to me these need not be flags. We can simply check an environment variable (example names: $SKIP_UPLOAD_TO_S3 and $SKIP_UPLOAD_TO_PACKAGECLOUD) and leave the flags alone.
That would make the change for this PR exclusively live in script/vsts/upload-artifacts.js. No need to change the .yml files.
@aminya Would you approve of this approach?

The UploadArtifacts job passes: https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=467&view=logs&j=fc7b8a14-4c68-54ee-1e90-04bf630ba360

Is this ready?

I made an upstream issue to try to get a discussion there, but it;s only been a couple of days, so they haven't had much time to respond... Still, I think they have other things on their plates at the moment keeping them busy.
+1 to move forward on this here.
I might want to check for some edge cases or additional scripts I missed, but honestly I found there are a lot of hard-coded URLs to the atom/atom repo, etc. I don't think I can get to that all in one PR without it taking forever. This is a good enough start.

I came up with a different approach you might want to take a look at: https://github.com/atom-ide-community/atom/compare/master...DeeDeeG:WIPPPPP
Just skip if S3 credential env vars aren't set. And then we can disable all the S3 and Linux (PackageCloud.io) uploads in yml and JS. I'm unclear of upstream wants these toggles, and they're honestly a bit tricky to get right, so we could bluntly disable them here and just revert to what upstream has some time in the future if we want S3 uploads again.

Here's something I realized we can do, a fully realized version of the WIPPPPP branch I posted above.
https://github.com/atom-ide-community/atom/compare/master...DeeDeeG:WIPPPPP2
If there is an unset Azure DevOps variable passed to a step with env: blocks of the yaml, it will show up in the environment as literally $(VAR_NAME_HERE). Since this useless value is predictable, we can check for it to determine that the variable was unset.
So basically we can just do a sanity check that we have credentials to authenticate to these services, otherwise skip uploading to them.
That's a way to do this without touching the YAML, and it should be reasonable enough for upstream too, but it works for us/forks without having to set any new environment variables. No new documentation fora new variable needed, less mental load for forks setting up their CIs, more automatic.
At this point I am happy with how this looks. If CI passes I want to overwrite this PR with that approach, change the PR title to "Skip uploading to paid services if credential env vars not set" or something, and we can merge it.

Going to upload (force push) the better version, the one that doesn't require us to set any variables.
Bookmarking the old commit, so we don't lose the original design/first pass attempt at this PR: 0793a758a6e08261e48932bdba54b728e6961783

@aminya We can do a similar technique to this, to skip code signing and notarization, meaning we (and hypothetically, other forks or individuals) wouldn't have to set a SHOULD_SIGN env variable.

This is working, at least for the "Nightly Release"/"Atom Nightly" pipeline.
See this release I made from CI at my personal fork: https://github.com/DeeDeeG/atom-nightly-releases/releases/tag/v1.51.0-nightly1
Note that your personal access token (https://github.com/settings/tokens) used in CI must have the public_repo setting checked/enabled in order to make releases via the API. Otherwise the API POST HTTP request will return a 404, indicating you aren't properly authorized to push a release via the API.

The double check isn't necessary here, strictly speaking, but not hurting anything either... This way we can set it and never have to change it regardless of changes in the tech stack going forward. Checking for !== '$(VAR_NAME_HERE)' is only applicable to Azure DevOps, but that's assuming they never decide to change how these vars work when unset. The double check would work even if we were trying to run in GitHub Actions or whatever. It's worth it.
Doing two of the exact kind of check (checking env var values) in the same if() check isn't confusing, it's not slow to run. It's literally two extra lines of code that make it more immune to problems. They are not risky or prone to failing, they actually reduce the scenarios where we can fail. It's worth it IMO.
Cost-benefit on CI and infrastructure is different from product code. It's more about conservative code that won't fail unexpectedly, as a CI bug, when the app itself is fine. Not optimizing everything down to the simplest/leanest checks if there is some realistic risk it could break.
It will matter more in script/lib/code-sign-on[mac/windows].js, with the signing env vars (example ATOM_MAC_CODE_SIGNING_KEYCHAIN_PASSWORD). People can set those on their local machines if they want, so it won't be set to $(VAR_NAME_HERE). I don't want to break local builds outside of CI there.

Unless we want to explicitly set ALL of these vars to be empty.

That's how you should use ENV variables. You should only check for truthy values. If the user sets some incorrect value for the ENV variable, the code will throw errors.

I just want to be absolutely clear that Azure DevOps sets these variables to literally hold the following characters, as strings:
$(ATOM_RELEASES_S3_KEY), $(ATOM_RELEASES_S3_SECRET), $(ATOM_RELEASES_S3_BUCKET), $(PACKAGE_CLOUD_API_KEY)
If we do not check for those values, we need to go in and override the default behavior by setting each of these four variables to be empty. I don't like this, because it's something tedious every fork would have to do, whereas we can just have the JS handle it.

Before we merge, we should double-check this works.
Please so these steps:

set the four vars to be empty in the shared variable group
Run either the Nightly or Release Branch pipeline against this branch
We'll watch CI and make sure the "Release" step skips uploading to these services.

set the four vars to be empty in the shared variable group

You don't need to set them to empty 😄. Read my comment here to understand how falsey values work in JavaScript: https://github.com/atom-ide-community/atom/pull/66#discussion_r471026684

Yes but Azure DevOps sets them Try it and see. Believe your own eyes, not me.

That's because the wrong type of variables is used in the vsts. We should use {{ }} instead of ()
https://docs.microsoft.com/en-us/azure/devops/pipelines/process/variables?view=azure-devops&tabs=yaml%2Cbatch#runtime-expression-syntax

I'm not sure if these files count as templates, since they are root yml files for the respective pipelines and not brought in with the - template thing, so it might not work, but no harm in trying it.

I'm not sure if these files count as templates, since they are root yml files for the respective pipelines and not brought in with the - template thing, so it might not work, but no harm in trying it.

I don't think these two are related. That's just the name of the variable.

Could you try linting? Your branch is out of date and uses different Node version than what is on our master.

I will say you underestimate the time I've put in checking all these alternative options. I'll see if I can get to a good machine to try this from.

CI did run the linter, and this is rebased on top of master already.
I can fix the lints though.

My bad. I have moved to a new system recently. The wrong Node was on my path. I am fixing it.

Thanks! It found out that it is a git issue. I can't fetch your branch.
It shows that the last commit is for two days ago.

Sory about that. I opened the PR against my fork, and also uploaded a branch here to run this project's CI against it.
You'll want to set my fork as a remote and pull from that, I suppose?
I'm getting in the habit of posting branches directly here for easier collaboration.

Now, I can't push to your branch, and I can't make a pull request to your branch.
Please apply this commit somehow: https://github.com/aminya/atom/commit/b5d6cb932f24aba1c2dd7ffb0102a40832a4cd37

I'm trying template variable syntax:
https://dev.azure.com/DeeDeeG/b/_build/results?buildId=272&view=results
This one is probably wrong but I'll let it run and see what happens:
https://dev.azure.com/DeeDeeG/b/_build/results?buildId=271&view=results

Please apply this commit somehow: aminya@b5d6cb9

Okay, I got it uploaded to my branch.

I am not seeing it here. Could you try your experiments here so I can give feedback on it?

I can't run the CI on your branch. We need to stick to your personal Azure this time.

It's uploaded here, but since the commit was authored before our most-recent comments, GitHub shows it just above our recent comments, visually.
It's there: https://github.com/atom-ide-community/atom/pull/66/commits

Oh whoops, I forgot "here" meant my fork. Sorry again. I didn't think ahead how many problems that would cause, or at least I didn't expect it to be this much. Arg.

The branch at this repo is also updated, but if we want to update the PR it's still the separate branch at my fork. :/ So you can run CI against atom-ide-community:CI-skip-paid-service-release-artifact-uploads if you want.
My experiments with this are still running, I linked them in a comment above.

Quick experiments.

Passing variables with the env: block, using macro syntax $(VAR)

https://dev.azure.com/DeeDeeG/b/_build/results?buildId=275&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=3324220a-e75a-5f85-b529-138c62778233
Reults: :x: "notset" variable resolves to "$(NOTSET)", but at least the secret variable is passed :heavy_check_mark:

Passing variables with the env: block, using template variable syntax ${{ VAR }}:

https://dev.azure.com/DeeDeeG/b/_build/results?buildId=275&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=8ed6c9da-5403-5e01-7f16-a821aee3b425
Results: :x: secret variable isn't passed, but at least "notset" resolves to being empty :heavy_check_mark:

I think on Azure DevOps we need to check for the $(VAR) value or else we can't pass the secret variables upstream needs. (Or we can convince upstream not to store these as secret variables, and then use template variable syntax?? Not the best for them though.)

That is strange and worth making an issue for. https://developercommunity.visualstudio.com/spaces/21/index.html
I will put this PR on hold until we find a fix for it.

I think it's intended.

I have opened a support ticket. https://developercommunity.visualstudio.com/content/problem/1151302/inconsistent-behavior-between-variablessecretvar-a.html

The solution to this problem is to first set the values in variables.
variables:
  ATOM_RELEASES_S3_KEY:  $[ variables['ATOM_RELEASES_S3_KEY'] ]
  ATOM_RELEASES_S3_SECRET:  $[ variables['ATOM_RELEASES_S3_SECRET'] ]
  ATOM_RELEASES_S3_BUCKET: $[ variables['ATOM_RELEASES_S3_BUCKET'] ]

Then it does not matter which syntax is used.
I don't have access to your branch so please update this accordingly.

That's a workaround. It's not any more compact than my solution before from the JavaScript.
I would like to see what they have to say, and I don't know why we couldn't have used my solution in the JavaScript if we're going to add a big number of lines to the yml to compensate.

It is not much about the compactness. We should not apply workarounds for the Azure's bugs in our JavaScript code. The CI code (Azure in this case) should be independent of the JavaScript software. Someday, If we decide that we want to move to another CI, this redundant string in the JavaScript code will become invalid. Hopefully, that makes sense.

I mean I feel silly being impassioned about this, since it doesn't matter dearly, but I worked hard on my version.
The scripts we are editing right now are strictly part of CI. (`script/vsts/upload-*.js). The way I had set it up was a bit more verbose (an extra line of code per var), but it would work outside of Azure. The workaround only helps us on Azure, but it doesn't hurt anywhere.
We would never see the $(VAR_NAME) value outside of Azure, and that's no problem. We are checking that that value is NOT set to that. That check behaves correctly outside of Azure, too. Checking for it not to be that weird value that only Azure would ever set && checking that the var is a truthy value, that is a valid check anywhere. It will still work outside of Azure, no problem.
I don't like having workarounds anywhere, but one place in the CI vs another place in the CI is all the same to me. So I am left wondering why I overwrite my code for another solution. I think it's an overboard type of solution... Hmm. But we will want to use the same thing for script/build too, so I begin to see your point.
I'd rather not commit a quick workaround if they can get back to us soon. If they don't get back to us quickly then I'll move forward with your suggestion.
By the way, I am validating that it works as intended. Once my testing happens I'll understand this workaround better.

I appreciate your effort, and certainly, that is not wasted. The code caused me to see the real problem. I just changed the solution slightly.
As a general note, if you could communicate the problem directly and compactly, I may be able to help in finding the solution, so you don't have to put so much time on it.

I don't want to be the bearer of bad news, and I was actually pretty sure your solution would work, but I don't think you tried what happens when a var is unset on the Azure Pipelines side of things.
In my tests, this doesn't fix the problem, it just changes what kind of output there is from $(VAR) to $[ variables['VAR] ]
https://dev.azure.com/DeeDeeG/b/_build/results?buildId=337&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=bd05475d-acb5-5619-3ccb-c46842dbc997

The Macro syntax works fine:
No secret var:

With secret var:

trigger:
- master

pool:
  vmImage: 'ubuntu-latest'

# Does not work without this:
variables:
  secrectVarCopy: $[ variables['secretVar'] ]

steps:
- pwsh: |
    echo secrectVar_Macro         $env:secrectVar_Macro
    echo secrectVar_CompileTime   $env:secrectVar_CompileTime
    echo secrectVar_Runtime       $env:secrectVar_Runtime

  env:
    secrectVar_Macro: $(secrectVarCopy)
    secrectVar_CompileTime: ${{variables.secrectVarCopy}}
    secrectVar_Runtime: $[ variables['secrectVarCopy'] ]

Hi again,
so as I understand we want these qualities:

[x] Secret vars that are SET will be passed to scripts
[x] Non-secret vars that are SET will be passed to scripts
[x] Non-secret vars that are UNSET will appear blank or unset to scripts
[ ] Vars passed as if secret, but UNSET in Azure UI will appear blank or unset to scripts

The last thing I have no idea how  to do.

For example, if we adequately pass the S3_KEY to the scipt, as needed by upstream... we won't get a blank var in JS. So we have to do that double check thing I coded up.

All of these work as expected. I just showed you.
Set:

[x]  Secret vars passed as is
[x]  Non-secret vars passed as is

Not set:

[x]  Empty string will be passed

I have been trying to run some version of our real CI with this, but sadly it fails for unrelated reasons in the Windows build step. Maybe I'll make a branch where I can experiment. I can disable Windows there just to see this work properly

For example, the $(macro) syntax passed in an env: block never produces a blank var on the script side. It produces the string'$(SOME_VAR_NAME_HERE)'
Isn't macro $(VAR) the syntax you were proposing to use? Maybe I am truly misunderstanding.

Yes. Just try my example. https://github.com/atom-ide-community/atom/pull/66#issuecomment-674610803

Your example in screenshots is working, so I just want to see Node running a js script, and acting like we expect CI to. I'll see if I can code up some minimal test for that, shouldn't be hard. Will take me some time, but hopefully not long.
I want to get this right, because when CI stops working in a way that I can't figure out how to debug, it's worse than having no CI. I have been there before on other projects and it's not fun. So just want to validate this in real-world-like conditions (Node running a JS script) then we should be good.

I tried this and haven't gotten it working quite yet.
https://dev.azure.com/DeeDeeG/b/_build/results?buildId=364&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=7dae026f-9e99-5d65-075e-3f7579577f94
Here are my files:
https://github.com/DeeDeeG/azure-pipelines-test/blob/master/console_log_js.yml
https://github.com/DeeDeeG/azure-pipelines-test/blob/master/thing.js
Here is my output:
SECRET_VAR is:undefined
NONSECRET_VAR is:hello this is not secret
UNSET_VAR is:

I am not sure what is going on with your example:
Set:

Not set:

trigger:
- master

pool:
  vmImage: 'ubuntu-latest'

# Does not work without this:
variables:
  secrectVar: $[ variables['secretVar'] ]

steps:
- pwsh: |
    node test.js   # https://github.com/aminya/threadsjs-test/blob/azure-pipelines/test.js

  env:
    secrectVar_Macro: $(secrectVar)

You have two different vars. Is this intended? secreVar and secre**c**tVar.
I am trying it, but oddly it seems both need to be set. I am trying a few combinations.

You have two different vars. Is this intended? secretVar and secrectVar .
I am trying it, but oddly it seems both need to be set. I am trying a few combinations.

That's a typo. Overwriting the old variable might not work. Azure has taken not a smart design for their macro processing. It is very fragile and opinionated.

secretVar
secrectVar
output

unset
unset
node:

set regular
unset
node: regular

set regular
set regular2
node: regular

unset
set regular2
node:

unset
set secret2
node:

set regular
set secret2
node: ***

set secret
set secret2
node: ***

set secret
set regular2
node: ***

set secret
unset
node: ***

Anyways, I don't want to rush for this PR for now. Please stop putting more time on this since it does not worth it. Azure should fix their broken syntax. Having workarounds although temporarily is not a good sign, and I don't consider it a good PR for the upstream repository.
We have other things to improve and maintain which directly affect the end-user.

I think the typo is actually extremely important to your example:
https://docs.microsoft.com/en-us/azure/devops/pipelines/process/variables?view=azure-devops&tabs=yaml%2Cbatch#variable-characters

Don't use variable prefixes that are reserved by the system. These are: endpoint, input, secret, and securefile. Any variable that begins with one of these strings (regardless of capitalization) will not be available to your tasks and scripts.

Any variable starting with secret isn't allowed.
(Might be losing my mind a bit trying to read these docs. And yeah, this PR is important to me so we can do releases.)

Okay, also due to this, we can't re-use variable names:

When you set a variable with the same name in multiple scopes, the following precedence applies (highest precedence first).

Job level variable set in the YAML file
Stage level variable set in the YAML file
Pipeline level variable set in the YAML file
Variable set at queue time
Pipeline variable set in Pipeline settings UI

https://docs.microsoft.com/en-us/azure/devops/pipelines/process/variables?view=azure-devops&tabs=yaml%2Cbatch#expansion-of-variables
When we re-used the variable names in the global variables: block, we were messing with how the values get inherited from the ones set in Azure Pipelines UI.

I have this working:
SECURED_VAR is: ***
truthy

PUBLIC_VAR is: thisisquitepublic
truthy

UNSET_VAR is:
falsy

CI link:
https://dev.azure.com/DeeDeeG/pipelines-tests/_build/results?buildId=546&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=9c939e41-62c2-5605-5e05-fc3554afc9f5

# minimal-variables-test.yml
# Modified from: https://developercommunity.visualstudio.com/content/problem/1151302/inconsistent-behavior-between-variablessecuredVar-a.html

variables:
  secured_var_mapped:  $[variables.SECURED_VAR]
  public_var_Mapped:  $[variables.PUBLIC_VAR]
  unset_var_mapped:  $[variables.UNSET_VAR]

steps:
- script: |
    echo "SECURED_VAR is: $SECURED_VAR"
    test "$SECURED_VAR" && echo "truthy" || echo "falsy"

    echo
    echo "PUBLIC_VAR is: $PUBLIC_VAR"
    test "$PUBLIC_VAR" && echo "truthy" || echo "falsy"

    echo
    echo "UNSET_VAR is: $UNSET_VAR"
    test "$UNSET_VAR" && echo "truthy" || echo "falsy"

  env:
    SECURED_VAR: $(secured_var_mapped)
    PUBLIC_VAR: $(public_var_mapped)
    UNSET_VAR: $(unset_var_mapped)

yml code: https://github.com/DeeDeeG/azure-pipelines-test/blob/c9bfb19/minimal-variables-test.yml

Closing this in favor of the local clone: https://github.com/atom-ide-community/atom/pull/99


Add accessibility improvement

Improve current Lighthouse accessibility check to score 100.

You're now in our contributors list! :tada:
https://github.com/bitcartcc/bitcart#contributors-
Thanks again!


feat: upgrade ethereumex with telemetry support

Ethereumex with telemetry support:
https://github.com/mana-ethereum/ethereumex/pull/68#event-3087252330

yeah, deleted mix.lock and ran mix.deps get


error

the annotations don't seem to be saving properly and I'm getting the following error:

I'm on my home computer but connected through the VPN.

that just looks like a 404 on a missing resource, what does the network tab look like?

I think that was the network resource I gave

right sorry - so that get on a missing resource won't affect behaviour for saving annotations. Any errors in the console tab? what url / steps to reproduce?

I'm getting the error just by loading the annotation page. The transcription page just hangs. Probably not much help but the console output is:

If I had to guess, I'd say Angular isn't correctly interpolating a url somewhere

Possibly here

annotation page worked for me when i selected the first voyage, http://demo.zooniverse.org/oldweather/#/annotate/2369/
Greg clear you local storage cache...and try again.

not sure exactly how to clear cache - went to incognito mode and am still getting the same error (this is on Chromium)

Also getting it with Firefox

Will check on a Mac in a minute

Tried with a Mac (just booted up too so hopefully no cache to begin with) and tried with Safari:

I can't reproduce at all..i've submitted annotations just fine this morning and the page works for me. Maybe something else is afoot here...

Naa, I suspect something is genuinely up - see https://github.com/zooniverse/old-weather/issues/58#issuecomment-131042452


[BUG] Issue with exporting PnP Field default values

Reporting an Issue or Missing Feature
FieldDefault values not included in Site Template export for text fields associated with Content Type, using Get-PnPSiteTemplate
Expected behavior
When defining a default value on a list with enabled content types, I noticed that default values specified on the list does not get exported into pnp:FieldDefault entries for text fields (it worked with term columns but haven't tested other fields so far)
Actual behavior
The field that you have changed default value for are included in pnp:FieldRefs, but the entry in pnp:FieldDefaults is missing
Steps to reproduce behavior
Create Site Content type that inherits from Document and add a text field ("test1") and a taxonomy field ("test2")
Create a new Document Library, enable content types, add the content type to the list
To prove that default values for text fields work, you may also add a text field ("test3") on the library and add an existing site column that is not associated to the content type ("test4")
Go to Change Default Column Values on library and set default values for the added fields
call Get-PnPSiteTemplate and output to template.xml
When you examine template.xml you will notice that pnp:FieldDefaults contains the non-CT associated field and the taxonomy default, but not the CT associated text field.
In pnp:Field the default for the field is also included:

<pnp:Fields>
                        <Field Type="Text" DisplayName="test3" Required="FALSE" EnforceUniqueValues="FALSE" Indexed="FALSE" MaxLength="255" ID="{a02448c9-2edb-4fdb-b8b6-b2303e59b330}" SourceID="{{listid:Documents}}" StaticName="test3" Name="test3" ColName="nvarchar17" RowOrdinal="0">
                            <Default>test3</Default>
                        </Field>
</pnp:Fields>
<pnp:FieldRefs>
                        <pnp:FieldRef ID="1b64e20a-952d-4ed4-a357-5c962ef16052" Name="test1" DisplayName="test1" />
                        <pnp:FieldRef ID="9879f367-3712-4d94-a24b-8a7da7daaf11" Name="test2" DisplayName="test2" />
                        <pnp:FieldRef ID="fa33e131-e798-45df-a438-cebda2168160" Name="test3" DisplayName="test3" />
</pnp:FieldRefs>
<pnp:FieldDefaults>
                        <pnp:FieldDefault FieldName="test2">1;#TEST2|edbb80ed-e9a5-4a2b-ad5c-6d9f6e5e3a82</pnp:FieldDefault>
                        <pnp:FieldDefault FieldName="test4">test4</pnp:FieldDefault>
</pnp:FieldDefaults>

If i manually add test1 to pnp:FieldDefaults and provision a site, it works fine, so I guess it is just a bug in the export?
What is the version of the Cmdlet module you are running?
1.9.17
Which operating system/environment are you running PnP PowerShell on?

[x] Windows
[ ] Linux
[ ] MacOS
[ ] Azure Cloud Shell
[ ] Azure Functions
[ ] Other : please specify

@AndersRask - Can you please update to the latest release 2.1.x (if not done already) however you will need PS 7.2 or later to benefit from all the bug fixes and improvements.
PS5 support has been dropped in the new release.
Feel free to reopen if the issue persists. Thanks!


[deliver,frameit] Support for new Apple devices

Added support for iPhone XR, iPhone XS, iPhone XS Max

Checklist

[x] I've run bundle exec rspec from the root directory to see all new and existing tests pass
[x] I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
[x] I've read the Contribution Guidelines
[x] I've updated the documentation if necessary.

Motivation and Context
Implement support for the latest Apple devices
Description
Added support for iPhone XR, iPhone XS, iPhone XS Max. Added new resolutions, added option to use legacy iPhone X instead iPhone XS, changed default 5.8" device to iPhone XS

I signed it!

@mirkokg Thank you for your contribution! ✨
For deliver and spaceship, @tattn did a great contribution in https://github.com/fastlane/fastlane/pull/13366.
We'd appreciate if you could update this PR for frameit 👍

Any update on this? I'd love to be able to use frameit to generate my iPhone XS Max screenshots 🙏

I've tried using fastlane from this branch, but not sure how to work around the fact that fastlane frameit download_frames appears to download the frames from the internet, so it's still not finding the new ones.

I've tried using fastlane from this branch, but not sure how to work around the fact that fastlane frameit download_frames appears to download the frames from the internet, so it's still not finding the new ones.

Until frames are updated, you need to modify files in local frameit folder manually.
Go to ~/.fastlane/frameit/latest/ and add attached png frame file.
You will also need to add offsets for iPhone XS Max to the offsets.json file in the same folder:
"iPhone XS Max": {
"offset": "+157+145",
"width": 2407
}
Please post your results 👍

I've tried using fastlane from this branch, but not sure how to work around the fact that fastlane frameit download_frames appears to download the frames from the internet, so it's still not finding the new ones.

Until frames are updated, you need to modify files in local frameit folder manually.
Go to ~/.fastlane/frameit/latest/ and add attached png frame file.
You will also need to add offsets for iPhone XS Max to the offsets.json file in the same folder:
"iPhone XS Max": {
"offset": "+157+145",
"width": 2407
}
Please post your results 👍

This did not work for me. I saved the file as Apple iPhone XS Max Space Gray.png in the latest folder and updated the offsets.json file as you describe, but without luck. Here's a sample of the errors I'm getting.
[⠋] Framing screenshot './en_US/iPhone XS Max-2-Calendar.png' [19:38:59]: Unknown device type for size iOS-6.5-in for path './en_US/iPhone XS Max-2-Calendar.png'
[19:38:59]: Unknown device type for size iOS-6.5-in for path './en_US/iPhone XS Max-2-Calendar.png'
[19:38:59]: Unknown device type for size iOS-6.5-in for path './en_US/iPhone XS Max-2-Calendar.png'
[19:38:59]: Couldn't find template for screenshot type 'Apple true Space Gray'

I followed the instructions, downloaded the image above and renamed to Apple iPhone XS Max Space Gray.png, then edited offset.js as stated, and /Library/Ruby/Gems/2.3.0/gems/fastlane-2.105.2/frameit/lib/frameit file to add this check you can find above:
when sizes::IOS_65
return 'iPhone XS Max'

Now I have framed screenshots for XS Max. Not perfect, because the bottom margin is a bit narrow in some of them, but they are better than nothing.

Could you fix it and merge?

Frames for the new devices are now available at facebook.design 🎉

This should make most of @joshdholtz's blocking review comments obsolete, thanks for the information @jonklo.
Could you take a look at the merge conflict @mirkokg (should not be hard, master just got some additional code) and maybe make sure the devices Facebook now offers really solve the problem? Then we should be able to get this merged!

I forked from @mirkokg, rebased, resolved the conflict and then had to add one more commit (based on comments in this thread) in order to get this working.
https://github.com/daveanderson/fastlane/tree/frame-latest-devices
The one other commit was adding
    "iPhone XS Max": {
     "offset": "+157+145",
     "width": 2407
     },

to frameit/frames_generator/offsets.json
I would see about issuing a PR against mirkokg:master except that my PR won't be able to resolve the conflict.

I resolved the merge conflict, hopefully I didn't mess up.
@daveanderson could you maybe just create a new PR from your branch?

I created a new PR https://github.com/fastlane/fastlane/pull/13774 but need @mirkokg to approve my inclusion of his commits.


Doctrine Commandline Tools

Hi,
I tried to use Commandline Tools. According to official guide, I can use this command to generate getters and setters:
php/vendor/doctrine orm:generate-entities
but I get this error:
You are missing a "cli-config.php" or "config/cli-config.php" file in your
project, which is required to get the Doctrine Console working. You can use the
following sample as a template:
<?php
use Doctrine\ORM\Tools\Console\ConsoleRunner;

// replace with file to your own project bootstrap
require_once 'bootstrap.php';

// replace with mechanism to retrieve EntityManager in your app
$entityManager = GetEntityManager();

return ConsoleRunner::createHelperSet($entityManager);

So I tried to create the cli-config.php but where is the correct bootstrap.php file?

if I understand correctly, the bootstrap.php file is needed to load EntityManager in cli. It contain the user credential to database access:

http://doctrine-orm.readthedocs.org/en/latest/reference/configuration.html

Thanks

@dannykopping's code integrates with laravel-doctrine better, however, I found this setup didn't allow me run doctrine reverse engineering commands with error "No Metadata Classes to process".
Steps:

convert mapping. This step worked well and all xml files were generated under app/models.

php vendor/doctrine/orm/bin/doctrine orm:convert-mapping --from-database xml app/models

generate entities. This step raised error "No Metadata Classes to process".

php vendor/doctrine/orm/bin/doctrine orm:generate-entities --generate-annotations=true --generate-methods=true app/models

After searching, I know should set xml metadata driver but there's no configuration entry in laravel-doctrine's config (am I wrong?)
So I updated @matiux's snippet by adding xml driver setup,  below shows the code. Then I could generate entities from database. Hope this may help someone likes me:)

require_once "vendor/autoload.php";

use Doctrine\ORM\Tools\Setup;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\Tools\Console\ConsoleRunner;
use Doctrine\ORM\Mapping\Driver\XmlDriver;

$paths = array(__DIR__.'/app/models');
$isDevMode = false;

// the connection configuration
$dbParams = array(
    'driver'   => 'pdo_mysql',
    'user'     => 'homestead',
    'password' => 'secret',
    'dbname'   => 'xxxx',
    'host' => 'localhost:33060'
);
$config = Setup::createAnnotationMetadataConfiguration($paths, $isDevMode, null, null, false);

// setup xml driver
$driver = new XmlDriver($paths);
$config->setMetadataDriverImpl($driver);

$entityManager = EntityManager::create($dbParams, $config);
return ConsoleRunner::createHelperSet($entityManager);

@zhouyixiang, I added your xml driver implementation in my snippet. So, after some previous change, I have only one file (that I have named cli-config.php) put into the root (at the same level of app) that contains this code:
<?php

require __DIR__.'/bootstrap/autoload.php';

use Doctrine\ORM\Tools\Setup;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\Tools\Console\ConsoleRunner;
use Doctrine\ORM\Mapping\Driver\XmlDriver;

$paths          = array(__DIR__.'/app/models');
$isDevMode      = Config::get('app.debug'); //false
$app =          require_once __DIR__.'/bootstrap/start.php';
$mysqlData      = Config::get('database.connections.mysql');

$app->setRequestForConsoleEnvironment();

$dbParams       = array(

    'driver'        => 'pdo_'.$mysqlData['driver'],
    'user'          => $mysqlData['username'],
    'password'      => $mysqlData['password'],
    'dbname'        => $mysqlData['database'],
    'charset'       => 'utf8',
    'driverOptions' => [1002=>'SET NAMES utf8'], //1002 is the integer value of PDO::MYSQL_ATTR_INIT_COMMAND
);

// setup xml driver
$driver             = new XmlDriver($paths);
$config             = Setup::createAnnotationMetadataConfiguration($paths, $isDevMode, null, null, false);

$config->setMetadataDriverImpl($driver);

$entityManager      = EntityManager::create($dbParams, $config);

$app->shutdown();

return ConsoleRunner::createHelperSet($entityManager);

For Laravel 5:
<?php
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Tools\Console\ConsoleRunner;
use Illuminate\Foundation\Application;

require __DIR__.'/bootstrap/autoload.php';

/** @var Application $app */
$app = require_once __DIR__.'/bootstrap/app.php';
$kernel = $app->make('Illuminate\Contracts\Console\Kernel');

$status = $kernel->handle(
    $input = new Symfony\Component\Console\Input\ArgvInput,
    new Symfony\Component\Console\Output\ConsoleOutput
);

$entityManager = $app->make(EntityManagerInterface::class);
return ConsoleRunner::createHelperSet($entityManager);

Thanks @dannykopping :+1:

Out of curiosity, is there a reason why the commands have to be ported, and we can't just proxy them through to a doctrine cli application?
(ex. $ php artisan doctrine orm:schema-tool:update --force)

Yes please, I'm going crazy trying to use Doctrine 2 on Laravel 5 D:

@DevNIX You should consider switching to laravel-doctrine/orm.

Yea, you should switch @DevNIX. I'm not maintaining this package anymore, because I don't think there's a need nor reason for such a package. The bootstrap of Doctrine 2 is insanely easy and then you'll have all the freedom you need.


SkyBlock profiles are not added to be scanned from players

The players.json schema generator is supposed to add up to 15 SkyBlock profiles as a postprocessing step, but this doesn't appear to be working currently.

This is done in the HypixelPlayer schema in schemas.ts, currently at line 237. The length of the skyblockProfilesToScan before and after the postprocessor are equal.


[Test-Proxy] Transformations pull from incoming request

@JoshLove-msft @mikeharder

Thank you for fixing this and sorry for causing it!


FIX Singleton creation

@tractorcow if tests pass this should be a better fix than https://github.com/silverstripe/silverstripe-framework/pull/5151
...Friggin' white space...

Excellent, but a couple of things:

We should probably replace usages of singleton($class) with $class::singleton(), rather than modifying the internals of singleton(), but only IF we know the $class in an Object subclass (some aren't).
Master branch only, since this is new API. Maybe we could back-port the method to 3 branch, but you'd need to have it on Object base class as well as DataObject.

Your other fix is probably fine for 3.x

I think this is SemVer compliant personally :grinning: Object::singleton() exists on Object in 3.2 thus it exists on DataObject as well.

We should probably replace usages of singleton($class) with $class::singleton()

I agree, but then it wouldn't be SemVer.

Well actually, replacing all those instances would be fine, but we couldn't remove the global function, just deprecate it

I think I’ve been added to this email chain by accident.
Good luck on - this!

On 7 Mar 2016, at 22:39, Jono Menz<EMAIL_ADDRESS>wrote:
I think this is SemVer compliant personally  Object::singleton() exists on Object in 3.2 thus it exists on DataObject as well.
We should probably replace usages of singleton($class) with $class::singleton()
I agree, but then it wouldn't be SemVer.
—
Reply to this email directly or view it on GitHub https://github.com/silverstripe/silverstripe-framework/pull/5154#issuecomment-193488062.

@tractorcow if you agree this can go in to 3.2 as a bugfix I'd suggest the only additional change I make here is marking the global singleton function as deprecated @ 4. Then once this has been merged up to master we can remove the global singleton function and replace all the calls to it as per your suggestion in a subsequent PR. Reason for that is there are around 68 files in framework that use the global singleton() function and 18 in CMS so might cause an unnecessary merge headache if we try to change this in an older version, with no real benefit?

Heh, Excellent.

I think I’ve been added to this email chain by accident. Could you remove me please?
Where did you get this email from - it’s my spam email?

On 7 Mar 2016, at 23:00, Jono Menz<EMAIL_ADDRESS>wrote:
Heh, Excellent.
—
Reply to this email directly or view it on GitHub https://github.com/silverstripe/silverstripe-framework/pull/5154#issuecomment-193494235.

Ugh, I'm sorry...
It's because I added your github name after the @ when I meant to write the literal Excellent.
https://github.com/silverstripe/silverstripe-framework/pull/5154#issuecomment-193487222
I've removed it... but you might stay a part of this conversation. You may need to click the "Unsubscribe" button and it will remove you. :D

Just smash this thing (or the link in your email)

thanks - sorry it was just I was getting 100 emails a minute - no problem, christ I must have had this account for years if it's using this email! Good luck.

At least you didn't call your account name todo.

I think this is SemVer compliant personally :grinning: Object::singleton() exists on Object in 3.2 thus it exists on DataObject as well.

For some reason I thought this was on master only... right you are!
@jonom I'm happy to deprecate the global method, but we'll need to solve the issue with this change breaking singleton() calls on non-object subclasses.

I'm happy to deprecate the global method, but we'll need to solve the issue with this change breaking singleton() calls on non-object subclasses.

I've updated the global method to restore the original behaviour for non-object subclasses but if this is a valid concern, I guess we can't really remove the global function? Unless in those cases we want to replace singleton($class) with Injector::inst()->get($class) which I suppose is okay. Can you give an example of when you would need a singleton for something that isn't a subclass of object?

Tests exploded

Restarted...

I know you'll hate me for this, but I just noticed you didn't actually add a test. :)
can you test that singleton("SomeDataObjectClass") correctly omits the populate defaults? That's what you're testing right?

@jonom also I'm impressed with your commitment to this. :) It must be a pain listening to my critique on end lol...

public function testSingleton() {
	$singleton = singleton('MyObject');
	$inst = MyObject::create();
	$this->assertEquals('Default Title', $inst->Title);
	$this->assertEquals('Default Name', $inst->Name);
	$this->assertEmpty($singleton->Title);
	$this->assertEmpty($singleton->Name);
}

class MyObject extends DataObject implements TestOnly {
	private static $db = array(
		"Title" => "Text",
		"Name" => "Text",
	);
	private static $defaults = array(
		"Title" => "Default Title",
		"Name" => "Default Name"
	);
}

Thanks for the free test! Have worked in something similar, just running locally but having some issues... if I do DataObjectTest_Fixture::create() the defaults are populated but if I do DataObjectTest_Fixture::create() they are not... is that because of something I did?

DataObjectTest_Fixture::create()

You mean doing the same thing exactly twice?

Updated the comment, made a mistake :)

::create() should trigger new defaults.... try testing this without your change (just the current 3.2 branch head) to test your assumption. :D

How about DataObjectTest_Fixture::singleton() ?

That's what I thought to do but after changing branches PHPUnit won't run test... eerrgggh

Yep looks like I've accidentally made sure populateDefaults() never works :) I'll close this for now and work on it more later.

Ah, so close!

Have pushed up the latest if you're interested... https://github.com/jonom/silverstripe-framework/commit/4332f3502fff94edf467c907271e6b09d3c26282#diff-79a102910bcc173b82e657e1926cfcc4R39
I'm momentarily stumped as to why this would have spread beyond singleton instances but throughout the Injector singleton support is pretty choppy - in many cases it's just assumed you want a singleton if you haven't specified a type so it's hard to know exactly what you're going to get.
I assumed everything was okay with this PR because tests passed but all the fixtures in DataModelTest use new instead of create() so I guess that's why this didn't get caught.
@chillu did you write the Injector? Don't suppose you have an idea what's going on here?

https://github.com/jonom/silverstripe-framework/commit/4332f3502fff94edf467c907271e6b09d3c26282#commitcomment-16575894

Hi there,
Any progress with this topic?
Thanks

Given that Object class no longer exists, I'm afraid there isn't going to be any more progress on this unless someone else picks it up for the master branch.

OK thank you for the feedback.

I ran in to this problem yet again, so opened a new PR to try to finally get this fixed in 3.x. https://github.com/silverstripe/silverstripe-framework/pull/7850


Oauth2 : Add the oauth2 client credentials bearer

First Check

[X] I added a very descriptive title to this issue.
[X] I used the GitHub search to find a similar issue and didn't find it.
[X] I searched the FastAPI documentation, with the integrated search.
[X] I already searched in Google "How to X in FastAPI" and didn't find any information.
[X] I already read and followed all the tutorial in the docs and didn't find an answer.
[X] I already checked if it is not related to FastAPI but to Pydantic.
[X] I already checked if it is not related to FastAPI but to Swagger UI.
[X] I already checked if it is not related to FastAPI but to ReDoc.

Commit to Help

[X] I commit to help with one of those options 👆

Example Code
oauth2_clientcredentials_scheme = OAuth2ClientCredentialsBearer(
    tokenUrl="oauth/clientcredentials_token",
    description="My client_id and my client_credentials",
    auto_error=False
)

Description
In Swagger UI, there are 4 types of oauth2 flow.
The client credentials flow is not present in FastAPI
https://swagger.io/docs/specification/authentication/oauth2/
Wanted Solution
Add client credentials flow in oauth2.py
Wanted Code
oauth2_clientcredentials_scheme = OAuth2ClientCredentialsBearer(
    tokenUrl="oauth/clientcredentials_token",
    description="My client_id and my client_credentials",
    auto_error=False
)

Alternatives
No response
Operating System
macOS
Operating System Details
No response
FastAPI Version
0.73.0
Python Version
3.8.7
Additional Context
No response

Any news?


Dev/juan

methodized valid user check and documented code

Give it a test with the merged version, but other than that; looks good!


give tips for timing; closes #103

I think it's better if we have more material but only select what to teach rather than cutting the material down further.

Good idea. I will add that and I will merge then.

I added some information in the intructor guide. Some exercises can be shorten and the episode on Github webpage can also be skipped if needed.

now this PR conflicts. Are the changes here now redundant after merging https://github.com/coderefinery/documentation/pull/127 ?

I re-arranged it so we have all inputs from everybody.

Shall we merge it now?


Keyboard mapping not working when launched from startup

Microsoft PowerToys version
0.56.2
Running as admin

[X] Yes

Area(s) with issue?
Keyboard Manager
Steps to reproduce
Bug report zip: PowerToysReport_2022-03-21-10-48-19.zip
I have mapped a singular key (print screen) to another key (VK210). This works fine, until I reboot the system. Powertoys is setup to start with Windows (as admin) but I have to exit powertoys and restart it manually for the keyboard remapping to work. Removing and re-applying the remap does not work. It only starts working again after restarting the app manually.
✔ Expected Behavior
Re-mapped keys to stay re-mapped after reboot
❌ Actual Behavior
The re-mapped keys return to their original function
Other Software
No response

Can't see anything suspicious from the logs.
Does it work just disabling and re-enabling the module in Settings?
Is your admin user the same as the user who's logged in?


Add indicator for runtime in toolbar

Fixes #1680.
Generic pipeline:

Cursor and style are the same when hovering over the button.
Note: There is a TODO comment because #1620 will introduce a function that provides a runtime icon given a runtime name, so if this gets merged before that it'll be good to rebase that PR and use that function instead.
Developer's Certificate of Origin 1.1
   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

Where's this shown when the panel is expanded?

It stays on the toolbar in the same location.


Add Teamleader user

At Teamleader we are using Keda in production 🎉

@YorickH We are looking for KEDA end-users who want to be interviewed by CNCF for our CNCF Graduation proposal.
Are you willing to share about your experiences on using KEDA? This can be anonymously if need be.
If so, feel free to fill in https://forms.gle/91dD4RVxifWsS2hc7


add promt for single replica volume

Add a note for the hot upgrade : Note : During upgrade if you create single replica volumes then you may loose them
Add upgrade_operator_endpoint to upgrade and upgrade status command  only.

 I tried `tracing::warn!("<<>>");` but this is only printed when we want the trace. The requirement here is to add this prompt as default , so that the user when the run `kubectl mayastor upgrade` knows about the warning.

bors try

I tried tracing::warn!("<<>>") but this is only printed when we want the trace. The requirement here is to add this prompt as default , so that the user when the run kubectl mayastor upgrade knows about the warning.

Oh I mean that maybe we should add our own, where we can customise how we show messages for the users, for example if we want to ask the user a yes/no question.

bors try

bors try

bors merge

bors merge


Cherry Pick upstream updates

I notice that there's been a bit of activity on Adobe's upstream, some of which we might care about: https://github.com/adobe/brackets/pulse/monthly.  It might make sense at some point to sort through the commits over the past year and figure out which ones we should take here.

We should wait until they release v1.8 and then take in the commits.

1.8 has shipped https://github.com/adobe/brackets/wiki/Release-Notes:-1.8

Will work on this on Monday

Dupe of #569.


[Class modifiers] No error if base class is indirectly implemented

The following co19 tests fail in both analyzer and CFE
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Class-modifiers/basic_restrictions_A03_t08.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Class-modifiers/basic_restrictions_A03_t09.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Class-modifiers/basic_restrictions_A03_t10.dart
The code looks like
// class_modifiers_lib.dart
base class BaseClass {}

// test.dart
import "class_modifiers_lib.dart";

base class ExtendsBaseClass extends BaseClass {}

base class IndirectlyImplementsBaseClass implements ExtendsBaseClass {}
//                                                  ^^^^^^^^^^^^^^^^
// [analyzer] unspecified
// [cfe] unspecified

No expected errors produced. But according to the spec

It's a compile-time error if:
...
A declaration implements another declaration, and the other declaration itself, or any of its super-declarations, are marked base or final and are not from the first declaration's library

Tested on the edge SDK (April 17, 2023) on Linux x64

If this is handled by _fe_analyzer_shared then 'area-meta' may be actionable. If it is handled separately by the common front end and the analyzer then we should have a sub-issue for each of those tools.
@johnniwinther, @stereotype441, WDYT?

@kallentu, maybe you're the one who knows best?

@eernstg FYI, I recently created an area-fe-analyzer-shared label for issues affecting the shared code.  I'll apply that label to this issue now.  (@kallentu I don't actually know if you implemented this logic in a shared way or not; feel free to adjust the labels as appropriate).

We'd have two-or-more issues in the case where a given implementation effort takes place in two-or-more different portions of source code, because it's difficult to determine the right time to close the issue if one job is done and another is not. I guess that's not important in this case.

Was fixed as https://github.com/dart-lang/sdk/issues/52160.


Factory Default Settings

An extra button on each different configuration screen that resets the options on that screen to the factory settings

Indeed, it's something I'd love to do (also one per field, like some apps have), but currently the way the options window is done it's not easily do-able unfortunately.
Although refactoring the options window is in my plans so I'll consider this when it's done! 😄


Add support for multi-line PackageReference format

In the recent push here: https://github.com/blackducksoftware/detect-nuget-inspector/pull/9, the fix was to identify a BadParseException and cascade back to PI when we hit that. But, the cause of this exception was whenever we had PackageReference in the below format:
<PackageReference Include="System.Formats.Asn1">  <Version>6.0.0</Version> </PackageReference>
The current code was only parsing over Attributes in the property so, I made changes to support Version tags if there are any. I am keeping the BadParseException check as it is just in case we encounter this in the future.

Could we add some tests for these changes?

Could we add some tests for these changes?

I can add tests but then the question is do we add for all the file types, currently the Nuget Inspector is supporting at this point. I would recommend doing it at later point as this has been already stretched. There are lot of cases we need to consider which are not is scope of this ticket.


CI: simplify by using a configuration matrix

Does this also apply to the gtk-rs repo?

It's what gtk-rs uses actually, just copied it back here


A short question regarding the version of Objects 365

Dear authors:
I notice that the pre-training code you shared recently includes a short instruction about downloading Objects365-v1. Just want to confirm that the following results are produced using Objects365-v1, rather than Objects365-v2, right?

Thank you

Yes, the Objects365 is the v1 not the v2, it is the 609K images version.


Processing is not filtered to run only wrongly encrypted objects

For example, using the command
poetry run app <bucket> <kms_key> --prefix l2_tourists/tourist_trajectories --dry-run

all objects in the directory are processed, and not only those which are encrypted with the wrong KMS key.
Output:
INFO:app:Found 'l2_tourists/tourist_trajectories/20190926/20191001/'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191001/_SUCCESS'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191001/part-00000-4815fe07-9f15-4c5a-a8ad-9e79266d11f6-c000.snappy.parquet'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191001/part-00001-4815fe07-9f15-4c5a-a8ad-9e79266d11f6-c000.snappy.parquet'
INFO:app:Found 'l2_tourists/tourist_trajectories/20190926/20191002/'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191002/_SUCCESS'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191002/part-00000-2973da3b-1d34-4d87-8441-3fdd87c0efa7-c000.snappy.parquet'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191002/part-00001-2973da3b-1d34-4d87-8441-3fdd87c0efa7-c000.snappy.parquet'
INFO:app:Found 'l2_tourists/tourist_trajectories/20190926/20191003/'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191003/_SUCCESS'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191003/part-00000-47ac8694-14c6-442b-b616-7cc183ac8f40-c000.snappy.parquet'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191003/part-00001-47ac8694-14c6-442b-b616-7cc183ac8f40-c000.snappy.parquet'
INFO:app:Found 'l2_tourists/tourist_trajectories/20190926/20191004/'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191004/_SUCCESS'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191004/part-00000-b0dc9f60-2f37-4fa8-ae15-2fa7df8f2278-c000.snappy.parquet'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191004/part-00001-b0dc9f60-2f37-4fa8-ae15-2fa7df8f2278-c000.snappy.parquet'
INFO:app:Found 'l2_tourists/tourist_trajectories/20190926/20191005/'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191005/_SUCCESS'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191005/part-00000-47e648cf-355c-4398-b95a-41a7b1d396de-c000.snappy.parquet'
INFO:app:Processing 'l2_tourists/tourist_trajectories/20190926/20191005/part-00001-47e648cf-355c-4398-b95a-41a7b1d396de-c000.snappy.parquet'
...

The above objects are processed even though they are already encrypted with the right KMS key.

Interestingly, the filtering does seem to work for other date ranges e.g. 2021 May. So it could be that the algo is right but the data encryption is wrong for 2019 data after all...


Create to-camilla-chen-from-vience-yu.txt

Mr.Berti, I miss you!!!

Shouldn't you be addressing your message to Mr Berti instead of Camilla? You completely ignore her in the message body! She must be so sad to see this.


"bin/oodt stop" won't stop tomcat causing system to go out of memory

I was testing OODT with DRAT and saw that the tomcat process created by the bin/oodt start command doesn't get killed/stopped on bin/oodt stop which causes the RAM to go out of memory if we run oodt start multiple times.
Below is an example in my machine where there were many tomcat processes running.

hmmm this sounds like a big problem @IMS94 how should we fix it?

@chrismattmann We can call "tomcat stop" like command in bin/oodt:158. But this depends on whether the Tomcat executable (or the command used to start Tomcat) also support stopping via a command.

got it. Well let's try and make this robust thanks @IMS94 i'm available to test


Palette Change Crashes

When trying to do a /tb palette change "name" I keep getting random game crashes.  Do the palette names need to contain no spaces so I can exclude the quotes around it, or should it not be crashing when I use that command when spaces are in the palette name?

The palette names shouldn't matter, and can contain spaces.  If the entire game is crashing, that's indicative of a deeper problem, and while it may potentially be outside the scope of tHotBar (running out of ram,vram for instance), more info is needed to diagnose:
-Does it crash every time you change palettes, or just occasionally?
-Does it happen more often around a certain palette change?
-Are you changing palettes with a thotbar macro?  If so, what is the macro you're using to change, and what's the macro on the same slot in the palette you're changing to?
-Is there any sort of error message or does the game just close outright?

-Does it crash every time you change palettes, or just occasionally?  No, it is random.  Mostly notice it within my first 30 minutes of playing
-Does it happen more often around a certain palette change?  It happens regardless of the palette change using thotbar keybind as command.
-Are you changing palettes with a thotbar macro? If so, what is the macro you're using to change, and what's the macro on the same slot in the palette you're changing to?  I have the macro commands to change palettes on a job global bar.  It is changing out a different row for the palette change.
Here is an example of one of the palette changes lua:
["1"] = {
ActionType              = 'Command',
Macro                   = T{
"/tb palette change "Debuffs Ele"",
},
Label                   = "Debuffs Ele",
Image                   = "jobs/smn.png",
ShowCost                = false,
ShowCross               = false,
ShowFade                = false,
ShowRecast              = false,
ShowName                = true,
ShowTrigger             = true,
ShowSkillchainIcon      = false,
ShowSkillchainAnimation = false,
ShowHotkey              = true,
},
-Is there any sort of error message or does the game just close outright?  When this happens the game crashes and closes 90% of the time.  The other 10% of the times it locks the game up.

I will look into it, but reproducing something like this can be tricky because it isn't likely to be anything inherent to the addon, so it will likely not have an immediate resolution.

Do you think it could be because I had around 100 binds?  Wondering if that maybe causes a memory issue.

I would assume not.. textures should be cached and everything else is relatively small.  I am thinking something more like a flaw in the underlying lua interpreter causing a stack overflow when reassigning everything.  But, tracing something like that would be close enough to impossible without a crash dump or the ability to reproduce it on my machine.

Just an update, it seems likely this is the same issue here:
https://github.com/ThornyFFXI/tCrossBar/issues/16
We did receive a crash dump in the Ashita channel, and found that crash to be happening inside d3d8 / graphics driver calls.  From the best I can tell, it is not a problem originating inside the addon's code or the lua engine itself, and I could not reproduce it even with extremely strenuous tests on my own machine(linked in that issue thread).
If anyone has any further information or a way to reliably reproduce this crash, I am open to trying things.  I'd also request sharing macro sets that are known to cause this problem for others, they are located at: Ashita/config/addons/thotbar/charname_charid/bindings/globals.lua
and
Ashita/config/addons/thotbar/charname_charid/bindings/job.lua

The original crash described was tied to changing palettes, not simply activating macros.  Unfortunately, at this point in time, it has been completely unreproducible and that continues to be the case when attempting to cause a crash using your own macro files.
I am forced to conclude this is a hardware issue, if you have a 13th or 14th gen intel CPU that would be the most likely cause and users in the ashita discord have had success resolving similar issues with HXUI by underclocking and/or undervolting.


Don't error out of FixVersionClash if service is dead

r? @gabriel

👍

CI failure is known flake.


Create transaction list (plus history per session)

We need to have a popup (called Toaster / sonner) for the transactions that happen in the background.
It would be nice to have per session (while browser is on), the "historical tranasctions" that one did - so that one can copy/paste the tx Id, or see the result of the transaction, instead of realying to check on the wallet.

The problem here is to know what you did in the past right?
I don't want to transform Delegit into a wallet though. Previous Txs is something where an indexer is absolutely needed, however I'm not sure this is really needed. If we concentrate on delegation for now, all you need to know is whether you're delegating. The locks from votes are displayed, bc we need this info and figured it's good for users to know, but it's actually not 100% related.
What actions do we have?

clean the lock -> locks should be 0
delegate -> see the delegation
undelegate -> the no delegation
change delegate -> see the new delegate

There's no action for which the consequence shouldn't be directly visible to users, and then having a list of past Txs doesn't appear as needed to me.

Understood. The idea behind initiating this, was, when i make a delegation, i want to know the result. And if it failed, why it failed. By "history" i mean only the actions that a user did in the session active , not past logins, meaning nothing will be saved locally or not

ok, so this: https://github.com/delegit-xyz/dashboard/issues/87
And make sure that when there's an error, it's not removed automatically maybe

closing as duplicate of #87