[WIP] external service registry for registry aggregator

during xDS api generation, pilot code needs to know whether a service is external or not.  treating v1alpha3.ExternalService objects as rules (as done in the first pass implementation) does not allow pilot to make this distinction.  This PR creates a new external service registry which will be added to the service registry aggregator in a followup PR.

@GregHanson: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-unit-tests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

https://github.com/istio/istio/issues/3901

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-unit-tests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-unit-tests

prow/e2e-simpleTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-simple

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-unit-tests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-unit-tests

prow/e2e-simpleTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-simple

prow/e2e-bookInfoTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-bookInfo

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-unit-tests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-unit-tests

prow/e2e-simpleTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-simple

prow/e2e-bookInfoTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-bookInfo

prow/istio-pilot-e2e.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-pilot-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-simpleTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-simple

prow/e2e-bookInfoTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-bookInfo

prow/istio-pilot-e2e.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-pilot-e2e

prow/istio-unit-tests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-bookInfoTests.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test e2e-bookInfo

prow/istio-pilot-e2e.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-pilot-e2e

prow/istio-unit-tests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test istio-unit-tests

prow/e2e-simpleTests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test e2e-simple

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e.sh
dba0768b6b738ce9cb45135d9a11ae0b4e9befec
link
/test istio-pilot-e2e

prow/istio-unit-tests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test istio-unit-tests

prow/e2e-simpleTests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test e2e-simple

prow/e2e-bookInfoTests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test e2e-bookInfo

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-simpleTests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test e2e-simple

prow/e2e-bookInfoTests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test e2e-bookInfo

prow/istio-unit-tests.sh
39370a6f0247111208f39275fb1222845f2ab9e6
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-bookInfoTests.sh
04a550bb1334488f2e573f82ffce124d12c63fb3
link
/test e2e-bookInfo

prow/istio-unit-tests.sh
39370a6f0247111208f39275fb1222845f2ab9e6
link
/test istio-unit-tests

prow/e2e-simpleTests.sh
39370a6f0247111208f39275fb1222845f2ab9e6
link
/test e2e-simple

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-simpleTests.sh
39370a6f0247111208f39275fb1222845f2ab9e6
link
/test e2e-simple

prow/e2e-bookInfoTests.sh
39370a6f0247111208f39275fb1222845f2ab9e6
link
/test e2e-bookInfo

prow/istio-unit-tests.sh
d01b9576f8ed3a7f16478c95083bbb3baa683c09
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-bookInfoTests.sh
39370a6f0247111208f39275fb1222845f2ab9e6
link
/test e2e-bookInfo

prow/istio-unit-tests.sh
d01b9576f8ed3a7f16478c95083bbb3baa683c09
link
/test istio-unit-tests

prow/istio-pilot-e2e.sh
d01b9576f8ed3a7f16478c95083bbb3baa683c09
link
/test istio-pilot-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e.sh
d01b9576f8ed3a7f16478c95083bbb3baa683c09
link
/test istio-pilot-e2e

prow/istio-unit-tests.sh
6ea22355e6e30c3a07d8c4ee69c7b6508546f8ed
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e.sh
d01b9576f8ed3a7f16478c95083bbb3baa683c09
link
/test istio-pilot-e2e

prow/istio-unit-tests.sh
6ea22355e6e30c3a07d8c4ee69c7b6508546f8ed
link
/test istio-unit-tests

prow/e2e-simpleTests.sh
6ea22355e6e30c3a07d8c4ee69c7b6508546f8ed
link
/test e2e-simple

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-e2e.sh
d01b9576f8ed3a7f16478c95083bbb3baa683c09
link
/test istio-pilot-e2e

prow/e2e-simpleTests.sh
6ea22355e6e30c3a07d8c4ee69c7b6508546f8ed
link
/test e2e-simple

prow/istio-unit-tests.sh
b9af5778a3aa0838724ac9d59cc73c4fd0818299
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-simpleTests.sh
6ea22355e6e30c3a07d8c4ee69c7b6508546f8ed
link
/test e2e-simple

prow/istio-unit-tests.sh
d8f227e1cf2e37823b12fe55c53e5ae36871ecb8
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

/test istio-unit-tests

@GregHanson: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/e2e-simpleTests.sh
6ea22355e6e30c3a07d8c4ee69c7b6508546f8ed
link
/test e2e-simple

prow/istio-unit-tests.sh
d8f227e1cf2e37823b12fe55c53e5ae36871ecb8
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

/test istio-unit-tests

@GregHanson: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-unit-tests.sh
d8f227e1cf2e37823b12fe55c53e5ae36871ecb8
link
/test istio-unit-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@ZackButcher @frankbu: I would like feedback before I merge this.

@ijsnellf If I'm not mistaken, merging this PR would be harmless because it's not being called yet. Is that right?

Yes, it shouldn't interfere with any existing logic.

@ZackButcher @frankbu: makes sense. An alternative is to have an additional Resolution type SDS (or EDS). I think that would be more logical. Most internal services would be SDS except for a few exceptions like headless services which would be passthrough. Thoughts?

cc @rshriram

@ZackButcher / @frankbu repeating my earlier comment
We might need to know about the external service property. I put this in because today we have the super clunky model where we expect external service accesses to be http only while we generate https upstream.
We could do this another way: ask people to explicitly create external service as http port only. And then tell them to create a destination rule that asks for https to the external service.
If we choose the latter approach, we can get rid of the external service property.
But keep in mind that the code is already doing a bunch of checks (if service.External() that checked if hostname was unset) .. I was merely trying to codify that into a boolean field to get more flexibility.

That's fair; I'm super opposed to it, just would've liked to do without if we could.

I'm not following the alternative suggestion:
We could do this another way: ask people to explicitly create external service as http port only. And then tell them to create a destination rule that asks for https to the external service. If we choose the latter approach, we can get rid of the external service property.
I thought that's already what we tell people to do, for example:
apiVersion: networking.istio.io/v1alpha3
kind: ExternalService
metadata:
  name: google-external-svc
spec:
  hosts:
  - www.google.com
  ports:
  - number: 443
    name: https
    protocol: http
---
apiVersion: networking.istio.io/v1alpha3
kind: DestinationRule
metadata:
  name: google-destination
spec:
  name: www.google.com
  trafficPolicy:
    tls:
      mode: SIMPLE

@frankbu / @ZackButcher thinking through this more, I think you guys are right.. Lets have a resolution type EDS or something, that will allow us to get rid of this special classification.
There is an associated piece of work that needs to be done in Envoy (allowing ports to be rewritten for original dst clusters). Barring that, the rest should work.
I am merging this PR with the guarantee that I will get rid of MeshExternal in the subsequent PR. I just need this code in place, to unblock other refactoring I am doing. Besides, this code is not activated yet.

@frankbu / @ZackButcher I think we now have a use case for MeshExternal.. We need to know if the service is an external one in order to setup the MixerFilter config for external services. Resolution/other fields will not be able to indicate that this is a special class of services that need the same mixer config as the one we setup on the inbound path.


Reacting to ResponseCookie Delete changes

Reacts to https://github.com/aspnet/HttpAbstractions/commit/594f55947f4c1d0a9d3122e3f39bcfa81199b12a
We now preserve Secure, HttpOnly, and SameSite on Delete.

@jkotalik,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot


ci(terraform): Test PR comment apply trigger

Replace draft PR approval triggering apply with adding a PR comment with the text: "terraform plan approved". Not all pull requests will relate to terraform and, because the pull_request_review trigger does not support the paths filter, pull request approval could trigger a terraform apply with the previous trigger.

terraform plan approved

terraform plan approved

terraform plan approved

apply -chdir=terraform/development -lock=true tfplan

Outline of changes.
Error: Saved plan is stale
Terraform CI by @chris3ware via issue_comment at undefined.

Plan Parity test is working


what is an appropriate bed file?

Hello,
I ran NanoSV like this: NanoSV -t 40 -o nanosv.vcf -b human_hg19.bed aln.bam, and it prompted The bed file may be inappropriate for your bam file.
The bed file is downloaded from your curation https://github.com/mroosmalen/nanosv/blob/master/nanosv/bedfiles/human_hg19.bed
The bam file is some ~8X ONT data downloaded from https://ftp.ncbi.nlm.nih.gov/giab/ftp/data/AshkenazimTrio/HG002_NA24385_son/UCSC_Ultralong_OxfordNanopore_Promethion/GM24385_1.fastq.gz, and then aligned to hs37d5, which can be perfectly ran on several other SV tools e.g. cuteSV, sniffles, svim.
I am wondering if I used NanoSV incorrectly and how can I adjust, and it seems to be the problem of the bed file. What I should do with the bed file?

I also have this problem. Even if I use Samtools to generate a new bed file, it is still inappropriate for BAM files

Bumping this.
I used bedtools to generate a random-spot BEDfile, after reading the scripts that are included to generate such. I did:
bedtools random -l 1 -seed 1938 -g Aedes_chr1.genome > Aedes_chrom1_randomspots.bed
Where "Aedes_chr1.genome" is just a two column file giving the length of Chromosome 1 -
1       310827022
I'm not sure if this is the right thing to do, but it has actually resulted in NanoSV giving me results, after 10 days of running...

If you align your data to GRCH37, it will work with the default bed file they have, which is mislabeled hg19


[ruff] Implement RUF102 (fastapi-redundant-response-model)

Summary
Implements ruff specific role for fastapi routes, and its autofix.
Test Plan
cargo test / cargo insta review

Thank you for this! Two reactions, which I'd in-turn like your reaction on:

I think we'll need to find someone who feels comfortable reviewing these rules for correctness and quality (e.g., is this a recommended pattern?). I'm not familiar enough with FastAPI to be a good judge here. Perhaps I can ask Sebastian what he thinks of it.
Ideally we'd implement multiple FastAPI rules before committing to supporting a one-off rule. This helps prevent Ruff from feeling like an unwieldy collection of one-off rules.

@charliermarsh Thanks for your comments.

