text:
Dask ReadParquet Performance Bottleneck

I link to performance reports, notebooks, and videos in this issue.  If you consume one piece of content, please watch this Loom video
Dask Dataframe ReadParquet performance is our clear bottleneck on TPC-H queries.  Here are performance reports for ...

cloud, scale 100
local, scale 10

Here is an accompanying explanatory Loom video
I've looked at bandwidths (ancient blogpost of mine here in case you're not used to thinking about and combining bandwidths (but I think everyone here is) and found that on an m6i.xlarge we have something like the following for data that is relatively warm on S3:

90 MiB/s for a single thread
300 MiB/s for parallel reads
1 GiB/s arrow deserialization (although this was on my laptop)

These experiments were run with this notebook and explained in this Loom.  If you watch/read one thing, watch this video.
In principle, I'd expect us to be able to reach almost 300 MiB/s when deserializing cloud data with arrow.  In practice, we get 8 MiB/s.  This is bad.
But it's also good!  So much room for improvement.  Also, conveniently, we're really slow on local data too, so we can spend all of our time developing locally!  I recommend going through the video.  We're only getting 16 MiB/s locally, so something basic is pretty wrong.
cc @rjzamora @phofl @fjetter @jorisvandenbossche

@milesgranger the data you created with duckdb have decimal dtype which is something we are round tripping through object. This is at least partially responsible for dasks bad performance locally. Can you convert those to float?

James just pointed me to the issue here as well.  I'm going to try the dtype_backend="arrow" keyword for now.
If we're sticking to the spirit of not changing things to suit us, it might be worth thinking about decimals inside of Dask itself.

This is a DuckDB specific thing, the source data are csv, so there is no type and float makes sense here as well.
polars uses floats as well for example
I wouldn’t bother with decimals at this stage

Running this locally points to the following bottleneck:
arrow reads all data into memory at once which is pretty fast. It looks like this operation has constant overhead that slows us down since we are doing this in 11 chunks, which is around 65-70% of the difference between dask and arrow

This is a DuckDB specific thing, the source data are csv, so there is no type and float makes sense here as well.
polars uses floats as well for example

OK, that's good to know.  And certainly yes it opens us up to use our own preferred datatypes.

I wouldn’t bother with decimals at this stage

Is it hard to identify this case?  It seems like we want to use arrow decimals if they're around in the data.  Same with dates.  In this case they're both getting converted to pyarrow strings, which isn't good.
>>> df = pd.read_parquet("tpch-data/scale-1/lineitem")
>>> df.dtypes
l_orderkey          int32
l_partkey           int32
l_suppkey           int32
l_linenumber        int32
l_quantity         object
l_extendedprice    object
l_discount         object
l_tax              object
l_returnflag       object
l_linestatus       object
l_shipdate         object
l_commitdate       object
l_receiptdate      object
l_shipinstruct     object
l_shipmode         object
l_comment          object
dtype: object

>>> list(df.iloc[0])
[3441604,
 71113,
 8635,
 3,
 Decimal('41.00'),
 Decimal('44448.51'),
 Decimal('0.07'),
 Decimal('0.00'),
 'N',
 'O',
 datetime.date(1995, 11, 10),
 datetime.date(1995, 11, 2),
 datetime.date(1995, 11, 12),
 'DELIVER IN PERSON',
 'SHIP',
 'nd the ironic packages. req']

This seems like something that we should fix regardless.  It doesn't need to be urgent, but it's probably important medium-term.

Running this locally points to the following bottleneck:

arrow reads all data into memory at once which is pretty fast. It looks like this operation has constant overhead that slows us down since we are doing this in 11 chunks, which is around 65-70% of the difference between dask and arrow

I'm not sure I fully understand this comment.
(also, no obligation to continue this conversation.  It's late.)

I agree with the medium term timeline, but they are not well tested in pandas, so I wouldn’t guarantee that the experience is smooth

We load the dataset in chunks (one chunk per parquet file in the lineitem scale 1 folder) while arrow loads everything at once.
it seems to me that the overhead in arrow for every chunk is responsible for 65-70% of the difference in runtime.

The chunks here seem large enough to me (20 MB) that I'd expect an SSD to handle them OK.
I agree that we're bound by arrow at this point

I'm also seeing reasonable bandwidth if I use dtype_backend="pyarrow"

Sequential ~ 250MB/s
Threads on a MBP - 500-1000 MB/s

Assuming we can handle the dtypes, I'm retracting my "dask dataframe parsing performance is horribly slow" concern.  My apologies for the drama earlier.

(ignore the core.py::get thing on the left of that chart, I think it's just the profile module being dumb)

Yeah that’s what I am seeing as well.
we could be smarter and read files in batches and then split them up, that might cause other issues though

Maybe @rjzamora already has code for that ?

OK, some observations from recent experiments:I've oversaturated threads and certainly seen an improvement.  Doubling gets us maybe a 30-40% reduction in q1 runtime

For now I'm switching to s3://coiled-data/uber, just to avoid the decimal/date dtype thing.  Our performance problems persist there to a lesser, but still quite significant degree.  Here's a quick demonstration:
import coiled
import s3fs

s3 = s3fs.S3FileSystem()
import time
from dask.utils import format_bytes
import dask.dataframe as dd
import pandas as pd

@coiled.function(
    vm_type="m6i.xlarge",
    region="us-east-2",
)
def time_dask(filename, n=20):
    nbytes = sum(s3.du(fn) for fn in s3.ls(filename)[:n])
    start = time.time()
    df = dd.read_parquet(filename).partitions[:n].persist(scheduler="sync")
    stop = time.time()
    return format_bytes(nbytes / (stop - start))

time_dask("s3://coiled-data/uber/", n=20)

This yields 11 MB/s, or up to around 25 MB/s once S3 gets warmed up.

I was curious what's taking up time, reading from S3, or arrow deserialization.  It's almost certainly the former, but it's hard to tell with the Dask profiles, which look like this:

So I decided to just read all of the bytes right away (we are pulling in the full partition in these simple examples, so different row groups or columns don't matter).
--- a/dask/dataframe/io/parquet/arrow.py
+++ b/dask/dataframe/io/parquet/arrow.py
@@ -250,6 +250,8 @@ def _read_table_from_path(
         precache_options=precache_options,
         **open_file_options,
     )[0] as fil:
+        import io
+        fil = io.BytesIO(fil.read())
         if row_groups == [None]:
             return pq.ParquetFile(fil, **pre_buffer).read(
                 columns=columns,

We find that, indeed, s3fs is taking up most of the time.

I've verified that things don't take significantly longer with this approach, which gives some confidence that it's representative.

Hmm.. I’d be curious to know if filesystem=“arrow” has any effect on performance.

Hmm.. I’d be curious to know if filesystem=“arrow” has any effect on performance.

Mildly positive, but not positive enough.
OK, new simpler experiment.  Simplifying in two ways:

Using the Uber-Lyft dataset (we wrote this, not DuckDB)
Just single-threaded for now (let's improve performance there, and then parallelize)

Video here
Notebook here
I'm observing the following, single threaded:

90 MB/s: S3FS Download speeds
50 MB/s: S3FS Download + Arrow deserialization    (this seems important)
25 MB/s: Arrow Download + Arrow deserialization   (this seems strange)
20-30MB/s: Dask (pretty variable)

I suspect that there are questions both for Arrow folks and for Dask folks with these numbers.
(obviously no pressure on the weekend, I'm just a bit more energetic this week than is typical)

I've updated the notebook with another experiment.

125 MB/s: Reading files from disk with Arrow (no bytes or s3fs objects involved)
6000 MB/s: Reading bytes directly from disk (clearly some filesystem caching involved)

This asks more questions of arrow

To be explicit I think that there are two big questions:

What does it appear that PyArrow deserialization works at GB/s speeds on my laptop, but only 100MB/s speeds on an m6i.xlarge?  I'm hoping that Arrow devs like @jorisvandenbossche can point us in a good direction.

What is Dask doing on top of the following:
for filename in filenames:
    file = s3.open(filename)
    pq.ParquetFile(file).read()

That would make it twice as slow.  I'm hoping that @rjzamora or others familiar with the dask.dataframe.io.parquet code can help to answer this question.

Also @jorisvandenbossche, please let me know if I should be escalating this up to the Arrow issue tracker.

In the latest notebook, for the cell with the "# Just let Arrow do it all" case, can you do a version with  pq.ParquetFile("s3://" + filename, pre_buffer=True).read() ?

After running it many times to make sure S3 files were hot I got it up to about 50MB/s

What is Dask doing on top of the following: ... That would make it twice as slow. I'm hoping that @rjzamora or others familiar with the dask.dataframe.io.parquet code can help to answer this question.

When you are not using filters or hive-partitioning, the "table read" code should be here.
The biggest thing that stands out to me is use_threads=False.
There are also still several ways to open the file at the moment. For example, it is sometimes helpful to pass in open_file_options={"precache_options": {"method": "parquet"}}. However, we never made much of an effort to make this method easy to access, because filesystem="arrow" was typically just as fast (or faster) around the time it was added.

(i.e. with enabling pre_buffer, that's also what dask does internally, I think)

Yes, dask should use pre_buffer=True by default (when the "parquet" method is not set under the "precache_options").

There are also still several ways to open the file at the moment.

Are there better defaults we should be using for cloud access?  If there is some keyword I can set and double my bandwidth, that would be welcome :)

The biggest thing that stands out to me is use_threads=False.

I played around directly with raw arrow here:
# Download bytes, hand bytes to Arrow

with bandwidth():
    for filename in filenames:
        file = s3.open(filename)
        bytes = file.read()
        pq.ParquetFile(io.BytesIO(bytes)).read(use_threads=False)  # play with this keyword

I saw maybe a 5-10% speed difference (from 52 MB/s to 58 MB/s or so)

For example, it is sometimes helpful to pass in open_file_options={"precache_options": {"method": "parquet"}}.

I'm curious, what does this mean?

However, we never made much of an effort to make this method easy to access, because filesystem="arrow" was typically just as fast (or faster) around the time it was added

Note, this may be broken now?
https://github.com/dask/dask/issues/10585

Are there better defaults we should be using for cloud access? If there is some keyword I can set and double my bandwidth, that would be welcome :)

In the recent pass, default behavior tended to be reasonable, and  filesystem="arrow" was the suggested change for clusters with many threads. However, It is probably about time to start re-evaluating.

I'm curious, what does this mean?

This uses the optimizations discussed in this blog.  These optimizations are used by default in dask-cudf, but seemed to be less helpful in dask.dataframe proper.

Note, this may be broken now?

Darn. I'll need to look into that.

OK, I'm surprised by the difference between PyArrow on my laptop vs PyArrow on any cloud machine (I've tried AWS, GCP, and even metal instances on AWS (albeit still with Docker on top)
import contextlib
import os

import s3fs
from dask.utils import parse_bytes, format_bytes
import pyarrow.parquet as pq

s3 = s3fs.S3FileSystem()
filenames = s3.ls("s3://coiled-data/uber/")[:10]
nbytes = sum(s3.du(filename) for filename in filenames)

@contextlib.contextmanager
def bandwidth(nbytes=nbytes):
    start = time.time()
    yield
    stop = time.time()
    print(format_bytes(nbytes / (stop - start)))

# Download files
os.mkdir("uber")
for filename in filenames:
    s3.get(filename, "uber/")

# Read with PyArrow
with bandwidth():
    t = pq.read_table("uber")

On my laptop this prints out numbers in the 1-2 GB/s range.  On a cloud machine, more like 75-150MB/s.  This data is small enough that it fits in the filesystem's cache (so disk reads are fast, but just to be paranoid, I ran this too:
byte_list = []
for filename in glob("uber/*"):
    with open(filename, mode="rb") as f:
        byte_list.append(f.read())

with bandwidth():
    for b in byte_list:
        pq.ParquetFile(io.BytesIO(b)).read()

And this (reading from memory) continues to print out around 100 MB/s on cloud (even metal instances!) and 1 GB/s+ on my MBP.

I wanted to rule out Coiled, so I also ran this on Colab.  Getting 100 MB/s.

OK, I opened up my Linux laptop and this is slow there as well.

I get around 500 MB/s on my Linux laptop (which feels as an expected number, as my 4-year old laptop is generally slower than a MacBook Pro). But will follow-up further on the Arrow issue you opened (-> https://github.com/apache/arrow/issues/38389)

@jorisvandenbossche have you built Arrow yourself, or are you using the conda installed version?  I am curious if maybe the conda build environment uses something suboptimal for Linux builds.

@mrocklin - Just leaving a minor note to avoid possible confusion in the future: The .partitions[:n] code you used here does not ensure that you are only reading in the first n files unless you also specify blocksize=None (just to make absolutely sure that we are partitioning by file, rather than row-group chunks).

Oh interesting.  Was that happening in this case?
On Mon, Oct 23, 2023 at 11:46 AM Richard (Rick) Zamora <
@.***> wrote:

@mrocklin https://github.com/mrocklin - Just leaving a minor note to
avoid possible confusion in the future: The .partitions[:n] code you used
here
https://github.com/coiled/benchmarks/issues/1125#issuecomment-1773569843
does not ensure that you are only reading in the first n files unless
you also specify blocksize=None (just to make absolutely sure that we are
partitioning by file, rather than row-group chunks).
—
Reply to this email directly, view it on GitHub
https://github.com/coiled/benchmarks/issues/1125#issuecomment-1775601358,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AACKZTBDCIZPGCXWS3SVSUDYA2NNXAVCNFSM6AAAAAA6JTWWTGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZVGYYDCMZVHA
.
You are receiving this because you were mentioned.Message ID:
@.***>

--
https://coiled.io
Matthew Rocklin CEO, Dask Maintainer

Was that happening in this case?

We should be fine for the "s3://coiled-data/uber/" dataset, because the total_byte_size metadata in the first file suggests that the uncompressed storage size of that afile is '37.71 MiB' (less than the default '128 MiB' threshold).

Ok, so I spent the past couple of days a lot on profiling. Instead of posting raw results, I just summarize that I strongly believe that our readers are bound by latency. We're neither utilizing network nor CPU. It was a little difficult getting the actual profiling snapshots but I was able to get some native traces of the pyarrow FS that show that we're spending almost the entire "IO time" just in a CURL poll. The actual read of the data is barely showing up on the trace (note that this wraps the arrow FS with a buffered IO class so you will not recognize the _fetch_blocks or the _ensure_range_loaded but this is irrelevant)

A single connection to S3 should carry about 50MB/s, i.e. transferring the 10MB (as in the example of Q1 since we have about 20-30MB partitions and we're only fetching halve of it) takes only about 200ms + latency.
Besides the actual read, I already pointed out in https://github.com/apache/arrow/issues/38389#issuecomment-1774777681 that there are a couple of unnecessary requests to S3 being made. Considering that latency of S3 is about 100-200ms and the payload data we're fetching here is about 10-20MB we end up at about
200ms (payload) + 3 x latency ~ 200ms + 3 * 150ms = 650ms which gives us an effective bandwidth of just ~15MB/s
What I observed recently when running this are network bandwidths of about 20-30MB/s with a four core worker. Considering that the tasks I'm running are fused tasks that also do a little compute, that is about the right ballpark to explain what I was seeing there.

Looking at the actual requests that are placed by arrow, this gets even more concerning. For instance, I micro benchmarked the read of a single file of the lineitem dataset and depending on what kind of columns are selected, the actual requests differ significantly.
The available columns of that file are
cols = [
    "l_orderkey", # int64
    # "l_partkey", # int64
    # "l_suppkey", # int64
    # "l_linenumber", # int64
    "l_quantity", # int64
    "l_extendedprice", # double
    "l_discount", # double
    "l_tax", # double
    "l_returnflag", # string
    "l_linestatus", # string
    "l_shipdate", # timestamp[ns]
    # "l_commitdate", # timestamp[ns]
    # "l_receiptdate", # timestamp[ns]
    # "l_shipinstruct", # string
    # "l_shipmode", # string
    # "comments", # string
]

and if one indeed includes l_orderkey (columns above are ordered as they are in the parquet file), pyarrow actually starts with a single request that only fetches 1.48 MiB
[DEBUG] 2023-10-24 11:44:46.767 CURL [140293531756288] (HeaderOut) GET /tpch-scale-1000/lineitem/part.255.parquet HTTP/1.1
Host: coiled-runtime-ci.s3.us-east-2.amazonaws.com
range: bytes=4-1556684

I'm not entirely sure if it is worth it to generate a single request like this. My gut feeling is that for small files like this, it would be more efficient to fetch everything from l_orderkey up to and including l_shipdate. I haven't looked up how much unnecessary data we'd load here yet.

Anyhow, instead of micro optimizing the read patterns here for small files, I went ahead and ran some of those benchmarks on two and four times larger files and I can indeed see significantly better network utilization. I'll provide a more thorough update on this once I had the chance to run a couple more tests

FWIW I believe the pq.read_table API would perform much better on smaller files if we read multiple files at once. At least, if I understand the fragment_readahead (https://arrow.apache.org/docs/python/generated/pyarrow.dataset.Dataset.html#pyarrow.dataset.Dataset.to_table) correctly it is fetching multiple files concurrently which should reduce some of the impact of this latency. However, dask is using the dataset interface with single files only (so far) so we cannot benefit from this optimization

Short term, can we get arrow to read the different payloads in threads?  How about with larger partitions?
Long term, I'm trying to think about how we would design this system from scratch today.  Is it just the current system, but we have lots of threads going in order to hide latency?  Do we need to separate out IO and deserialization in some way?
I think my ideal solution says that we use many threads to hide latency.  We identify these in workers by noticing when we have incoming bytes but nothing large scheduled on comms, and when that happens we look at what tasks are active and have some simple model to learn which task-prefixes are IO based.  Then we put those in some other larger thread pool.
We can get better bandwidth by super-saturating cores.  However, I'm surprised that we can't get much better.  Maybe this is GIL-related stuff from s3fs?  We should try benchmarking without any joins or groupbys happening in the background.  Maybe Arrow Filesystem could do better, but it's bound by some IO ThreadPool in Arrow?  I'm curious about this.

Some random notes:

make sure sure you use pre_buffer=True when using the Parquet read APIs (this should be the default, but who knows?)
you can increase the number of IO threads in Arrow using pa.set_io_thread_count (though I'm not sure that would help because the AWS SDK has its own thread pool AFAIR)
Arrow does separate IO threads and CPU threads already

Oh, and please don't use io.BytesIO with Arrow. Use pa.BufferReader or pa.py_buffer instead, otherwise you'll pay for the Python overhead (including some GIL acquisitions).

@pitrou here are my current efforts to find something fast: https://gist.github.com/mrocklin/9eca141688d03bbb6c375e53bee35c6d
Any suggestions welcome or pointers on anything I'm doing that's dumb.

Summary:

S3 Bandwidth: 500MB/s
Fastest arrow parquet reads: 250MB/s using more cores than threads, and s3fs rather than Arrow filesystem.

Any help here would be very welcome.  I'm trying to make things fast before a talk at PyData NYC.

FWIW I believe the pq.read_table API would perform much better on smaller files if we read multiple files at once. At least, if I understand the fragment_readahead correctly it is fetching multiple files concurrently which should reduce some of the impact of this latency. However, dask is using the dataset interface with single files only (so far) so we cannot benefit from this optimization

@fjetter Some notes on this: from my understanding dask actually has two possible code paths: one using the dataset API with a frag.to_table(..) and one with raw ParquetFile(..).read(..) API (the _read_table_from_path implements the latter, and a comment there says that this code path is used when there is no filter, although I don't know how up to date that is).
The mentioned fragment_readahead keyword is only relevant for the dataset API, and so is probably not used in the current cases that are being benchmarked here (which don't use a filter).
Further, looking at our C++ documentation, it says that batch_readahead/fragment_readahead is not used in case of use_threads=False.

The mentioned fragment_readahead keyword is only relevant for the dataset API, and so is probably not used in the current cases that are being benchmarked here (which don't use a filter).
Further, looking at our C++ documentation, it says that batch_readahead/fragment_readahead is not used in case of use_threads=False.

By now, I'm ont running on main any longer but am setting use_threads=True and currently I'm using the pq.read_table API with a pyarrow FileSystem. Please forget whatever there is right now. If necessary, we'll burn everything and rebuild it. However, so far we haven't found any configuration that is really fast.

The configuration I described above is currently running on https://cloud.coiled.io/clusters/299493/information?account=dask-benchmarks&tab=Metrics (I think that account is public
I only get about 40-50% of the CPU used and am not saturating network, by a longshot

This is literally just doing
            with fs.open_input_file(path_or_frag) as fd:
                arrow_table = pq.read_table(
                    fd,
                    columns=columns,
                    schema=schema,
                )

[on fragment_readahead] However, dask is using the dataset interface with single files only (so far) so we cannot benefit from this optimization

You can split a single-file fragment into multiple rowgroup-based fragments with the dataset API (list(ds.dataset(fd).get_fragments())[0].split_by_row_group()), in case you have a file with multiple fragments, this might enable readahead of those row group fragments? (didn't check, just a random idea)

For the handling of latency / coalescing reads, it might be relevant to check out https://github.com/apache/arrow/blob/main/cpp/src/arrow/io/caching.h to understand what arrow is doing here (although unfortunately this CacheOptions is not really exposed in Python AFAIK, making it difficult to play with those parameters)

Sorry for that. Our Parquet APIs are a mess.

Sorry for that. Our Parquet APIs are a mess.

Ours too :)

Yeah, I know it's a bit of a mess. Summarizing:

pq.ParquetFile is the lower level single-file Parquet reader interface (wrapping libparquet). This is not based on the datasets API, and therefore has not the same feature set
pq.read_table nowadays is mostly just a convenience wrapper around pyarrow.dataset.dataset(..., format="parquet"), but handling parquet specific keyword for you (doing that manually with pyarrow.dataset is a bit more complicated ..). This is great if you just want to read the whole thing, and don't need other methods from the dataset API
pyarrow.dataset is a generic dataset API (so not just Parquet), and also abstracting over single / multi files etc. When using this API, it has its own execution logic regarding parallelization and readahead, etc. And it also gives other features like accessing/inspecting fragments, counting rows, returning a stream of batches, etc

My guess is that we still want to stick with pq.ParquetFile.  I think that in my ideal world we would have each Dask task be assigned to either a single-file, or a row group within a file (for very large files) and then Arrow would download the right parts of that file with as much parallelism as makes sense (we've seen targetting 16 MB/s download chunks to be good).  Then Arrow would deserialize that data using as many threads as possible.
We would be running several of these concurrently, but because each task switches between IO and deseriailzation it's probably ok to super-saturate compute cores (they're unlikely to collide).
@fjetter are you seeing parallel downloads when using use_threads=True?  I'm not seeing much in my benchmarks, but maybe that's because the files are small (trying again soon).

So, I resorted to cave man debugging (aka print). I effectively wrapped the opened arrow filesystem with a python proxy class that would print whenever we are reading something. This is running on my local machine on a single thread, i.e. dask.compute(..., scheduler='sync')
The print below shows [thread ID / replaced with name] ArrowRead size={nbytes-to-read} tell={position of buffer}
[MainThread] READING fjetter-playground/128MiB/scale-100/lineitem/lineitem_0a8c7425-cd0a-46cc-80a0-2b0d1b7adf3e.parquet

[Thread0] ArrowRead size=65536 tell=131790372
[Thread1] ArrowRead size=2116545 tell=1285941
[Thread2] ArrowRead size=2116866 tell=7697994
[Thread2] ArrowRead size=2115965 tell=59001926
[Thread3] ArrowRead size=2116944 tell=14110395
[Thread4] ArrowRead size=2116591 tell=26937249
[Thread5] ArrowRead size=2115593 tell=33349194
[Thread6] ArrowRead size=2116086 tell=39762681
[Thread0] ArrowRead size=2116423 tell=46174822
[Thread1] ArrowRead size=2115721 tell=52589217
[Thread7] ArrowRead size=2116373 tell=20523104
[Thread2] ArrowRead size=2116552 tell=65419418
[Thread3] ArrowRead size=2117685 tell=71830583
[Thread4] ArrowRead size=2116442 tell=78248593
[Thread5] ArrowRead size=2116105 tell=84665595
[Thread6] ArrowRead size=2116208 tell=91077482
[Thread0] ArrowRead size=2116557 tell=97491347
[Thread1] ArrowRead size=2116735 tell=103908961
[Thread7] ArrowRead size=2116287 tell=110323000
[Thread2] ArrowRead size=2116152 tell=116734731
[Thread3] ArrowRead size=2117118 tell=123147875
[Thread4] ArrowRead size=1172279 tell=128985507

what this shows is that I have indeed 8 concurrent threads reading (just like pa.io_thread_count() says)
It also shows the first thread as it is fetching the footer (size=65536 tell=131790372) of a ~120MiB file.
What I don't understand is why every read is split into 2MiB chunks. Can I configure this somehow?

There are some options in the ReadRange CacheOptions (https://github.com/apache/arrow/blob/main/cpp/src/arrow/io/caching.h), but as mentioned above, those are not exposed in Python AFAIK (if you think those might be relevant and could test with a custom arrow build, I am happy to provide a branch exposing those you can test with)

According to @shughes-uk (who I generally trust to know this well) "S3 is happiest with 16 MB chunks, 2-3 threads per core".

Well, larger chunks is not necessarily the answer. This file has 21 row groups (maybe too many?) and there are 21 read requests, i.e. it fetches the data of the columns from every row group and for every fetch it likely fetches the minimal amount of data which is 2MiB. Those ranges are not sequential.
Larger chunk sizes may be the answer but only if we downloaded the entire file.
Are there any best practices about how large row groups should be? The ones I'm looking at are currently 23461856 in total_byte_size.

The 2MiB reads you see above is from reading a single column from the file?

@fjetter the files here s3://coiled-runtime-ci/tpch_scale_100/lineitem have only 1 row group per file if you want something with less row group for comparison

les here s3://coiled-runtime-ci/tpch_scale_100/lineitem have only 1 row group per file if you want something with less row group for comparison

Ok, in this case the read is using larger chunks but it cannot utilize parallelism since the file is just too small
[8222088704] READING coiled-runtime-ci/tpch_scale_100/lineitem/part.0.parquet
[6319632384] ArrowRead size=65536 tell=24086575
[6316191744] ArrowRead size=5239502 tell=7770128
[6317912064] ArrowRead size=2378335 tell=21763847
[8222088704] DONE coiled-runtime-ci/tpch_scale_100/lineitem/part.0.parquet

The 2MiB reads you see above is from reading a single column from the file?

I believe that's a couple of columns from a single row group but I haven't verified this. The count just matches the number of rowgroups and I am reading a couple of columns (about half of all columns)

If we can't use parallelism well within a file then we'll want to use lots of parallelism outside, and call pyarrow many times from Python threads.  For this we'll want the PyArrow threadpool to be fairly large (but I guess we can set this ourselves now?).  I think that we'll still want the larger chunks.
I do this today with s3fs (I get faster performance than with arrow's filesystem, maybe because of the small chunks?) but the GIL is active about 50% of the time, which is annoying.  I'd love to use something with less GIL activity if possible.

Some notes that may be helpful

I noticed some worries about only getting 125MB/s downloading s3 files to disk. This is expected and matches the provisioned throughput on the EBS volumes we mount. It's low because anything else is very expensive. For faster stuff you use a large (maybe even metal) storage optimized instance type, and write/read to the /scratch directory where we mount the SSD. Or just don't involve the filesystem in these tests
If you're using boto anywhere (s3fs does), and using it as you're supposed to, with a unique session per thread, there is a large initialization cost to each session, it loads up to 70MB of json (mostly docstrings.. dont ask). We monkey patched botocore recently for the platform to avoid this.
Client/bucket being located in the same region is very important to performance. No matter what i did in the past, cross region performance never came close to same region.

OK, so I've just ripped out dask.dataframe's parquet implementation (it was hard to reason about) and replaced it with a minimal implementation, and now I think I can convince myself that I'm mostly bottlenecked on arrow deserialization performance, which again I'm seeing as being oddly slow on these machines (around 50 MB/s per core).
I suspect that there is still some improvement to be made with things like super-saturation, especially when we're subselecting columns, but not more than say, 50%.

Matt, if you try “metal” instances those have much less interference from
being virtualized and sharing tenancy. Id also try amd/intel and arm
chipsets. Maybe it’s something to do with the way arrow is compiled?
Im not sure if arrow is one of those packages that has a “generic” slow
version and then faster versions that get installed if conda detects
specific chipsets.
On Fri, Oct 27, 2023 at 10:53 AM Matthew Rocklin @.***>
wrote:

OK, so I've just ripped out dask.dataframe's parquet implementation (it
was hard to reason about) and replaced it with a minimal implementation,
and now I think I can convince myself that I'm mostly bottlenecked on arrow
deserialization performance, which again I'm seeing as being oddly slow on
these machines (around 50 MB/s per core).
I suspect that there is still some improvement to be made with things like
super-saturation, especially when we're subselecting columns, but not more
than say, 50%.
—
Reply to this email directly, view it on GitHub
https://github.com/coiled/benchmarks/issues/1125#issuecomment-1783296267,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAEFQSYDXQJ2A5ADNQ3WRATYBPYLLAVCNFSM6AAAAAA6JTWWTGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBTGI4TMMRWG4
.
You are receiving this because you were mentioned.Message ID:
@.***>

virtualized and sharing tenancy

I'd expect this to show up as "steal" on cpu util.

I've already tried Metal.  I've also tried my Linux laptop.

authors: fjetter jorisvandenbossche mrocklin ntabris phofl pitrou rjzamora shughes-uk

repo: coiled/benchmarks


text:
Scrub dashed lines from Terraform output before parsing

Resolves issue: #37
Terraform output can contain dashed lines followed by new lines before output match by regexp.
This scrubs the dashes and new lines before parsing the output and moves the no changes match above the other parsing code.
More details in issue comment: https://github.com/coinbase/terraform-landscape/issues/37#issuecomment-336341627
TF Output before scrubbing:
------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

TF Output after scrubbing:
No changes. Infrastructure is up-to-date.

Probably want to look at updating the test inputs as well.

@mikeflisher can you add a test to confirm it is not fixed?

Closing due to lack of response, as well as the fact that I don't believe this output is coming from Terraform. Feel free to reopen with a reproducible example. Thanks!

I've run a variety of tests against TF 0.10.5 output with the terraform_landscape 0.15 and 0.16 and couldn't reproduce the same error so assuming fixed as well.

source: gharchive/pull-request

authors: krobertson mikeflisher sds

repo: coinbase/terraform-landscape

license_type: permissive

license_source: github-api


text:
Related Posts

Could we do a simple implementation of "related posts"?
A basic idea could be loading more posts from the category of the active post
They could be added at the bottom of the page. More from [category]

hey @colbyfayock I was thinking on this one and I think I have an implementation. Not sure if you are already working on it... if not, happy to send a PR 😃
Looks like this:

hey @colbyfayock I was thinking on this one and I think I have an implementation. Not sure if you are already working on it... if not, happy to send a PR 😃
Looks like this:

that looks good to me! maybe a little extra spacing above the header to separate it similar to the "last updated" line from the original content
but yeah, go ahead with a PR!

that looks good to me! maybe a little extra spacing above the header to separate it similar to the "last updated" line from the original content
but yeah, go ahead with a PR!

Perfect! Making that change, and sending the PR 💪

Perfect! Making that change, and sending the PR 💪

source: gharchive/issue

authors: GuilleAngulo colbyfayock

repo: colbyfayock/next-wordpress-starter

license: MIT


text:
gif_animation only working for Background-Layer

It seems that the gif_animation-Option is only working for the background-layer of the compilation. If I am adding a ImageLayer with animated gif inside, the animation is not there anymore.

See https://github.com/coldume/gd-enhancer/issues/12

authors: coldume dirkritters

repo: coldume/imagecraft

license: mit

license_source: bigquery


text:
Update only part of version number in AssemblyInfo file?

I'm looking to use semantic versioning in our application, where we would be manually updating the version number in source control, and then just have the VSTS build update the revision (4th part of the version number) so that the version number is unique, likely by using the $(Rev:r) VSTS Build variable. Is it possible to do that using the Version Assemblies task?
i.e. The ApplicationInfo.cs file has the version 2.3.4.0 defined in it, and I want the build system to update it to 2.3.4.123
I'm guessing it is possible to do by using some custom regular expressions for the capturing and replacing of the version number, but I'm not certain what the regex would look like for those 2 fields. Any suggestions? Thanks!

Hi @deadlydog - how often to you update the first 3 numbers? If you hard-code your build number format to be 2.3.4$(rev:.r) and use the default 1.0.0.0 format for the VersionAssembly task, you'll get exactly what you want. Of course you'd have to update the build number format whenever you change the other numbers (the 2.3.4 portion) but if you don't do that often, you'll be good to go.
The other option is to do a custom numbering script before VersionAssemblies - but keep the logic out of VersionAssemblies is the point!

authors: colindembovsky deadlydog

repo: colindembovsky/cols-agent-tasks


text:
fix: Use or in partial

Changes
The gitinfo template should use an OR instead of an AND for the checks on lastMod and editPage
The contents of the main div already had an exist check, so it already supports ui logic for showing one or both.
However the AND used in the class attribute forced the use of both  lastMod and editPage

Dark mode

[ x ] The UI has been tested both in dark and light mode-->

Good catch. Thank you. 👍

authors: arjenvdhave colinwilson

repo: colinwilson/lotusdocs


text:
HLS Transcription Stops After A Couple Minutes

I set up a local HLS stream playing a long video of someone talking.
Everything seems great until after exactly 2 minutes in the transcription stops completely.
INFO:faster_whisper:Processing audio with duration 00:07.936
INFO:faster_whisper:Processing audio with duration 00:02.984
INFO:faster_whisper:Processing audio with duration 00:03.032
INFO:faster_whisper:Processing audio with duration 00:01.432
INFO:faster_whisper:Processing audio with duration 00:03.480
INFO:faster_whisper:Processing audio with duration 00:05.272
INFO:faster_whisper:Processing audio with duration 00:01.152
INFO:faster_whisper:Processing audio with duration 00:03.200
INFO:faster_whisper:Processing audio with duration 00:02.548
INFO:faster_whisper:Processing audio with duration 00:04.596
INFO:faster_whisper:Processing audio with duration 00:01.796
INFO:faster_whisper:Processing audio with duration 00:03.844
INFO:faster_whisper:Processing audio with duration 00:02.484
INFO:faster_whisper:Processing audio with duration 00:02.484
INFO:faster_whisper:Processing audio with duration 00:02.484
INFO:faster_whisper:Processing audio with duration 00:02.484
INFO:faster_whisper:Processing audio with duration 00:02.484
INFO:faster_whisper:Processing audio with duration 00:02.484
INFO:faster_whisper:Processing audio with duration 00:02.484
In the server logs i can see that chunks of variable length are processed by the server. However the problem starts when the "00:02.484" chunks keep getting processed. I'm unsure if its just continuing to send the same chunk and it keeps translating it therefore the client appears to be "stuck" or if its stuck in a different loop of some sort.
Setting use_vad to True doesn't seem to make a difference.
I have tried both on Mac (M3 Max chip) and Windows 10. Both docker and python server. Both produce the same results.
This is the client code:
from whisper_live.client import TranscriptionClient
client = TranscriptionClient(
  "localhost",
  9090,
  lang="en",
  translate=False,
  model="small",
  use_vad=False,
)

client(hls_url="http://localhost.m3u8")

@austinm1120 Thanks for reporting the issue, does whisper-live behave similarly with other input types as well or is it only HLS?

same happens to me, but this time it's exactly 10 minutes every time:
[INFO]: Server disconnected due to overtime.
[INFO]: Websocket connection closed: 1000:

Investigating on my own and if I will come to any fixes, will let you know.

OK, I found the "issue" :) It's not an issue, it's was designed to work that way: https://github.com/collabora/WhisperLive/blob/main/whisper_live/server.py#L28
So in my case I'll do refactoring to strip that part.

repo: collabora/WhisperLive


text:
stateManager.getCollection - review / generalize options

options could be:
Generalize existing parameter filtering feature (apply to SharedState and SharedCollection):

whitelist - same behaviour as current filter argument
blacklist - inverse of whitelist

Allow to model some hierarchies / relationships (apply to SharedCollection):

contraint - arbitrary function to filter the states that live in the collection, could be useful e.g. to model sort of simple "join"

const parent = stateManager.attach('parent');
const children = stateManager.getCollection('child', {
   constraint: values => values.parentId === parent.id
});

This could be quite simply done on the client side, just detaching early from the state if the value does not match, or even by propagating init state values in OBSERVE_NOTIFICATION to avoid additional network roundtrips.
The constraint could also be a pure facade, allowing to insert / remove a state from the collection when one of it's param changes and match the constrain... to be discussed
Maybe provide both behaviours:

staticContraint - check is done only on init values when the state is created (better in terms of network load)
liveContraint - The state is attached but added / removed from collection dynamically at any moment

Would also benefit from #93

constraint could even be a pure facade, allowing to insert / remove a state from the collection when one of it's param changes and match the constrain... to be discussed

Why not keep the whitelist (and maybe blacklist) for the subscriptions? constraint sounds more like a requirement to me. (If constraint' is to be used, do not forget the s`.)

dynamicWhitelist
staticWhitelist

Is there a way to integrate that with the filtering of attributes, to get only a subset (regardless of their values)?

filter rather than constraint

Actually the dynamicFilter behavior is not very relevant as we already have the SharedStateCollection#filter method. This would also create problems such as how to be notified when the collection change, etc.
For now, let's just stick with something like:
stateManager.getCollection('test', {
  paramWhitelist: ['a', 'b'],
  paramBlacklist: ['a', 'b'],
  filter: (currentValues) => currentValues.group === 'group-1',
});

authors: b-ma jipodine

repo: collective-soundworks/soundworks


text:
Queue priority in config

Queue priority is not settable via a config file. This causes complications when you want to set queue priority and leverage ActiveJob in Rails. In order to allow setting queue priority I have created a queue_attributes attribute that falls off the worker. The attribute is then accessed and the priority is set when setting up the job prior to enqueuing.
I have extracted the enqueueing code into it's own class to try and clean things up a little into a JobPreparer for a lack of a better name.
Some side notes it may be better to rather use the queue attribute on the worker, but I thought this was a good start.
This is my first public PR to someone else's repo. I have read the guidelines ... I am happy if it's rejected, but input on where I can improve in future or how I can change things to provide similar functionality would be much appreciated.
Someone also started a thread requesting this functionality.

Can you please rebase from master and fix the outstanding RuboCop style guide violations in your code?

I have rebased from master ... I hope I have done it correctly. Let me know if there is anything I need to fix. There were also a couple of rubocop issues that weren't mine which I included as well.

I hope I have done it correctly.

@ryannealmes This doesn’t look quite right. Try this in your fork:
git remote add upstream git@github.com:collectiveidea/delayed_job.git
git checkout master
git pull upstream master
git checkout queue-priority-in-config
git rebase master
# after each conflict is resolved resolved:
git add .
git rebase --continue
# after all conflicts are resolved:
git push --force origin queue-priority-in-config

Also, ideally all of the changes in your branch would be squashed into a single commit. Let me know if you need instructions on how to do that.

Cool, I think I am there. Let me know what you think. Thanks for the help.

Hey @sferik, is there anything I need to do to get this merged? I know guys are probably pretty busy with other stuff, but I am not really sure if I should just wait until someone says something or if I should actively be chatting to someone. Thanks.

@ryannealmes Thanks for this PR, this is really a crucial bit! (I have to up an app to Rails 4.2 now and without this feature, I'll be forced to switch to another backend.)
@sferik Any chance this will be merged soonish?

Sorry for the long delay on merging this pull request. Other than the few minor questions/suggestions I just made related to RuboCop, this looks good to me.
I’d like to merge this and release it at the same time as Rails 5 compatibility. Of course, Rails 5 is not yet released but I’m hoping it will come later this year or early 2016.
In the mean time, after this is merged, you’ll be able to use it by specifying a Git source in your Gemfile:
gem 'delayed_job', :git => 'https://github.com/collectiveidea/delayed_job.git'

@sferik @ryannealmes
There might still be a problem lurking. Using the with this. With the version from the queue-priority-in-config branch of ryannealmes/delayed_job, I experience the following behaviour:
# initializers/delayed_job.rb
Delayed::Worker.queue_attributes = [
  { name: :low_priority, priority: 10 }
]

"test".delay(queue: :low_priority).to_s
job = DelayedJob.last
job.queue      # => "low_priority"   <-- OKAY
job.priority   # => 10               <-- OKAY

UserMailer.password_reset(user).deliver_later(queue: :low_priority)
job = DelayedJob.last
job.queue      # => "low_priority"   <-- OKAY
job.priority   # => 0                <-- FAIL

@sferik @ryannealmes
I found the cause for the problem mentioned in my previous comment. While the ActiveJob documentation uses symbols for queue names, the actual queue name is converted to a string internally. Therefore, when setting default priorities per queue, you must currently use a string to reference the queue. Now, I won't be the only one tripping over this :smile:
I suggest adding a to_s to the priority discovery method in this PR:
Delayed::Backend::JobPreparer#set_priority:
  def set_priority
    options[:priority] ||= Delayed::Worker.default_priority
-   queue_attributes = Delayed::Worker.queue_attributes.select { |queue| queue[:name] == options[:queue] }
+   queue_attributes = Delayed::Worker.queue_attributes.select { |queue| queue[:name].to_s == options[:queue] }
    options[:priority] = queue_attributes.first[:priority] if queue_attributes.any?
  end

@ryannealmes I've added a PR to your fork with this fix and a little addition to the README explaining your feature. The rubocop directive cleanup suggested by @sferik is also included: https://github.com/ryannealmes/delayed_job/pull/2

Thanks for the help @svoop and @sferik . I have rebased the commits. Let me know if there is anything else.

Looks good to me! Thanks for the patch!

@sferik , @svoop do you guys know why this PR wouldn't show up in my public contributions? Thanks.

@ryannealmes No idea, after all, collectiveidea/delayed_job is listed as a repo you've contributed to. Maybe a Github glitch.

@svoop I was looking forward to being a contributor :/ At least the functionality is there now :)

Maybe it uses the timestamp of the commit, and the public activity thing doesn't scroll back to September

@ryannealmes It’s because this is a fork and GitHub only counts contributions to the original repo.

Any chance of getting a release with this change before all of the rails 5 stuff @sferik?

We already have the rails 5 stuff in master. I don't like releasing something against in flux dependencies. Have been bitten by that too much.

fair enough.
I wonder if I'm missing a simple way to get what I want anyway without risking the bleeding edge.
Is there a way to set the priority with a rails4 mailer?
something like:
#doesn't work
MyMailer.signup(message,recipient).deliver_later(priority: 5)

I know that activeMailer supports queues - so default queue priorities will allow
MyMailer.signup(message,recipient).deliver_later(queue: "high_priority")

Be aware I have simplified the configuration structure on this in master and the about to be released gem
https://github.com/collectiveidea/delayed_job#named-queues

authors: ConfusedVorlon albus522 maxjacobson ryannealmes sferik svoop swrobel


text:
Points 3d

Thank you for the python binding :)
I have the keypoints of the two images that I have matched and also the pose information of one of the images. How do I obtain point3d for the absolute pose estimation?

You will need a way to lift the 2D points from the image with pose to 3D - depth or a sparse 3D model for instance.
In this repository, we suppose you already have access to 2D<->3D correspondences for absolute pose estimation so building the sparse model is out of scope.

@mihaidusmanu I have not managed to retrieve the 3D points from the 2D points using the pose information of the image. Could you please suggest me a method that I can use to retrieve the 3D points from a 2D image with a known pose?

Given an image with camera pose (world-to-image) P and intrinsics matrix K, you can back-project a pixel location (x, y) to 3D by computing P^{-1} @ K^{-1} @ [x, y, depth(x, y)] where @ denotes matrix multiplication.
As I mentioned above, in order to do this, you will need the depth of pixel x, y. This can be obtain for instance using an RGB-D camera.
If you do not have access to an RGB-D camera, you might want to look into Structure-from-Motion - i.e., building a 3D model from a sequence of images.

@mihaidusmanu thank you! I have managed to extract the 3D points using the depth image.
import cv2

# camera matrix
mtx = [8.6767822265625000e+02, 0., 4.8268579101562500e+02, 0.,
       8.6760119628906250e+02, 2.7117459106445312e+02, 0., 0., 1.]

# parse calib matrix
fx = mtx[0]
cx = mtx[2]

fy = mtx[4]
cy = mtx[5]

depth_img = cv2.imread("depth.png", cv2.IMREAD_ANYDEPTH)
depth_row, depth_column = depth_img.shape

# to store 3d points
pts3d = []

for i in range(depth_row):
    for j in range(depth_column):

        # retrieve depth value
        depth = depth_img[i][j]
        x, y = i, j

        if depth > 0.0:
            x3D = (x - cx) * depth / fx
            y3D = (y - cy) * depth / fy
            z3D = depth

            # populate 3d points array
            pts3d.append([x3D, y3D, z3D])
        else:
            pts3d.append([-1, -1, -1])

I have key points from SIFT. Any idea on "points2D" which is the first parameter?

In this case, points2D would be the 2D keypoints in a different image corresponding to the 3D points lifted in the current image. I am unsure what you are trying to achieve. Maybe if you explain that, I could be of more help. What is your current evaluation scenario? Do you want to get the relative pose between two images or do you want to get the absolute pose of an image given a pointcloud?

I would like to obtain the absolute pose of an image given a point cloud. I'm still having a hard time trying to get the perfect absolute position :( Is there any example code?

Do you have descriptors for both the point-cloud and the local features in the image? If so, then all you need to implement yourself is matching between the descriptors. Next you will have to give the 2D points and their 3D point correspondences to pycolmap.absolute_pose_estimation as shown in the README and that will return you the pose.

I have implemented the first portion so I have points2D from the image and points3D from the depth image.
In terms of params, I have used the values of the image corresponding to the points2D.  (fx,cx,cy) values.
The resultant absolute pose has a pose estimation error of about 50-70cm. am I doing it properly?

@mihaidusmanu may I know how I can obtain the rotation vector in same format as that of the solvepnp's rotation vector? I think the resultant output of pycolmap is Quaternion

hey can you provide the solution you implemented? I am having the same trouble as you

authors: StanlyHardy Zumbalamambo bgramaje mihaidusmanu

repo: colmap/pycolmap


text:
Add Aspire Component / DI Extensions

This PR:

Adds DI extensions to register LlamaParseClient with DI.
Fixes reading InMemory files in Aspire sample.

@colombod ready for review when you get a chance. Given that this is standard DI, I don't think it needs Aspire branding. Also, instead of having a separate project for this, maybe it's worth just adding another class to the LlamaParse project with these extensions. Thoughts?


text:
Move locations data to backend

Currently the data for the locations is hardcoded as geojson into the frontend (src/api). It should ideally be moved to the backend, even if hardcoded there, for consistency.
For the sake of automating the ingest of this data later in #74, see #19 for how the data was initially scraped and added to the frontend.

Hey @vincerubinetti, I've started working on this. I wanted to mention one caveat: our GIS database, PostGIS, doesn't support FeatureCollections or Features directly as geometry. Instead, you're supposed to "fully normalize" the structure by breaking out the individual elements of the FeatureCollection as tables, down to actual geometry elements (the "geometry" entry under a Feature in GeoJson), which can be stored as geometry. If you want to serve GeoJSON FeatureCollections/Features after normalizing, you do so by reconstituting that data structure when you return it from the API.
While I definitely see the benefits of fully normalizing the structure in the database, I also don't know if we'll ever need it and I would prefer to get this done quickly; we can revisit it if we find that we need it later on. Instead, I plan to store each FeatureCollection as a JSON blob in the backend, which I'd just pass off to you verbatim from the API, like how you currently load it from a JSON file in the frontend.
No action required from you, but I just wanted you to know the design decision I'm planning to take. I'm also willing to fully normalize the structure if you prefer, and I can fill you in on the pros and cons of each approach you like.

Ok, sounds good; I think my current implementation in #82 matches what you asked for above.
Well, the main selling point of PostGIS is that you can store geometry and perform geometric operations on it; having it parse GeoJSON and create hierarchies of tables for you isn't something it's designed to do. It's really not a lot of work for me to implement the tables (it's like 15 lines of model definition code), but since don't need to deeply index the FeatureCollection structure at the moment, I think just storing and returning it as JSON is fine.

authors: falquaddoomi vincerubinetti

repo: colorado-cancer-center/ecco


text:
Attempt at having a parallel tests test project

not yet working, opening an issue at aruba (https://github.com/cucumber/aruba/issues/699) to figure out how to
do this :thinking_face:
The issue being that my thought of having a separate Gemfile doesn't work as it appears that the bundle commands are executed in the scope of the top level simplecov project.
for parallel_tests I might add a development dependency, but in the future I'd also like to test against different Rails versions or something so while feasible that might not be ideal.
#781

It works :tada:
It might fail though... because - surprise! - the merging of parallel tests might fail/is timing dependent. It's hard to reproduce. Sometimes it happens 4 times in a row. Sometimes it doesn't happen at all in 10 runs. Classic flaky (but in this case I blame our implementation so it's a "good" flaky).
Probably should then skip it for the time being and attempt to fix it in another PR... which kinda defeats the purpose but probably better than having to rerun all other PRs n times because of a known flaky.

Collected some failures here for future reference. In all of them it seems that the correct report is generated but is then overridden by a subsequent incorrect one that doesn't take everything into account.
Which I believe is a great find to have semi reproducible in a test :tada:

4 processes for 4 specs, ~ 1 specs per process
..

Finished in 0.00217 seconds (files took 0.09603 seconds to load)
2 examples, 0 failures

.

Finished in 0.00292 seconds (files took 0.09621 seconds to load)
1 example, 0 failures

...

Finished in 0.00245 seconds (files took 0.0952 seconds to load)
2 examples, 0 failures

Finished in 0.00224 seconds (files took 0.09621 seconds to load)
1 example, 0 failures

Coverage report generated for (2/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 23 / 33 LOC (69.7%) covered.
Coverage report generated for (1/8), (2/8), (3/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 37 / 43 LOC (86.05%) covered.
Coverage report generated for (1/8), (2/8), (3/8), (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 42 / 47 LOC (89.36%) covered.
Coverage report generated for (1/8), (2/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 32 / 39 LOC (82.05%) covered.

6 examples, 0 failures

Took 0 seconds

(::) failed steps (::)

expected: 9
     got: 7

(compared using ==)
 (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/html_steps.rb:14:in `block (3 levels) in <top (required)>'
./features/step_definitions/web_steps.rb:5:in `block in with_scope'
./features/step_definitions/web_steps.rb:5:in `with_scope'
./features/step_definitions/html_steps.rb:12:in `block (2 levels) in <top (required)>'
./features/step_definitions/html_steps.rb:11:in `each'
./features/step_definitions/html_steps.rb:11:in `/^I should see the groups:$/'
./features/step_definitions/parallel_tests_steps.rb:4:in `"I should see the results for the parallel tests project"'
features/parallel_tests.feature:12:in `Then I should see the results for the parallel tests project'

Failing Scenarios:
cucumber features/parallel_tests.feature:9 # Scenario: Running it through parallel_tests produces the same results as a normal rspec run

4 processes for 4 specs, ~ 1 specs per process
....

Finished in 0.00174 seconds (files took 0.0938 seconds to load)
1 example, 0 failures

.

Finished in 0.00186 seconds (files took 0.09369 seconds to load)
2 examples, 0 failures

Finished in 0.0018 seconds (files took 0.0936 seconds to load)
2 examples, 0 failures

.

Finished in 0.00237 seconds (files took 0.09319 seconds to load)
1 example, 0 failures

Coverage report generated for (1/8), (2/8), (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 37 / 43 LOC (86.05%) covered.
Coverage report generated for (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 19 / 31 LOC (61.29%) covered.
Coverage report generated for (1/8), (2/8), (3/8), (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 42 / 47 LOC (89.36%) covered.
Coverage report generated for (1/8), (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 28 / 37 LOC (75.68%) covered.

6 examples, 0 failures

Took 0 seconds
[DEPRECATED] use `bundle info rspec-core` instead of `bundle show rspec-core`
[DEPRECATED] use `bundle info rspec-core` instead of `bundle show rspec-core`
[DEPRECATED] use `bundle info rspec-core` instead of `bundle show rspec-core`
[DEPRECATED] use `bundle info rspec-core` instead of `bundle show rspec-core`
.F

(::) failed steps (::)

expected: 9
     got: 7

(compared using ==)
 (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/html_steps.rb:14:in `block (3 levels) in <top (required)>'
./features/step_definitions/web_steps.rb:5:in `block in with_scope'
./features/step_definitions/web_steps.rb:5:in `with_scope'
./features/step_definitions/html_steps.rb:12:in `block (2 levels) in <top (required)>'
./features/step_definitions/html_steps.rb:11:in `each'
./features/step_definitions/html_steps.rb:11:in `/^I should see the groups:$/'
./features/step_definitions/parallel_tests_steps.rb:4:in `"I should see the results for the parallel tests project"'
features/parallel_tests.feature:19:in `Then I should see the results for the parallel tests project'

Failing Scenarios:
cucumber features/parallel_tests.feature:16 #

4 processes for 4 specs, ~ 1 specs per process
.

Finished in 0.00221 seconds (files took 0.09079 seconds to load)
1 example, 0 failures

..

Finished in 0.00172 seconds (files took 0.09245 seconds to load)
2 examples, 0 failures

..

Finished in 0.00195 seconds (files took 0.09657 seconds to load)
2 examples, 0 failures

.

Finished in 0.00179 seconds (files took 0.0979 seconds to load)
1 example, 0 failures

Coverage report generated for (3/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 19 / 31 LOC (61.29%) covered.
Coverage report generated for (1/8), (3/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 28 / 37 LOC (75.68%) covered.
Coverage report generated for (1/8), (2/8), (3/8), (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 42 / 47 LOC (89.36%) covered.
Coverage report generated for (1/8), (2/8), (3/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 37 / 43 LOC (86.05%) covered.

6 examples, 0 failures

Took 0 seconds

(::) failed steps (::)

expected: 9
     got: 8

(compared using ==)
 (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/html_steps.rb:14:in `block (3 levels) in <top (required)>'
./features/step_definitions/web_steps.rb:5:in `block in with_scope'
./features/step_definitions/web_steps.rb:5:in `with_scope'
./features/step_definitions/html_steps.rb:12:in `block (2 levels) in <top (required)>'
./features/step_definitions/html_steps.rb:11:in `each'
./features/step_definitions/html_steps.rb:11:in `/^I should see the groups:$/'
features/parallel_tests.feature:12:in `Then I should see the groups:'

Failing Scenarios:
cucumber features/parallel_tests.feature:9 # Scenario:

4 processes for 4 specs, ~ 1 specs per process
..

Finished in 0.00186 seconds (files took 0.09117 seconds to load)
2 examples, 0 failures

..

Finished in 0.00198 seconds (files took 0.09406 seconds to load)
2 examples, 0 failures

.

Finished in 0.00271 seconds (files took 0.09533 seconds to load)
1 example, 0 failures

.

Finished in 0.00258 seconds (files took 0.09796 seconds to load)
1 example, 0 failures

Coverage report generated for (2/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 23 / 33 LOC (69.7%) covered.
Coverage report generated for (1/8), (2/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 32 / 39 LOC (82.05%) covered.
Coverage report generated for (1/8), (2/8), (3/8), (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 42 / 47 LOC (89.36%) covered.
Coverage report generated for (1/8), (2/8), (4/8) to /home/tobi/github/simplecov/tmp/aruba/project/coverage. 37 / 43 LOC (86.05%) covered.

6 examples, 0 failures

Took 0 seconds

(::) failed steps (::)

expected: 9
     got: 8

(compared using ==)
 (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/html_steps.rb:14:in `block (3 levels) in <top (required)>'
./features/step_definitions/web_steps.rb:5:in `block in with_scope'
./features/step_definitions/web_steps.rb:5:in `with_scope'
./features/step_definitions/html_steps.rb:12:in `block (2 levels) in <top (required)>'
./features/step_definitions/html_steps.rb:11:in `each'
./features/step_definitions/html_steps.rb:11:in `/^I should see the groups:$/'
features/parallel_tests.feature:12:in `Then I should see the groups:'

Failing Scenarios:
cucumber features/parallel_tests.feature:9 # Scenario:

repo: colszowka/simplecov


text:
Make Evaluator Closeable

Cleaning-up the worker cache upon close

I decided to open a PR for that for https://github.com/com-lihaoyi/mill/pull/3579 (https://github.com/com-lihaoyi/mill/pull/3579#discussion_r1772373344 in particular), but I'm now realizing this might not be needed for it after all. Don't know what you think of that, I think it's a good idea to clean up things when the Evaluator isn't needed anymore.

(This makes Evaluator extends AutoCloseable, so this breaks bin compat…)

I'm not sure this is the right thing to do. Right now in production, the workerCache is this long lived thing that is shared across main0 invocations, and I think can out-live the individual evaluator that created it to be used in the next Mill command's evaluator. If that's the case, then having the workerCache be closed together with the evaluator at end of life would be wrong.
That code around sharing stuff across main0 commands is pretty gnarly, but that's what I last remember it being like

It seems this works at the end (CI is green), as in the watch loop, the previous RunnerState isn't closed, so that underlying Evaluators aren't either, and workerCaches are kept intact and can keep being used. But I don't think I did that on purpose 😅

Yeah that's why it's green, and then the next question is more what should the logic be here? I don't actually know, but although the current logic of injecting the workerCache into the Evaluator is pretty weird and roundabout, I'm inclined to leave it for now. I suspect that refactoring things enough to do things "properly" will require breaking bincompat, so we should wait until after 0.12.0 has landed and we're working towards 0.13.x and have the flexibility to make more sweeping changes without being restricted by bincompat requirements

authors: alexarchambault lihaoyi

repo: com-lihaoyi/mill


text:
Difference between docker images

There are 2 docker images
combustml/mleap-serving
combustml/mleap-spring-boot
from what i can tell these now share the same API in the latest versions (0.13.1-SNAPSHOT). Could someone clarify if there are any differences. It would be simpler for people looking to get started if the difference are documented.
Also the documentation for mleap-serving is out of date in http://mleap-docs.combust.ml/mleap-serving/. This works for the 0.9.0 tag of mleap-serving  but not the latest.
i've tested the documentation https://github.com/combust/mleap/tree/master/mleap-spring-boot, it is correct for both images and i would recommend it being put into the main documentation.

Hey @jwgwalton,
combustml/mleap-serving starts both the gRPC server and HTTP server, using a single MLeap executor whereas combustml/mleap-spring-boot starts only the HTTP server. Indeed, the HTTP API in both cases is the same.
We have https://github.com/combust/mleap-docs/pull/17 raised to update the main documentation, in the meantime, you can take a look at the documentation in the sub projects, that should already be up to date.

Cheers @ancasarb

authors: ancasarb jwgwalton

repo: combust/mleap


text:
Add additional fields

This adds genres, tags, rating and manga to ComicIssue and adds genres field to ComicSeries to allow for filtering of search results from the cache.

I've added a check for manga and rating. Not sure if it's worth doing the manga? With the rating, should fixes it to 2 decimal points be done?

The MacOS build seems to have stalled. Does it need a reset?

They finally got rid of macOS 10.15

https://github.com/comictagger/comictagger/pull/433/commits/d65a120eb5b00c594a43e4f7738e30af9ad89ad0 Adds issue_count from count_of_issues. I believe we talked about this before and couldn't rely on CV to have a valid number. I believe it should be up to the talker to validate this number. If the series has ended or been cancelled then the number can be considered valid for example.

With additional talkers comes additional information. Should everything in metadata (that makes sense) be added the ComicSeries and ComicIssue types? It looks like these are the ones left:
language
country
maturity_rating
CoMet-only items (ignore these?)
price
is_version_of # Not sure what this is?
rights # Copyright holder? Same as publisher is so.
identifier # ISBN etc.?
last_mark # No idea?

language
country
maturity_rating

you can just do these three

language
country
maturity_rating

you can just do these three

Looking at GCD they put the language and country IDs in the series table. I was thinking that putting it in the issue level makes more sense but now I'm not so sure. Country will come from the publisher but a publisher could publish in multiple languages? I'm probably over thinking it as it gets thrown to metadata and sits with the issue anyway.
Any strong opinion on series or issue for language or country attachment?

Any strong opinion on series or issue for language or country attachment?

Put it in with the issue, it could be stored on either one but issues are the only option that is guaranteed to be correct every time. for example ComicVine separates the Japanese manga from it's english (or other translation) into a separate series, MangaUpdates has a separate place where they mention the english publisher.

When it comes to checking/converting the language and country I think it would be best if talker_utils handled it. Taking either the 2-letter code (and checking it against pycountry) or the language/country name and converting it to the 2-letter code. Either map_comic_issue_to_metadata handles it or add some functions to talker_utils (this happens either way, it's whether the talkers are expected to use them or not.)
Do you agree talker_utils is the best place and/or when map_comic_issue_to_metadata processes the comic data?
Some info on sources:

MU the publisher type field is language when "original" is returned for the publisher (this can be Japanese, Korean etc.) (https://www.mangaupdates.com/publisher/91qzdxd/viz).
From the GCD DB, both 2 letter code and full name is available for language and country of the series. (https://docs.comics.org/wiki/Current_Schema)
CV has the country on the web page of the publisher but does not provide it via the API (back to scrapping the page like used to do with alt images if we want the data). (https://comicvine.gamespot.com/api/documentation#toc-0-26 vs https://comicvine.gamespot.com/viz/4010-428/)

In general it seems an additional API call will be required to infer either country or language so I plan to make it an option with the requisite help message.

I've noticed now that language is 2-letter country code and country is the name. This is to do with them being ComicInfo and CBL respectively. Annoying but there go.
I'm wondering for down the line (metadata plugins) if both should be in the generic metadata:

languageISO
language_name
countryISO
country_name

That way the exporter can choose?

I also missed format which I've now added.
It might make sense to add to utils a function to return format/manga/maturity rating list so they can be checked against in talker_utils when processing the ComicIssue/ComicSeries to genericmetadata. That is probably better for another PR?
Also adding countryISO and renaming language to languageISO in genericmetadata (add on on to store date PR with whatever new fields are needed also?).

Ubuntu build test machine failed getting a package. Might be temporary?

Ubuntu build test machine failed getting a package. Might be temporary?

That's because I need to tell it to fetch the latest package list before telling it to install a package. That's one reason why I hate the apt package manager.

Other than my outstanding question about removing get_country_iso and get_country_iso_name I believe I've covered everything?

authors: lordwelch mizaki

repo: comictagger/comictagger


text:
docs: write up "how we develop openpilot"

Most of the internal workflow exists in this repo. include stuff like:

how to run the tests
how to use useradmin
useful tools

scripts in selfdrive/debug
replay + plotjuggler

Opened #22483, no need to also track in an issue

repo: commaai/openpilot


text:
Add chime to lane changed blocked alert

Description: Changed the lane changed blocked alert to be more prominent, it's now orange and makes an alert sound.

I like this change, what do you think @pd0wm?

Yes. I like it!

Merging this one, let's do that in a separate PR


text:
paramsd: use real logMonoTime

Anything rely on this @nuwandavek?

No. Is this more correct? Only case I can think of is in the downstream analysis if the carstate logmonotime is very far off from actual logmonotime

Yes, logMonoTime should be timestamped in the process building/sending the message. For cases where we want to track the source info, we add a field with a frame ID or logMonoTime, e.g. lateralPlan.modelMonoTime.

authors: adeebshihadeh nuwandavek


text:
UI: poll modelV2

The issue before was that sm->receive was finishing either too early sometimes, causing the model path drawn to be the same as the previous model packet. This reduces the frames behind camera from 1-2 due to chances of timings to just 1 (50-100ms to 50ms)
Difference is very noticable when going over bumps, path sticks to the road more (as we're removing the inconsistencies).

When there's no map view up and the user isn't touching the screen causing a redraw, model is always drawn on the camera frame (at least from C3 being onroad with CAN replay for 45 minutes).
However the problem is that on the second C3 with the map view, it randomly draws the camera 1 frame ahead, and when touching and dragging the map view, camera view lags significantly which doesn't happen on master. Will see what's going on. Any ideas for someone who knows an obvious fix/sees an issue with the PR?

Has some timing issues, think if we start too late/early we end up polling for the previous model packet on start up. Need to dynamically adjust the update loop so we're consistently getting the latest.

Closing for now.

authors: adeebshihadeh sshane


text:
Cabana: add window icon

@deanlee is there a better way to do this?

it's good.:D

Lol, let's at least make this a bit nicer. Maybe just bigger comma and make it a circle?

@deanlee do you want to make a PR to add support for a tools/cabana qrc file? That's the only thing left for this :)

@sshane  added qrc resource file in https://github.com/commaai/openpilot/pull/27386

@deanlee away from my desktop and cabana doesn't yet work on Win11, can you test this out?

authors: adeebshihadeh deanlee sshane


text:
Provide more details for serverless deployment + Do not ensure-resource by default to be consistent and document the changes

Additionally:

Document token cache usage.

Not related to this PR: I found in index.googleFunction.js commercetools-adyen-integration/notification/index.googleFunction.js that there's no return added in all the places where response is used. Can you add return everywhere? Right now the code might have a weird behaviour.

https://github.com/commercetools/commercetools-adyen-integration/pull/566/commits/d6fb74052fc2065239d3f760ef9d30072869976e


text:
More understandable validation errors

Is your feature request related to a problem? Please describe.
When sending a variant creation request, there might be problems related to product type attribute constraints. The following is an example error message from submitting a variant with over 40 of such attributes :
child "resources" fails because ["resources" at position 0 fails because [child "attributes" fails because ["attributes" at position 0 fails because [child "value" fails because ["value" must be a boolean], child "value" fails because ["value" must be an array], child "value" fails because ["value" is not allowed to be empty], child "value" fails because ["value" must be an array], child "value" fails because ["value" is not allowed to be empty], child "value" fails because ["value" must be an array], child "type" fails because ["type" must be one of [enum]], child "value" fails because ["value" must be an array], child "type" fails because ["type" must be one of [lenum]], child "value" fails because ["value" must be an array], child "value" fails because ["value" must be an object], child "value" fails because ["value" must be an array], child "value" fails because ["value" must be an object], child "value" fails because ["value" must be an array], child "value" fails because ["value" must be a number], child "value" fails because ["value" must be an array], child "value" fails because ["value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object, "value" must be an object], child "value" fails because ["value" must be an array], child "value" fails because ["value" is not allowed to be empty], child "value" fails because ["value" must be an array], child "type" fails because ["type" must be one of [time]], child "value" fails because ["value" must be an array]]]]]
The error does not mention which attribute and what value caused the problem.
Describe the solution you'd like
It would be essential to see the name of the attribute causing the error, plus(if possible) any other information such as the value causing the error.
Describe alternatives you've considered
At least the  name of the attribute should be mentioned.

Sorry for the late reply. But this error is not directly related to the SDK and comes back from the import API. I will forward this feedback to the respective team.

authors: dpasdar jenschude


text:
Using Client Credentials flow from a public client (web browser, mobile app, desktop app) creates a cybersecurity vulnerability

re: https://github.com/commercetools/commercetools-sdk-typescript/blob/master/examples/react-vite/src/lib/ClientBuilder.ts
The client credentials flow is a confidential client flow, yet it is being used in a public client. This creates a security vulnerability by exposing the client secret to anyone who cares to read the source code. The client secret doesn't expire - it can only be replaced administratively.
The client credentials flow is intended for use by server applications authenticating themselves to a protected resource.
I strongly recommend that commercetools implement an appropriate authentication flow for public client applications. The current recommendation is the Authorisation Code Flow with PKCE (ref: https://auth0.com/docs/get-started/authentication-and-authorization-flow/which-oauth-2-0-flow-should-i-use).

Hi @mikehans
The SDK is not recommended to be used on it own, it should be integrated into an application that is already managing environment/secrets properly.
Some suggestion is to use the SDK behind a proxy, a server or a frontend reading its data of being served by a server.
Using the SDK on a plain javascript build is not recommended.

Hi @ajimae
I'm glad to hear you say that. However, you have frontend examples that do exactly this.

authors: ajimae mikehans


text:
use $default_branch instead of hardcoding "master"

The input branch should use the actual default branch.  I think you can grab this either by:

removing the default key in the action.yaml file
using $default_branch

I have an open PR awaiting reviews 😅
https://github.com/commitizen-tools/commitizen-action/pull/17

Released

This already solved now!

authors: Woile docwhat

repo: commitizen-tools/commitizen-action


text:
Caption lost after accepting "Is this a picture of..."

Steps:

Prepare two images, the second with coordinates near a Wikidata item (you can download them from Commons to test)
Enter a caption. Imagine the user is taking the time to write a great/accurate/descriptive very long caption.
Go to the second image. The dialog "Is this a picture of..." appears, click Yes
Problem: You loose everything you had typed.

https://github.com/user-attachments/assets/15c92af5-0ed7-4b0b-9305-aea3b9f1ae5a
Ideally the app would read the coordinates of all pictures of the multi-upload, find the first one whose coordinates are near a Wikidata item (something smarter can be devised in the future such as barycenter or clustering but let's keep this task simple for now), ask "Are these pictures of...", and only then let the user enter captions.

actually, i have ran the application i cannot see any error as such. is it this problem still there or is it closed

@QuantumPineapple68 Still happening to me. Are you using the GitHub main branch? Can you post a screencast? Thanks!

I checked it so many times and the issue seems to be solved. I am using the main branch itself. Can you please check once again from your end, and let me know ?? Thanks https://github.com/user-attachments/assets/a17c37c2-a116-4414-ab44-dc45f4060ab5

I’d like to work on this issue, can I be assigned to it?

@savsch Did you manage to  reproduce the issue? 🙂

@savsch Did you manage to reproduce the issue? 🙂

Yes, I was able to reproduce this issue. As a note, I typically do that part before requesting for assignment 😁

Fantastic, thanks! 🙂

authors: QuantumPineapple68 nicolas-raoul savsch

repo: commons-app/apps-android-commons


text:
Fix Lint errors/warnings #171

Delete unused imports.
Extract conditions into one method for readability.
Extract multiple used string into constant and minor change to check if list is empty.
Extract permission testing in a separate method for better readability.
Delete unused variable.
Delete unused code.
Description
Fixes #{GitHub issue number}
{Describe the changes made and why they were made.}
Tests performed
Tested on {API level & name of device/emulator}, with {build variant, e.g. ProdDebug}.
{Please test your PR at least once before submitting.}
Screenshots showing what changed
{Only for user interface changes, otherwise remove this section. See how to take a screenshot}

Codecov Report

Merging #1402 into master will not change coverage.
The diff coverage is 0%.

@@          Coverage Diff           @@
##           master   #1402   +/-   ##
======================================
  Coverage     3.3%    3.3%
======================================
  Files         128     128
  Lines        6837    6837
  Branches      669     668    -1
======================================
  Hits          226     226
  Misses       6596    6596
  Partials       15      15

Impacted Files
Coverage Δ

...n/java/fr/free/nrw/commons/MediaDataExtractor.java
0% <ø> (ø)
:arrow_up:

...fr/free/nrw/commons/settings/SettingsActivity.java
61.11% <ø> (ø)
:arrow_up:

...c/main/java/fr/free/nrw/commons/AboutActivity.java
0% <ø> (ø)
:arrow_up:

...free/nrw/commons/upload/MultipleShareActivity.java
0% <0%> (ø)
:arrow_up:

...fr/free/nrw/commons/settings/SettingsFragment.java
28.57% <0%> (ø)
: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 e23f752...d7fe5e4. Read the comment docs.

authors: codecov-io gabrielaradu


text:
Added Help link in Alert Dialogue

Description
Added help link to each alert dialogue with custom tab.
Fixes #3433
In each of the help popups, it would be great to have a link that a wiki page that explains things much more deeply. So I added a link to each info button that shows an alert dialogue and there I add a READ ME button from where the user can open the following custom webpage.
**Tests performed **
Tested 3.0.0-debug-helpLink~afd977f6c on Pixel 3a with API level 27.
Screenshots (for UI changes only)

Need help? See https://support.google.com/android/answer/9075928

Codecov Report

Merging #4315 (44de6a8) into master (d0f8954) will decrease coverage by 0.06%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #4315      +/-   ##
============================================
- Coverage     10.38%   10.31%   -0.07%
+ Complexity      473      471       -2
============================================
  Files           342      342
  Lines         13108    13124      +16
  Branches       1061     1061
============================================
- Hits           1361     1354       -7
- Misses        11678    11702      +24
+ Partials         69       68       -1

Impacted Files
Coverage Δ
Complexity Δ

...ons/profile/achievements/AchievementsFragment.java
0.00% <0.00%> (ø)
0.00 <0.00> (ø)

...mons/upload/mediaDetails/UploadMediaPresenter.java
55.88% <0.00%> (-6.26%)
15.00% <0.00%> (-2.00%)

...e/nrw/commons/explore/ExploreListRootFragment.java
0.00% <0.00%> (ø)
0.00% <0.00%> (ø%)

...upload/mediaDetails/UploadMediaDetailFragment.java
0.00% <0.00%> (ø)
0.00% <0.00%> (ø%)

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 d0f8954...44de6a8. Read the comment docs.

Thanks @4D17Y4 @nicolas-raoul , For this great review. I will keep these things in mind for my next PRs. I hope I fixed all minor issues as listed. Sorry for the inconvenience.


text:
Fix - No Precise Error Message After Error Due to Password Change

Description (required)
Fixes #5642
What changes did you make and why?
I identified the source of all these Errors (Which is No Accurate Information Provided to the User as to why an operation has failed because of a Single Type Of Exception that is getting lost due to Multiple layers of method calls), Hence I made a separate Exception Class for that Exception called - IvalidLoginTokenException so that it is easier to Identify and Propagate it.
Then I handled that Exception for each of the cases and since all of these Exceptions are the same their action is also Similar which is to Notify the user that Their Login Session has Expired Due to Password Change and They Must Login Again
Tests performed (required)
Tested on Xiaomi 11 Lite NE  with API level 33.

@nicolas-raoul @sivaraam @whym Please Review this PR

@nicolas-raoul @psh @whym @sivaraam Fixed all merge conflicts Please Review

Even though the above says 1 check passed : Run tests and generate APK, somehow the two unit tests below started failing in branch main from the present commit.
fr.free.nrw.commons.description.DescriptionEditActivityUnitTest > testUpdateDescription FAILED
    java.lang.reflect.InvocationTargetException at DescriptionEditActivityUnitTest.kt:120
        Caused by: fr.free.nrw.commons.auth.csrf.InvalidLoginTokenException at DescriptionEditActivityUnitTest.kt:120

fr.free.nrw.commons.description.DescriptionEditActivityUnitTest > testOnSubmitButtonClicked FAILED
    java.lang.reflect.InvocationTargetException at DescriptionEditActivityUnitTest.kt:132
        Caused by: fr.free.nrw.commons.auth.csrf.InvalidLoginTokenException at DescriptionEditActivityUnitTest.kt:132

Any idea what might be causing this, or even how to fix the tests? Thanks! :-)

@nicolas-raoul Seems like this error is due to Unchecked Exception at editDescription() Method that is added later on in another commit, looking into it.
Should I create a new PR for the fix or commit here?

@nicolas-raoul added a PR fixing this #5685

authors: nicolas-raoul shashankiitbhu


text:
Improve external dependency handling

Using an externally build DCMTK with CMAKE_DEBUG_POSTFIX set to "d" led to problems in the CTK CMake scripts.
Further, targets from external projects build within the CTK superbuild were not available when calling find_package(CTK). For example, the CTKXNATCore library depends on qRestAPI and when linking an external library to CTKXNATCore, CMake computes the transitive link dependency and links qRestAPI as well. However, there is no imported target available which leads to "library not found" errors thrown by the linker.

@jcfr Would you mind reviewing my changes please?

@jcfr I changed the PR according to your suggestions and also handle the variables in CTKConfig.cmake now differently for build and install trees. What do you think?

:+1:

Thanks for the good discussion :-)

authors: jcfr saschazelzer

repo: commontk/CTK


text:
DT-455: Add new jaspersoft setup instructions; remove ldap bind from …

…jaspersoft

Yeah the release process will be something like:
Create the new users + roles in Jaspersoft -> deploy Delta -> deploy common-infra


text:
Project Console Gerrit API

Summary
Write following api

Add Gerrit
Delete Gerrit
List Gerrit

User Story

As a project manager, I want to manage gerrits for cla-group

Acceptance Criteria
The "done" criteria when this feature or problem is resolved. Such as:

Unit Tests added and running in CI
Functional Tests updated to cover feature, if applicable
Demonstrate the set of capabilities to the product team while the code is running in the STAGING environment.

References
(Optional) Provide any code or specification references that would be helpful for the developer implementing this feature.

Tested and observed below issues
Add Gerrit
1. 422 is returned empty object is provided in the payload or when mandatory fields are not provided in the payload while adding gerrit

2. Appropriate error message is not displayed when string value is provided for groupIdCcla or groupIdIcla is provided in the payload

3. Able to Add gerrit instance with gerritName and gerritURL having only spaces

4. gerritUrl accepts free text and there is no validation for url format

5. Value of groupIdCcla is returned for groupIdIcla in the response

6. Able to add duplicate gerrit instance

7. Appropriate error message is not displayed when invalid groupIdCcla or groupIdIcla is provided in the payload

8. Able to add gerrit instance with only groupIdIcla or groupIdCcla when CLA group is enabled for both

9. groupIdIcla is not returned in the response when trying to add gerrit instance with only groupIdIcla

10. version is V1 while adding gerrit

------------------------------------------------------------------------------------------------------------------
Get Gerrit
1. Value of groupIdCcla is returned for groupIdIcla in the response

2. 200 with empty list is returned instead of 400 when cla-group and projectsfid provided in the url are not associated with each other or when invalid cla-group/projectsfid is provided in the url
3. groupIdIcla is not returned in the response when gerrit instance with only groupIdIcla exists

Tested and observed below issues. Except below issues all other issues mentioned above are fixed
Add Gerrit
6. Able to add duplicate gerrit instance
8. Able to add gerrit instance with only groupIdIcla or groupIdCcla when CLA group is enabled for both
Get Gerrit_
4. Able to add gerrit instance with url http://geerit, However while fetching the gerrit instance with Get action observed below issue

@rinkeshbhutwala to summarize the Gerrit validation:
Project Manager Provides this as input:

Via API

Gerrit Name: Can be any string value based on the Swagger regex rules.  Reject if we identify a duplicate Gerrit name in the database - don't allow.
Gerrit URL: we only allow Gerrit Instances from a known list. I have submitted a PR with this Swagger Doc update - see swagger doc.  Please add validation logic to check the values and reject if the value does not match from our list.
Group ICLA - currently the values are numeric, but we don't restrict - string is OK - the numeric-only value may change in the future.  Ensure value differs from submitted Group CCLA and that we do not have a duplicate value in the database for ICLA or CCLA group ID - should be unique across both column values in the database. If there is a duplicate, then I'm 100% sure this is a mistake by the Project Manager. This value is obtained from IT and should be just plugged in.  We can't validate further.
Group CCLA - currently the values are numeric, but we don't restrict - string is OK.  Ensure value differs from submitted Group ICLA and that we do not have a duplicate value in the database for ICLA or CCLA group ID - should be unique across both column values in the database.  This value is obtained from IT and should be just plugged in.  We can't validate further.

@nirupamav please provide comments/feedback if you know otherwise.

https://github.com/communitybridge/easycla/pull/1615

Tested and observed below issues
1. Appropriate error message is not displayed when 12 characters are provided for both groupIdCcla and groupIdIcla

2. Able to add duplicate gerrit instances for same project with same gerritName, groupId and gerritUrl

3. Able to gerrit instance with same gerritName, groupId and gerritUrl for different projects that are associated to same CLA Group

4.Able to add gerrit instance with only groupIdIcla or groupIdCcla when CLA group is enabled for both

Issue4 is still reproducible. Able to add gerrit instance with only groupIdIcla or groupIdCcla when CLA group is enabled for both

@nirupamav Can you please check the above screenshot which @dealako is posted in that mention that only one id is required to create a Gerrit instance for that can you please confirm with @dealako issue no. 4?

Verified below scenarios. Testing is still In progress

Duplicate gerrit instance with same ICLA or CCLA groupId cannot be created
If CLA Group is enabled for both ICLA and CCLA, validation is implemented to check if both ICLA and CCLA group Id is provided

Verified that It is working as expected

authors: dealako nirupamav prasannamahajan rinkeshbhutwala

repo: communitybridge/easycla


text:
DSND-3203: Change retry handling

Use Supplier for InternalApiClient.
Remove interface and abstract class for api client service.
Upgrade pom deps.
Remove public healthcheck.

DSND-3203

CI: No security warnings found

CI: No security warnings found

authors: ch-code-analysis sthompsonCH


text:
Web components → React POC

As we face the possibility of the design system needing to support “vanilla” HTML/CSS/JS in addition to multiple front-end frameworks, we should determine the easiest way to avoid duplication of UI components.
Let’s investigate the complexity of wrapping a non-trivial web component—one that implements LitHtmlElement and relies on click events, for instance—in a React component.
See: https://codesandbox.io/s/lion-web-components-react-rok79?file=/src/helpers/reactifyLitElementComponent.js

We've decided to go the other way around:

React components are converted to web-components
In case a specific React component doesn't work as intended, we still have the flexibility to write a web component from scratch, while reusing the theme tokens

authors: kendrick kidroca

repo: compassion-gds/compassion-gds


text:
Feat: "What We've Done" section - first two cards

Part of #212
This PR implements the "What We've Done" section with just the first two cards.
Dev analysis of spacing and decisions to round:

Desktop non-image card
Mobile non-image card

Rebased onto latest feat/our-work

Sorry, found one more spacing thing to fix

🙏

I wonder if @machikoyasuda has any ideas?

This is going to be tricky, but I think ideally these Outcome section headers / sections should line up (independent of the size of the description above).

I noticed that too just yesterday while looking at Figma. I agree it'll be tricky... I haven't thought of a way to do it yet. 🤔
edit: maybe we can set a minimum height on things? idk

Yeah, a min-height with a percentage would work - only when the screen width is wide enough / containers are set to be side by side:

Sidenote: I think there's something funky going on with the responsiveness. At a certain width, the layout goes down to 1 container per row, but if you go further - it goes to 2 containers per row again.

@machikoyasuda is helping with the alignment of card contents

authors: angela-tran machikoyasuda thekaveman


text:
Admin DataBase Repository;

resolves #121

closes | fixes | resolves

Что сделано?
Репозиторий для админов

Комментарии
Хочу согласовать, какие поля необходимы админу.

В качестве хеш функции, перед добавлением юзера в бд, можно использовать вот эту
https://stackoverflow.com/questions/1137368/what-is-default-hash-algorithm-that-asp-net-membership-uses

Думаю зайдет то решение из ссылки на СО, оно адекватным кажется

repo: complexitybot/Quiz