APNG Encode

As @HeroicKatora suggested, I made a PR directly here, this is supposed to continue the work done on image-png#99.
As I said over there, this PR takes the existing code and adds APNG encoding functionality, maintaining backwards compatibility, but is not supposed to be merged in it's current state. I'd like to hear suggestion from others on how to make it better.
APNG related features:

[x] encode frames
[x] set a separate default image
[x] set different frame rate for each frame
[x] encode frames that don't fill the frame
[x] set the default dispose and blend operation
[x] set the dispose and blend operation for each frame
[x] set the filter type for each write

So I've thought a bit on your comments @HeroicKatora and this is what I came up to:
About single images
Currently the ChunkWriter is kind of useless as it's only purpose is to bufferize the data coming from the StreamWriter, and add an IDAT chunk on every write, that could be done by using a BufWriter on the base writer itself and by writing the chunk directly on the StreamWriter. Another thing is that it doesn't even really need the Writer as it only needs it's Info to work. Also you can specify a buffer smaller that the in_line size and that wouldn't make much sense as there are two additional buffers of that lenght on the StreamWriter.
Having said that I think we can get rid of the StreamWriter and ChunkWriter objects at the same time by putting in the Writer the prev and curr buffers + a counter for the data written (or the lines written). Then the write_image_data method will just continue to append the data and write IDAT chunks once those are filled, without allocating the two buffers each time, until the complete image gets written.
Note: Right now it is possible to write additional image data as long as it fills the image completely (it can be used to hide an
image in another) so this solution will solve this "problem" too.
If backwards compatibility is required, StreamWriter could just become a wrapper around Writer that implements io::Write (or maybe it could be implemented directly in Writer)

Regarding animations
I was thinking about making a new struct that would implement Iterator:
the way I intended it to work is that it would return an AnimationEncoder for each frame, that could be used to set the properties of that frame, and that could then be made into a AnimationWriter so it could be used to write the frame data. Not sure if this would work, but if both the structs have a mutable reference to the base iterator than it would be impossible (with safe code) to have two AnimationWriters to the same image, so that on each iteration the writer has to be dropped in order to be able to encode the next frame.
The way I imagined all of this to work is like that:
// For single images
{
    let mut encoder = Encoder::new(100, 100);
    // set encorder options
    let mut writer = encoder.write_header().unwrap();

    writer.write_image_data(&data).unwrap(); // First part
                                                // ...
    writer.write_image_data(&data).unwrap(); // Last part
    writer.finish().unwrap(); // Because it can silently fail while dropping
}
// For animations
{
    let mut encoder = AnimationEncoder::new(100, 100, 20); // 20 frames
    // set encoder options
    let mut frames = encorder.write_header(&default_image).unwrap(); // or write_animation_header() if only Encoder is used

    for frame_enc in frames {
        // set frame options
        let writer = frame_enc.write_header(); // or something on that line

        writer.write_image_data(&data).unwrap(); // First part
                                                    // ...
        writer.write_image_data(&data).unwrap(); // Last part
        writer.finish().unwrap(); // Because it can silently fail while dropping
    }
}

Like that it might be better to call the two types of writers ImageWriter and AnimationWriter
Obviously it's still very generic and some more thinking should be done in order to avoid code duplication, I'm not an expert so I don't expect this to be completely right.

While I'm here, I've seen that the write_chunk method is public and the field in ChunkType is also public so anything could be written between the image data. That is right as if you have a propetary extension you want to be able to write it, but I feel like this is not the best option: (mine probably isn't too)
If we put ChunkType as a trait that has a const TYPE: [u8; 4] (and maybe implements Deref<[u8;4]>) then we would be able to make custom constructor for each chunk, for example:
struct IDATChunk;

impl ChunkType for IDATChunk {
    const TYPE: [u8;4] = *b"IDAT";
}

// The above part wouldn't change much the current state/usage

impl IDATChunk { // or maybe impl io::Write
    pub fn write<W: io::Write>(
        writer: &mut W,
        filter: FilterType,
        bpp: BytesPerPixel,
        curr: &[u8],
        prev: &[u8],
    ) {
        // ...
    }
}

Then we could avoid code duplication between the types I mentioned above (Writer/AnimationWriter) and propertary extension could still be made by implementing ChunkType and maybe by exposing a method like get_raw_writer (again it's still too general to get a perfect idea of how things get together).

That's it, I should give it another thought but I want to make progress and can't spend much time on this during the week so I'm going to throw it in and read your opinions

I'm sorry that it took a while to answer but I wanted to be thorough. However, now this PR becomes more complicated with each commit, at least in my perception,—the chunk encoding logic is duplicated, and so is the {Animation,Image}Encoder interface with configuration for gamma, chromaticities,—and I want to respond more quickly to ensure that it doesn't become unreviewable and over engineered. So, this may be slightly unfinished but at least it's a direction.

If backwards compatibility is required, StreamWriter could just become a wrapper around Writer that implements io::Write (or maybe it could be implemented directly in Writer)

It isn't. Currently the plan is for 0.17 to have all breaking changes necessary to improve the API for full APNG support while keeping the rough structure we currently have. And the 0.16 interface is more or less finished, the master changes go into 0.17.

While I'm here, I've seen that the write_chunk method is public and the field in ChunkType is also public so anything could be written between the image data. That is right as if you have a propetary extension you want to be able to write it, but I feel like this is not the best option: (mine probably isn't too)

The png crate should have low-level accessors, I'd even like to give control over the chunk checksums to the user at some point such that broken images can be replicated accurately (or to do stenography, or to disable checksums entirely because they don't have a very useful purpose in this age). The interface is however poor: It should allow assembling the chunk from multiple slices which would avoid some allocations, it should make writing incorrect data (i.e. chunks too long, unknown chunk types, chunk types with incorrect order) more explicit. I'd keep the primitive though in some capacity. An explicit call to a wrapper such as get_raw_writer might be a middle-ground worth looking at. In an independent PR.

Having said that I think we can get rid of the StreamWriter and ChunkWriter objects at the same time by putting in the Writer the prev and curr buffers + a counter for the data written (or the lines written). Then the write_image_data method will just continue to append the data and write IDAT chunks once those are filled, without allocating the two buffers each time, until the complete image gets written.

Point well taken. I do agree that the exposed ChunkWriter shouldn't be the same owning primitive that is used internally, and that it should borrow from the main writer in the same way as StreamWriter. The only negative aspect is that the borrow restricts the downstream interface. There should be a way to stop writing a chunk, drop the borrow, but later continue writing at the exact same state. Maybe these two interfaces would help:

StreamWriter::pause(self) -> PartialChunkData
Writer::continue_chunk(&mut self, _: PartialChunkData) -> StreamWriter<'_>

Another good idea for another PR :)
I'm much more critical of merging the two types however. The image line state is necessary only for writing image data and the chunk length restriction is on the length of compressed data which we can't necessarily predict from the raw sample buffers alone, as it also depends on the prediction functions etc. Using less buffer copies could work though.

I was thinking about making a new struct that would implement Iterator: [..]
Like that it might be better to call the two types of writers ImageWriter and AnimationWriter

I don't like the idea of additional owning types, it's already too many. It really complicates many designs downstream to have two incompatible types. An example for this is the image::ImageBuffer (and inflexibility of DynamicImage). It works, but isn't ergonomic. The basic idea of having a new, borrowing adaptor for each frame as a sort of temporary type state sounds great. Here's a diff which actually could make it work, instead of implementing Iterator:
-    for frame_enc in frames {
+    while let Some(frame_enc) = frames.next_animated() {

…where fn next_animated(&mut self) -> FrameWriter<'_> is a method on that type that borrows self mutably. This solves the soundness problem, it isn't really necessary to write two frames at once. The interesting thing about this would be that the main writer could be a single type. In the non-animated case we simply would always return None on that method. Plus it lends itself to an workflow where the default image is not part of the animation.
// set encoder options
let mut encoder = Encoder::new(100, 100)
  .animated(20); // Or don't call it, for static PNG.
let mut frames = encorder.write_header().unwrap();

// This will never be part of the animation.
// If you don't call it on animated PNG, the first animated frame will write IDAT chunks.
frames.write_image_data(&default_image).unwrap();

// Or don't loop, for static PNG.
while let Some(frame_enc) = frames.next_animated() {
    // set frame options
    let writer = frame_enc.write_header(); // or something on that line

    writer.write_image_data(&data).unwrap(); // First part
                                                    // ...
    writer.write_image_data(&data).unwrap(); // Last part
    writer.finish().unwrap(); // Because it can silently fail while dropping
}

If you don't loop then you've naturally written a non-animated PNG. If you don't call the default write_image_data then all frames are part of the animation.

I agree with you that this is starting to look over engineered, I was going to write a little review of my code addressing the code duplication problem but you preceded me.
Regarding the merging of the {Animation,Image}{Writer,Encoder} I think it can easily be done, I didn't think about it before because I felt that having two distinct type would have been better, but considering the code duplication problem and the downstream API conflicts, it really might be the best option. The only thing that I would change from your example is setting the separate default image data flag with a method so that it could work like now, where the first frame is actually the default image data and the following frames would be considered as proper frames.
I'd like to have more details on the Stream::pause and get_raw_writer part: as I understood you want this PartialChunkData to permit access to the internal writer so that raw data could be written but should the writer flush before or the internal state should remain untouched?
Also could you take a look at chunk new.rs file, I was experimenting with the chunks data type.
I'm going to start working on merging the writers/encoders, thank you for the useful advice

Got inline with the latest commit on master and merged the Writers/Encoders, now it works like your example but with some differences:
For encoding an image
    let mut enc = Encoder::new(file, width, heigth);
    // enc.set...
    let mut ctrl = enc.write_header().unwrap();
    let mut wrt = ctrl.write_frame_header().unwrap();
    wrt.write_image_data(&data).unwrap();
    // ... Nth write
    wrt.finish().unwrap();
    ctrl.finish().unwrap();

For encoding an animation
    let mut enc = Encoder::new(file, width, heigth);
    // enc.set...
    enc.animated(frames, repeat).unwrap();
    let mut ctrl = enc.write_header().unwrap();
    while ctrl.remaining() > 0 {
        // ctrl.set...
        let mut wrt = ctrl.write_frame_header().unwrap();
        wrt.write_image_data(&data).unwrap();
        // ... Nth write
        wrt.finish().unwrap();
    }
    ctrl.finish().unwrap();

For encoding with a separate default image
    let mut enc = Encoder::new(file, width, heigth);
    // enc.set...
    enc.animated(frames, repeat).unwrap();
    enc.with_separate_default_image().unwrap();
    let mut ctrl = enc.write_header().unwrap();

    // this could go in the while loop if the data is already lined up
    {
        let mut def_img_wrt = ctrl.write_frame_header().unwrap();
        def_img_wrt.write_image_data(&def_img_data).unwrap();
        def_img_wrt.finish().unwrap();
    }

    while ctrl.remaining() > 0 {
        // ctrl.set...
        let mut wrt = ctrl.write_frame_header().unwrap();
        wrt.write_image_data(&data).unwrap();
        // ... Nth write
        wrt.finish().unwrap();
    }
    ctrl.finish().unwrap();

Regarding the ChunkWriter, I've been trying things out a bit, and came to the conclusion that we cannot go any further with its implementation as long as we can't access the inner writer in the ZlibEncoder, that's because we need to be able to write uncompressed IDAT and fdAT chunks in between the compression stream. Having access to the inner writer will make possible have the pause method you were talking about. If you agree with me I'm going to open a PR on the deflate-rs crate to add that method. BTW I haven't studied the compression algorithm so tell me if I'm missing something that would make this worng/unfeasible.
Right now I'm using a BufWriter with an inner ChunkWriter that adds the chunk data on each write (by the BufWriter) and can change between writing an IDAT chunk and a fdAT chunk with a flag. One thing that I have noticed, in fact, is that even with the method I talked above, the ChunkWriter would need a wrapper because it still need the additional chunk data (like the sequence number) and couldn't be generalized to each chunk without having a specific type for each one (that will till need a wrapper enum for allowing to swap between IDAT and fdAT).

A note: You're still doing many, many superfluous edits left-and-right (changing set_palette, changing return types to Self from an explicit name etc, leaving a lot commented code). It detracts from the goal of apng encoding. Please consider that reviewing code takes time, a lot especially if you're introducing a new idea. Ensure that reviewers can focus on that idea instead of having to spend attention on anything else.

Sorry for making your job more difficult, it's my first time contributing to an open source project and I have still a lot to learn. I'm going to remove anything that hasn't to do with APNG encoding (like the time chunk that I forgot to remove after trying things out with the "chunk new" file). Also I thought that using Self instead of the explicit name was a good practice (not referring to the errors on the test, but to return type and constructors). Though, with my comment I was more interested on the ZlibEncoder part.

The main challenge right now is that the pull request adds/modifies over 1300 lines. The general advice is to break pull requests down as much as possible since even when changes might be an improvement on their own (like improving formatting/naming/consistency) they can become overwhelming when trying to make sure a complex PR is correct.

It's my first time contributing to an open source project and I have still a lot to learn.

Welcome then :slightly_smiling_face:
On teaching how to contribute to open source, there have already been two good demonstration on why to keep PRs small and self-contained. The first being the error rework which you had to adjust for. Chances are, other changes are in progress in parallel to your changes and every line or idea is a potential point of conflict. Minimizing those means quicker progress.
The second instance, now, the CI failure. Doing a ton of changes only to discover that the first is incompatible with some requirement feels bad and is very unfortunate as it requires extra work picking out those changes that are unaffected. Again, small changes lower the chances of that happening.

Did some cleaning of changes that were not needed, hopefully this makes your work easier. Let me know what to do for the ZlibEncoder problem/enhancement and thank you for being kind with me, I already understood my errors and I'll try to not repeat them again in future. I've learned (and I'm going to lear) a lot from this experience.

Please also remove the Zlib stuff, especially since you don't know how/if it works out. I've opened #267 as a dedicated tracking issue as it is currently merely a draft.

It works, when I said I didn't understand the compression algorithm I wasn't referring to the usage of the library but to the actual implementation because I don't know if exposing the inner writer invalidates the compression stream. The ZlibWriter just encapsulates what was previously done inside write_image_data (and previously in deflate_image_data) but allowing to load scanlines in more writes.
I'm not sure if you noticed but StreamWriter right now is just a wrapper around Writer, and that's because there isn't really much of a difference between those two. You don't get much of an advantage by having the complete image data all at once instead of randomly sized chunks as you'd still have to filter each scanline individually, compress the filtered data and write it in a PNG chunk that might split the compressed stream at an arbitrary point (the maximum chunk size).
Also, currently in the StreamWriter implementation, after writing the compressed data the two buffers get swapped but the curr_buf contains the filtered data not the original unfiltered one, so on the next scanline the prev_buf will contain wrong data.
Having said that, if you think this should not be done, taking those changes back it's easy

Compared to Writer, the StreamWriter allows multiple incremental writes while ensuring¹ that those IDAT chunks are not interspersed with any other chunks as required by the specification. Any interface on Writer directly would not give such convenience as it offers too many low-level methods. Reducing the  interfaces—and thus adding new invariants—is the essence of the common newtype concept and it was not unnoticed or surprising even though no additional fields have been added. In addition, requiring two lifetimes on a struct is a smell of design complexity when they should be effectively based on each other.
¹not 100% ensuring if you have a shared writer but very much encourages it.

The need of a double lifetime comes from the fact that currently the Writer borrows the inner writer of the Controller (as we discussed previously). We can't insert the into_stream method inside the Controller because that would make the animation control really difficult on the StreamWriter side. The only thing we could do is to make the Writer take full ownership of the Controller, that would solve our problems but will make the usage a bit strange as to continue on with the animation you'd have to get back the controller from the finish method of the Writer.
It would look like this:
pub struct Writer<W: Write> {
    // ...
    ctrl: Option<Controller>,
}

impl<W: Write> Writer<W> {
    // ...
    pub fn finish(&mut self) -> Result<Controller> { /* ... */ }
}

It does not reflect my understanding of what needs to own the stream. I'm trying to pick out your conceptual changes of ownership but there are quite a lot and some unclear to me.
The current concepts are:

Encoder: Setup the operation, gather all input which we need for the IHDR etc. Then do that write call, transforming it into a Writer.
Writer: For the raw chunk-leve operations, with one utility method of writing several consecutive IDAT chunks.
StreamWriter: It's currently for writing image chunks incrementally, ensuring through type interface that they are not interspersed with other chunks.

Can you make a similarly clear differentiation for each struct in the PR and justify the difference instead of purely an addition? If not, then don't change them. As far as I'm willing to invest into following the changes:

Writer corresponds your PR to Controller with less methods and no more low-level chunk writes?
StreamWriter is in your PR now Writer? I honestly can't tell. The backwards compatibility justifications for its conversion to a StreamWriter is brittle at best because you've change the type definition of Writer itself, hence there will be no compatibility anyways. I'd also be interested why you want to differentiate between your versions of Writerand StreamWriter? What guarantees or ergonomics does the distiction afford?

Please focus on the last point in particular since that is the source for your two-lifetime borrows. There's a substantial amount of code dedicated to that complexity: The Option for taking the stream, a lot handling for then calling methods after the stream was taken, and configuring and tracking different aspects of the writing in three different structs.

Ok so, my idea was to remove the Writer and use only StreamWriter as the only type of writer, but now I don't think this is a good idea anymore (forgive me). The current StreamWriter is just an abomination I came up with to fill the gaps and to make all tests work.
For the understanding of the current mechanism, all the structs have the same functions as before, what is new is the Controller which stands in the middle between the Encoder and the Writer. It simply counts the number of images you write and writes the "image header" each time you ask for a new one; this header is either nothing, if it's an image or if it's the separate default image of the animation, or the fdAT chunk for the animation frames. The current Controller spits out Writers that borrow it's inner writer, so that it's not possible to have to writers on the same controller at the same time. That, though, will make the StreamWriter unable to own the writer (because it "doesn't own itself"). The only way to make it work is by having the Writer take ownership of the Controller for as long as it lives, so that it could be later taken back (with the finish method) and used to write other images.
On the Option<W> part, it's the only way to get rid of the need of code execution in the destructor, it's even used in the deflate-rs crate for the same reason.
I'm currently taking back the StreamWriter and making the Writer own the Controller, it's not going to take that much, then you'll probably understand better what is going on. (I understand I made really poor decisions)

I completely forgot to post a comment, sorry for taking so long. I think it's ready for a review, I did a test yesterday and the encoding of animations was working

@HeroicKatora I was wondering is there any progress going on? I don't want to sound annoying, just asking

@HeroicKatora I was wondering is there any progress going on? I don't want to sound annoying, just asking

I understand all the points on the cleverness of the code I wrote, and please don't get me wrong I didn't wrote that because I don't care about those things, it's just how I'm used to. The article you linked was helpful and I've been encountering many of those aspects since when I started working as programmer because the codebase I'm working on is really full of those clever lines and I have to go back and forth every time to understand what is going on and talking with my collegues started to make me think more on this aspect of writing code.

I will open the pull requests for the encode methods as you suggested as soon as I can. Should I implement them in the encoder.rs file like I did here or directly in common.rs? Should it be a single PR or one for each struct?

Regarding the other questions I feel like many of them have already been answered in my previous comment. I think there has been a misunderstanding in that situation as the reaction to the comment and the fact that no other comment was submitted in the following day made me think that you were ok with what I wrote and that's why I started working on it.
Regardless, I'm going to explain it again:
First of all the only two addition are the Controller struct and the ChunkData enum, all the others (Writer, Encoder, StreamWriter, ChunkOutput, ChunkWriter) have been modified just to match the other code and to allow the new functionalies.
I feel like the Controller name is kind of confusing you as I've seen you often comparing it to the Writer but it's not what it is. I cannot think of a better name for it but hopefully this will help.
The Controller as 3 functions:

select the parameters for the image that is going to be written next (this includes the filter, the blend op, the frame delay, etc.);
write an image "header" depending on if it's an image or an animation and other things (separate default image, first frame, etc.);
provide a writer that allows the image data to be written;

+1 that will be added, as you suggested: write chunks in-between frames directly (through Controller::write_chunk).
Here comes in the new ChunkData: it allows to store the state of the image/animation so that neither the Writer nor the StreamWriter have to know what they are writing, they just call encode and it will take care of writing IDAT or fdAT chunks, incrementing the sequence number. BTW I'd like some more explanation on the comment about the state machine as I didn't quite get that.
The Writer and StreamWriter didn't see much of a change other that the use of an Option<_<W>>. This allows to move the inner writer without having to drop all the struct and that is needed to allow for the Writer and StreamWriter to own the controller. As an example I pointed to the deflate-rs crate that does the same thing for the same reason. And yes the public interface can easily be modified so that it consumes the Writer.
Now, the StreamWriter is supposed to be able to either own the inner writer or borrow it exclusively, but on your comment you suggested removing the Option around the inner writer so that it could be borrowed, but by doing so then it would be impossible to have a StreamWriter that owns the inner writer. You are right that it would allow for the controller to not be dropped when an error occurs during a finish call, but how would you recover from a probably corruped PNG stream during the encoding? Wouldn't you just have to start all over again? Currently there is no error recovering system so I don't think it's important, let me know about that.
On the note about not being scared of making dynamic checks in order to favour simplicity I'll try to come up with an implementation that uses only Writer and StreamWriter. When I did it the first time I found it confusing, now that I understand the library a bit better I might find a better way.
Let me know what you think of everything, also if you can give me other tips or anything it will be highly appreciated.

I understand all the points on the cleverness of the code I wrote, and please don't get me wrong I didn't wrote that because I don't care about those things, it's just how I'm used to. The article you linked was helpful and I've been encountering many of those aspects since when I started working as programmer because the codebase I'm working on is really full of those clever lines and I have to go back and forth every time to understand what is going on and talking with my collegues started to make me think more on this aspect of writing code.

I will open the pull requests for the encode methods as you suggested as soon as I can. Should I implement them in the encoder.rs file like I did here or directly in common.rs? Should it be a single PR or one for each struct?

Regarding the other questions I feel like many of them have already been answered in my previous comment. I think there has been a misunderstanding in that situation as the reaction to the comment and the fact that no other comment was submitted in the following day made me think that you were ok with what I wrote and that's why I started working on it.
Regardless, I'm going to explain it again:
First of all the only two addition are the Controller struct and the ChunkData enum, all the others (Writer, Encoder, StreamWriter, ChunkOutput, ChunkWriter) have been modified just to match the other code and to allow the new functionalies.
I feel like the Controller name is kind of confusing you as I've seen you often comparing it to the Writer but it's not what it is. I cannot think of a better name for it but hopefully this will help.
The Controller as 3 functions:

select the parameters for the image that is going to be written next (this includes the filter, the blend op, the frame delay, etc.);
write an image "header" depending on if it's an image or an animation and other things (separate default image, first frame, etc.);
provide a writer that allows the image data to be written;

+1 that will be added, as you suggested: write chunks in-between frames directly (through Controller::write_chunk).
Here comes in the new ChunkData: it allows to store the state of the image/animation so that neither the Writer nor the StreamWriter have to know what they are writing, they just call encode and it will take care of writing IDAT or fdAT chunks, incrementing the sequence number. BTW I'd like some more explanation on the comment about the state machine as I didn't quite get that.
The Writer and StreamWriter didn't see much of a change other that the use of an Option<_<W>>. This allows to move the inner writer without having to drop all the struct and that is needed to allow for the Writer and StreamWriter to own the controller. As an example I pointed to the deflate-rs crate that does the same thing for the same reason. And yes the public interface can easily be modified so that it consumes the Writer.
Now, the StreamWriter is supposed to be able to either own the inner writer or borrow it exclusively, but on your comment you suggested removing the Option around the inner writer so that it could be borrowed, but by doing so then it would be impossible to have a StreamWriter that owns the inner writer. You are right that it would allow for the controller to not be dropped when an error occurs during a finish call, but how would you recover from a probably corruped PNG stream during the encoding? Wouldn't you just have to start all over again? Currently there is no error recovering system so I don't think it's important, let me know about that.
On the note about not being scared of making dynamic checks in order to favour simplicity I'll try to come up with an implementation that uses only Writer and StreamWriter. When I did it the first time I found it confusing, now that I understand the library a bit better I might find a better way.
Let me know what you think of everything, also if you can give me other tips or anything it will be highly appreciated.

The Controller as 3 functions: [..]

This is pretty much what I had meant for and described the current Writer to provide, except the parameter selection was never added. The other two points apply directly: (2) by write_image and (3) by the borrow/conversion to StreamWriter. Plus the additional functionality of writing arbitrary chunks independent of an image because the PNG specification does not place inherent order constraints on chunks, except for:

IHDR (shall be first; enforced by write_header)
IEND (shall be last; enforced by drop)
the standard chunks, not consistently enforced and doing all of them by type system leads to the problem address in the following section on state machine.

On owning vs. non-owning I'm very confused by your statement that an Option is required—mostly for that fact that you have kept ChunkOutput. That enum models a solution for precisely this problem: the ability to either own or borrow a controller and its contained writer W. And before your PR the StreamWriter already does this. (stream_writer(&mut) vs. into_stream_writer). It that is no longer clear to yourself then cleaning up and rebase could give the breather to take another look at the current types, unconfounded by the lifetime troubles of the first approach.
On state machine: Rust's type system makes it so that you can use parameters zero or one time but you can not arbitrary copy them (this propery is called affine). When writing methods that take their parameter by value your code places a restriction on the caller: They can not use that object again, and they must have owned it. So, the methods taking self do encode a statemachine through the type system. (Clarification: I think this is an anti-pattern):
Encoder --[write_header]--> Controller --[next_image]--> Writer --[into_stream_writer]--> StreamWriter

By doing this, any caller that wants to call next_image twice must own a controller, or encode in its own struct the state for 'this controller is currently wrapped in a Writer', even if they never intend to do a partial write of an image. In other words, they must Option-wrap the Controller. Confusingly, the methods to get the controller back (Writer::finish) do not take by value so you still remaing with a Writer afterwards. Now that's plainly inconsistent. And no caller can rely on ever getting a Controller back, at least as far as the type system is concerned.
All the Option wrapping troubles come from a similar fact: By choosing the signature finish(&mut self) -> Result<W>, you promise to be able to produce a value of type W and the only way to do is to take one from Controller, but you must keep a valid controller. Hence, ZlibEncoder and my comment proposes not to choose such a signature.
Now you, proposing that dropping the Controller in an error case is fine, ask:

But how would you recover from a probably corrupted PNG stream during the encoding

However, consider that most errors do not signal a corrupted PNG stream. Except for IO errors, most are checked before any write is performed. Your own added checks for APNG are of that sort. Mostly, all produced chunks will be intact and complete. Recovery would be possibly, in libpng et.al place great value in error recovery, even when not perfect or even when it means dropping a chunk. (The situation could be improved in a few places).

The Controller as 3 functions: [..]

This is pretty much what I had meant for and described the current Writer to provide, except the parameter selection was never added. The other two points apply directly: (2) by write_image and (3) by the borrow/conversion to StreamWriter. Plus the additional functionality of writing arbitrary chunks independent of an image because the PNG specification does not place inherent order constraints on chunks, except for:

IHDR (shall be first; enforced by write_header)
IEND (shall be last; enforced by drop)
the standard chunks, not consistently enforced and doing all of them by type system leads to the problem address in the following section on state machine.

On owning vs. non-owning I'm very confused by your statement that an Option is required—mostly for that fact that you have kept ChunkOutput. That enum models a solution for precisely this problem: the ability to either own or borrow a controller and its contained writer W. And before your PR the StreamWriter already does this. (stream_writer(&mut) vs. into_stream_writer). It that is no longer clear to yourself then cleaning up and rebase could give the breather to take another look at the current types, unconfounded by the lifetime troubles of the first approach.
On state machine: Rust's type system makes it so that you can use parameters zero or one time but you can not arbitrary copy them (this propery is called affine). When writing methods that take their parameter by value your code places a restriction on the caller: They can not use that object again, and they must have owned it. So, the methods taking self do encode a statemachine through the type system. (Clarification: I think this is an anti-pattern):
Encoder --[write_header]--> Controller --[next_image]--> Writer --[into_stream_writer]--> StreamWriter

By doing this, any caller that wants to call next_image twice must own a controller, or encode in its own struct the state for 'this controller is currently wrapped in a Writer', even if they never intend to do a partial write of an image. In other words, they must Option-wrap the Controller. Confusingly, the methods to get the controller back (Writer::finish) do not take by value so you still remaing with a Writer afterwards. Now that's plainly inconsistent. And no caller can rely on ever getting a Controller back, at least as far as the type system is concerned.
All the Option wrapping troubles come from a similar fact: By choosing the signature finish(&mut self) -> Result<W>, you promise to be able to produce a value of type W and the only way to do is to take one from Controller, but you must keep a valid controller. Hence, ZlibEncoder and my comment proposes not to choose such a signature.
Now you, proposing that dropping the Controller in an error case is fine, ask:

But how would you recover from a probably corrupted PNG stream during the encoding

However, consider that most errors do not signal a corrupted PNG stream. Except for IO errors, most are checked before any write is performed. Your own added checks for APNG are of that sort. Mostly, all produced chunks will be intact and complete. Recovery would be possibly, in libpng et.al place great value in error recovery, even when not perfect or even when it means dropping a chunk. (The situation could be improved in a few places).

You did not understand my statement on owning/borrowing because you were not considering the Controller but this doesn't matter anymore because now that I understood clearly your point on the state machine and the Rust type system it's obvious that it will have to be removed.
BTW I when I wrote "And yes the public interface can easily be modified so that it consumes the Writer"  I meant that the finish method could have the signature you asked because it doesn't really matter on the implementation side, I probably should have been more explicit.
Right now the main thing I'm not sure of is how to handle animations on the StreamWriter side and also how to decide when the parameters(blend op, frame delay, ...) can be changed. Because on the Writer side is pretty straghtforward: set the parameters, write the data, go on with the next frame, but on the StreamWriter there are more calls to write for the same frame. Solutions that come to my mind are:

keep track of the amount of data written and write fcTL when a new frame is added, and allow changing parameters only when the data written for the new frame is 0;
Add a method like next_frame that and takes and writes an fcTL with the parameters for that frame without considering the amount of data already written (as it does right now);

Related to the last point is why the Writer side checks that the data you provide fills the entire image while allowing to write more data after that but on the other end the StreamWriter does nothing.
Also why check the palette to be present on the write_image_data if the color type is Indexed as if one of the two can change while the image is being written, while also not performing any check on the StreamWriter.
What is confusing me here is the line between the need of checking the correctness of what it's going to be written and blaming who is using the library for using it wrong.
If you could answer those questions and the other ones I asked on the previous comment about what to do with the encode methods I could start working on it tomorrow.

You did not understand my statement on owning/borrowing because you were not considering the Controller but this doesn't matter anymore because now that I understood clearly your point on the state machine and the Rust type system it's obvious that it will have to be removed.
BTW I when I wrote "And yes the public interface can easily be modified so that it consumes the Writer"  I meant that the finish method could have the signature you asked because it doesn't really matter on the implementation side, I probably should have been more explicit.
Right now the main thing I'm not sure of is how to handle animations on the StreamWriter side and also how to decide when the parameters(blend op, frame delay, ...) can be changed. Because on the Writer side is pretty straghtforward: set the parameters, write the data, go on with the next frame, but on the StreamWriter there are more calls to write for the same frame. Solutions that come to my mind are:

keep track of the amount of data written and write fcTL when a new frame is added, and allow changing parameters only when the data written for the new frame is 0;
Add a method like next_frame that and takes and writes an fcTL with the parameters for that frame without considering the amount of data already written (as it does right now);

Related to the last point is why the Writer side checks that the data you provide fills the entire image while allowing to write more data after that but on the other end the StreamWriter does nothing.
Also why check the palette to be present on the write_image_data if the color type is Indexed as if one of the two can change while the image is being written, while also not performing any check on the StreamWriter.
What is confusing me here is the line between the need of checking the correctness of what it's going to be written and blaming who is using the library for using it wrong.
If you could answer those questions and the other ones I asked on the previous comment about what to do with the encode methods I could start working on it tomorrow.

If you could answer those questions and the other ones I asked on the previous comment about what to do with the encode methods I could start working on it tomorrow.

One PR for all the encoding methods would be fine. The impls can live where each struct is defined, but that's not a strong preference. I'll get back to the rest of the response later.

If you could answer those questions and the other ones I asked on the previous comment about what to do with the encode methods I could start working on it tomorrow.

One PR for all the encoding methods would be fine. The impls can live where each struct is defined, but that's not a strong preference. I'll get back to the rest of the response later.

Side note on terminology: We're both discussing current semantics and wanted semantics. For obvious reason using the type names of this PR is confusing for those cases. Try to avoid renames such that semantics collide with current structs of the same name in the rework. Or we discuss the renames as a dedicated preceeding PR. If you want to do this, see: renaming in gif and its current name design. In any case I'll use the terminology of this PR for now so that we can also refer to any additional types..

Right now the main thing I'm not sure of is how to handle animations [..]

Then don't. Have each just write at most a single frame, just like it is now. The main functionality of not requiring a full allocation is still there even when one needs to call the Controller::next_image method multiple times.

Right now the main thing I'm not sure of is how to [..] decide when the parameters(blend op, frame delay, ...) can be changed.

It should be clear that changing a parameter has effect on its next usage, not immediately or retroactively. That is, there is little reason to stop it from being changed or to require it to be only changed once. So I would add setters of frame parameters to Controller and setters for parameters relating to image data—well, only FilterType—also to Writer/StreamWriter. Then the former are used for fcTL chunks (blend, dispose, etc.) and the latter can be changed when different scanlines should prefer different filters.

What is confusing me here is the line between the need of checking the correctness of what it's going to be written and blaming who is using the library for using it wrong.

An interesting little point here regarding Writer vs. StreamWriter: It's not necessary to have them as separate types. In fact, their methods may be merged. All that is necessary is to make the methods that write full frames return an error if any partial write through the stream methods has been performed. The only 'risk' involved for the caller with StreamWriter is that they might forget writing part of the frame, which corrupts the PNG. But neither the merge nor the split types addresses this risk differently with regards to their finish call nor their implicit Drop. In both cases a partial frame, i.e. having partial pixel data or having no pixel data, is incorrect according to the PNG specification but not fatally incorrect for our own program state. It's hard to say if we should even impose a hard requirement on image correctness here as long as the chunk framing is proper—which it is when IO itself succeeds. Then a lenient decoder might just skip or zero-fill bad frames, the rest of the image is still intact.
I briefly touched on this above. I don't think it is necessary to enforce total correctness of the PNG, except where we can make a specific guarantee and the caller explicitly requests us to do so. Accidentally making something impossibleis worse than incorrectness on wrong usage. That's also the reason why the write_chunk method is exposed in the first place. After all, it would allow arbitrary incorrect chunks to be inserted at arbitrary places. But that is clear to the caller.
The write_image_data is meant as a short cut, relative to StreamWriter. As for the check on indexed, there was an idea floating around of having the palette write being delayed—not automatically done in write_header—hence the duplicate check. That didn't materialize so it's probably in a weird spot. It wouldn't hurt to check it for the StreamWriter construction, too.

Side note on terminology: We're both discussing current semantics and wanted semantics. For obvious reason using the type names of this PR is confusing for those cases. Try to avoid renames such that semantics collide with current structs of the same name in the rework. Or we discuss the renames as a dedicated preceeding PR. If you want to do this, see: renaming in gif and its current name design. In any case I'll use the terminology of this PR for now so that we can also refer to any additional types..

Right now the main thing I'm not sure of is how to handle animations [..]

Then don't. Have each just write at most a single frame, just like it is now. The main functionality of not requiring a full allocation is still there even when one needs to call the Controller::next_image method multiple times.

Right now the main thing I'm not sure of is how to [..] decide when the parameters(blend op, frame delay, ...) can be changed.

It should be clear that changing a parameter has effect on its next usage, not immediately or retroactively. That is, there is little reason to stop it from being changed or to require it to be only changed once. So I would add setters of frame parameters to Controller and setters for parameters relating to image data—well, only FilterType—also to Writer/StreamWriter. Then the former are used for fcTL chunks (blend, dispose, etc.) and the latter can be changed when different scanlines should prefer different filters.

What is confusing me here is the line between the need of checking the correctness of what it's going to be written and blaming who is using the library for using it wrong.

An interesting little point here regarding Writer vs. StreamWriter: It's not necessary to have them as separate types. In fact, their methods may be merged. All that is necessary is to make the methods that write full frames return an error if any partial write through the stream methods has been performed. The only 'risk' involved for the caller with StreamWriter is that they might forget writing part of the frame, which corrupts the PNG. But neither the merge nor the split types addresses this risk differently with regards to their finish call nor their implicit Drop. In both cases a partial frame, i.e. having partial pixel data or having no pixel data, is incorrect according to the PNG specification but not fatally incorrect for our own program state. It's hard to say if we should even impose a hard requirement on image correctness here as long as the chunk framing is proper—which it is when IO itself succeeds. Then a lenient decoder might just skip or zero-fill bad frames, the rest of the image is still intact.
I briefly touched on this above. I don't think it is necessary to enforce total correctness of the PNG, except where we can make a specific guarantee and the caller explicitly requests us to do so. Accidentally making something impossibleis worse than incorrectness on wrong usage. That's also the reason why the write_chunk method is exposed in the first place. After all, it would allow arbitrary incorrect chunks to be inserted at arbitrary places. But that is clear to the caller.
The write_image_data is meant as a short cut, relative to StreamWriter. As for the check on indexed, there was an idea floating around of having the palette write being delayed—not automatically done in write_header—hence the duplicate check. That didn't materialize so it's probably in a weird spot. It wouldn't hurt to check it for the StreamWriter construction, too.

This is taking very long, these weeks have been quite busy for me but I've got some work done, this weekend I will finish it. If that's okay with you I'll open a new pull request where I start again from the current master (as you suggested) in order to clean up the messy commit history this one has.

After another week I did it even if there was no answer, I opened a new PR and I'm closing this one


Listing resources is taking too long

The call to fetch the dashboards in the UI takes >2s to respond in some cases, causing the UI to feel slow. We should figure out how to speed up this API.

Update:

The call to fetch dashboards doesn't return the content of the cells, just the position. it probably doesn't need that though IMO, although it won't do much to speed this up probably
Looks like most of the time is spent waiting for the backend. So we should investigate why the api takes so long to return.

@russorat can you post the size of the response you are referring too in your comment this could be a really large JSON response, which then takes time to process and render?
My best guess would say it is not the payload size that is the issue, but rather the access pattern in the API.

553kb
The browser timings seem to indicate the delay is in waiting for the api to respond, not it processing the jsonn.

Ahh that confirms it. The size of the response is misleading in the description of this issue. Its almost certainly the access patterns of the meta store.
Can you elaborate on what you mean by create specific list APIs. We have these already from what I understand. If the payload is 20kb vs 500kb the user wont' see a huge difference in most situations.
This issue likely a dead end here. It all resides on our meta store access patterns. Which is something that is part of a larger body of work in the platform.
Sadly, limiting the meta data returned won't do much of anything 😢.

I've updated the original issue to reflect the updated information.

to add a little more insight into this one. The current list page is returning just the high lvl dashboard data, no view data, using hte endpoint /api/v2/dashboards?orgID=:orgID. The show page, is adding the /api/v2/dashboards/:dashID?properties=true query param to include the views within the dashboard show call.
There is not optimization to be had here by not including the metadata unfortunately. It is already doing this.
One possible way to make this faster is to address the access pattern directly.  It would be to make an additional index on the dashboard orgID, and list over that specific org IDs keys. Or we put this data in a store that is more adept with our access patterns. That is a much larger body of work, consisting of migrations and other fun state management stuff 🙃

Filtering dashboards on id/dashboards-list page has some responsiveness issues too.


Docker does not work with certain property keys

Alluxio Version:
2.0.0-SNAPSHOT
Describe the bug
$ bin/alluxio runClass alluxio.cli.GetConfKey ALLUXIO_MASTER_MOUNT_TABLE_ROOT_OPTION_AWS_ACCESSKEYID
alluxio.master.mount.table.root.option.aws.accesskeyid

and alluxio.master.mount.table.root.option.aws.accesskeyid will not work but only alluxio.master.mount.table.root.option.aws.accessKeyId will work
To Reproduce
passing ALLUXIO_MASTER_MOUNT_TABLE_ROOT_OPTION_AWS_ACCESSKEYID as the property to docker
Expected behavior
We should have one way to pass the property
Urgency
High
Additional context
One solution might be making Alluxio properties case independent by fixing alluxio.conf.PropertyKey#isValid and alluxio.conf.PropertyKey#fromString

As an alternative solution, instead of relying on camel case I think we should always . between words as is convention for (probably) > 90% of the property keys. I think we should follow the same convention for all keys.
While not affected by the casing of the characters I'm specifically thinking about tiered storage keys that use level0, level1, etc.. instead of level.0, level.1...

@ZacBlanco , I discussed this issue with @aaudiber , another approach might be just pass all properties to docker run  through env variable like $ALLUXIO_JAVA_OPTS, $ALLUXIO_MASTER_JAVA_OPTS or $ALLUXIO_WORKER_JAVA_OPTS. Then we don't need to modify the code but only the recommended way to run docker

Looks like there are some keys that must be in mixed cases https://github.com/Alluxio/alluxio/blob/12d159a0d74369e256b3ded8cdf129d4b3e0a8c2/shell/src/main/java/alluxio/cli/GetConfKey.java#L31
I think we should just use JAVA_OPTS for everything.

Created https://github.com/Alluxio/alluxio/pull/8993 to support spaces in environment variables, and also recommend using -e ALLUXIO_JAVA_OPTS="-Dkey1=value1 -Dkey2=value2 ..." for everything

Both 1.8 branch and master branch have this fix. Close this ticket now


permission denied when accessing libvirt_pool location, files getting saved as root.

System Information
NAME="Ubuntu"
VERSION="20.04.2 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.2 LTS"
VERSION_ID="20.04"
Consuming v0.6.11 (latest)
Running as regular user "demo" (non sudo user)
Linux distribution
openSUSE 42.2/ Centos7/ Ubuntu..
Ubuntu
Terraform version
terraform -v

demo@dev:~$ terraform -v
Terraform v1.0.11
on linux_amd64

provider registry.terraform.io/dmacvicar/libvirt v0.6.11
provider registry.terraform.io/hashicorp/template v2.2.0

Provider and libvirt versions
terraform-provider-libvirt -version

v0.6.11
If that gives you "was not built correctly", get the Git commit hash from your local provider repository:
git describe --always --abbrev=40 --dirty

Checklist

[ ] Is your issue/contribution related with enabling some setting/option exposed by libvirt that the plugin does not yet support, or requires changing/extending the provider terraform schema?

[ ] Make sure you explain why this option is important to you, why it should be important to everyone. Describe your use-case with detail and provide examples where possible.
[ ] If it is a very special case, consider using the XSLT support in the provider to tweak the definition instead of opening an issue
[ ] Maintainers do not have expertise in every libvirt setting, so please, describe the feature and how it is used. Link to the appropriate documentation

[ ] Is it a bug or something that does not work as expected? Please make sure you fill the version information below:

Description of Issue/Question
libvirt_volume.ubuntu-qcow2: Creating...
libvirt_cloudinit_disk.commoninit: Creating...
libvirt_cloudinit_disk.commoninit: Still creating... [10s elapsed]
libvirt_volume.ubuntu-qcow2: Still creating... [10s elapsed]
libvirt_cloudinit_disk.commoninit: Still creating... [20s elapsed]
libvirt_volume.ubuntu-qcow2: Still creating... [20s elapsed]
libvirt_volume.ubuntu-qcow2: Creation complete after 22s [id=/var/lib/libvirt/images/ubuntu-qcow2]
libvirt_cloudinit_disk.commoninit: Creation complete after 22s [id=/var/lib/libvirt/images/commoninit.iso;bfdc388a-5da9-4a0f-873a-c14319a9468c]
libvirt_domain.domain-ubuntu: Creating...
╷
│ Error: Error creating libvirt domain: internal error: process exited while connecting to monitor: 2021-11-27T18:02:45.717932Z qemu-system-x86_64: -blockdev {"driver":"file","filename":"/var/lib/libvirt/images/ubuntu-qcow2","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}: Could not open '### /var/lib/libvirt/images/ubuntu-qcow2': Permission denied
│
│   with libvirt_domain.domain-ubuntu,
│   on main.tf line 86, in resource "libvirt_domain" "domain-ubuntu":
│   86: resource "libvirt_domain" "domain-ubuntu" {
Running as 'demo' user.
demo@dev:~$ id
uid=1001(demo) gid=1001(demo) groups=1001(demo),108(kvm),138(libvirt)
Setup
(Please provide the full main.tf file for reproducing the issue (Be sure to remove sensitive information)
demo@dev:~$ cat main.tf
terraform {
required_version = ">= 0.13"
required_providers {
libvirt = {

  source  = "dmacvicar/libvirt"

  version = "0.6.11"

}

}
}
instance the provider
provider "libvirt" {
uri = "qemu:///session"
}
We fetch the latest ubuntu release image from their mirrors
resource "libvirt_volume" "ubuntu-qcow2" {
name   = "ubuntu-qcow2"
pool  = "default"
source = "https://cloud-images.ubuntu.com/releases/xenial/release/ubuntu-16.04-server-cloudimg-amd64-disk1.img"
format = "qcow2"
}
data "template_file" "user_data" {
template = file("${path.module}/cloud_init.cfg")
}
data "template_file" "network_config" {
template = file("${path.module}/network_config.cfg")
}
for more info about paramater check this out
https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/website/docs/r/cloudinit.html.markdown
Use CloudInit to add our ssh-key to the instance
you can add also meta_data field
resource "libvirt_cloudinit_disk" "commoninit" {
name           = "commoninit.iso"
user_data      = data.template_file.user_data.rendered
network_config = data.template_file.network_config.rendered
pool           = "default"
}
Create the machine
resource "libvirt_domain" "domain-ubuntu" {
name   = "${terraform.workspace}-test-10"
memory = "512"
vcpu   = 1
cloudinit = libvirt_cloudinit_disk.commoninit.id
network_interface {
network_name = "default"

}
IMPORTANT: this is a known bug on cloud images, since they expect a console
we need to pass it
https://bugs.launchpad.net/cloud-images/+bug/1573095
console {
type        = "pty"

target_port = "0"

target_type = "serial"

}
console {
type        = "pty"

target_type = "virtio"

target_port = "1"

}
disk {
volume_id = libvirt_volume.ubuntu-qcow2.id

}
graphics {
type        = "spice"

listen_type = "address"

autoport    = true

}
}
IPs: use wait_for_lease true or after creation use terraform refresh and terraform show for the ips of domain
Steps to Reproduce Issue
(Include debug logs if possible and relevant).
terraform apply with main.tf

Additional information:
Do you have SELinux or Apparmor/Firewall enabled? Some special configuration?
Have you tried to reproduce the issue without them enabled?
No they are not enabled.

This issue was resolved by means of documentation (22f096d9).

The steps documented here https://github.com/dmacvicar/terraform-provider-libvirt/commit/22f096d9 addresses the issue.


"realistic" military organization

I would like to see a realistic military organization
Like being able to move brigades separately away from thier division and being able to change the division that a specific brigade is attached to
Ps: I can provide more info if this interesting for you.
Btw, great job on the mod I really love it 😍

Tnx.
In Vic2 the basic unit is "regiment"(brigades). Vic2 Regiments(brigades) are organized in armies. Divisions are virtual subdivisions within a Vic2 army.  The limitations of Vic2 applies to NWO.
So I am sure I understand, you are able to change as it is today..


package.json dependencies on the provisioning plugin on the branch 4.15 looks to be outdated

When I'm checking out the branch 4.15, I'm expecting to find the version 1.0.0 for the dynamic plugin sdk, but I'm seeing  0.0.12    https://github.com/openshift/console-plugin-template/blob/8376b271fe422530fb7b96e71354d49fca9a8d1e/package.json#L24
What about the other dependencies in the file, are they up-to-date too?

We should verify that the README is correct and, if so, update the dependency in the 4.15 branch. It would be good to verify the older branches, too.
cc @jhadvig @vojtechszocs

It seems to be inconsistencies in the other branches as well, e.g release-4.16. The release notes for 4.16 says:

Source: https://docs.openshift.com/container-platform/4.16/release_notes/ocp-4-16-release-notes.html#ocp-4-16-web-console_release-notes
but in package.json react-router 5 is used:
https://github.com/openshift/console-plugin-template/blob/d5a16f6e9d843bb06cc6dd6336fc9819c56521d0/package.json#L59-L60

@mikaello Thanks for raising this. I believe we still expect to have the older package in the dependencies to give developers time to migrate to v6. It's deprecated, but still there for compatibility. You can use the react-router-dom-v5-compat dependency to begin migrating to v6 if your plugin targets only OpenShift 4.15+.
See https://github.com/openshift/console/blob/master/frontend/packages/console-dynamic-plugin-sdk/README.md#changes-in-shared-modules
@vojtechszocs @jhadvig Can you confirm? I think we should improve the docs around the react-router migration.


[cmd][diff] Ignore detection status for tags

We already do this on the GUI, but on the cmdline, we default the
detection status to NEW, REOPENED and UNRESOLVED. This makes sense for
runs (where we check whether a report is outstanding at the time of the
query), but not for tags (as a report can have a RESOLVED detection
status but still be outstanding at the time of the tag).
Depends on #3996 and #4006.

I cherry-picked the docs changes into #4006, so it no longer has any dependencies.


Support for UIOCO

IOCO has some issues with simulator and tester
See e.g. attached file
UIOCO is theoretically better.
Support UIOCO (as well or instead of IOCO)
ioco.zip

for more info on uioco: http://www.ub.utwente.nl/webdocs/ctit/1/00000123.pdf

I'm not familiar with the strengths and weaknesses of both implementation relations, but as an end user I would be in favor of supporting only one of these to avoid adding any more cognitive load.

I prefer UIOCO over IOCO
but I don't know what Jan's preference is?


In programs with a main() function, do not support non-declaration top-level statements.

go+ allows instructions to be at top level, so a program like this is valid:
println("hello")

this is reasonable, as it allows small projects to write less code to get started.
currently, a program like this will not compile:
println("hello")
func main() {
    println("goodbye")
}

This is good. For one, I am often alerted to a mistake by syntax error: non-declaration statement outside function body. Far more important though, is readability. larger projects will likely have func main() to group all of their code that runs at the start together. You should be able to look in main() and see all of the code that will run. Otherwise ypu would have to check every file.

In the Go+ language, we allow that only one .gop file exists global statements.


Create dedicated unstable flag for asymmetric-token

Asymmetric tokens are gated by -Zcredential-process. Since we're considering stabilizing that soon, this moves asymmetric token support to have its own unstable flag.
It was previously gated by -Zregistry-auth, and some of the docs were not updated when it moved.
r? @Eh2406

@bors r+

:pushpin: Commit 8c13e9ae3fc758f5298d9e150ed404d7808a7f6e has been approved by Eh2406
It is now in the queue for this repository.

:hourglass: Testing commit 8c13e9ae3fc758f5298d9e150ed404d7808a7f6e with merge 3b209079261da420576a50b238fb44fa76bc3030...

:sunny: Test successful - checks-actions
Approved by: Eh2406
Pushing 3b209079261da420576a50b238fb44fa76bc3030 to master...


Gzip files after generating them

For https://github.com/acteng/atip/issues/57. My plan is to upload new bin.gz files and leave the existing .bin alone, then cutover ATIP to use bin.gz, and only delete the old .bin files after we update atip.uk files.
For context for Pete, this repo is for generating the route snapper and info files. https://github.com/acteng/abstreet-to-atip/ README should hopefully be good docs; if not, it's a good time to add to them!

Thanks for the review! I found an s3 command to set the content-encoding header, and it's working e2e, so merging! All files now exist as .bin.gz alongside the old .bin


Fixing package order

I pasted below the CMake error log that Irina was getting.
CMake Error at
cmake/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake:141 (MESSAGE):
Error, the package 'DataTransferKitC_API' is listed as a dependency of the
package 'DataTransferKitSTKMeshAdapters' is in the list
'DataTransferKitSTKMeshAdapters_TEST_REQUIRED_DEP_PACKAGES' but the package
'DataTransferKitC_API' is either not defined or is listed later in the
package order.  Check the spelling of 'DataTransferKitC_API' or see how it
is listed in Trilinos_PACKAGES_AND_DIRS_AND_CLASSIFICATIONS in relationship
to 'DataTransferKitSTKMeshAdapters'.

I was not able to reproduce the error, with the same CMake version and Trilinos Git revision...
root@f66abd5b442e:/scratch/source/trilinos/12.8.1/DataTransferKit/build# ../irina_cmake

Configuring Trilinos build directory

-- PROJECT_SOURCE_DIR='/scratch/source/trilinos/12.8.1'
-- PROJECT_BINARY_DIR='/scratch/source/trilinos/12.8.1/DataTransferKit/build'
-- Trilinos_TRIBITS_DIR='/scratch/source/trilinos/12.8.1/cmake/tribits'
-- TriBITS_VERSION_STRING='0.9 (Dev)'
-- CMAKE_VERSION='3.0.2'
-- Reading in configuration options from /scratch/source/trilinos/12.8.1/cmake/StdDevEnvs.cmake ...
-- Trilinos_USE_BUILD_ENV=''
-- CMAKE_HOST_SYSTEM_NAME='Linux'
-- Trilinos_HOSTNAME='f66abd5b442e'
-- PYTHON_EXECUTABLE='/usr/bin/python'

Setting up major user options ...

Processing list of POST extra repos from Trilinos_EXTRA_REPOSITORIES='DataTransferKit' ...

-- Setting Trilinos_ENABLE_TriKota=OFF because '/scratch/source/trilinos/12.8.1/packages/TriKota/Dakota' does not exist!

Trilinos repos versions:
--------------------------------------------------------------------------------
*** Base Git Repo: Trilinos
938691e [Fri Oct 21 20:41:38 2016 -0700]<EMAIL_ADDRESS>Piro: finished hooking up Thyra Piro interfaces to MatrixFree Decorator. This la
 --------------------------------------------------------------------------------

Reading list of native packages from /scratch/source/trilinos/12.8.1/PackagesList.cmake

-- Trilinos_NUM_PACKAGES='56'

Reading list of native TPLs from /scratch/source/trilinos/12.8.1/TPLsList.cmake

-- Trilinos_NUM_TPLS='102'

Reading list of POST extra packages from /scratch/source/trilinos/12.8.1/DataTransferKit/PackagesList.cmake ...

-- Trilinos_NUM_PACKAGES='57'

Reading list of POST extra TPLs from /scratch/source/trilinos/12.8.1/DataTransferKit/TPLsList.cmake ...

-- Trilinos_NUM_TPLS='104'

Processing Project, Repository, and Package dependency files and building internal dependencies graph ...

-- Trilinos_NUM_SE_PACKAGES='159'
-- Tentatively enabling TPL 'Pthread'
-- Tentatively enabling TPL 'DLlib'

Explicitly enabled packages on input (by user):  Tpetra DataTransferKit 2

Explicitly enabled SE packages on input (by user):  Tpetra DataTransferKit 2

Explicitly disabled packages on input (by user or by default):  Domi Claps Trios TriKota NewPackage MeshingGenie 6

Explicitly disabled SE packages on input (by user or by default):  KokkosExample Domi Claps ShyLUHTS ShyLUTacho ShyLUBDDC ShyLUBasker Trioscommsplitter Triossupport Triosnnti Triosnssi Triosprograms Triosexamples Triostests Triosnetcdf-service Trios TriKota STKExp NewPackage MeshingGenie 20

Explicitly enabled TPLs on input (by user):  Pthread MPI BLAS LAPACK Boost BoostLib DLlib 7

Explicitly disabled TPLs on input (by user or by default):  Netcdf MOAB Libmesh 3

Disabling all packages that have a required dependency on disabled TPLs and optional package TPL support based on TPL_ENABLE_<TPL>=OFF ...

-- Setting Trilinos_ENABLE_SEACASExodus=OFF because SEACASExodus has a required library dependence on disabled TPL Netcdf
-- Setting Trilinos_ENABLE_SEACASExodus_for=OFF because SEACASExodus_for has a required library dependence on disabled TPL Netcdf
-- Setting Trilinos_ENABLE_SEACASExoIIv2for32=OFF because SEACASExoIIv2for32 has a required library dependence on disabled TPL Netcdf
-- Setting Trilinos_ENABLE_DataTransferKitMoabAdapters=OFF because DataTransferKitMoabAdapters has a required library dependence on disabled TPL MOAB

Fixes #108 (hopefully...)

@bartlettroscoe I even tried using the latest TriBITS instead of the one snapshotted in Trilinos but I still did not get the configure error that Irina reported in #108.  Am I missing anything?

Damien,
Can you please provide your full configure script?
-Ross

Stuart posted a configuration script in #108 but I used this (pasted below) with Trilinos trilinos/Trilinos@938691e and DTK 883c622.
#!/bin/bash
EXTRA_ARGS=$@
ARGS=(
    -D CMAKE_BUILD_TYPE=Debug
    -D BUILD_SHARED_LIBS=ON
    ### COMPILERS AND FLAGS ###
    -D CMAKE_CXX_FLAGS="-Wall -Wextra -Wpedantic -Weffc++"
    ### TPLs ###
    -D TPL_ENABLE_MPI=ON
    -D TPL_ENABLE_BLAS=ON
    -D TPL_ENABLE_LAPACK=ON
    -D TPL_ENABLE_Boost=ON
        -D Boost_INCLUDE_DIRS=$BOOST_DIR/include
        -D Boost_LIBRARY_DIRS=$BOOST_DIR/lib
    -D TPL_ENABLE_BoostLib=ON
        -D BoostLib_INCLUDE_DIRS=$BOOST_DIR/include
        -D BoostLib_LIBRARY_DIRS=$BOOST_DIR/lib
    -D TPL_ENABLE_Netcdf=OFF
        -D Netcdf_INCLUDE_DIRS=$NETCDF_DIR/include
        -D Netcdf_LIBRARY_DIRS=$NETCDF_DIR/lib
    ### ETI ###
    -D Trilinos_ENABLE_EXPLICIT_INSTANTIATION=ON
    ### PACKAGES CONFIGURATION ###
    -D Trilinos_ENABLE_ALL_PACKAGES=OFF
    -D Trilinos_ENABLE_ALL_OPTIONAL_PACKAGES=OFF
    -D Trilinos_ASSERT_MISSING_PACKAGES=OFF
    -D Trilinos_ENABLE_TESTS=OFF
    -D Trilinos_ENABLE_EXAMPLES=OFF
    -D Trilinos_ENABLE_Tpetra=ON
        -D Tpetra_INST_INT_UNSIGNED_LONG=ON
    ### DTK ###
    -D Trilinos_EXTRA_REPOSITORIES="DataTransferKit"
    -D Trilinos_ENABLE_DataTransferKit=ON
        -D DataTransferKit_ENABLE_DBC=ON
        -D DataTransferKit_ENABLE_TESTS=ON
        -D DataTransferKit_ENABLE_EXAMPLES=ON
        -D TPL_ENABLE_MOAB=OFF
        -D TPL_ENABLE_Libmesh=OFF
    )
cmake "${ARGS[@]}" $EXTRA_ARGS $TRILINOS_DIR

 -D Trilinos_ASSERT_MISSING_PACKAGES=OFF

That is the problem.  That disables all strong checking.  If you look at the detailed cmake STDOUT, you will see that the package DataTransferKitSTKMeshAdapters is never even listed in the final sets of enabled or disabled or SE packages.
That option should never be turned on when doing development against the repos.  See:

https://tribits.org/doc/TribitsBuildReference.html#cmake-configure-time-development-mode-and-debug-checking

How did that option get added?  Was this added from back in the days of the old filtered Trilinos public clone?
This is the second example that I have seen of people setting this option when doing development.  I need to revise that section to be more accurate about that this actually does and I need to add more verbose warnings about the danger of setting this option and then trying to do develoment.  This option should only be set when using a reduced tarball (and it is enabled by default in tarball releases of Trilinos, for example).

THANK YOU!

How did that option get added? Was this added from back in the days of the old filtered Trilinos public clone?

This is from DTK build guidelines (which comes from the old README that was there before I started working on DTK)

I updated the guidelines and removed the flag from the configuration script on Docker.

@bartlettroscoe

How did that option get added? Was this added from back in the days of the old filtered Trilinos public clone?

This was a hold over from when you would clone the Trilinos repo and some packages wouldn't be cloned - this was from some internal packages or something. This flag was added to allow the library to configure.


How to register model in admin panel

I have created one extra app and inside of which I have created several models, how can I register them into the admin site so that they become visible on the frontend.

@pandeyroshan Hey! Are you talking about Dashboard 2.0? If yes, then please keep in mind that all modules use graphql API to communicate with the dashboard. If you would like to extend it, you need to become more familiar with saleor.graphql module.

Ok, now I'm a lot more clear than previously.


Handle error in prediction of non-cached models

When we try to perform prediction in a non-cached models, we get the following error.
We should handle this error, and try not to download model in prediction UDFs.

SQL Error [22002]: VM error: F-UDF-CL-LIB-1127: F-UDF-CL-SL-PYTHON-1002: F-UDF-CL-SL-PYTHON-1026: ExaUDFError: F-UDF-CL-SL-PYTHON-1114: Exception during run
File "/usr/local/lib/python3.8/dist-packages/transformers/configuration_utils.py", line 601, in _get_config_dict
resolved_config_file = cached_path(
File "/usr/local/lib/python3.8/dist-packages/transformers/utils/hub.py", line 282, in cached_path
output_path = get_from_cache(
File "/usr/local/lib/python3.8/dist-packages/transformers/utils/hub.py", line 470, in get_from_cache
os.makedirs(cache_dir, exist_ok=True)
File "/usr/lib/python3.8/os.py", line 213, in makedirs
makedirs(head, exist_ok=exist_ok)
File "/usr/lib/python3.8/os.py", line 223, in makedirs
mkdir(name, mode)
OSError: [Errno 30] Read-only file system: '/buckets/bfsdefault/default/container/SUB_DIR'

During handling of the above exception, another exception occurred:

TE_QUESTION_ANSWERING_UDF:8 run
File "/usr/local/lib/python3.8/dist-packages/exasol_transformers_extension/udfs/models/base_model_udf.py", line 41, in run
predictions_df = self.get_predictions_from_batch(batch_df)
File "/usr/local/lib/python3.8/dist-packages/exasol_transformers_extension/udfs/models/base_model_udf.py", line 55, in get_predictions_from_batch
for model_df in
File "/usr/local/lib/python3.8/dist-packages/exasol_transformers_extension/udfs/models/base_model_udf.py", line 90, in extract_unique_model_dataframes_from_batch
self.load_models(model_name)
File "/usr/local/lib/python3.8/dist-packages/exasol_transformers_extension/udfs/models/base_model_udf.py", line 127, in load_models
self.last_loaded_model = self.base_model.from_pretrained(
File "/usr/local/lib/python3.8/dist-packages/transformers/models/auto/auto_factory.py", line 423, in from_pretrained
config, kwargs = AutoConfig.from_pretrained(
File "/usr/local/lib/python3.8/dist-packages/transformers/models/auto/configuration_auto.py", line 680, in from_pretrained
config_dict, _ = PretrainedConfig.get_config_dict(pretrained_model_name_or_path, **kwargs)
File "/usr/local/lib/python3.8/dist-packages/transformers/configuration_utils.py", line 553, in get_config_dict
config_dict, kwargs = cls._get_config_dict(pretrained_model_name_or_path, **kwargs)
File "/usr/local/lib/python3.8/dist-packages/transformers/configuration_utils.py", line 641, in _get_config_dict
raise EnvironmentError(
OSError: Can't load config for 'WRONG_MODEL_NAME'. If you were trying to load it from 'https://huggingface.co/models', make sure you don't have a local directory with the same name. Otherwise, make sure 'WRONG_MODEL_NAME' is the correct path to a directory containing a config.json file
(Session:<PHONE_NUMBER>052043776)

This ticket is handled in #25 and the problem here has been addressed. Therefore, this ticket is closed.


macOS doesn't have wget

Make building easier on macOS by using curl instead of wget
because wget isn't part of macOS, and wget installed from
brew has trouble resolving certificates appropriately.

Can one of the admins verify this patch?

Thanks @dmaclach! Do you mind also resolving the conflicts?

@dmaclach any luck with pull rebase and push?


Allow configuration of generation return time

ValueTask should only be used when the duration of a task in the majority of cases will be below 20ms. Not sure in which article i read this exactly (Stephen Cleary, i think), but in the majority of cases where networks are involved this is probably not the case. ValueTask returns force me to do async+await if i am interacting with a task, to get around ValueTask. But when you use async+await a state machine is returned which also isn't really ideal. Having control over ValueTask vs Task would be good.

I think that would be difficult given the interfaces aren't generated (the ones contained in Mediator.Abstractions)

When talking about abstractions you are talking about for example the return type of the send method for example, yes?

Hm, yeah, but in places where you interact with Send or similar methods it being a ValueTask would not be an issue. If it just changed for the RequestHandler interface it would be good enough. Because then you can just do
return HttpClient.GetStringAsync("somerequest"); if the returntype is string, instead of having to do async+await everywhere, just so it compiles (when you don't want to generate the statemachine but just directly return the task instead).


Execution failed for task ':clean' after gradle upgraded from 3.5.1 to 4.3.1

Expected Behavior

Execute task ':clean' successfully.
Current Behavior

Execution failed for task ':clean'  because of the error Caused by: org.gradle.api.file.UnableToDeleteFileException: Unable to delete file: D:\_APPS\git_apps\groovy\target\libs\groovy-3.0.0-SNAPSHOT.jar  ( BTW, sometimes the file failed to delete is D:\_APPS\git_apps\groovy\target\bootstrap\groovy-3.0.0-SNAPSHOT-bootstrap.jar )
Context

We fail to continue run gradlew clean dist, no workaround found yet...
Steps to Reproduce (for bugs)

clone https://github.com/apache/groovy.git
run gradlew clean dist  (BTW, gradlew dist fails as well)
get the error FAILURE: Build failed with an exception., i.e. org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':clean'

Your Environment

Win10, Java 8, Gradle 4.3.1

Build scan URL:  https://gradle.com/s/dx7bxzoe5tpmg

Is there a reason why you even need to run the clean task? Usually you don't need to do so. Gradle will figure out necessary changes for you.

Even if we do not run clean task,  dist task fails with same reason because clean task will run automatically.

@danielsun1106 I am bit confused. What exactly makes the clean task run? Does one of your tasks depend on the clean task?

@bmuschko As you see, dist task(https://github.com/apache/groovy/blob/master/gradle/assemble.gradle#L644) does not depend on the clean task.
P.S. even if I search all gradle script files, only one clean task found: https://github.com/apache/groovy/blob/master/subprojects/performance/build.gradle#L45

I'd try to remove the line https://github.com/apache/groovy/blob/master/subprojects/performance/build.gradle#L45. I see no reason why you should run clean here.
@melix Do you have any background information on this?

@bmuschko @melix  gradlew clean dist runs well on CentOS 7, but fails on Win10

4.4-rc-2 has the same issue:
java.io.IOException: Unable to delete file: D:\_APPS\git_apps\groovy\target\bootstrap\groovy-3.0.0-SNAPSHOT-bootstrap.jar
> Unable to delete file: D:\_APPS\git_apps\groovy\target\bootstrap\groovy-3.0.0-SNAPSHOT-bootstrap.jar

After @melix improved the build of Groovy, the issue is gone :-)


[BUG] gradle precommit is broken due to transportClientRatio

Describe the bug
Gradle precommit on local desktop is broken
/home/ubuntu/sarat-opensearch/server/src/test/java/org/opensearch/cluster/routing/MovePrimaryFirstTests.java:23: error: cannot find symbol
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0, transportClientRatio = 0)
                                                                                ^
  symbol:   method transportClientRatio()
  location: @interface ClusterScope

Not sure why GHA action for precommit is going through. But its quite reproducible on local machine.
To Reproduce
Steps to reproduce the behavior:

Checkout OpenSearch
Run ./gradlew precommit

Host/Environment (please complete the following information):

OS: Mac OS BigSur, Ubuntu (Linux)

This is now fixed in main.
The changes are not backported yet to 1.x: https://github.com/opensearch-project/OpenSearch/pull/1445
@jainankitk can you take care of this when you back port it to 1.x.
Another follow up question, I do not know why GHA for Precommit passes consistently while on local it consistently fails.

@saratvemulapalli - Sure, thanks!

@saratvemulapalli - My bad, I missed it. Thanks for taking care of this!


Add CompletionTriggerKind.QuickSuggestions or CompletionTriggerKind.Manual

Related: #10098
For whatever reason, it appears that both editor.quickSuggestions's suggestions and the user manually invoking completions via CTRL + Space have the same CompletionTriggerKind, which is... nonsense.
It would be great if a new enumeration was added to differentiate.
Use case
If my trigger characters are [")", ".", ":"] I simply do not want to return any completions for quick suggestions (which appears to eagerly ignore any trigger character rules for completion item provider subscriptions.)
However, I DO want to provide completions if the user manually presses CTRL + Space to invoke completions manually.
Both quick suggestions and manual invokation have the same CompletionTriggerKind and CompletionContext.triggerCharacter

If my trigger characters are [")", ".", ":"] I simply do not want to return any completions for quick suggestions (which appears to eagerly ignore any trigger character rules for completion item provider subscriptions.)

Quick suggestions and trigger characters are not related, quick suggestion triggers suggestion when type at a word end, trigger characters happen when you type one of those characters.

If my trigger characters are [")", ".", ":"] I simply do not want to return any completions for quick suggestions (which appears to eagerly ignore any trigger character rules for completion item provider subscriptions.)

Quick suggestions and trigger characters are not related, quick suggestion triggers suggestion when type at a word end, trigger characters happen when you type one of those characters.


nixos/sshd: fix socket activated ports when using ListenAddress

Description of changes
Noticed that issue while reviewing #275633: when declaring ListenAddress host without a port, all ports declared by Port/cfg.ports will be used with host according to sshd_config(5).
However, if this is done and socket activation is used, only a socket for port 22 is created instead of a sockets for each port from Port/cfg.ports. This patch corrects that behavior.
Also added a regression test for this case.

Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[ ] sandbox = true

[x] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[x] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

@ofborg test openssh


Defer script insertion until referenced elements are present in DOM

Fixes #692

Thanks a lot for reporting!
I though the DOM was guarenteed to be populated when the use effect callback is populated.
It doesn't seem to be the case.
I'll fix this in the useScripts hook by observing the DOM so we do not introduce breaking changes for pepole.

Thanks you very much for diagnosing this. I've fixed it on every pages that requires scripts.
@all-contributors please add @marvinruder for bug


Change in README.md command [issue 10]

Hi @xdebbie! I have changed the readme installation command. I hope that now works well!

This pull request is referenced to issue #10.


Documentation bug: break-match-filter or break-match-filters

DO NOT REMOVE OR SKIP THE ISSUE TEMPLATE

[X] I understand that I will be blocked if I intentionally remove or skip any mandatory* field

Checklist

[X] I'm reporting a bug unrelated to a specific site
[X] I've verified that I'm running yt-dlp version 2023.03.04 (update instructions) or later (specify commit)
[X] I've checked that all provided URLs are playable in a browser with the same IP and same login details
[X] I've checked that all URLs and arguments with special characters are properly quoted or escaped
[X] I've searched known issues and the bugtracker for similar issues including closed ones. DO NOT post duplicates
[X] I've read the guidelines for opening an issue

Provide a description that is worded well enough to be understood
Documentation is inconsistent about --break-match-filter (singular) or --break-match-filters (plural).
Command: yt-dlp -h | egrep -C1 break-match
Output:
    --no-match-filter               Do not use any --match-filter (default)
    --break-match-filters FILTER    Same as "--match-filters" but stops the
                                    download process when a video is rejected
    --no-break-match-filters        Do not use any --break-match-filters
                                    (default)
--
    --break-per-input               Alters --max-downloads, --break-on-existing,
                                    --break-match-filter, and autonumber to
                                    reset per input URL

There's a couple of other inconsistencies about this in the README.
Provide verbose output that clearly demonstrates the problem

[X] Run your yt-dlp command with -vU flag added (yt-dlp -vU <your command line>)
[ ] If using API, add 'verbose': True to YoutubeDL params instead
[X] Copy the WHOLE output (starting with [debug] Command-line config) and insert it below

Complete Verbose Output
[debug] Command-line config: ['--ignore-config', '-vU']
[debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8
[debug] yt-dlp version<EMAIL_ADDRESS>[392389b7d] (zip)
[debug] Python 3.8.10 (CPython x86_64 64bit) - Linux-5.4.0-148-generic-x86_64-with-glibc2.29 (OpenSSL 1.1.1f  31 Mar 2020, glibc 2.31)
[debug] exe versions: ffmpeg 4.2.7, ffprobe 4.2.7, phantomjs 2.1.1, rtmpdump 2.4
[debug] Optional libraries: Crypto-broken 2.6.1, certifi-2022.12.07, mutagen-1.45.1, pyxattr-0.6.1, secretstorage-3.3.3, sqlite3-2.6.0
[debug] Proxy map: {}
[debug] Extractor Plugins: Youtube_AgeGateBypassIE
[debug] Loaded 1787 extractors
[debug] Fetching release info: https://api.github.com/repos/yt-dlp/yt-dlp/releases/latest
Available version<EMAIL_ADDRESS>Current version<EMAIL_ADDRESS>Current Build Hash: 91cad9f121c1f6f0a81b747415c46ecba0ff331ed38cc6433040b4ac7b6e15ca
yt-dlp is up to date<EMAIL_ADDRESS>

ERROR: [twitter]<PHONE_NUMBER>713894402: Unable to download JSON metadata: HTTP Error 404: Not Found (caused by <HTTPError 404: 'Not Found'>); please report this issue on  https://github.com/yt-dlp/yt-dlp/issues?q= , filling out the appropriate issue template. Confirm you are on the latest version using  yt-dlp -U


Support @CheckForNull annotation

error: [NullAway] method returns @Nullable, but superclass method com.github.benmanes.caffeine.cache.AsyncLoadingCache.getIfPresent(java.lang.Object) returns @NonNull
  public @Nullable CompletableFuture<V> getIfPresent(@Nonnull Object key) {

However, this method is annotated with @CheckForNull which should implicitly mean @Nullable.

Which library provides the @CheckForNull annotation?

javax.annotation (jsr305 / findbugs)

Cool, this should be easy to support

Are you ok with a pull request that adds support for a @ChecksForNull annotation? From the code it looks like that I just need to update https://github.com/uber/NullAway/blob/cc7869b6c89e388a6e0f8d0a6faba342f7827be4/nullaway/src/main/java/com/uber/nullaway/Nullness.java#L149 and provide a test.
Or is there something conceptional wrong with this issue? Supporting ChecksForNull would help projects a lot that use NullAway and SpotBugs

I would be open to a PR that just adds @CheckForNull support.  @lazaroclapp please comment if you see an issue with that.

I am fine with it. The idea is just to make CheckForNull another supported nullable annotation, right? As an exact alias for @Nullable, correct? Reading the previous discussion, we were a bit concerned about overhead of equality checking so many annotations if we just supported a custom list. But given that there seems to be interest for this particular annotation, I'd say go ahead.
We can open a separate task to optimize isNullableAnnotation and profile later.

I am fine with it. The idea is just to make @CheckForNull another supported nullable annotation, right? As an exact alias for @Nullable, correct?

Yes, just a synonym for @Nullable. (The reason is that SpotBugs treats @Nullable differently: such references might be null but should not be flagged by a tool. In Spotbugs the semantics of @CheckForNull  is the same as your @Nullable annotation). Then teams can use SpotBugs and NullAway to check the code...


Feature Request - Unable to access DataResponse in should function during retry

What did you do?
Tried retrying a failed request and it works.
What did you expect to happen?
I expected to have access to the failed request DataResponse object so that I can make decisions to update certain data before retrying a new request. I would like to get data in the DataResponse
    /// The data returned by the server.
    public let data: Data?

What happened instead?
Cannot find a way to access the DataResponse object in the
 func should(_ manager: SessionManager, retry request: Request, with error: Error, completion: @escaping RequestRetryCompletion) { }

or the

func adapt(_ urlRequest: URLRequest) throws -> URLRequest { }

Alamofire Environment
**Alamofire version:4.8.1
**Xcode version:11.1
**Swift version:4
**Platform(s) running Alamofire:iOS
**macOS version running Xcode:Catalina

In Alamofire 4, retry is triggered before any response parsing has occurred, so you're request isn't possible. In Alamofire 5, retry is now triggered twice: once in the place as Alamofire 4 (after response validation but before serialization) and once after each failing serializer. In both of these cases (and in Alamofire 4), it's intended that your Error includes all of the information you need to reason about retry.


OSOE-107: Updating Meziantou.Analyzer, SonarAnalyzer.CSharp, StyleCop.Analyzers to latest

OSOE-107

Next time please also add the issue key to the PR description like I've done so it links back to the issue, and update the PR's title if it becomes invalid or incomplete.


Publish v1.1.0 to NPM

NPM is currently blocking this module from being published because of the following:
npm ERR! code E403
npm ERR! 403 403 Forbidden - PUT https://registry.npmjs.org/tusd-static-win32 - Package name triggered spam detection; if you believe this is in error, please contact<EMAIL_ADDRESS>npm ERR! 403 In most cases, you or one of your dependencies are requesting
npm ERR! 403 a package version that is forbidden by your security policy.

I've reached out to NPM Support for help on this

wtf


Add a dialog notifying that the app is prototype

Add popup screen: "This mobile app is still in its prototype stage with many features being experimental. We currently recommend using this app only for testing."
The popup should only display once

@iamareebjamal  working on this issue

Hi @iamareebjamal should i work on this issue?

Hey Sir, I would like to work on this issue

I would like to work on this issue

Please don't just comment I want to work on this issue. Send a PR after commenting. 4 people have commented in a week and not a single PR

@iamareebjamal The Project is not able to build .I have raised the concern multiple types on Gitter Chanel.

https://github.com/fossasia/open-event-organizer-android/runs/1323277639
https://circleci.com/gh/fossasia/open-event-organizer-android/4071?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
It is able to build in both these places. You have to either update the project to latest gradle or downgrade your tooling to older version which is used in the project. First is easier to do


Force docker repo names to lowercase

Think this should fix this example failure:
https://github.com/WardCunningham/seran-wiki/runs/526417213?check_suite_focus=true#step:4:11

Nice - Shame GitHub doesn't just have a function for this.

There may be better ways to solve this with these official actions.
https://github.com/sdras/awesome-actions#official-actions
I also found interesting things in the toolkit, but don't much feel like learning how to use those tools just now: https://github.com/actions/toolkit


GH-40366: [C++] Remove const qualifier from Buffer::mutable_span_as

Rationale for this change

The const qualifier on Buffer:mutable_span_as prevents it from calling the non-const Buffer::mutable_data_as
What changes are included in this PR?

See issue title
Are these changes tested?

No, I believe this is a simple oversight
Are there any user-facing changes?

Yes, Buffer::mutable_span_as loses it's const qualifier

GitHub Issue: #40366

@github-actions crossbow submit -g cpp


Add subproject "examples"

In this subproject there would be simple projects with examples of usage of qtcsv.

Add section Examples in Readme.md file


Project 1. Issue 11: Method Flow::Send().

Closes issue #11

Sender: получает пакеты от Flow. При генерации пакета Flow вызывает Sender::Enqueue(packet). Sender после обработки (по истечении очереди и задержки) отправляет пакет к следующему устройству (например, Switch).

I can't understand why flow directly calls Sender::Enqueue(packet) instead of creating an event with Sender::ProcessPacket(packet) as handler.

Flow creates event with Sender::ProcessPacket(packet)
Scheduler executes event's handler -- sender enqueues packet to the queue and then creates a new event (sending packet to next device) with delay.

Is it a good solution for Flow::Send?

I can't understand why flow directly calls Sender::Enqueue(packet) instead of creating an event with Sender::ProcessPacket(packet) as handler.

I think you have misunderstood the role of scheduler. The purpose of the scheduler is to order the events (arrival of packets coming from different sources). In your example, when everything happens on one server there is no need of using scheduler because we can order the issue of packets directly one by one.