It would be great if Sebastian could confirm that rule. Could you please ask for his opinion?
Another rule I'm considering is to use the new Annotated style for dependencies declaration. Other rules I have in mind require complicated type inference and cross-file analysis, so they are probably out of scope for this PR. Do you think multiple rules are mandatory for merging this PR?

@charliermarsh Any chance you could provide feedback on my previous message when you have a moment?

Thanks @TomerBin for emailing me! This looks great.
And recommending Annotated would be awesome as well.
Additionally, if there was a way to auto-fix code that uses the old style to now use Annotated in Ruff, that would be a dream come true. It applies to dependencies or to Security as well (another type of dependency.
An old dependency and Security param would look like:
@app.get("/items/")
def get_items(
    current_user: User = Depends(get_current_user),
    some_security_param: str = Security(get_oauth2_user),
):
    # do stuff
    pass

That would be ideally transformed into:
@app.get("/items/")
def get_items(
    current_user: Annotated[User, Depends(get_current_user)],
    some_security_param: Annotated[str, Security(get_oauth2_user)],
):
    # do stuff
    pass

Additionally, the same transformation would apply to other types of parameters, like Query, Path, Body, Cookie.
The only caveat is that if one of those has a default argument (or the first argument), as in Query(default="foo") then that would become the new default value. For example:
@app.post("/stuff/")
def do_stuff(
    some_query_param: str | None = Query(default=None),
    some_path_param: str = Path(),
    some_body_param: str = Body("foo"),
    some_cookie_param: str = Cookie(),
    some_header_param: int = Header(default=5),
    some_file_param: UploadFile = File(),
    some_form_param: str = Form(),
):
    # do stuff
    pass

would ideally be transformed into:
@app.post("/stuff/")
def do_stuff(
    some_query_param: Annotated[str | None, Query()] = None,
    some_path_param: Annotated[str, Path()],
    some_body_param: Annotated[str, Body()] = "foo",
    some_cookie_param: Annotated[str, Cookie()],
    some_header_param: Annotated[int, Header()] = 5,
    some_file_param: Annotated[UploadFile, File()],
    some_form_param: Annotated[str, Form()],
):
    # do stuff
    pass

Note: please keep pinging me on email if I miss a GitHub notification around this, I miss most GitHub notifications as I still have around 10k unread. :sweat_smile:

Thank you @TomerBin and @tiangolo 🙏

@TomerBin, thanks for your work on this!
Is this rule intended to apply only to route functions? I ask because at my company, we have a pretty comprehensive set of free functions that are used as dependency providers, for example:
controller.py:
@app.post("/")
def route(dependency: Annotated[str, Depends(providers.base_provider)]) -> None:
    ...

providers.py:
def base_provider(sub_provider: Annotated[str, Depends(sub_provder)]) -> str:
    ...

def sub_provider() -> str:
    ...

Currently, this rule only detects and fixes the route function, and not the provider function in providers.py (which would be pretty nice).


Add Ruff linter configuration

Description
This change introduces Ruff as a linter and code formatter for the project. It adds the necessary configuration in pyproject.toml and includes a poetry.lock file to manage dependencies.
Key updates include:

Adding Ruff as a development dependency in pyproject.toml.
Configuring Ruff with specific settings:

Set line length to 120 characters.
Enabled all rules with some specific ignores.
Configured autofix settings.
Set up exclusions for common directories.
Defined per-file ignores for test files.
Adopted Google-style docstring conventions.

Created a poetry.lock file to lock dependency versions.

These changes aim to improve code quality and consistency across the project by enforcing a standardised linting and formatting approach.
fixes #4

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

#26
#25
#24  👈
#23
#22
#19
#18
#17
#16
main

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

Join @JackPlowman and the rest of your teammates on  Graphite

Merge activity

Sep 19, 11:32 AM EDT: @JackPlowman merged this pull request with Graphite.


Pr/23

Working off PR #23

Coverage increased (+5.0%) to 82.938% when pulling 9be641577120665f3fd1afe9bddc9dd09cdac756 on pr/23 into a12605dfb223c4604540216e69794a58adad4238 on master.

Coverage increased (+5.0%) to 82.938% when pulling 6500f4f5be0d3ade9aba2e1244a86452c4fe9b50 on pr/23 into a12605dfb223c4604540216e69794a58adad4238 on master.

Coverage increased (+5.9%) to 83.886% when pulling e17ac599840cb95dc40648f7aebdbda905d57387 on pr/23 into a12605dfb223c4604540216e69794a58adad4238 on master.

Coverage increased (+6.1%) to 84.038% when pulling edd53dc7e61d1f56c036791860357646a6d99cec on pr/23 into a12605dfb223c4604540216e69794a58adad4238 on master.


How to fine-tune the 4-bit model?

First a big thanks for this amazing effort!
I was just trying to fine-tune this 4-bit model under the transformers framework. The model could be loaded successfully and the training process worked well, however, the loss just became nan after one single loss.backward().
Here is the my code:
import sys
from pathlib import Path
sys.path.insert(0, str(Path("/efs-storage/text-generation-webui/repositories/GPTQ-for-LLaMa")))
import llama

load_quant = llama.load_quant

model = load_quant(
    "/file/llama-7b-hf",
    '/file/llama-7b-hf-int4/llama-7b-4bit.pt',
    4
    )

model.to(device)
....
for batch in tqdm(dataloader):
        model.train()
        input_ids = batch[0]['input_ids'].squeeze(1).to(device)
        attention_mask = batch[0]['attention_mask'].squeeze(1).to(device)
        tgt_labels = batch[1].squeeze(1).to(device)
        loss = model(
                    input_ids = input_ids,
                    attention_mask = attention_mask,
                    labels=tgt_labels
                    ).loss
        model.zero_grad()
        loss.backward()
        optimizer.step()
        torch.cuda.empty_cache()

And here is how my loss looks like after training one batch:
tensor(nan, device='cuda:1', dtype=torch.float16, grad_fn=<NllLossBackward0>)
I wonder is there any way to fine-tune the 4-bit model? thanks!

gptq quantization is designed for inference and does not support training.

Not sure if it is possible to have A @ B.T adapter for vecquant4matmul_cuda. That is needed to compute gradient for lora finetuning.
https://github.com/tloen/alpaca-lora/issues/2#issuecomment-1472302525

@johnsmith0031
Would training quantized models ever be possible due to precision requirements for the gradients? From what I understand, PEFT reduces VRAM usage by training a limited a # of parameters but training is still being done in floating point numbers and not integers right. Also, even if it could work wouldn't the latency of quantizing/dequantizing the model severely hinder training performance?

You could train LoRA modules. Those are more efficient to train anyway. Imagine each layer as a 4096 x 4096 matrix of 4-bit weights. You superimpose a lower-rank 4096 x 4 matrix of FP32, which expands over the larger matrix via an outer product. After finishing the training session, you can de-quantize the 4096 x 4096, add the LoRA, then re-quantize. You can also not fuse anything, and leave the LoRA as a separate layer. That allows switching different variations of the model at runtime without loading in new weights.
Most weights (~16 GB): 4-bit integer (non-differentiable), not changed during gradient descent
LoRa weights (~64 MB): 32-bit float (differentiable), changed during gradient descent

Also, even if it could work wouldn't the latency of quantizing/dequantizing the model severely hinder training performance?

You typically train with batches of several inputs at once. Switching 16 bit -> 4 bit improves the arithmetic intensity by 4x, meaning you can fully utilize hardware with only 1/4x as large batches. That should cancel the per-batch (not per-input) overhead of dequantizing.

@philipturner
That sounds very promising! Would be really cool to see a RLHF training scheme utilize this kind of setup for the 30B or 65B models to try and replicate ChatGPT's results using LoRA.

You typically train with batches of several inputs at once. Switching 16 bit -> 4 bit improves the arithmetic intensity by 4x, meaning you can fully utilize hardware with only 1/4x as large batches. That should cancel the per-batch (not per-input) overhead of dequantizing.

This will probably require custom CUDA kernels or maybe even dedicated hardware support for 4 bit matmul and other BLAS operations right? Similar to how tensor cores in Turing and newer architectures accelerate 8 bit operations? I'm a bit skeptical of the linear performance increases since 8 bit inference in the transformers library has been around 40% - 60% slower than fp16, I would expect 4 bit to be lower.

This will probably require custom CUDA kernels or maybe even dedicated hardware support

No, this is already possible with PyTorch function calls. It also doesn't need to be restricted to CUDA/Nvidia-only. I was considering asking GPT-4 to draft me a Metal kernel that dequantizes in-place. Take geohot's kernel, inject decompression in place of the following lines, them reinterpret-cast two adjacent elements as the thread's components of a simdgroup_matrix.
// old
    simdgroup_load(A[0], data1, {N}, ulong2(k, 0));
    simdgroup_load(A[1], data1, {N}, ulong2(k, 8));
    simdgroup_load(A[2], data1, {N}, ulong2(k, 16));
    simdgroup_load(A[3], data1, {N}, ulong2(k, 24));
// new
   // Read a bunch of 4-bit weights from contiguous device memory.
   // Use SIMD-scoped operations to transpose certain chunks in-place
   // to the different threads' SIMD-group matrices.
   // Decompress to 32-bit and reinterpret cast to SIMD-group matrix.e

8 bit inference

8 bit inference will have vastly different performance patterns than 8 bit training. One is memory-bound, the other compute-bound.

try and replicate ChatGPT's results using LoRA.

Sorry to disappoint, but GPT-3.5 outperforms LLaMa by a large margin, without fine-tuning. And GPT-4 makes every LLM before it essentially obsolete. I've been using it to help me generate code instead of setting up local models for that purpose. It's more efficient to rephrase whatever your goal is - instead of a low-accuracy local model with high batch throughput, ask Bing GPT-4 some hard questions. Then it shows you how to implement the goal yourself.

Thanks for the deep insights into LLMs. I had some hopes of maybe LoRA adapter for the larger 30B or 65B LLaMa models  that were trained Stanford's Alpaca could be a match against text-davinci-003 since it generally showed promising results for the smaller models. I guess some dreams were just not meant to be :(

And GPT-4 makes every LLM before it essentially obsolete.

Perhaps GPT-5 could give us the answers to achieving the previous model's iterations on consumer hardware someday? That would really be something.

I was planning to build an application that uses LLaMa as an AI-powered transpiler. It would port massive CUDA code bases to Metal, processing batches of ~32 files at once. I attached the raw data gathered while planning the application.
AI Data.xlsx

Fun fact: Before LLaMa-65B-3bit data was available, I predicted its Wikitext2 performance to within rounding error. I guessed ~4.16; the actual was 4.17. The only reference measurement was 16-bit, scoring 3.53.
The HumanEval extrapolations are less accurate; 65B-3 probably performs equal to 33B-4. I wanted to try though, because it's the largest model my 32 GB GPU could fit (projected 30 GB deserialized). Regardless, its codegen performance pales in comparison even to untuned GPT-3.5.

Changed to support 2, 4, 8 bit backward using triton.

We also try to implement 4bit-qlora, thanks to the optimized kernel implementation of back-propagation, the fine-tuning speed is similar to 8-bit lora at present. Welcome to use and issue: https://github.com/megvii-research/Sparsebit/tree/main/large_language_models/alpaca-qlora


Fix Service Release Notes - Oct 3 update

hi there is a bug in yesterday's update to the service release notes in the link. instead of:
Hash rules now support [256 for OpenTelemetry collectors] (docs/send-data/collection/processing-rules/hash-rules).
it should be:
Hash rules now support [256 for OpenTelemetry collectors](/docs/send-data/collection/processing-rules/hash-rules).
Slack Message

@nmohantyctr , if this is fixed, can you please close it out?

@nmohantyctr closing this out as I've not heard back from you, and followed up w/the requester


feature: add a subdirectory synchronization file

[ ] save files to ${workspace}/.idea/code-remark

https://github.com/wenzewoo/jetbrains-code-remark/releases/tag/1.4.0


[Bug] Async actions inside loops are evaluated before their effects

Description
When running actions like storeValue inside a loop, and referring to the store at the same time, the values in the store don't reflect the effects those functions have created
Steps to reproduce the behaviour:
Example trigger call
{{
  Api1.run(() => {
  	const arr = [1,2,3];
        const len = arr.length;
        storeValue('inc', len);
        arr.map((row) => {
  	  Api2.run((res, params) => {
  		  storeValue('inc', appsmith.store.inc - 1);
                  console.log("inc " + appsmith.store.inc);
                  storeValue(params.name, res, false);
                  if (appsmith.store.inc == 0) {
        	        AfterAllApi.run();
                  }
          },undefined, {...row, len: len})
      })
  })
}}

Here the inc value in the store does not update as expected as the storeValue function has not run yet and the loop continues

@hetunandu this is a big problem for workflows because there is no way to know when 3 parallel actions are complete. Can we solve this with JS Editor


Document troubleshooting steps

We have seen many cases where users don't know how to troubleshoot a failed KET execution.
Usually, the first thing to do is check the ansible logs. We should document this.

Closed via #429


Display Cards not Working

IMPORTANT: Before you create an issue, please take a look at our Issue Reporting Guide.
Briefly summarize your issue:
The display cards will not show up, but show the code underneath the command in the sample app. Is there any way to enable it? I did enable display cards with media in my AVS account. Also, how do you have an led turn on when I say Alexa, and turns off afterwards.
What is the expected behavior?
I was expecting the display cards to pop up in a new window, or something like that.
What behavior are you observing?
No display cards, and also Alexa acting if my device doesn't have a screen.
Provide the steps to reproduce the issue, if applicable:
Follow the instructions for Raspberry pi, and have the the display cards enabled.
Tell us about your environment:
What version of the AVS Device SDK are you using?
  Version that was only couple days old.

Tell us what hardware you're using:

[ ] Desktop / Laptop
[+] Raspberry Pi
[ ] Other - tell us more:

Tell us about your OS (Type & version):

[ ] Linux
[ ] MacOS
[ ] Raspbian Stretch
[ ] Raspbian Jessy
[+] Other - tell us more: AIY Voice image from Google, with Raspbian Stretch

We're spreading the same question across multiple threads.  Please refer to here: https://github.com/alexa/avs-device-sdk/issues/642


[GEARPUMP-339] Fix unknown tag parameter

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

[x] Make sure the commit message is formatted like:
[GEARPUMP-<Jira issue #>] Meaningful description of pull request
[x] Make sure tests pass via sbt clean test.
[x] Make sure old documentation affected by the pull request has been updated and new documentation added for new functionality.

+1

Codecov Report

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

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
- Coverage   68.98%   68.91%   -0.07%
==========================================
  Files         191      191
  Lines        6171     6171
  Branches      546      546
==========================================
- Hits         4257     4253       -4
- Misses       1914     1918       +4


extensible key bindings

It would be useful to be able to add additional key bindings to trees. For example I would like to implement a delete action when the delete key is pressed that receives the focused item and additional state.
It could also be useful to allow 3rd party libraries to implement their own keybindings handling instead of using the built-in one here. This is useful when the 3rd-party library is used to display all keybinding in an app and/or allow the user to customize the key bindings.

A nice way to do this could be to add a renderKeybindingsContainer similar to the other render functions. The default implementation would just call useTreeKeyboardBindings() and custom implementations could chose to call this and add additional key bindings or could skip calling it and completely implement their own key binding system.

Hm I thought about your suggestion, but at the moment you can also pretty much write custom keyboard handlers by just listening to keydown events yourself and use an imperative handle of the tree to manipulate the tree state yourself (https://rct.lukasbach.com/docs/guides/refs) or directly changing the state of a controlled tree environment. And if custom keybindings clash with existing ones, you can disable them by passing [] as keybinding to them. Would this be enough for your scenario?

but at the moment you can also pretty much write custom keyboard handlers by just listening to keydown events yourself

The problem I had with this is that there is no access to the internal state of each item to know when an item should respond to a key event or not. Most of the internal keybindings use isActiveTree && !dnd.isProgrammaticallyDragging && !isRenaming which is quite a chore to try to recreate and would need to be kept up-to-date with any internal changes to the react-complex-tree library.

Would this be enough for your scenario?

The reason I suggested an extra render hook is that because (as stated above) the key bindings need to have access to the tree state. In other words, they need to be able to use the useTree() hook. What I did for now is use the renderTreeContainer hook to wrap the default renderTreeContainer with the key bindings.
    return
{renderers.renderTreeContainer(props)}
;

So, in summary, exposing a state of when to enable/disable the key bindings is the major missing feature. A cleaner way to be able to inject the key binding element as a child of the Tree element would helpful but is not as critical.

there is no access to the internal state of each item to know when an item should respond to a key event or not

You should be able to get everything from the treeref, with ref.isRenaming, ref.isProgramaticallyDragging and ref.isRenaming you have access to all pieces of data as is internally used for internal keybindings. So you can use existing outwards-facing APIs to compose your hotkey logic rather than using internal logic which is likely more convinent.

which is quite a chore to try to recreate and would need to be kept up-to-date with any internal changes to the react-complex-tree library

The thing is, which properties you want to use to enable or disable depends on the keybinding you're implementing. Yes, for a lot of keybindings it makes sense to listen for those properties specifically and then I agree that it might be inconvenient to repeat this check everytime, but if I provide predefined logic for writing custom hotkeys that specifies this check automatically, you will not be able to write keybindings that e.g. work while renaming or during drag events, which might be important for specific use cases.


External Kafka SASL_SSL is using TLS true

Hi Team,
When we are using SASL_SSL for external kafka its expecting tls configuration rather it should go pass with SASL user based authentication only. please check the below snapshot and from the link. looks like we need to remove this piece of code.
https://github.com/zilliztech/milvus-operator/blob/main/pkg/external/kafka.go

Below is the kafka configuration. its going good in case of PLAINTEXT but expecting TLS in case of SALS_SSL.

@navneetrajput06 em... correct me if I'm wrong:
SSL is just an old name of TLS see https://www.cloudflare.com/learning/ssl/transport-layer-security-tls/ .
So just as the names suggests,
if you only need SASL, then you can use SASL_PLAINTEXT?
and if you need SASL together with TLS, then you can use SASL_SSL.


[D435 Not Working] Outdated UDEV-Rules after apt upgrade librealsense

Required Info

Camera Model
D435

Firmware Version
(Open RealSense Viewer --> Click info)

Operating System & Version
Ubuntu 14.04.6 LTS

Kernel Version (Linux Only)
4.4.0-148-generic

Platform
PC

SDK Version
{ legacy / 2.. }

Language
{C/C#/labview/nodejs/opencv/pcl/python/unity }

Segment
{Robot/Smartphone/VR/AR/others }

Issue Description
Today, an update was available for the librealsense package, so I executed apt upgrade command. After the update, RealSense is throwing the following error-
Timestamp:<PHONE_NUMBER>700.077637
Severity: Warn
Description: RealSense UDEV-Rules file:
 /etc/udev/rules.d/99-realsense-libusb.rules
 is not up-to date!
UDEV-Rules permissions configuration
 for RealSense devices.

Missing/outdated UDEV-Rules will cause 'Permissions Denied' errors
unless the application is running under 'sudo' (not recommended)
In case of Debians use:
sudo apt-get upgrade/install librealsense2-udev-rules
To manually install UDEV-Rules in terminal run:
$ sudo cp ~/.99-realsense-libusb.rules /etc/udev/rules.d/99-realsense-libusb.rules && sudo udevadm control --reload-rules && udevadm trigger

I downloaded the latest file from here (Github) and executed following commands as suggested but nothing happened-
ravi@lab:~/Desktop$ ll
total 508
drwxr-xr-x  3 ravi ravi   4096 Nov 23 19:09 ./
drwxr-xr-x 43 ravi ravi   4096 Nov 23 19:11 ../
-rw-rw-r--  1 ravi ravi   5285 Nov 23 19:09 99-realsense-libusb.rules
ravi@lab:~/Desktop$ sudo rm /etc/udev/rules.d/99-realsense-libusb.rules
[sudo] password for ravi:
ravi@lab:~/Desktop$ sudo cp 99-realsense-libusb.rules /etc/udev/rules.d/99-realsense-libusb.rules
ravi@lab:~/Desktop$ sudo udevadm control --reload-rules
ravi@lab:~/Desktop$ udevadm trigger

Please note that RealSense was working fine previously but not now.
Any workaround, please?

Chirping in: I have the same problem with freshly installed librealsense on NVidia Xavier (arm64 Ubuntu 18.04)

@ravijo hi, what SDK version are you running?
Can you also clarify whether you're using Debian packages or building the SDK from source as those methods have separate installation methods and targets.
In any event you should verify that a single udev-related file is installed, either with sudo apt install librealsense2-udev-rules or with sudo cp ~/.99-realsense-libusb.rules /etc/udev.....

@lostmsu , please provide the problem description:

SDK version used/ installation method
Error message encountered

@ev-mp , thank you very much for the response.

what SDK version are you running?

librealsense version shown by relasense-viewer is 2.28.1

Can you also clarify whether you're using Debian packages or building the SDK from source.

I am using a prebuild package via apt install.

you should verify that a single udev-related file is installed

How to verify it? I see a 99-realsense-libusb.rules file inside /etc/udev/rules.d/ directory.
PS: I noticed that I can ignore this message. I can get the images from the camera surprisingly.

@ravijo , I think it could be related to an issue with debian package that was fixed in v2.29, so I'd suggest to upgrade to v2.29/v2.30. (#4990)

PS: I noticed that I can ignore this message. I can get the images from the camera surprisingly.

The message is informative to make the user aware of having multiple and potentially non-synchronized udev-rules. It does not affect the SDK functionality.
With that being said  - if you stick to Debian packages then make sure to delete /etc/udev/rules.d/99-realsense-libusb.rules as recommended.

Hi @ravijo,
Any update? Is your issue resolved per @ev-mp's suggestion?

Thanks, @RealSenseCustomerSupport, and @ev-mp for the support. It worked.


Export jsonReferenceLoader or proper interface

Why you're not exporting it? If you don't want to export the struct directly, define an interface type for it.

Seems like JSONLoader is the exported interface. It would be better if you use it at function interface signature level to avoid doubts.
Example:
func NewReferenceLoader(source string) JSONLoader

+1 on this - would be very helpful.

The function signature was changed to return JSONLoader with #202.


initramfs-test-image: enable gpsd support

Bundle gpsd and gps-utils into initramfs-test-image. This cause
compressed image growth of 1.6 MiB.
Signed-off-by: Dmitry Baryshkov<EMAIL_ADDRESS>

well.. where do we set the limit? the description for this image is 'a small ramdisk'. Are we not adding too much here? I understand it's convenient, but how often do we really need gpsd in there? perhaps test-full instead?

Ack, test-full seems fine to me.
Maybe we'd need some light utility for working/validating GPS on db410c (and hopefully db820c)

Switched to initramfs-test-full-image


3rd party libs

@raix You're library for importing 3rd party libs is awesome. I think it might be nice if you made this a separate package, f.e. raix:library without any default libs. And it can be a proof of concept for Meteor to possibly adopt or make something like it.
@dandv ^ Check out raix:famono on Atmospherejs and see how it loads non-Atmosphere libraries for your project. This would prevent from having to PR organizations to make Atmosphere wrappers just for the sole purpose of shipping a library into Meteor; useful for importing libraries that don't add any meteor-specific functionality.

@raix Oh, is that what raix:include is for?

Yep, raix:include is just a tiny helper package for the meteor bower/github release i did. It's a proof of concept - the Nice thing about that approach is the use of the version resolver on 3rd party packages.
The include package should/could be removed by adding scope detection and isolation (we can do alot of thing here)


OpenApiSchema.Items can be a array

For tuple representation as array, OpenApiSchema.Items can be array of schemas - https://json-schema.org/understanding-json-schema/reference/array.html

JSON Schema Items can be an array, however OpenAPI Schema Items specifically prohibits this behavior https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#schema-object  One of the reasons being that it makes it because hard to create a model in strongly typed languages.


Discussion

This package defines a simple Extent object that gives you the bounds for each dimension covered by any spatial object - like the edges or a raster, or the min/max values for each dimension in a vector or table of points.
@evetion, this should work for the GeoInterface extent object. Although I'm wondering where we should put it...
A question I have with the design is if we can always assume we know the dimension names, and how often/where we don't know them. Currently the Extent object can hold a Tuple or a NamedTuple, to handle both cases. But I'm not sure how necessary that is.
@Tokazama if you have any input on this that would be useful at this early stage - especially to tie in with SpatioTemporalTraits.jl. The idea here is that all spatial objects have extents / bounding boxes, and we can have a generic Extent object that defines these in a standard way, returned by extent(obj). As much as possible an Extent will also have dimension names, as Symbols. Comparing named extents compares by name, ignoring order.
I guess extent will provide your spatial_order, spatial_first and spatial_last, although it doesn't distinguish the space/time difference.
Primarily this is for GeoInterface.jl and Rasters.jl and other GeoSpatial packages to be able to share extent information in a generic way. I will also use it in DimensionalData.jl.

I'm hoping we can separate the spatial vs temporal trait from other individual traits like this, so that they are more flexible, but I'll have to think about this some more.

I would assume we always have dimension names or packages could generate symbols on the fly.
Mixing both named and unnamed seems to complicate things, as the comparison order changes between them. We could split it off into an UnnamedExtent?

There was a discussion a while back in NamedDims and I was kind of pushing for NamedTuple to be used for more of the interface. There are two problems with this though:

We can't have repeating names, so if we have a default name value it doesn't work as a NamedTuple. One could argue that is a sign of an incorrect implementation, but I'd rather not get caught up in that.
We also want to support dynamically named dimensions sometimes. You could probably argue that you wouldn't need this package unless you new the names statically, but, again, I don't think it's necessarily worth the debate when a tuple would do.

Just my two cents.

@Tokazama thanks for the comments.  Tuple probably won't be enough for geospatial data because the dimension order is not consistent, but the dimension names are. And we want interop accross different point and dimension orders - even equality of Extents is defined to ignore order because its the area we care about most.
Maybe we can get around the repeated names problem by using a Tuple and type parameter Symbols directly without using the NamedTuple? DimensionalData/Rasters.jl can also have repeated dimension names after some statistical operations (and everything still works) so thats a good point.
For dynamic name changes, in what circumstances do the names of spatial dimensions need to change but you can't rebuild a new object with new names?
Having all of this work based on named comparisons, but also compile away completely, is very nice for e.g. writing fast rasterization algorithms with arbitrary source and target dimension order.
@evetion it is easier if we assume/force dimension names. Maybe we can start with that and see if it breaks anywhere.

Are the dimension names used purely to determine the order

Not purely order, also so we know they are the same kinds of dimension. For catching errors, like not using time and spatial Z as the same thing.


orb-ui: diamond: self-serve with cone support

UX with cone in Diamond self-serve mode.
fixes ORBP-113

@fouge shall we rebase on main and mark as ready for review?

@TheButlah no, not yet

@TheButlah i'm gonna have a look today

still relevant?

closing for now
will reopen when needed


Add navigation layers to AStar2D, AStar3D, and AStarGrid2D

Adds navigation layers to AStar2D, AStar3D, and AStarGrid2D. By default, all points will be on layer one and all functions that now take a layer argument will default to layer 1, so existing implementations should not be impacted.
AStar2D::add_point and AStar3D::add_point now have an optional argument to specify the layer bitmask when adding the point.
get_id_path, get_point_path for all support specifying a layer to use. Points that are not on that layer are ignored for pathfinding, as if they were disabled. This also means you can actually disable a point if you set it's layer to 0, but I kept the disabled functionality as a way to disable a point for all layers and preserve the point's layer data at the same time.
get_closest_point for AStar2D and AStar3D will respect the provided layer, and only return the closest point with the matching layer.
AStarGrid2D seems to constantly remake the points whenever you call update. I believe this will wipe out any custom layers you set as well, which may be an issue and should maybe be mentioned in the docs? I couldn't think of a way to preserve this data given the update function first clears all points. Open to suggestions on this!
Rational:
Instead of having to make duplicate AStar objects if some agents have slightly different path options, you can now use the same overall map data for 32 distinct path options. This greatly simplifies code for users.
For example, in my project the map is the same once generated but some units can only travel on some tiles. Water tiles, for example, cannot be traversed by land units and vice versa. Air units can traverse all types. Previously I've had to use 3 separate AStar objects and switch which one I am using based on the unit. After adding layers, I can just specify which layers the points on are to represent land, sea, air during generation, and then for pathfinding specify which layers the agent uses and use one map instead of duplicating the entire thing.
Example:
Download: astar_test.zip
This project features two agents, a Red and Blue. The Red and Blue tiles can be traversed only by those agents respectively. The project also has use UI elements to view the layers of the point the mouse is over and showcase the get_closest_point function respecting the layer option. It pulls the layer from the currently selected character in the option box.
astar_navigation_layers.webm
By default all "open" tiles have been set to layer 2 and 3 (so the value will display 6). Gray "obstacle" tiles are set to layer 0. The red character is on layer 3 (value 4) and the blue character is on layer 2 (value 2). Towards the end, you can that I have my mouse over tile 6,7 (id 497) but the "closest point" in the UI is "496" as that tile is on a different layer from the red character.

Outstanding questions:

I noticed some of the physics layer implementations have a distinct mask and mask_value function, where the former takes a bitmask and the later takes a layer as an int 1-32. Should I mirror that and have two functions, one to change the bitmask directly and one to specify the layer? Setting layer by direct int is a bit easier, but would not support multiple layers for "shared" points, so I'm not sure if that extra function would be considered a nice utility or just bloat.
What should we do about AStarGrid2D::update wiping out any saved layer changes? Just add a documentation note about it? Change that function to remember all the layers for all points and somehow reapply them?

Previous questions, leaving here for community knowledge reasons

I need to add the compat functions, but I am not sure the proper procedure when a function already has one. I added one for get_id_path and get_point_path in a previous PR, and this one will be modifying those functions again. Is the correct approach to entirely remove those old compat methods and replace them with the new ones, or do I create separate compat functions and bind them again? I couldn't find a clear instruction on this.
-> I was told they get additional ones, which I have now added.

Closes: https://github.com/godotengine/godot-proposals/issues/5188

Thanks for opening a pull request!
Feature pull requests should be associated to a feature proposal to justify the need for implementing the feature in Godot core. Please open a proposal on the Godot proposals repository (and link to this pull request in the proposal's body).

Thanks for opening a pull request!
Feature pull requests should be associated to a feature proposal to justify the need for implementing the feature in Godot core. Please open a proposal on the Godot proposals repository (and link to this pull request in the proposal's body).

I believe the closest one would be https://github.com/godotengine/godot-proposals/issues/5188
Specifically: https://github.com/godotengine/godot-proposals/issues/5188#issuecomment-1219349720
I couldn't find precisely one "feature proposal" that was purely for navigation layers in AStar, but it's scattered around in a few.
There was an initial tackle of this issue with PR https://github.com/godotengine/godot/pull/64641 however it did not target master and also had additional changes involved so was not accepted.
Would you like me to open a feature that is specific to navigation layers for AStar?

That seems to cover this well so I'd say to link it with Closes: https://github.com/godotengine/godot-proposals/issues/5188 in the fist message of the PR

Hm, I've added compat functions for the expected changes in 4.3, but it seems the test fails for 4.2, 4.1, and 4.0 with

size changed value in new API, from 2 to 4.

There is already a compat function in the 4.2 file to handle the change from 2 to 3 arguments.
Does this not combo with the compat function then introduced here from 3 to 4?
Do I need an additional set of functions to handle 2 to 4?

The main thing preventing this from passing the tests is the issue I mentioned above.
I'm not sure how to handle the compat errors for older versions when the API changes again.


add aria-label to announce checkbox status for checkbox column

This PR addresses the checkbox status being announced part of https://github.com/microsoft/azuredatastudio/issues/22156 should also address https://github.com/microsoft/azuredatastudio/issues/22187.
The aria-label for the checkbox select column was originally added back in https://github.com/microsoft/azuredatastudio/pull/7008, moved in https://github.com/microsoft/azuredatastudio/pull/13644, then was removed last year in https://github.com/microsoft/azuredatastudio/pull/20514. This PR brings back the aria-label so that "checkbox checked" or "checkbox unchecked" gets announced when the focus is on the checkbox in a table.
before:

after:

closing this since I switched to using the DeclarativeTableComponent in #22390, which announces checkbox status


Update IEnderZooMob to extend IMob

If they're mobs, they're mobs.

I forgot that was a thing, I mean most of the EZ mobs already extend IMob classes I believe, but this will be good for the future.


Update session id logic for dtls13

Ci failure looks irrelevant

This pull request is ready to merge

Pushed.


CASSANDRA-9352

Change all instances of 3.0 to 2.2. Tests here https://cassci.datastax.com/job/beobal-9352-dtest/

PR by @beobal. +1 from me.


Issue with this line

https://github.com/francojc-lin380s24/lab-04/blob/3e5fa3b80e4562b345e89e2d3216faa0ed8ca5ba/README.md?plain=1#L3
I can't figure out why this isn't working!

@francojc could you help me with this?


A bunch of EU resorts and one japanese resort.

First attempt at this Pull request stuff, hope that it is right. :)

@pirxpilot ?

Thanks for the PR. Sorry it took me so long to review it.
I am going to add some general remarks here - and also add some comment and suggestions inline later on.

it makes things easier for me if PRs contain one resort or one type of resorts at a time - in this way if there is a problem with one resort it does not block another - a separate branch for each newly added resort and a PR based off that branch makes things much easier; obviously resorts that share the same parser can and should share a PR
resort name in the descriptor is something that users see in the UI, resort slug (the name of the directory in which the resort.json resides) is a part of the URL and liftie is always using kebab-case for it - so alta-badia and not Alta-Badia and les-4-valles and not les_4_valles
if several resorts have the same parser the common code should end up in /lib/tools  - see for example how /lib/tools/vail.js is used in the code
ll is not optional and it cannot be set to null - location is used be weather forecast and - where available - webcams
please remove the code that you are removing instead of commenting out the old code and adding the new one
make sure that the tests are passing before submitting PR

If you can submit a single resort PR we can work out the kinks and land it in liftie. If not I'll try to cherry pick from what I see here. Thanks again.

Thanks for the PR. Sorry it took me so long to review it.
I am going to add some general remarks here - and also add some comment and suggestions inline later on.

it makes things easier for me if PRs contain one resort or one type of resorts at a time - in this way if there is a problem with one resort it does not block another - a separate branch for each newly added resort and a PR based off that branch makes things much easier; obviously resorts that share the same parser can and should share a PR
resort name in the descriptor is something that users see in the UI, resort slug (the name of the directory in which the resort.json resides) is a part of the URL and liftie is always using kebab-case for it - so alta-badia and not Alta-Badia and les-4-valles and not les_4_valles
if several resorts have the same parser the common code should end up in /lib/tools  - see for example how /lib/tools/vail.js is used in the code
ll is not optional and it cannot be set to null - location is used be weather forecast and - where available - webcams
please remove the code that you are removing instead of commenting out the old code and adding the new one
make sure that the tests are passing before submitting PR

If you can submit a single resort PR we can work out the kinks and land it in liftie. If not I'll try to cherry pick from what I see here. Thanks again.

I see, thanks for the information :)
I was thinking that maybe the whole of the "interwebs" domain could be made into a tool ? under the "tools" folder, similarily to how boyne, aspen and intrawest are. As the way each resort is setup is obv the same and adds a 60-100 ~ areas in especially Austria, but also some other european and international places ?
Then the other ones could be added individually like requested ?
What do you think ?

I was thinking that maybe the whole of the "interwebs" domain could be made into a tool ? under the "tools" folder, similarily to how boyne, aspen and intrawest are. As the way each resort is setup is obv the same and adds a 60-100 ~ areas in especially Austria, but also some other european and international places ?
Then the other ones could be added individually like requested ?
What do you think ?

Yup. That would be my preference as well.

I am pretty sure that all new resorts that were included here (not based on intermaps) are now released. I am going to close this PR now. Please open new/separate PRs once you have more fixes or changes ready. Thanks again!


Why Box instead of plain T?

Why does Managed required you to put the contents on the heap? Why doesn't it just look like this:
pub enum Managed<'a, T: 'a + ?Sized> {
    Borrowed(&'a mut T),
    #[cfg(/* Box available */)]
    Owned(T), // <--- no Box
}

Oh, I assume mainly for trait objects?

Correct.

Ok thanks!
I was irritated by the EthernetInterface of smoltcp, which uses managed to store a generic device type: https://github.com/m-labs/smoltcp/blob/f64a99a4e6d744c3a6f3d9a54a0fb3f94e4b0ec8/src/iface/ethernet.rs#L28
Is this a mistake or am I still missing something?

A mistake I think.


Automated CAPV(CD) controller updates

automatically consuming the latest upstream changes for our CAPV and CAPVCD controllers

Do we really need this considering the upstream situation?

Considering this, probably no


Feature request: static var

I would like static var within a function to create a variable scoped outside the function which can only be accessed from within the function.  The intent is to act as a variable within the function that retains its value between invocations.
An example is below:
Input:
function newId() {
    static var id = 0;
    return id++;
}

Desired compiled output:
var newId = ( function() {
    var id = 0;
    return function newId() {
        return id++;
    };
}() );

Usage:
newId(); // 0
newId(); // 1
newId(); // 2

I don't believe static let or static const make sense or would be useful; static var should be sufficient.

Very good syntactic sugar idea for declaring closures!
Indeed closures are static in nature since there's only 1 copy of it and it's scope shared.
Although I don't see why a scope couldn't be let or const since JS allows those to be closures as well.

@GoToLoop Thank you.
I suggested not supporting let and const because it occurred to me that block scoping could be problematic for a variable that is actually scoped outside (and therefore across) the function.  This might be solvable with name mangling (as when block-scoped variables compile down to ES3 or ES5) but I thought it might be easier to get a feature passed if the use-case was kept as simple as possible.  Beyond that, I really couldn't think of a place where a static variable would be useful scoped to a block within a function.
It would be really useful for methods as well, but unfortunately due to the prototypal nature of methods, a static var would end up being shared across all instances (just like a static property).  This might be useful for some scenarios, but none I can think of.

All those 3 declaration styles: var, let & const create a scope which exists not only for the declared current scope, but for any inner scopes created below them. That's how closure works btW.
And since both let & const create closures just like var, I don't see why they couldn't be allowed.
Even if it might  make the whole feature implementation a little more complex.
Of course var is hoisted to the top of the function no matter where it was declared within that.
While let & const respects the {} limits where they're declared in.

The implementation I suggested performs a similar "hoist" as var, it just hoists outside the function, inside an implicit closure surrounding it.  That's why it seems to make more sense as with var, because the behavior is more similar to how var works.

Your excerpt but replacing var id w/ let id:
var newId = ( function() {
    'use strict';

    //var id = 0;
    let id = 0; // static closure

    return function newId() {
        return id++;
    };
}() );

for ( var i = 3; i--; console.log(newId()) );  // 0 1 2

As you can see, it works regardless id is declared as var or let.
That'd the point I'm trying to make that if JS itself allows closures w/ let & const, they shouldn't be left out!

The example is too simplistic for let or const, and your example output is ES6.  The example function does not contain any blocks.  Try a more complex example:
function newId( flag: boolean ) {
    if( flag ) {
        let static id = 10000;
        return id++;
    } else {
        let static id = 0;
        return id++;
    }
}

This would become in ES5:
var newId = ( function() {
	var id_1 = 10000,
		id_2 = 0;
	function newId( flag: boolean ) {
		if( flag ) {
			return id_1++;
		} else {
			return id_2++;
		}
	}
}() );

You end up with a variable name clash that needs to be resolved by mangling (even in ES6, because these end up in the same scope after hoisting).

Indeed a good point. Although it seems a road block, it's actually easy to solve!
No matter how many times a closure static variable w/ the same name happens to be redeclared, it still should exist only 1 declaration in some scope above them.
That is, all static declarations are erased and hoisted into the parent scope.
No matter whether they're static var, static let or static const, if they're the same name, they'll all be hoisted as 1 unique closure declaration.
When emitting for ES6, the parser gonna honor the static declaration type.
That is, if they happen to be static const id, it's gonna be hoisted as const id.
Now for ES5 & ES3, that's even easier: they're all hoisted as a simple var no matter whether they're happen to be let or const.

While this is clearly useful for the "make a counter" function, it doesn't seem more broadly useful enough to justify adding a new runtime feature to the language. If this actually has a bunch of use cases I'm not aware of, it's probably worth taking it to the ES Discuss group as a proposal for ES7+ since it's a runtime feature rather than a type system feature.

Indeed it's just syntactic sugar to make easier to create closures w/ a more simple syntax by declaring a local variable as static.
I'm not the OP, but I pretty knew it wouldn't be approved. But I'm not getting the reason given to it.
This feature is costly prohibitive for runtime. In no way I'd ask such feature for the JavaScript language!
This is merely a smart pre-compiler text replacement that only a transpiler such as TypeScript would be fit to do it.

If it's as simple as a text replacement, I don't see how it could be prohibitively costly for runtimes to implement.
The set of expression-level features that should exist in compile-to-JS languages that don't belong in the JS language itself ought to be very small. Outside of the type system, we have a very short list of runtime syntax sugar: namespaces, enums, class property initializers, and JSX. If there's a runtime feature proposal, the ES committee should usually be the first to take it.
It's been discussed a few times (as early as five years ago) there without any concrete action being taken, which generally confirms that this isn't necessary sugar.
https://esdiscuss.org/topic/static-keyword-from-c-c-as-own-closured-var-declaration
https://esdiscuss.org/topic/static-local-variables

I had no idea that would be also an old JS proposal feature.
Nonetheless, I still think doing it at compile time is much more efficient performance wise.
Analyzing the whole scope looking for local static declarations and replacing them as 1 parent closure seems a daunt job for me.

@RyanCavanaugh I concede your argument that this feature may not be appropriate for TypeScript.
I do, however, think that the applications are much more broad than "make a counter" functions.  It can be used for any function that maintains an internal state.  (Though some may argue a class would be more appropriate; I like that it can keep its internals hidden from external view.)
For example, memoization:
function fibonacci( n: number ) {
	type Memo = { [ input: number ]: number; }
	static var memo: Memo = { 0: 1, 1: 1 };
	if( n in memo ) return memo[ n ];
	return memo[ n ] = fibonacci( n - 1 ) + fibonacci( n - 2 );
}

I respect your decision for not including it in the language, but I think that there are many such uses.   Luckily, the feature request is just sugar.  I can still accomplish this by using a closure.


Add bluedroid SBC encoder/decoder to Zephyr as an external library

Origin
The Android bluedroid SBC:
https://android.googlesource.com/platform/external/bluetooth/bluedroid/+/refs/heads/main/embdrv/sbc
Purpose
A2DP support in Zephyr requires encoder/decoder support of SBC. Initial A2DP support in Zephyr added with #31583 .
Mode of integration
This would be a new Zephyr module with libsbc as the name. I am using the name libsbc similar to liblc3. There will be very limited code changes and add one porting file based on bluedroid code to support building.
Demonstration integration PR:

#70531

Maintainership
Maintainers - @MarkWangChinese
Description
The SBC encoder and decoder is mandatory component to complete support of A2DP in Zephyr.  Android bluedroid maintains the SBC project and is Apache2.0 licensed.
I am implementing one middle layer libsbc/sbc.c that is in line with zephyr's style and used in Zephyr. it calls the bluedroid SBC.
Dependencies
It is only used by A2DP now.
Revision
Temporary repository https://github.com/MarkWangChinese/libsbc for review.
License
Apache License, Version 2.0
For example: the license of https://android.googlesource.com/platform/external/bluetooth/bluedroid/+/refs/heads/main/embdrv/sbc/encoder/srce/sbc_encoder.c is as follow:
/******************************************************************************
 *
 *  Copyright (C) 1999-2012 Broadcom Corporation
 *
 *  Licensed under the Apache License, Version 2.0 (the "License");
 *  you may not use this file except in compliance with the License.
 *  You may obtain a copy of the License at:
 *
 *  http://www.apache.org/licenses/LICENSE-2.0
 *
 *  Unless required by applicable law or agreed to in writing, software
 *  distributed under the License is distributed on an "AS IS" BASIS,
 *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 *  See the License for the specific language governing permissions and
 *  limitations under the License.
 *
 ******************************************************************************/

I will wait it is approved and the repository is created, then I can push the codes and create PR.

It would help us make a decision if there is a draft PR that shows how this will be integrated.

I create one repository as https://github.com/MarkWangChinese/libsbc, please help to check. And one porting related file is put in zephyr repository (modules/libsbc/data_types.h) in pr https://github.com/zephyrproject-rtos/zephyr/pull/31583.

@nashif, see #70531 as a reference.
We would like to get things setup. It is unclear to me if we actually voted on this in the TSC or not but I do remember multiple discussions. What should the next steps be?

@nashif @carlescufi
The TSC notes said we were waiting on a PR reference. @MarkWangChinese posted one so need to figure out what we need to do next to get this moving.

@nashif @carlescufi
The TSC notes said we were waiting on a PR reference. @MarkWangChinese posted one so need to figure out what we need to do next to get this moving.

the PR needs to be referenced i nthis issue, there is a section dedicated to that.

@nashif @carlescufi
The TSC notes said we were waiting on a PR reference. @MarkWangChinese posted one so need to figure out what we need to do next to get this moving.

the PR needs to be referenced i nthis issue, there is a section dedicated to that.

The issue description is updated, please help to check.

@nashif What do we need to do next on this?

@dleach02 @nashif I have removed the Stale Label. Has this been discussed in the TSC

@MarkWangChinese we discussed this request in TSC today (Dec 11, 2024). Please reopen and refresh your example integration PR #70531

@MarkWangChinese we discussed this request in TSC today (Dec 11, 2024). Please reopen and refresh your example integration PR #70531

Thanks @dleach02 I create one new pr https://github.com/zephyrproject-rtos/zephyr/pull/82942 because the old pr use discarded  nxp-zephyr repo.


trimaps used for testing

I saw you used dilation/erosion to generate trimaps during testing. However, Adobe provided trimaps which should be used for testing for fair comparison. Correct me if I am wrong. thanks.

@yxt132 You are right, please take a look at about the result.


no published sdist for 2.4.2

I'm trying to prepare a build of sacrebleu from source but there is no sdist on pypi for the 2.4.2 release. Is that an oversight or are you no longer publishing sdists?

I confirm https://pypi.org/project/sacrebleu/2.4.2/#files is missing sacrebleu-2.4.2.tar.gz (it includes only sacrebleu-2.4.2-py3-none-any.whl). I guess @mjpost will confirm it is an oversight.
Meanwhile, you can obtain https://github.com/mjpost/sacrebleu/archive/refs/tags/v2.4.2.tar.gz from GitHub.
BTW: sacrebleu is a pure-Python package (no compilation of C/C++ is needed), so even the whl file contains (just) all the source codes.

Thanks, @martinpopel. The tool we're working with automates builds from sdists, so I thought it was worth checking and I appreciate the quick response. We can do some work to configure it to build from that source archive if we have to, but if you consider it an oversight I may just wait a day or so to see if you have time to upload the sdist.
I'd be happy to help you set up a GitHub action for publishing releases from tags, too. I have it configured in several repos, and find it really simplifies things. If you're interested, I could send you a PR and help you debug any issues setting up the authentication. Let me know.

Hi @dhellmann, this would be great to have. We'd be very grateful you'd contribute a PR and tell us how to make use of the feature. It's one of those things I can never seem to make time for.

Great, I'll be happy to help. Today is pretty full, but I should have time later this week to prepare a PR for you based on what I do in other repos like this: https://github.com/python-wheel-build/fromager/blob/main/.github/workflows/python-publish.yaml
There's a link in the top of that file that points to the GitHub.com docs for publishing python packages.
The main thing that I stumbled over when setting up the trusted publishing hook was failing to set the "environment" value.


Set command converts the value to string by default

if one JSON object is stringfied by  JSON.stringify  and saved to redis by set. When it is get and been JSON.parse, it's still string.
Then by testing I find the value is converted to string by default, I don't know if this is a feature by design. there're no description in API document.

Using JSON.stringify() and JSON.parse() is exactly how I'm using ioredis/redis and when I get the value and pass it through JSON.parse() I get an object.  JSON.parse() returns an object and does not modify the original input.
origValue = { a: 1, b: 2 };
key = 'abc';
redis.set(key, JSON.stingify(origValue));
value = redis.get(key);
parsedValue = JSON.parse(value);

value is still a string.  parsedValue equals origValue.


Variable expansion doesn't work

One of our prod query panels uses
${Services:singlequote}

to expand a list of select-able services as a list of single-quoted strings.
It seems like the backend datasource needs to understand how to handle these on its own.
https://grafana.com/docs/grafana/latest/dashboards/variables/variable-syntax/#advanced-variable-format-options

@flobernd is this something that can be done from the frontend before the query is sent to the backend? It seems like the front end ought to have the information available to do this. I'm not sure the backend does.

Relevant: https://community.grafana.com/t/how-to-use-template-variables-in-your-data-source/63250

@philhofer I will have a look! I remember there was a frontend service that can be used for variable expansion. Not sure if it handles this specific case by default, but I'll test it.

The linked article says this is what you have to do for backend datasources:
export class DataSource extends DataSourceWithBackend<MyQuery, MyDataSourceOptions> {
  constructor(instanceSettings: DataSourceInstanceSettings<MyDataSourceOptions>) {
    super(instanceSettings);
  }

  applyTemplateVariables(query: MyQuery, scopedVars: ScopedVars): Record<string, any> {
    const interpolatedQuery: MyQuery = {
      ...query,
      rawQuery: getTemplateSrv().replace(rawQuery, scopedVars),
    };

    return interpolatedQuery;
  }
}

@philhofer Yes, that looks like the code I used in the initial frontend version of the plugin. Probably forgot to add it again while converting to a backend plugin. I'll create a PR soon.


Communication: Show number of unread messages

Checklist
General

[x] Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
[x] I chose a title conforming to the naming conventions for pull requests.

Server

[x] I followed the coding and design guidelines.
[x] I added multiple integration tests (Spring) related to the features (with a high test coverage).
(https://docs.artemis.ase.in.tum.de/dev/guidelines/server/#rest-endpoint-best-practices-for-authorization) and checked the course groups for all new REST Calls (security).
[x] I implemented the changes with a good performance and prevented too many database calls.
[x] I documented the Java code using JavaDoc style.

Client

[x] Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
[x] I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
[x] I documented the TypeScript code using JSDoc style.
[x] I added multiple screenshots/screencasts of my UI changes.

Motivation and Context

When directly communication with other students or tutors we can only see an icon that there is new message but not how many messages. In this PR I have implemented a solution where now its possible to count how many messages are unread for a specific participant in a conversation. This feature was discussed with @stefanwaldhauser
Description

This PR introduces a new field unread_messages_count to conversation_participant table that will allow to store the number of unread messages of each participant.
The number of unread messages is found by querying all messages after the last_read message of participant date.
last_read is compared with creation_date of the message and if the creation_date of a message is bigger than last_read it will count as unread message.
Steps for Testing

Prerequisites:

1 Course
2 Users register at the same course

Log in to Artemis in 2 different browsers with each user
Navigate to a Course and then go under messages tab
user1 sends message to user2
user2 checks if the number of unread messages increases ( have the conversion with user1 unselected! )

Review Progress

Code Review

[x] Review 1
[x] Review 2

Manual Tests

[x] Test 1
[x] Test 2

Test Coverage

Class/File
Branch
Line

MessageService.java
62%
92%

ConversationService.java
93%
99%

ConversationParticipant.java
n/a
91%

unread-messages.pipe.ts
75%
100%

conversation-sidebar.component.ts
84.61
88.63

Screenshots

Before:

After:

Codecov Report
Base: 79.90% // Head: 75.54% // Decreases project coverage by -4.36% :warning:

Coverage data is based on head (428042d) compared to base (a1ba7f2).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #5858      +/-   ##
=============================================
- Coverage      79.90%   75.54%   -4.37%
=============================================
  Files           2115     1108    -1007
  Lines          83728    43748   -39980
  Branches       12539     8268    -4271
=============================================
- Hits           66906    33051   -33855
+ Misses          9351     5582    -3769
+ Partials        7471     5115    -2356

Flag
Coverage Δ

client
75.54% <0.00%> (-0.02%)
:arrow_down:

server
?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

...ersation-sidebar/conversation-sidebar.component.ts
74.46% <0.00%> (-5.08%)
:arrow_down:

.../artemis/domain/metis/ConversationParticipant.java

...java/de/tum/in/www1/artemis/domain/metis/Post.java

...ww1/artemis/service/metis/ConversationService.java

.../in/www1/artemis/service/metis/MessageService.java

...vice/connectors/bamboo/dto/BambooBuildPlanDTO.java

...n/www1/artemis/domain/enumeration/DiagramType.java

...n/www1/artemis/web/rest/dto/PageableSearchDTO.java

...ecurity/ArtemisInternalAuthenticationProvider.java

...m/in/www1/artemis/security/PBEPasswordEncoder.java

... and 998 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

The number of unread messages increases as expected when sending a new message to a user who does not have the conversation open. Is the number of unread messages supposed to stay the same if the user who sent the message deletes it before being seen by the other user, or should it decrease? As it is now it stays the same.

It should decrease in my opinion, it seems that i missed this case. I will make the changes to support it.
Thank you for the remark.

Request: Could you also please test the following on the testserver in a testsession: What happens if the users really message quickly to each other. Will there be a conflict message?
Because with channels , group chats we will have hundreds of users potentially messaging in a channel.

Hi @tobias-lippert ,
Thanks for the remarks.

A lot of internal server errors occurred.

When exactly those error occurred? When trying to get messages?

Message counter showed two new messages although I only sent one.

Maybe the same message was sent twice, if so counter counts it.

I would also like the counter to be more visible, e.g. using a red color as it is done in other messaging apps.

Yes red is more noticeable, but some apps like slack use greyish color in desktop or browser view and thats why i did not use red color.  But yeah maybe its a better idea to change to red.

It doesn't work for me on ts 1

Hi @Yunqing-Wang
Normally it should have worked but i don't know. Can you try again? There was a problem with database change log file so maybe that was the problem.

please add the test coverage in the required table format into the PR description

Did you also make sure that the performance is good? It's not enough to just tick the checkmark!
Can you increase the coverage in unread-messages.pipe.ts? 50% is too low.
Also, there is a merge conflict now. After fixing the remaining issues, please ask for re-approvals

Did you also make sure that the performance is good? It's not enough to just tick the checkmark!

@krusche  I used Jmeter tool for testing the performance:
The performance of new changes is almost same as develop branch. The conversation was populated with 500 messages.

Performance test Requests
Load Time (develop)
Load Time (feature/unread-messages)

Get Messages
155 ms
130 ms

Send new Message
54 ms
46 ms

Here you can find some screenshots from Jmeter.
jMeter.zip

@pellumbbaboci please resolve the merge conflict in the client tests

@pellumbbaboci please resolve the merge conflict in the client tests

@krusche conflict is resolved

Codecov Report

Merging #5858 (c9c6f58) into develop (f1abaca) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #5858   +/-   ##
==========================================
  Coverage      79.91%   79.91%
- Complexity     12379    12383    +4
==========================================
  Files           2124     2125    +1
  Lines          84491    84505   +14
  Branches       12676    12675    -1
==========================================
+ Hits           67518    67536   +18
+ Misses          9440     9438    -2
+ Partials        7533     7531    -2

Impacted Files
Coverage Δ

...ersation-sidebar/conversation-sidebar.component.ts
80.24% <ø> (+0.70%)
:arrow_up:

.../artemis/domain/metis/ConversationParticipant.java
93.33% <100.00%> (+1.66%)
:arrow_up:

...ww1/artemis/service/metis/ConversationService.java
97.01% <100.00%> (+2.98%)
:arrow_up:

.../in/www1/artemis/service/metis/MessageService.java
85.07% <100.00%> (+2.93%)
:arrow_up:

...in/webapp/app/shared/pipes/unread-messages.pipe.ts
100.00% <100.00%> (ø)

Flag
Coverage Δ

client
75.76% <100.00%> (+<0.01%)
:arrow_up:

server
84.53% <100.00%> (+0.01%)
:arrow_up:

Continue to review full report at Codecov.

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

Codecov Report

Merging #5858 (bb25ff2) into develop (6a7bfe2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #5858   +/-   ##
==========================================
  Coverage      79.91%   79.91%
- Complexity     12379    12383    +4
==========================================
  Files           2124     2125    +1
  Lines          84491    84505   +14
  Branches       12676    12675    -1
==========================================
+ Hits           67518    67536   +18
+ Misses          9440     9438    -2
+ Partials        7533     7531    -2

Impacted Files
Coverage Δ

...ersation-sidebar/conversation-sidebar.component.ts
80.24% <ø> (+0.70%)
:arrow_up:

.../artemis/domain/metis/ConversationParticipant.java
93.33% <100.00%> (+1.66%)
:arrow_up:

...ww1/artemis/service/metis/ConversationService.java
97.01% <100.00%> (+2.98%)
:arrow_up:

.../in/www1/artemis/service/metis/MessageService.java
85.07% <100.00%> (+2.93%)
:arrow_up:

...in/webapp/app/shared/pipes/unread-messages.pipe.ts
100.00% <100.00%> (ø)

Flag
Coverage Δ

client
75.76% <100.00%> (+<0.01%)
:arrow_up:

server
84.53% <100.00%> (+0.01%)
:arrow_up:

Continue to review full report at Codecov.

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


C:\shells\msys2bash.cmd fails due to BOM

Description
In the Windows images, there is a file C:\shells\msys2bash.cmd, probably created from https://github.com/actions/virtual-environments/blob/main/images/win/scripts/Installers/Configure-Shell.ps1. It is meant to run a command in a MSYS2 bash. This fails to run, probably due to a UTF BOM at the start of the CMD file:
2021-04-02T04:57:06.0149259Z ##[section]Starting: CmdLine
2021-04-02T04:57:06.0382299Z ==============================================================================
2021-04-02T04:57:06.0383399Z Task         : Command line
2021-04-02T04:57:06.0384475Z Description  : Run a command line script using Bash on Linux and macOS and cmd.exe on Windows
2021-04-02T04:57:06.0385317Z Version      : 2.182.0
2021-04-02T04:57:06.0386245Z Author       : Microsoft Corporation
2021-04-02T04:57:06.0387323Z Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/command-line
2021-04-02T04:57:06.0389026Z ==============================================================================
2021-04-02T04:57:07.6896215Z Generating script.
2021-04-02T04:57:07.7044907Z Script contents:
2021-04-02T04:57:07.7054035Z C:\shells\msys2bash.cmd -c "echo C:\shells\msys2bash.cmd: success"
2021-04-02T04:57:07.7552674Z ========================== Starting Command Output ===========================
2021-04-02T04:57:07.7946594Z ##[command]"C:\Windows\system32\cmd.exe" /D /E:ON /V:OFF /S /C "CALL "D:\a\_temp\7661d88f-f5b2-4d0e-88ed-7b1e88336786.cmd""
2021-04-02T04:57:07.8243816Z '��@' is not recognized as an internal or external command,
2021-04-02T04:57:07.8246375Z operable program or batch file.
2021-04-02T04:57:07.9254634Z ##[error]Cmd.exe exited with code '1'.
2021-04-02T04:57:07.9972484Z ##[section]Finishing: CmdLine

The same CMD file created manually without BOM works.
Area for Triage:

Question, Bug, or Feature?:

Bug
Virtual environments affected

[ ] Ubuntu 16.04
[ ] Ubuntu 18.04
[ ] Ubuntu 20.04
[ ] macOS 10.15
[ ] macOS 11.0
[ ] Windows Server 2016 R2
[x] Windows Server 2019

Image version

Azure Pipelines, 2021-04-01
Expected behavior
The CMD file shall run a MSYS2 bash.
Actual behavior
The CMD file fails to start.
Repro steps
Minimal example, including working version:
https://github.com/dg0yt/superbuild/tree/azp-msys
https://dev.azure.com/dg0yt/superbuild/_build/results?buildId=293&view=results

Hi @dg0yt! Thanks for raising this issue since we were not aware of the problem.
We will have a look and make a fix.

Hello, @dg0yt
Thank you for mention about the issue. It will be fixed - https://github.com/actions/virtual-environments/pull/3086
jobs:
  build:
    runs-on: windows-2019
    steps:

      - name: change encoding
        run:  |
          $shellPath = "C:\shells"
          @'
          @echo off
          setlocal
          IF NOT DEFINED MSYS2_PATH_TYPE set MSYS2_PATH_TYPE=strict
          IF NOT DEFINED MSYSTEM set MSYSTEM=mingw64
          set CHERE_INVOKING=1
          C:\msys64\usr\bin\bash.exe -leo pipefail %*
          '@ | Out-File -FilePath "$shellPath\msys2bash.cmd" -Encoding ascii
      - name: echo "Hello"
        shell: C:\shells\msys2bash.cmd {0}
        run: echo "Hello"

Thanks! From a long journey to using vcpkg for native MSYS2 builds...

The image with fix has been deployed.


Use ocp location instead ocp-dev-preview for mirror

Since 4.8 have rc bits available it is good time to switch to
latest 4.8 candidate directory then using ocp-dev-preview location.

This is the same as #443 isn't it?

/close


Should provide helpers for quickly standing up the required database structure

via Notion API

Closing in favor of #30


Installation fails

This package is failing to install for me on:
Atom 1.0.19
Windows 8.1 x64
Installing<EMAIL_ADDRESS>failed.

><EMAIL_ADDRESS>install C:\Users\Landon\AppData\Local\Temp\apm-install-dir-115921-231128-1opv8ga\node_modules\language-markdown\node_modules\pathwatcher\node_modules\runas
> node-gyp rebuild

C:\Users\Landon\AppData\Local\Temp\apm-install-dir-115921-231128-1opv8ga\node_modules\language-markdown\node_modules\pathwatcher\node_modules\runas>if not defined npm_config_node_gyp (node "C:\Users\Landon\AppData\Local\atom\app-1.0.19\resources\app\apm\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node  rebuild )
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
MSBUILD : error MSB4132: The tools version "2.0" is unrecognized. Available tools versions are "4.0".

gypnpm ERR! Windows_NT 6.2.9200
npm ERR! argv "C:\\Users\\Landon\\AppData\\Local\\atom\\app-1.0.19\\resources\\app\\apm\\bin\\node.exe" "C:\\Users\\Landon\\AppData\\Local\\atom\\app-1.0.19\\resources\\app\\apm\\node_modules\\npm\\bin\\npm-cli.js" "--globalconfig" "C:\\Users\\Landon\\.atom\\.apm\\.apmrc" "--userconfig" "C:\\Users\\Landon\\.atom\\.apmrc" "install" "C:\\Users\\Landon\\AppData\\Local\\Temp\\d-115921-231128-m1k57s\\package.tgz" "--target=0.30.7" "--arch=ia32"
npm ERR! node v0.10.40
npm ERR! npm  v2.13.3
npm ERR! code ELIFECYCLE

npm ERR<EMAIL_ADDRESS>install: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the<EMAIL_ADDRESS>install script 'node-gyp rebuild'.
npm ERR! This is most likely a problem with the runas package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node-gyp rebuild
npm ERR! You can get their info via:
npm ERR!     npm owner ls runas
npm ERR! There is likely additional logging output above.

From a bit of searching it almost looks like I would need to install VS2013 (or higher)? https://github.com/chjj/pty.js/issues/60
As this hasn't been required for any other Atom package that seems a bit odd...
If you need any more information please let me know.

That is weird. I will look into this tomorrow.

@Arcanemagus I've checked and updated the dependencies (I saw we used an older version of pathwatcher) so perhaps this might do the trick. Could you un- and re-install the package to see if this solves this?

@Arcanemagus Another user on Windows had the same issue, and it appeared to be because of a wrong version of Python. I have no idea where that comes into play. Anyway, try running npm install runas from the language-markdown directory, or perhaps install it globally. If things go according to plan, it will fail building runas and show some more specific output. After that I'm afraid I can't help you any further than by directing you to their repository. Let me know how things play out.

Yep, definitely looks like an issue with the runas repository. Although I'm not seeing the python issue (I have 2.7.10 installed).
I've filed https://github.com/atom/node-runas/issues/19 over there. Marking this as closed since it's not an issue with this package :wink:.

Thanks for reporting the issue anyway!

So it might be quite simple to work around this in this package. You are requiring node-pathwatcher, which is what is requiring node-runas, which is what is causing all these issues.
From what I could see looking over the code, the only place you are utilizing this is here to synchronously get a listing of files. Is there a reason you are using this package instead of a simpler method?

I found the Directory class as part of the Atom API so I thought it would be safe to use. Apparently not, or I'm requiring the 'wrong' pathwatcher. But as you suggest, there might be a simpler way of getting the list of files, probably via fs. I'll have a look at that. Thanks for your help!

Hey @Arcanemagus, I've just published a v0.1.2 version that doesn't depend on pathwatcher directly, but requires Directory as provided by Atom. I assume this should work better. Would you mind giving the update a try?

Sure, I'll try it when I get into work tomorrow, thanks!

Sorry it took so long to get back to you, v0.1.2 installed with no issues.

Not a problem, and thanks for checking!


[Package Request]: Service Bus Explorer

Package Requested

[ ] I would like someone else to build the manifest.
[X] I would like help so I can submit the manifest.
[X] I have performed a search and couldn't find this package.
[X] I have checked that the installer for this package is not contained in a .zip file
[ ] I think there is a new version available and I have provided the URL.

Please provide the following information
Publisher:
Publisher Url: https://github.com/paolosalvatori/ServiceBusExplorer
Package Name: Service Bus Explorer
Description: Service Bus Explorer
Package Version: 5.0.14
Installer URL: n/a (https://github.com/paolosalvatori/ServiceBusExplorer/releases/latest)
Current Package Identifier (for new versions):

Based on https://github.com/microsoft/winget-cli/issues/140, it should be possible to use portable zip-packaged deployment.
I'm looking for information on how to build and submit a zip-based manifest.

Based on microsoft/winget-cli#140, it should be possible to use portable zip-packaged deployment. I'm looking for information on how to build and submit a zip-based manifest.

Zip installers are still an experimental feature in WinGet. https://github.com/microsoft/winget-pkgs/issues/80892#issuecomment-1260057099.

@msftbot .zip

@MSFTbot .zip


Feature request: Switch to ESM --moduleResolution node12 for Typescript

Similar requests

[X] I have searched and found no existing similar requests

How are you using Studio?
No response
What problems are you trying to solve?
Typescript 4.5 supports ESM via --moduleResolution node12 or --moduleResolution nodenext - See their release note. One of the biggest change is the requirement for specifying extensions for imports.
However, perhaps, as they also noted in their release, there are still several concerns around the support and stability of this - See https://github.com/microsoft/TypeScript/issues/46452
Describe the solution you would like to see implemented

[ ] Create a script to scan import statement by regex and add the extension .js
[ ] Consider changes to be done in package.json
[ ] TODO investigate other potentially issues

Describe alternatives you have considered
No response
Documentation, Design, Adoption, Migration Strategy
No response
Contribution

[ ] I would like to work on this feature

Closed by https://github.com/finos/legend-studio/pull/1190


Yaml Format CI Test

Following https://github.com/meltano/hub/issues/949 we should add a little CI test that makes sure new additions and updates conform to the style guide that we decide on.

I've used yamllint before with some success


Added Air Quality Index Calculator

Fixes Issue🛠
Closes #631
Description👨💻
Added the AQI Level Calculator.
Takes the Air Quality Index as the input and outputs level of air as good, satisfactory etc.
Type of change📄

[x] New feature (non-breaking change which adds functionality)
Added the AQI Level calculator with gradient background representing the various color codes of air quality.

How this has been tested✅

[x] Tested locally on my pc.
The screenshots of the same have been attached.

Checklist✅

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have added demonstration in the form of GIF/video file
[x] I am an Open Source Contributor

Screenshots/GIF📷

Merged 🎉 @Aarzookhunger


Comments with Spaces cause failure

Description
Fixes issue where comments with a space were not quoted and would cause iptables to fail to start

Fixed rule resource
Added test to ensure it will not break again

Signed-off-by: Jason Field<EMAIL_ADDRESS>Issues Resolved

Check List

[ ] All tests pass. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/TESTING.MD
[X] New functionality includes testing.
[ ] New functionality has been documented in the README if applicable
[X] All commits have been signed for the Developer Certificate of Origin. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/CONTRIBUTING.MD

@tas50 / @robbkidd  could we get this one merged and shipped as it is causing breaks in my environment.
Also it would be great if travis was not a required check anymore ... :+1: