Miscellaneous bugs in Tableview

I did some tests (Linux Mint x64 Cinnamon X11), some suggestions for change:
Sorting image was cut off when doing autofit_columns. I changed
pad = utility.scale_size(self, 10)
for this and fixed the problem:
pad = utility.scale_size(self, 20)

Changed menu post method to tk_popup, several lines. Thus, the dropdown menu is displayed when hovering over the parent.
And mainly the menu is closed when clicking anywhere on the screen outside of it
example:
def post(self, event):
to: (in every 'post' to tk_popup)
def tk_popup(self, event):

I also changed the way to write csv.
writer = csv.writer(f)
for
CSV_DELIMITER = ';'
CSV_QUOTECHAR = '"'
writer = csv.writer(f, delimiter=CSV_DELIMITER, quotechar=CSV_QUOTECHAR, quoting=csv.QUOTE_NONNUMERIC)

Because depending on the country's local currency, using ',' as a delimiter can cause csv conflicts
ex: In Brazil the float separator is ',' and not '.'
bugs:
https://www.mediafire.com/file/0oil09c4i9vcif6/bug_tableview.mp4/file
modified:
https://www.mediafire.com/file/50e4b5qog148l9u/modified_tableview.mp4/file

There is also a bug with 'hide select rows'.
After applying this filter and trying to clean. It does not return the hidden line.
https://www.mediafire.com/file/hyhakz2oyi8lmd3/bug_hide.mp4/file
Another problem if the user hides all the rows there is no way for him to reload the data. It would be nice to have a reload/refresh data button as suggested before

A complement.
The header separator could be in a different color than the header. For the program user, it is difficult to identify where this separator to double-click

See the image. When the program is loaded. The separator is in an unexpected position for the user to find.
If the separator were a line with a different color from the header, it would be easier to find.
Originally posted by @antrrax in https://github.com/israel-dryer/ttkbootstrap/pull/110#issuecomment-1004079814

Feature implementation suggestion.
Add option to copy selection to clipboard.
A code example (maybe there are other better ways), I don't know what it's like on Windows and Mac. But on Linux you need to use pyperclip to keep the data on the clipboard after closing the program. And on linux you also need to do this 'update()' twice and need the
return 'break'
example:
import pyperclip

def tvw_item_ckick(self, event=None):
    self.tvw.selection()
    str_all_columns = ''
    for n in self.tvw.selection():
        str_columns = '\t'.join(self.tvw.item(n, 'values'))
        str_all_columns += f'''{str_columns}\n'''

    self.clipboard_clear()
    self.update()

    try:
        pyperclip.copy(str_all_columns)
    except TclError as e:
        raise e

    self.update()
    return 'break'

self.tvw.bind('<Control-c>', self.tvw_item_ckick)
self.tvw.bind('<Control-C>', self.tvw_item_ckick)

[ ] change autofit pad scaling to 20 pad = utility.scale_size(self, 20)
[ ] investigate post vs tk_popup
[ ] change csv delimiter to something internationally accepted
[ ] investigate bug with hide select rows & clear filter combination
[ ] make vertical separator visible on Tableview header (also treeview?)
[ ] add refresh type button to pagination frame (similar to searchbar)

Actually Tk's clipboard stuff works really weird on Linux (KDE neon with Plasma). Sometimes everything's fine, sometimes nothing happens, sometimes only within the Tk application and outside the it, the previous content is on the clipboard, and sometimes everywhere but it doesn't show up in the clipboard history. However, I haven't experienced that it keeps the content only until closing.

@israel-dryer

make vertical separator visible on Tableview header (also treeview?)

I found this widget has the grid drawn in the treeview, maybe it will serve as an inspiration
https://github.com/ragardner/tksheet/blob/master/tests/test_tksheet.py

I haven't experienced that it keeps the content only until closing.

On the desktop this is my experience with Linux Mint 20.2 x64 Cinnamon X11, Python 3.8.10. If you use tk's default clipboard when closing the program the clipboard is cleared.

I was able to get something that looks ok on the headers while still appearing as a 'flat' theme.

Question is... should I apply this to Treeview as well?
@antrrax, @rdbende, @daniilS, @jongbatax
Another question of style... sort indicator on the left or right of the text?

Indicators to the right is what I've usually seen elsewhere. No personal preference for Treeview atm.

Question is... should I apply this to Treeview as well? @antrrax, @rdbende, @daniilS, @jongbatax

In the widget I commented on earlier:
https://github.com/ragardner/tksheet
pip install tksheet
Demo:
https://github.com/ragardner/tksheet/blob/master/tests/test_tksheet.py
on line 23 and 24 of the demo we have the option to show or hide the grid lines
#show_vertical_grid = False,
#show_horizontal_grid = False,
In the file below see line 3820:
https://github.com/ragardner/tksheet/blob/master/tksheet/_tksheet_main_table.py
The grid lines option used in this example looks very smooth. But they seem very complex to be carried out.
For the complexity. I think not just the gridline in the header would be enough.

Another question of style... sort indicator on the left or right of the text?
More suitable is to be to the right of the text.

I'll look into gridlines in the Tableview. It appears that table above is being drawn with labels, so it's a different build altogether.
A "reset" button is configured in several areas. The reason is that the searchbar is optional, as well as the pagination frame.

A reset table option is now available in the right-click header menu
The reset filters is still available in the cell right-click menu (now fixed)
A reset button will appear in the pagination frame to the right
A reset button will appear in the searchbar (ONLY IF THERE IS NO PAGINATION FRAME)

A reset button appears in the pagination frame (not in the searchbar).

A reset button appears in the searchbar when there is no pagination frame.

A reset table button is now in the header right-click menu and will remove all filters and sorts.

Also, I decided to comment out the "pagesize" combobox for the moment. That option is configurable via the constructor, and the pageframe was getting really cluttered.

@antrrax, how does post behave differently than tk_post?
At least on Windows, there doesn't appear to be a noticeable change in behavior.

With tk_popup you can specify the menuitem index menu should pop up
def tk_popup(self, x, y, entry=""):
    """Post the menu at position X,Y with entry ENTRY."""
    self.tk.call('tk_popup', self._w, x, y, entry)

with post you also can, but it's not implemented in tkinter
def post(self, x, y):
    """Display a menu at position X,Y."""
    self.tk.call(self._w, 'post', x, y)

In behavior the only difference (on Linux) is how it disappears
https://user-images.githubusercontent.com/77941087/147980349-139f1dbb-9ae3-479f-be33-9dd4f84e321a.mp4

https://www.mediafire.com/file/pko7umva7b5waup/bug_post.mp4/file
See the behavior of the post in the video, pay attention to mouse clicks.
When the menu is displayed and hover over the menu items.
Submenus are not displayed.
If I click anywhere on the screen outside the menu, the menu doesn't close. It remains visible.
The menu is only responsive from the moment I click on it. Then it displays the submenus when hovering over the items
Now if I click outside the menu I can close it.

With tk_popup the menu is responsive right when it is displayed.

I have a suggestion for the tableview, but I don't think it would be the case to open a new issue.
The suggestion would be, in the entry of the number of pages restrict to typing only numbers.
The original code looks like this:
index = ttk.Entry(pageframe, textvariable=self._pageindex, width=4)
just change it like this:
vcmd = (self.register(self.only_digits))
index = ttk.Entry(pageframe, textvariable=self._pageindex, width=4, validate='all', validatecommand=(vcmd, '%P'))

def only_digits(self, P):
    if str.isdigit(P) or P == "":
        return True
    else:
        return False

@antrrax, I think this is related to the validation issue and can probably be implemented with that addition.


Prevent Double Commenting

In the addComment function (or whatever the name of the function is that we use everywhere in the code to add a comment) we should check the last comment in the conversation. If the last comment is from the bot, then the bot should edit that comment instead of posting another one.

Updated the wallet address for @pavlovcik successfully!	 Your new address: 0x4007CE2083c7F3E18097aeB3A39bb8eC149a341d

Originally posted by @ubiquibot[bot] in https://github.com/ubiquity/ubiquibot/issues/410#issuecomment-1610792247

I see double comment by the bot is not happening always. It happens when you edit your comment.
Is removing the double comment in this case a good idea? Here is my analysis from my limited knowledge of the bot.
A bot comment is usually just not a comment. It has side effects, e.g. saving wallet to DB. If you remove the double comment, you need to undo its side effects.
If you change wallet, and then edit the comment to assign, both have side affects. I think double comment in this case should stay.  Here is an example https://github.com/web4er/ubi-iss-rep/issues/4#issuecomment-1612722021
Github already doesn't trigger the change if the comment was editing to same old comment. However, if you add space or something invisible to old comment, a double comment by the bot happens. In this case the double comment should be prevented. But instead of removing the double comment, a better approach is to improve the comment parser.  I see two approaches to improve the parser.

Make it very strict e.g. if there are extra spaces, just ignore it.
Use regex and allow multiple white characters

We can see the entire edit history and cross reference timestamps if necessary to track things down. Ultimately, when reviewing a conversation I think that the only information thats relevant is the final state of e.g. the registered wallet.
Furthermore this proposal also solves for when a new contributor comments on a repository and the bot will reply with a help menu. If that new user edits their first comment, the bot will continue to post those tutorial comments again and again.

Intuitively, the bot should adapt its response based on what it was responding to.
In practice, this means that we should detect which comment was edited, and then edit the response of the bot in the immediately following comment.

I have started work on this. Based on what is available through GitHub API, this is my approach.
Let's assume what the user is commenting are commands

We already have information in the bot if it is edited comment or newly created comment by the user.
If it is an edited comment, we retrieve 2 comments since the created_at time of this edited comment.
The first comment is the original initial comment by the user, and the second is the response by the bot. We do confirm the response by checking the username of the bot.
We update the response comment by the bot.
We still use new comment from the bot as a fallback in case we can't find a response from the bot to an edited comment.  More about this below:

There are some valid reasons why we will not find a response from the bot.

someone else comments on the issue before the user gets a response from the bot.
a user edits a normal comment to a command comment.
bot was down and never responded to a command

There are some ways to handle the above cases, but it gets complicated and still leaves a chance of error.
It is a work in progress so anything can change. I am testing it for edge cases.

/assign

Skipping /assign since no time labels are set to calculate the timeline

Oops, I will go ahead anyway.
This was a tough one. The reason is, I couldn't find a way to retrieve the exact comment I wanted through Github API.  Github doesn't offer flexible serverside orderby and filters for comments.
The solution we have is very much like I have described above. It is working well. However, I would like to describe a case, where there is clearly a bot response next to a command and the bot will not edit it but will go to fallback mode and add a new comment. We retrieve 30 comments since the created_at datetime of the edited comment. Unfortunately, Github uses this datetime to compare updated_at datetime of comments instead of created_at. In most cases, this works because the first comment we get is the edited comment, and next is the response the bot has to edit. But if users have edited about 30 comments or more above the target comment, these comments end up between the edited comment, and its response and we miss the response that we have to edit.
The above case can be made less likely by increasing the comments we receive to more than 30, but I think in most cases, that would be unnecessary. Loading so many comments will just put a load on the bot.
Here is the QA https://github.com/web4er/ubiquibot/issues/35

@web4er I'm not sure how I forgot to set a price for this but your efforts are appreciated. I'll generate a reward for you in a bit.


Implement new MotorAxis module

The main goal of this module is to quickly respond to switches and protect the hardware from damage - even if, for example, no RoSys is running, a different serial interface is being used, or a developer sends "manual" commands via the Lizard console.
Tasks:

[x] control a StepperMotor via position/speed/stop commands
[x] use Input switches to stop the motor if a switch becomes active
[ ] prevent the motor from moving in the wrong direction if switch is already active
[ ] support one or two switches

Maybe later:

[ ] reference() with reference switch: find reference switch with predetermined speed, send message to RoSys
[ ] approach reference again more slowly?
[ ] set the internal position to zero when referenced?
[ ] allow using inverted switches
[ ] handle the conversion between motor and output

Test configuration on RB28 (motor on socket 2, sensors on socket 4):
rdyp = Output(15)
en3 = Output(12)
en3.on()
rdyp.on()

m = StepperMotor(5, 4)

serial = Serial(26, 27, 115200, 1)
p0 = Expander(serial, 25, 14)

in1 = p0.Input(32)
in2 = p0.Input(33)

a = MotorAxis(m, in1, in2)


Changes to DependencyInjection requirements

From @davidfowl on Tuesday, July 19, 2016 9:17:38 PM
Moving the discussion from https://github.com/aspnet/DependencyInjection/pull/416
The tricky part with supporting DI is that not all of the things can be codified into the interface definition. That would make it easier for sure but some of it is  just impossible.
Right now we're researching a couple of things:

We're looking to see which components depend on specific features of the DI container (via some automation) and as part of it, we want to see if we can automate any of it.
We're looking at @tillig's suggestions (like writing a chaos container, one of our devs already wrote a second one)
We're trying to see if we build ordering on top via another service type (playing around with IOrdered<T>)
We're looking at the implementations of other adapters to see what things are impossible to implement (We could use everyone's help/feedback on this one).
As a result of the previous exercise, we're also looking to see what other requirements we can relax (last registration wins, closed generics falling back to open generics etc)

As for the a specific list of things we think can be candidates for potential removal:

Ordering - We're trying to build a service on top with some that can potentially handle this.
Last registration wins - We'll likely change the API around service collection to make sure that if multiple registrations make it into the container, resolving it as a single service fails.

Copied from original issue: aspnet/DependencyInjection#433

From @davidfowl on Tuesday, July 19, 2016 9:21:47 PM
From @dotnetjunkie

We're looking at the implementations of other adapters to see what things are impossible to implement (We could use everyone's help/feedback on this one).

There is a lot of implicit behavior in the adapter that will have to be explicitly unspecified. In other words, the contract should explicitly state that in certain conditions an adapter is free to behave as it chooses. For instance, an adapter is allowed to throw an exception.
When considering the current version of Simple Injector, the behavior as specified by the abstraction should be undetermined in the following scenarios:

A captive dependency is registered. Simple Injector will throw an exception when an object graph contains captive dependencies.
A disposable transient component is registered. Simple Injector throws an exception during verification when a component that implements IDisposable is registered as transient.
A component with ambiguous lifestyles is registered. Simple Injector throws an exception during verification when it finds multiple registrations for the same components (e.g. using different interfaces) that are registered with different lifestyles.
Multiple registrations for the same component and same lifestyle, but with different interfaces. The current version of Simple Injector throws an exception during verification when there are multiple registrations for the same component (e.g. using different interfaces) for the same lifestyle, when the registrations will result in having multiple instances of that component within the same scope. In a future version, we are likely to automatically merge these registrations into one to prevent this (much like Unity already does), but this means that it should be undetermined how the system behaves when this happens. i.e. the options are: registrations could be merged, the container can resolve multiple instances per scope, or an exception can be thrown.
Registration of types with multiple constructors. Simple Injector throws an exception when a type with multiple constructors is registered.
Registration of open generic types that overlap with closed generic registrations. Simple Injector does not treat open generic types as fallback, but will instead throws an exception where registrations overlap.
Resolving transient components from ApplicationServices. The built-in container seems to resolve transient components as singleton, when requested from the ApplicationServices property. Simple Injector would resolve them as transient.
Resolving scoped components from ApplicationServices. Simple Injector does not allow resolving scoped instances outside the context of an active scope. It throws an exception instead.

Besides the above list of behavior that has to become explicitly unspecified, there are other behaviors of the abstraction that I’m currently unsure of how it should be specified for the current version of Simple Injector to conform. These included:

Registration and resolving of collections. In its API, Simple Injector explicitly separates the registration of collections from one-to-one mappings. Letting an adapter to Simple Injector register everything as collection (with possibly one element) makes users lose out on one of Simple Injector verification capabilities. Disallowing a single ‘default’ to be resolved when there are multiple registrations (as proposed earlier in this thread) doesn’t change this fact.
Scopes are explicit. In Simple Injector, scopes are ambient (just like TransactionScope) and the services are always resolves from the container; never directly from a scope instance.

This is what I’ve been able to come up with at this point and that holds for the current version of Simple Injector. As already noted above, there is already a plan to change some behavior of Simple Injector in the next major version. I will likely have missed some things, so I will update this comment as I discover more items.

From @davidfowl on Tuesday, July 19, 2016 9:22:28 PM
From @slaneyrw
@dotnetjunkie Looking at the perspective of a Unity user I echo most of your points, and explicitly call out the Cardinality problem.
I have been a user of the Unity container for many years and i have a fork of the GitHub that I'm using to convert it to netstandard1.6.  When i was trying to build an Adapter for IServiceCollection/IServiceProvider I immediately came across the issue of how to distinguish between a replacement registration and a multiple registration.
TryAdd is easy as you check for an existing registration and add if not present
Add becomes complicated... Do I replace the existing registration or add another one.  Without understanding of the intent of a registration at the time of registration the container will have to hedge both bets.
It become vastly more complicated with IServiceScope.  It doesn't make sense to me have registrations for any type that is designed for a single instance when resolved to have both a Scope lifetime and singleton/transient lifetime.  In Unity a Scoped lifetime would be implemented using the HierarchicalLifetimeManager.  If a type was resolved from the composition root ( i.e. IServiceProvider ) or from the scope (IServiceScope ? ) then the same build plan would be used but the instances will be stored in their respective owner container.  I would not be possible to store two separate registrations for two different implementation without creating a new registration in the scope container when it is created ( or black magic to "migrate" a parent registration marked as "Scope" ).  Once again, having this distinction for a single instance resolve does not make sense to me.
Without any cadinality metadata at registration I will be forced to maintain two sets of registrations.  If Add is used then I will have to add a default registration AND a named registration.  If Add is used multiple times for the same interface/concrete then last in wins for the default registration.
Having to maintain a specific order for resolved IEnumerable<T> should be easy to accomplish by adding a post resolve build policy for those interfaces what have Order metadata, like IOptions<T>.
However is there anything in the spec that indicates whether the order should be evaluated across the entire resolve set... i.e.

Resolve both composition root AND scope, then ordering
Resolving and ordering from scope and composition root respectively, in isolation, then concatentating, and in what order
Resolving IEnumerable<T> from IServiceScope should ignore composition root

From @davidfowl on Tuesday, July 19, 2016 10:30:28 PM
The problem with having cardinality defined up front for everything is that you lose the ability to distribute where registrations come from. There might be some default implementations in the box and the user might provide some. I'm not sure how that would work if you were forced to declare everything up front in a single call. I'm not even sure why that matters to be honest.
We're currently going to change things so that an IServiceCollection with multiple entries of the same service type indicate the service is IEnumerable<T>. There's some gotcha's though:

To support optional services in constructor injection some components use IEnumerable<T> and check for empty as a pattern. We didn't plan to break that but that will conflict with features like this:

Registration and resolving of collections. In its API, Simple Injector explicitly separates the registration of collections from one-to-one mappings. Letting an adapter to Simple Injector register everything as collection (with possibly one element) makes users lose out on one of Simple Injector verification capabilities. Disallowing a single ‘default’ to be resolved when there are multiple registrations (as proposed earlier in this thread) doesn’t change this fact.

We'd need to take a look at places where we do this and see if we can break that pattern.
@slaneyrw about unity

Resolve both composition root AND scope, then ordering

Resolving and ordering from scope and composition root respectively, in isolation, then concatentating, and in what order
Resolving IEnumerable from IServiceScope should ignore composition root

Always preserve registration order (currently) using what. As I said before, we're looking at relaxing that requirement. You need to look at both the "composition root" and the "IServiceScope" depending on the lifetimes of the resolved services.

From @mattnischan on Wednesday, July 20, 2016 7:09:28 AM

To support optional services in constructor injection some components use IEnumerable and check for empty as a pattern.

That's crazy evil. Collection resolution is pretty important, especially when you have strategy pattern type services like something resembling an IEnumerable<IFilter> or IEnumerable<IRule>. You might want all of them to throw your data at.
Optional services should still be able to be injected by either expanding the expression generator to mock up a default class for that service interface or by injecting default(T). That would make it more compatible with most other containers.

From @dotnetjunkie on Wednesday, July 20, 2016 7:24:42 AM

or by injecting default(T). That would make it more compatible with most other containers.

Think again. Most containers disallow injecting null references into constructors. Constructor arguments should never be optional.

From @mattnischan on Wednesday, July 20, 2016 8:22:39 AM
Not a huge fan of optional constructor injection either, honestly. Just looking for the most easily cross container compatible answer without resorting to something like IOptional<T> and a static Optional<T>.Empty.

From @slaneyrw on Sunday, July 24, 2016 7:49:03 PM
Almost finished a ServiceCollection adapter for Unity and came across a few nasties.

There is no way of getting a IServiceProvider from IServiceCollection, so I tried using your BuildServiceProvider() extension.  However your implementation uses hardcodes a ServiceProvider instance passing all the ServiceDescriptors, instead of resolving it or asking ServiceCollection.  Rounding tripped registrations via ServiceDescriptor means I'll lose information.
TryAdd extension iterates through all the ServiceDescriptor in the ServiceCollection. Maybe IServiceCollection should have an IsRegistered ( ServiceDescriptor ) => bool

I think both of the aspects should be on the ServiceCollection interface so implementors can override the behaviour

From @davidfowl on Sunday, July 24, 2016 9:20:21 PM

You're supposed to create your own service provider implementation over the list of service descriptors. Look at the other adapters for an example.
Why? Also changing the interface is breaking so that's not going to be happening in the short term. We can look in a future release.

From @slaneyrw on Monday, July 25, 2016 5:30:34 AM
Yes, I am creating my own service provider, but NOT over the original ServiceDescriptors.  The ServiceDescriptor is too simplistic for all but the basic mapping type to implementation.  What about property injection, method invocation, interface and virtual method interception.  These are all tools in the arsenal that are not supported using ServiceDescriptor.
Asking the ServiceCollection for all the descriptors and giving it to a ServiceProvider feels wrong to me.  It believe should be the responsibility of the ServiceCollection to initialise the ServiceProvider.
So at the moment I have to return an IServiceProvider from the ConfigureServices method instead of relying on MVC to create it.  I hope you don't remove that capability!  Maybe the default MVC template should be changed to make it more explicit and return IServiceProvider from ConfigureServices.

From @davidfowl on Wednesday, July 27, 2016 9:00:36 PM

Yes, I am creating my own service provider, but NOT over the original ServiceDescriptors. The ServiceDescriptor is too simplistic for all but the basic mapping type to implementation. What about property injection, method invocation, interface and virtual method interception. These are all tools in the arsenal that are not supported using ServiceDescriptor.

I don't know why that affects any of those features.

Asking the ServiceCollection for all the descriptors and giving it to a ServiceProvider feels wrong to me. It believe should be the responsibility of the ServiceCollection to initialise the ServiceProvider.

The service collection is just metadata for your actual container implementation. The service collection isn't your container registration API. So its the container implementors job to take the "metadata" and turn it into container specific API calls. The container implementor also needs to return an IServiceProvider that calls through the container implementation.

So at the moment I have to return an IServiceProvider from the ConfigureServices method instead of relying on MVC to create it.

That's how it works and will continue to work.

I hope you don't remove that capability! Maybe the default MVC template should be changed to make it more explicit and return IServiceProvider from ConfigureServices.

We landed on leaving it implicit for the default case.

From @davidfowl on Wednesday, July 27, 2016 9:08:58 PM
We've started on some of the proposed changes here:
https://github.com/aspnet/DependencyInjection/pull/437

Preserving ordering is no longer a requirement - There are new APIs and a new type IOrdered<T> to get a list of ordered things described via services.AddOrdered<T> calls. Specific containers can choose to override the default implementation and do something more efficient.
Explicit IEnumerable<T> services - If you ask for IEnumerable<T> without saying the service is IEnumerable<T> it will fail.
New service descriptor (breaking change) implementation - ServiceDescriptor is the base class and there's derived ServiceDescriptor(s) for each of the service types (Instance, Type, Factory, Enumerable etc).

/cc @tillig @alexmg @nblumhardt @seesharper @jeremydmiller @khellang

From @tillig on Thursday, July 28, 2016 10:28:19 AM
I'm a bit torn on the explicit IEnumerable<T> services. I can see enforcing that adding multiple services of the same type to a ServiceCollection may fail unless you specify that you're adding to a member of a collection (intentionally), but the resolution of the IEnumerable<T> may need to be unspecified.
For example, Autofac intentionally returns an empty IEnumerable<T> if you don't have any members registered so things like message processors that want a list of handlers won't just fail out of hand - you won't have to check for null-or-empty, just empty. And if you only register one thing, great, you get an enumerable of one.
We couldn't even really add anything to the service provider to track which explicit IEnumerable<T> contributors have been registered because we'd need to push the tracking down into Autofac (so things registered by assembly scanning or whatever are also tracked)... and then it'd break out of the box behavior.
Long story too long - I think it's cool to enforce the different registration mechanism as part of ServiceCollection but not so much for the actual backing IServiceProvider doing the resolving.

From @davidfowl on Thursday, July 28, 2016 10:36:47 AM

Long story too long - I think it's cool to enforce the different registration mechanism as part of ServiceCollection but not so much for the actual backing IServiceProvider doing the resolving.

We have to, it's one of the things that ends up being hard to implement in some containers. We can't enforce IEnumerable<T> is ok to resolve if the service wasn't registered as IEnumerable<T>, that can be an autofac thing but we won't rely on it anymore. Also, we need to call out the behavior of GetService vs ctor injection so that it's mostly consistent (where possible).

From @tillig on Thursday, July 28, 2016 10:38:58 AM
I think it's fine if you don't rely on it as long as it's not also wrong if it isn't explicitly disallowed.

From @khellang on Thursday, July 28, 2016 12:12:13 PM
@davidfowl Just thought about something... Does ctor injection go through the ISupportRequiredServices path? Or does it fail on ctor selection?

From @khellang on Thursday, July 28, 2016 2:05:39 PM
Uh, never mind. Just realized that it all depends on the call at the composition root and what the individual container does from the on.

From @davidfowl on Thursday, July 28, 2016 2:26:48 PM
@tillig That's always a problem right? What should "the spec" recommend? If any framework relies on it then it won't work with whatever container doesn't support it.

From @tillig on Thursday, July 28, 2016 2:43:34 PM
@davidfowl Yeah, that is the problem. Of course, if part of this is in an attempt to loosen the requirements, then saying "behavior is unspecified and shouldn't be counted on" is loosening the requirements and I'm for it. Adding the notion that "if a component isn't registered via the ServiceCollection with an explicit bent toward IEnumerable<T> then the backing container should also explicitly fail to resolve the IEnumerable<T>" is actually a new requirement and I'm not keen on that.
Just off the top of my head, that one would actually be harder to implement on Autofac with more breaking changes than the ordering requirement that was previously handled.
So... "don't count on it because it's intentionally unspecified" == cool; "don't count on it and container must implement a behavior to disallow it" == not so cool.

From @slaneyrw on Thursday, July 28, 2016 3:12:56 PM

Explicit IEnumerable services - If you ask for IEnumerable without saying the service is IEnumerable it will fail.
New service descriptor (breaking change) implementation - ServiceDescriptor is the base class and there's derived ServiceDescriptor(s) for each of the service types (Instance, Type, Factory, Enumerable etc).

For what it's worth, makes me happy.  Should be a lot easier to implement container adaptors.
Will the new ServiceDescriptors be sealed ?
What is the expected behaviour if a new ServiceDescriptor subclass is presented to ServiceProvider ?

From @davidfowl on Friday, July 29, 2016 8:27:04 AM

Will the new ServiceDescriptors be sealed ?

@slaneyrw sealed? No.

What is the expected behaviour if a new ServiceDescriptor subclass is presented to ServiceProvider ?

Explode.

From @davidfowl on Friday, July 29, 2016 8:29:28 AM
@tillig

So... "don't count on it because it's intentionally unspecified" == cool; "don't count on it and container must implement a behavior to disallow it" == not so cool.

That's fine. Our spec tests won't require it to throw but our default container will throw. At the same time, we need a way to make sure that frameworks don't rely on things like this. It's fine for applications to rely on it though.

From @davidfowl on Saturday, July 30, 2016 1:23:54 AM
/cc @dadhi @seesharper

From @davidfowl on Monday, August 1, 2016 11:16:56 PM
Some questions for the thread as we go further with the current plan:

What should the behavior be if the adapter is passed an unknown service descriptor? I mentioned earlier it should blow up but I'm having second thoughts on that one. We're implementing IOrdered<T> as a specific ServiceDescriptor that keeps track of the registration order of things. It means if somebody adds a new derived service descriptor, it'll be silently ignored by container adapters so error detection is harder. On the other hand, allowing different kinds of service descriptors lets different containers light up behaviors that others may not be able to handle. It would be nice to have a way for containers to replace behaviors built on top with native implementations.
For things that resolve as IEnumerable<T>, can the lifetimes be mixed? In the current prototype we only support transient services in IEnumerable<T>. How do other containers handle this and what are the side effects?

From @tillig on Tuesday, August 2, 2016 7:14:16 AM
I think I like the ability to have custom derived service descriptors as an extension mechanism, though admittedly I don't have a completely thought out use case right now. Maybe the ability to register decorator types? Just brainstorming.
I think lifetimes should be mixable in IEnumerable<T>.
By way of example, in Autofac if you register a singleton, request scope, and transient into the same container and resolve the enumerable from the request scope, you'll get all three.
For example, day you have some message processor and you register handlers for messages. One handler may be expensive to instantiate or otherwise initialize but is stateless, so you can optimize by registering it as singleton. Another handler may be cheaper or stateful, so it's transient. When you resolve all the handlers you still want both.

From @ajcvickers on Tuesday, August 2, 2016 9:38:37 AM
Just a note about EF. EF relies on collections of scoped services being constructor injected as IEnumerable<T>. So are far as I can tell these changes will break EF. It will also break all our providers since provider resolution also relies on this.
Question about versioning: since these are very significant breaking changes shouldn't it bump the version of D.I. to 2.0? Otherwise NuGet is going to make it very easy for people to get combinations of packages that don't work together.

From @davidfowl on Tuesday, August 2, 2016 11:43:25 AM

Just a note about EF. EF relies on collections of scoped services being constructor injected as IEnumerable. So are far as I can tell these changes will break EF. It will also break all our providers since provider resolution also relies on this.

What's the lifetime of the thing it's being injected into? Is that itself scoped? Would be good to look at some concrete examples.

Question about versioning: since these are very significant breaking changes shouldn't it bump the version of D.I. to 2.0?

This was one of the breaking changes we were going to take for 1.1 because of the impact on 3rd party containers.

Otherwise NuGet is going to make it very easy for people to get combinations of packages that don't work together.

Calling it 2.0 doesn't make it any harder for nuget to give you invalid combinations of packages. NuGet will roll you forward if any package depends on 2.0.

From @dotnetjunkie on Wednesday, August 3, 2016 11:25:52 AM
Since I can't update this comment, here's a little update about Simple Injector and IEnumerable<T>:
In Simple Injector IEnumerable<T> dependencies are streams. What this means is that those components will only be created when the IEnumerable<T> is iterated and components will be resolved according their registered lifestyle. What this means is that a transient component will be created everytime the collection is iterated. In other words, the IEnumerable<T> dependency itself is a singleton that has factory-like behavior. It can be safely injected into a singleton component, even though its elements might be transient, scoped or singleton.  So obviously Simple Injector allows mixed lifestyles within a single IEnumerable<T>, but the behavioral difference goes even further.
Since other DI containers behave differently in this respect, the DI abstraction should make the exact behavior undetermined.

From @davidfowl on Tuesday, August 30, 2016 12:49:20 AM
Just an update on this thread. We've been busy trying to come up with a proposal that lets these changes happen in 1.1 while not breaking everyone by default. It's looking like we're going to be adding new APIs parallel to IServiceCollection and ServiceDescriptor to account for the changes we need to make for other container authors to plug in this new behavior. Since the new behavior is a subset of the old behavior, it's tricky to make them work in the same application without awful hacks. The API split is the cleanest approach we've come up with to date. I'll update this thread with the proposal as soon as we get more clarity.
In addition to this @pakrym has been doing a bunch of research on 3rd party containers to what other problems we can uncover. Here's his summary:
Requirements lifted

Enumberable descriptors are now implicit lists of services instead of being by convention.
Order requirement for enumerable
Next registered service overrides previous
IEnumerable resolving of non registered services

We are thinking about lifting

Disposing transient services
Default argument values in constructor selection
Nested scopes

Current state of porting DI container adapters
Autofac
Works with minimal changes
StructureMap
Works with minimal changes
Ninject

Does not dispose transient services
The way we implemented scopes in adapter might need overhaul

Unity

Uses longest constructor by default, requires constructor selector implemenatation that choses longest satisfiable constrcutor.
Does not dispose transient services
Unity would dispose singleton resolved from scoped service provider. E.g this test fails:

[Fact]
public void ChildContainerDoesNotDisposeSingleton()
{
    var container = new UnityContainer();
    container.RegisterType<IFakeSingletonService, FakeService>(new ContainerControlledLifetimeManager());
    container.RegisterType<IFakeScopedService, FakeService>(new HierarchicalLifetimeManager());

    FakeService singleton = null;
    FakeService scoped = null;
    using (var childContainer = container.CreateChildContainer())
    {
        singleton = (FakeService) childContainer.Resolve<IFakeSingletonService>();
        scoped = (FakeService)childContainer.Resolve<IFakeScopedService>();
    }

    Assert.True(scoped.Disposed);
    Assert.False(singleton.Disposed); //fails here
}

Castle.Windsor

Does not allow creating scoped services without being in scope
Scope implementation is not directly compatible. When you do BeginScope in windsor entire container switches to net scope so there is no way to resolve from scoped and root container at the same time.

From @khellang on Tuesday, August 30, 2016 1:45:13 AM
Lifting "Resolving transient services"? What does that mean?

Uses longest constructor by default, requires constructor selector implemenatation that choses longest satisfiable constrcutor.

FYI; StructureMap also does this. We have a custom constructor selector.

From @seesharper on Tuesday, August 30, 2016 2:09:01 AM
Are the specification tests updated to reflect changes made in 1.1.0 so that we can start to prepare/test for the next version?

From @pakrym on Tuesday, August 30, 2016 8:19:47 AM

Lifting "Resolving transient services"? What does that mean?

That's a mistake it should read "Disposing transient services"

From @ipjohnson on Tuesday, August 30, 2016 8:44:19 AM
I understand the drive not to create a conforming container standard but at some point doesn't it become a detriment to the end user not to have some core functionality that can be expected? Transient disposal seems like it would be one of those features a user would want as a staple (if the container doesn't who would?).

From @khellang on Tuesday, August 30, 2016 10:50:11 AM

That's a mistake it should read "Disposing transient services"

Ah, good.

From @dotnetjunkie on Tuesday, August 30, 2016 1:23:15 PM
@davidfowl
I admire your desire to improve the DI contract and it's great to see your dedication to solving the issues that exist today.
I would like to know what the current progress is in improving of the experience of the non-conformers as you stated in https://github.com/aspnet/DependencyInjection/pull/416#issuecomment-232106346:

We'll (ASP.NET) make sure that there are adequate composition roots per framework to make integration as smooth as it can be.
We'll work with container authors to figure out how to bridge the 2 containers (because it is necessary)

If you have work that we could review and discuss, please provide us with the details.

From @slaneyrw on Wednesday, August 31, 2016 10:44:11 PM
For Unity, registering the same concrete type for a Parent singleton and Child scope is not a supported scenario.  Unfortunately Unity reuses the same build plan because the service implements both interfaces
You will get the correct behaviour if the classes were different.
You also get the correct behaviour if you use ResolveAll and supply a "name" argument to the registration - equivalent to your planned IEnumerable service descriptor.

From @naasking on Tuesday, September 13, 2016 10:31:13 AM
+1 to closed generics overriding an open generic instance.

From @pakrym on Monday, September 26, 2016 10:30:07 AM
/cc @muratg

From @dotnetjunkie on Thursday, October 13, 2016 11:31:20 PM
Here are my suggestions for improving integration for non-conformers.

From @dotnetjunkie on Thursday, November 17, 2016 2:44:11 AM
Did anything discussed above end up in the just released ASP.NET Core 1.1?

From @khellang on Thursday, November 17, 2016 3:17:41 AM
I think (at least) the following was fixed in 1.1 (with #430):

A captive dependency is registered. (This might not be until resolution though. Not sure)
Resolving transient components from ApplicationServices.
Resolving scoped components from ApplicationServices.

From @nvivo on Wednesday, February 8, 2017 8:11:13 AM
Any news on removing the tracking of transient disposables outside of scopes?

From @slaneyrw on Friday, March 10, 2017 10:19:06 PM
Is this still to be done?  I cannot find any subclasses of ServiceDescriptor on the Dev branch

From @ErikSchierboom on Friday, March 31, 2017 5:51:01 AM
This looks great. I would love to have improved integration when using SimpleInjector, preferrably such that SimpleInjector would be able to inject dependencies as action parameters (which is a brilliant feature).
By the way, SimpleInjector 4 has just been released. Don't know if that helps.

From @slaneyrw on Monday, August 14, 2017 3:54:49 PM
Just looking at the API for v2, looks like none of the service descriptor differentiation still hasn't surfaced
Also new issues with bad constructor discovery

LoggerFactory now has 2 constructors of length 2 and Unity is throwing an ambigious constructor exception.
ConsoleLoggerProvider's longest constructor takes a Func<string, LogLevel, bool> for the first argument, which is not injectable.
DebuggerLoggerProvider has the same problem
HttpContextFactory longest constructor takes an IHttpContextAccessor, which is not registered in the default service collection.  AppInsights also has this pattern.

From @davidfowl on Monday, August 14, 2017 7:43:55 PM
Realistically, we can't change anything here. The best bet for making things like unity, castle and other containers work would be to go to @dotnetjunkie approach outlined here https://github.com/aspnet/Mvc/issues/5403.
TL;DR if the container implementation can't conform then there's another approach that includes adapting various composition roots. I think this is the best path forward to making containers with completely incompatible semantics work. It does mean that both containers need to be bridged but that's basically how things used to work in the older days (pre-ASP.NET Core).

From @slaneyrw on Tuesday, August 15, 2017 10:38:54 PM
Just had a look at @dotnetjunkie 's repo and I think there are major problems trying to run both side-by-side.  I prefer that the container OWNS the composition root, and that is how I've got unity to work
Using the IServiceProviderFactory approach make the most architectural sense to me.  I ended up creating a subclass of ServiceDescriptor where I can add all the Unity specific registration metadata and handle the registration in the CreateBuilder phase.  I've created extensions off IServiceCollection to build the new descriptor, now I can support all the more complicated DI aspects, like Property and Method injection, parallel dependency graphs, as well as interception mechanisms ( transparent caching )
I've solved the issues with the constructor selection by switching the constructor selector policy to match the default DI's behaviour, although I can't work out how it makes a distinction between LoggerFactory's 2 constructors - I think there is some undocumented behaviour in there.
Cardinality hints ( or lack there of ) is still an issue, I was really hoping the v2 API was going to support this, but I can work around it.

We looked at this in detail and decided to do nothing here. We have a bunch of 3rd party implementations and our goal is to not break those and as a result, we'll shy away from adding features that aren't ubiquitous.

We looked at this in detail and decided to do nothing here. We have a bunch of 3rd party implementations and our goal is to not break those and as a result, we'll shy away from adding features that aren't ubiquitous.

As has been pointed out multiple times, the default DI abstraction in .NET Core is a poor cousin to the existing full featured libraries.  Given it is virtually impossible to replace the DI layer inside ASP.NET every has had to "conform" to your API surface, and therefore your statement has become a self fulfilling prophecy.


Add retentionDays option to the action

Thanks for taking this up further. From my very limited knowledge changes looks good.


clboss-withdraw?

If I want to pull some funds out of my node, I currently turn off clboss, close a channel at random, and withdraw.  It'd be nice if clboss figured out how to get enough onchain funds to make the payment for me, instead.

I would second this feature, as I have to do the same procedure :)

Suggest clboss-withdraw be a wrapper around the submarine swap with user supplied address and amount.
Cons

using a third party so is unreliable, and subject to their amount restrictions
Pros
Withdraw creates remote channel capacity
Good for privacy, doesn't expose your lightning node or balance on chain

You can use clboss-ignore-onchain then close channels and withdraw, without disabling clboss, that is the intent of that command after all, to ignore onchain funds temporarily.  This still requires multiple commands as well as waiting.
@AutonomousOrganization has good idea I think.  Though it would require some restructuring of the code.

the onchain-funds-announcer should use a different new message for announcing onchain funds.
move the ignore-onchain mechanism to a new module that accepts the "raw" onchain funds announce and emits the "pickled" onchain funds announce (i.e. the current message).
Make the new module a swapper.
When a clboss-withdraw command is received, put it in the db as a set of payments to make.  Then trigger a swap of the value.
If the raw onchain funds announcement is received, check if we have pending payments that can now be fulfilled, and withdraw if so (possibly with some heuristics to hold off for future swaps if there are still other pending payments as well, to automatically merge multiple withdraws).  Otherwise check for ignore-onchain timeout and if not ignore-onchain, announce the "pickled" onchain funds announcement (which will trigger the channel creator).


Typos on Corretto

Description:
The codes should use Corretto rather than Coretto.
For example, src/distributions/corretto/installer.ts contains the below line,
console.time('Retrieving available versions for Coretto took'); // eslint-disable-line no-console

Coretto is a typo, and Corretto is expected.
Task version:
v3
Platform:

[x] Ubuntu
[x] macOS
[x] Windows

Runner type:

[x] Hosted
[x] Self-hosted

Repro steps:
Expected behavior:
Uses Corretto in codes.
Actual behavior:
Uses Coretto in some codes.

Hello @johnshajiang 👋,
Thank you for creating this issue. We will investigate it and get back to you as soon as we have some feedback.

Thanks for triaging this issue.
I already submitted a PR #666 for it, please take a look at this PR.

Thanks for merging the patch from my PR #666.


FLOC-3913 Remove curls for missing files and postgres dir

fixes https://clusterhq.atlassian.net/browse/FLOC-3913

As discussed in person, request code review to involve testing https://s3-us-west-1.amazonaws.com/mewert-chq-work-bucket/FLOC-3913-cloudformation.json with the current test Richard is working on, in addition to usual code review.

Thanks @sarum90
I ran my automated test on the template from your branch and it passed.
CLOUDFORMATION_TEMPLATE_URL=https://s3-us-west-1.amazonaws.com/mewert-chq-work-bucket/FLOC-3913-cloudformation.json AWS_DEFAULT_REGION=us-west-1 KEY_PAIR=... ACCESS_KEY_ID=... SECRET_ACCESS_KEY=...  trial flocker.acceptance.endtoend.test_installer
flocker.acceptance.endtoend.test_installer
  DockerComposeTests
    test_docker_compose_up_postgres ... Warning: Permanently added '<IP_ADDRESS>' (ECDSA) to the list of known hosts.
                                   [OK]

-------------------------------------------------------------------------------
Ran 1 tests in 601.465s

PASSED (successes=1)

I'll merge this.


ci: run locally

copilot:all

Current dependencies on/for this PR:

main

PR #3026   👈

This comment was auto-generated by Graphite.


UNLOAD, SET TBLPROPERTIES, SERDEPROPERTIES, EXECUTE USING, SET LOCATION, DELIMITED, EXTENDED / FORMATTED

Some updates, I tried to find references and included them below.  I'm not all too familiar with this repo. so feedback would be appreciated.
If needed, I can look into breaking out these changes into separate PR's.
SET TBLPROPERTIES -- https://docs.databricks.com/en/sql/language-manual/sql-ref-syntax-ddl-tblproperties.html#set-tblproperties
Hive Delimited Properties -- https://spark.apache.org/docs/latest/sql-ref-syntax-hive-format.html
Hive SERDEPROPERTIES -- https://spark.apache.org/docs/3.0.1/sql-ref-syntax-ddl-create-table-hiveformat.html
Hive EXTENDED / FORMATTED -- https://docs.cloudera.com/cdw-runtime/cloud/materialized-view-commands/topics/hive_describe_materialized_view.html
Added support for DESC version of DESCRIBE
EXECUTE USING -- https://www.postgresql.org/docs/current/plpgsql-statements.html#PLPGSQL-STATEMENTS-EXECUTING-DYN
Unload -- https://docs.aws.amazon.com/redshift/latest/dg/r_UNLOAD.html
Athena only, should this be its own dialect?
Athena SET LOCATION -- https://docs.aws.amazon.com/athena/latest/ug/alter-table-set-location.html
Doesn't require SET, also may be Athena only
Included is a possible bugfix where supporting columns after partition could consume a query

Hi @jonathanlehto  -- thank you for the contribution and I apologize for the delay in reviews.
If you could please split this PR into smaller ones for easier review I would be most appreciative -- it will be much easier for me to verify that each feature is properly tested and documented in smaller PRs rather than one large one

Marking as Draft to signify this PR is no longer waiting on review. Please mark it as ready for review when it is ready for another look.

Okay, this pull request has been split into the following.  I'll leave this this on draft for now
https://github.com/sqlparser-rs/sqlparser-rs/pull/1150
https://github.com/sqlparser-rs/sqlparser-rs/pull/1151
https://github.com/sqlparser-rs/sqlparser-rs/pull/1152
https://github.com/sqlparser-rs/sqlparser-rs/pull/1153
https://github.com/sqlparser-rs/sqlparser-rs/pull/1154
https://github.com/sqlparser-rs/sqlparser-rs/pull/1155
https://github.com/sqlparser-rs/sqlparser-rs/pull/1156
https://github.com/sqlparser-rs/sqlparser-rs/pull/1157

Okay, this pull request has been split into the following. I'll leave this this on draft for now

Thank you very much 🙏

Most of the split pr's are in.  This pr is no longer necessary as a reference, closing..


HTTPS

Let's encrypt, yo.
@matthewbentley preemptively assigning to you; figured you would be interested. If you don't want to take this, feel free to unassign

This is on my longer term todo list for things running in docker on the acm server.
Right now, for each service it creates an interface w/ the proper MAC to get the required hostname, and then puts that interface into the container.
I'd like to have all these interfaces on the main VM and then use nginx to reverse proxy to each app container depending on the hostname.
This would give us the opportunity to serve multiple sites per interface (/ip address) and get lets encrypt certs for each site.
So I'll see if I can get around to this some time this or next week.

Cool! Lemme know if there's anything you need from me for this.

bump :)

No longer deployed under this infrastructure (RIP krall) so closing.

No longer deployed under this infrastructure (RIP krall) so closing.


Fix Waypoint Generate Random Whole Numbers Test

Closes #4194

@ltegman This challenge does not have a solution. Do you mind adding one?

Had to get creative with the tests since I couldn't use editor and get them to pass the test suite.

Sorry about that. Didn't realize this one required the editor. The solution will have to wait until all the challenges are updated to be wrapped. Can you remove the solution and revert back to original tests?

Done.

Sorry to make you jump through hoops.

Not a problem. It was actually fun to see how I could make a test that was more functionality based rather than based on checking exactly what was typed.

Two issues:
The last test incorrectly detects Math.floor, maybe from the example above?

While you're at it, move the display function out to the tail?

Okay, done :smile:

:+1:

@SaintPeter and @ltegman


chore: use codecov actions instead of deprecated NPM package

Codecov Report

Merging #528 (835b826) into main (b704989) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #528   +/-   ##
=======================================
  Coverage   97.86%   97.86%
=======================================
  Files          13       13
  Lines        1545     1545
  Branches      317      317
=======================================
  Hits         1512     1512
  Misses         15       15
  Partials       18       18

Continue to review full report at Codecov.

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


[QUESTION] Export related models

Prerequisites

[X] Able to reproduce the behaviour outside of your code, the problem is isolated to Laravel Excel.
[X] Checked that your issue isn't already filed.
[x] Checked if no PR was submitted that fixes this problem.

Versions

PHP version: 7.3
Laravel version: 5.7
Nova version: 1.3.1
Package version: - current -

Description
Actually this is no issue but a question. I have two models: Projects, Members. Each project has many members.
What I want to do? I want to export a list (xlsx) of all members for one project. I've created a Member's action. Then I have to select all members and run my action.
What I want to do instead? I want to run an action on a single project, getting all members. It this possible?
If this is the wrong place for questions like this, I apologize. Thank you for your help!

I think it's possible to execute an action on a relation resource. Create a hasMany field for Project -> Members and then if you go to show Project, you should see a list of Members. If you have a Member Excel action, then it should be there to execute.

@patrickbrouwers Thank you! Yes, this is exactly how I'm doing that.
I'm sorry, I wasn't clear. For better user experience I would like to skip the select-all step. Let me explain a little more:
If I'm managing a project I (almost always) want to have a list of all active members. I've created a filter for that. So now I have to do lots of things to get the list:

Open the project's detail view.
Choosing the correct filter.
Selecting all (filtered) members
Select Member's Excel action.
Run the action.

What I would prefer:

Open the project's detail view / Selecting project in project index view.
Run action on project.

I'm sorry for the ugly English, I'm not a native speaker. I hope, it gets clear now, what I want to do.
Thank you very much for your help!

@simonedelmann the "select-all" step is a Nova thing, so we can't really do a lot about that. Currently Nova only allows us to run actions on selected rows.

OK. :( Anyway thank you very much!

Similar question here (and staying in Simons example): when I use an excel export action on members via the the detail view of projects (members as relation resource on a HasMany-Field), the export ignores all selections and filters I apply to the relation resources before. No matter how many members I select for my export, I always get all related members in my excel file. However, if I run this action on the index view of members it works just fine and respects my selections for the export. Any idea what's going on here, @patrickbrouwers ? I use the WithMapping concern btw. Many thanks!

I think it's easier to just call Laravel Excel directly in a Nova action in those cases.

I faced the same issue, and I found a way around this. By implementing FromQuery in your action as well, and passing the resource variable to the action you can create a custom query. My result looks something like this:
`class ExportUsers extends DownloadExcel implements FromQuery, WithMapping, WithHeadings
{
public $onlyOnDetail = true;
public function __construct(Request $request)
{
    $this->project_id = $request->get('resources');
}

public function query()
{
    return Users::where('project_id', $this->project_id);
}

public function map($product): array
{
        return [
           $user->name,
           $user->project->name
        ];
}

}`
Assuming that there is 1 project with many users.


dependency missing for react-tap-event-plugin

The example requires it.

Removed the better tap plugin usage from the example bcbeccfbd959e04757726aeae1c8a28cfc006a9f


 IF YOU DO NOT FOLLOW THE FORMAT OR MAKE A GENERIC TOKEN REQUEST YOUR ISSUE WILL BE DELETED WITHOUT COMMENT
YOUR JUSTLIST MUST FOLLOW THE JSON SPECIFICATION
https://github.com/justswaporg/justlists/blob/main/example.justlists.ts
Checklist

[x] I understand that this is not the place to request a token listing.
[x] I have tested that my JustList is compatible by pasting the URL into the add a list UI at justswap.org.
[x] I understand that filing an issue or adding liquidity does not guarantee addition to the justlists website.

Please provide the following information for your token.
JustList URL must be HTTPS.
JustList URL: https://presale.metamenacoin.com/MtmSunswapList.json
JustList Name:  SunSwap MTM List
Link to the official homepage of the JustList manager:https://metamenacoin.com/

Thanks for reaching out.
We are currently very tight on development resources and adding new lists on a case-by-case basis.
If timing is a key issue for you, we recommend you join the existing community SUNSWAP list, for example: Tron Alliance List: https://github.com/listtronalliance/tronalliance, you can submit a new issue at it.
Thanks.

Sorry, this issue will be closed . If you want to add a new list, please resubmit.


nPrint logging/information printed to STDOUT breaks CSV output

nPrint appears to print all auxiliary information to STDOUT, such as this error:
error while trying to create ''

and this verbose info:
Superpacket {
Eth Header: src: 28:16:a8:64:08:c4, dst: 28:80:a2:0e:43:32, ether_type: 2048
IPv4Header: src_ip: <IP_ADDRESS>, dst_ip: <IP_ADDRESS>
TCPHeader: raw data not set
UDP header: srcprt: 50887, dstprt: 56833, len: 83, cksum: 55160
ICMPHeader:: raw data not set
Payload: length: 75
}

This might be fine, except that nPrint features printing its results to STDOUT, as well.
When I invoke nPrint without the --write_file option, the above verbose info gets mixed into the resulting CSV, which as a result is invalid and cannot be parsed. (This is particularly vital for pooled/buffered/etc. optimizations involving nPrint, which expect to be able to read from PIPE.)

It might make sense for all such non-CSV output to be printed to STDERR, (at least whenever an output file isn't specified).

To reproduce:
$ nprint --version
nprint 1.1.6
$ nprint -V -P what/ever/pcap.pcap --tcp --ipv4
c,s,v
Superpacket {
Eth Header: src: 28:80:a2:0e:43:32, dst: 28:16:a8:64:08:c4, ether_type: 2048
IPv4Header: src_ip: <IP_ADDRESS>, dst_ip: <IP_ADDRESS>
TCPHeader: raw data not set
UDP header: srcprt: 56833, dstprt: 50887, len: 197, cksum: 16954
ICMPHeader:: raw data not set
Payload: length: 189
}
c,s,v
…

(Note: the --verbose option. And, e.g.: I used webrtc-handshakes/snowflake/windows_firefox_snowflake_389.pcap above.)

Confirmed – thanks :+1:


Clarify behavior for removing "container" labels

As per the title, can you please update the "Removing and deleting labels" section in the document to clarify what happens when you (try to) remove a label with active container assignments?

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 57908eea-077f-363c-8d51-3b8335914f4c
Version Independent ID: 0d1421f1-b546-677f-ef3b-7e3da294b5ef
Content: Create and publish sensitivity labels - Microsoft 365 Compliance
Content Source: microsoft-365/compliance/create-sensitivity-labels.md
Service: o365-seccomp
GitHub Login: @cabailey
Microsoft Alias: cabailey

Investigating

@michevnew  Apologies this has taken awhile because we investigated all scenarios for deleted labels (vs removed from policy).  It seems it can take SharePoint longer than our usual 24hrs to stop enforcing the settings from the now deleted label and this information is now added to the section: https://docs.microsoft.com/en-us/microsoft-365/compliance/create-sensitivity-labels?view=o365-worldwide#removing-and-deleting-labels

Thanks Carol.


Module '"dexie"' has no exported member 'KeyPaths',  Module '"dexie"' has no exported member 'TableProp'

I have installed dexie 3.2.0 and dexie-react-hooks 1.1.1 and I got this error. The project uses Typescript 4.3.5. I have tried to find KeyPaths and TableProp in the Dexie repository and I couldn't. It's strange) Where are these types from?

I've figured out that it's because dexie-react-hooks 1.1.1 isn't compatible with dexie 3.2.0. With dexie 3.2.0 you should use dexie-react-hooks 1.0.7. I think this information should be explicitly described in https://dexie.org/docs/libs/dexie-react-hooks page.


Original src before async/await babel transform

The source is hard to read with the babel async/await transform applied. Can you provide the source before this transformation? Thanks!

I also tried to use this code but it did not work out of the box for me. I converted the code back to es6 and replaced some duplicates with join-monster internals.
Gist.
I plan to put it into a repo with some integration tests at some point....

I only need keyset paging for now, so I attempted to implement that. Here's the gist.
I copied some code from the mariadb implementation. AFAIK the only reason it's not supported is because of the (a,b) > (1,2) syntax? That's silly because it's pretty trivial to unroll. Comes out like this:
SELECT
  `branchConn`.`id` AS `id`,
  `branchConn`.`startDate` AS `startDate`,
  `branchConn`.`baseUrl` AS `baseUrl`,
  `branchConn`.`browser` AS `browser`
FROM (
  SELECT `branchConn`.*
  FROM Branches `branchConn`
  WHERE (`branchConn`.`startDate` ><PHONE_NUMBER>760 OR (`branchConn`.`startDate` <=><PHONE_NUMBER>760 AND `branchConn`.`id` > X'0d9d964a4a205c94e7a087512237b5bc'))
  ORDER BY `branchConn`.`startDate` ASC, `branchConn`.`id` ASC
  LIMIT 4
) `branchConn`
ORDER BY `branchConn`.`startDate` ASC, `branchConn`.`id` ASC

Not certain why we need the nested query though. Maybe it matters when the query gets more complex...?

Hi @mnpenner I am still interested in improving this. I just had a basic pagination use case so I only got that working. Should I create a repo so we can do some collaboration/testing? I have seen some PR's getting merged in join-monster again recently so that would be my end goal. I was also a little wary of this dialect because the nested query is definitely not ideal but I am not sure how else to do it with mysql.
Ciaran

Hi all - I am very sorry I haven't been able to respond much to this original comment as i've been so busy with other work but i'm getting back to this now. Thank you for the enthusiasm in the project!
To answer the original question, that's my bad sorry. I was just lazy and copied a lot of the code from the original dialects but obviously took the transform code instead of the original. I can probably fix this but it sounds like @cliedeman already has so thank you!
The nested query approach is a requirement, i'm struggling to remember why but i've wondered this myself and I did figure it out at the time. It's not something I have done, it's already part of the join-monster project. I have since made some modifications which improve on this so when I get a moment i'll post them here.
Unfortunately I only needed a simple pagination use case myself so that's all I built into this. Proper keyset paging would be great if someone else wants to add it though?

@mposborne on the nested query: Its probably because the default implmentation uses a LATERAL JOIN which is not supported by mysql.

The problem I'm having now is that we can't override arrToConnection within the dialect, nor objToCursor. Also, can we even avoid the nested query within the context of handlePaginationAtRoot? It looks like we can only push additional tables into the join list.
This is my first attempt at "keyset" paging but it seems that some of the restrictions that JoinMonster imposes are unnecessary. It's baked into arrToConnection that after you page forward, you can no longer move backwards. objToCursor does not handle Buffers (UUIDs) nicely.
And then pagination in general requires that you perfectly conform (including nullability) to the Relay spec.
I think I'd have to fork the project to get it to behave the way I like.


nixUnstable: pre20210317_8a5203d -> pre20210324_d1cb956

Motivation for this change
To bring back nix flake info.

[ ] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)

[ ] NixOS
[ ] macOS
[ ] other Linux distributions

[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
[ ] Tested execution of all binary files (usually in ./result/bin/)
[ ] Determined the impact on package closure size (by running nix path-info -S before and after)
[ ] Ensured that relevant documentation is up to date
[x] Fits CONTRIBUTING.md.

/cc @edolstra

Warnings by @r-rmcgibbo are unrelated.

Warnings by @r-rmcgibbo are unrelated.

Even if they are unrelated they should be addressed. In this case it is just the patch comment warning which can be ignored at times.

Please wait until the openssl change is merged to merge this.

Superceded by https://github.com/NixOS/nixpkgs/pull/117758.


Feature/merge together aeron write squence and inner subscriber

Hi @ronenhamias ,
Could you please share you plans regarding reactor-aeron?


Autoprefixer command giving unexpected token error in sublime text 3

when try to run the autoprefixer from my sublime text 3 , getting the following error.
error: Autoprefixer
Error: /Users/Mohamed_Hussain_SH/Library/Application Support/Sublime Text 3/Packages/Autoprefixer/node_modules/get-stdin/index.js:2
const {stdin} = process;
^
SyntaxError: Unexpected token {
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:413:25)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/Mohamed_Hussain_SH/Library/Application Support/Sublime Text 3/Packages/Autoprefixer/autoprefixer.js:2:16)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)

![Screenshot 2020-02-26 at 17 01 36](https://user-images.githubusercontent.com/4039657/75347401-181df800-58ba-11ea-8c4b-bb4ee65f990e.png)

screenshot attached. thanks in advance for any help

You have a very old Node.js version. Upgrade that.

Thanks @sindresorhus  I am also suspecting that one since I start using nvm long time back. Is there a way to update the nodejs version that sublime to use

It works!!!  after I made the symliunk for the latest node bin which is in the path(usually in this path for mac os) to /usr/local/bin/ folder like below
ln -s /Users/Mohamed_Hussain_SH/.nvm/versions/node/v10.11.0/bin/node /usr/local/bin/node
trick is your node binary or its symlink should be in the /usr/local/bin/folder , It didnt work even it is in $PATH


The "multidispatcher" does not work with compiled classes, NotImplementedError: Could not find signature for init:

Hi, it looks like the multipledispatch library is not working for me
python -m nuitka --version
1.2.2
Commercial: None
Python: 3.10.8 (main, Nov 24 2022, 13:21:08) [GCC 10.2.1 20210110]
Flavor: Unknown
Executable: /usr/local/bin/python3.10
OS: Linux
Arch: x86_64
Distribution: Debian 11

Output of errror:
Traceback (most recent call last):
  File "/home/user/dispatch.dist/dispatch.py", line 14, in <module>
  File "/home/user/dispatch.dist/multipledispatch/dispatcher.py", line 433, in __call__
NotImplementedError: Could not find signature for __init__: <str>

How did you install Nuitka and Python
I used the following command to install nuitka:
python3.10 -m pip install nuitka

I downloaded and compiled python myself. Not using pyenv.
I have a very barebones debian setup.
Version of multipledispatch I'm using
multipledispatch==0.6.0

Reproducible code
Confirmed working in python as expected
# nuitka-project: --standalone
from multipledispatch import dispatch

class TestClass:
	@dispatch(str)
	def __init__(self, id: str):
		self.id = id

	@dispatch(str, int)
	def __init__(self, id: str, count: int):
		self.id = id
		self.count = count

ins1 = TestClass("abc")
ins2 = TestClass("xyz", 4)

print(ins1.id)
print(ins2.id, ins2.count)

Bonus points for using project options, the project happens in accelerated mode too. it just "hates" compiled methods it seems.

The critical code is this one, the prints are added by me:
    def _(func):
        name = func.__name__
        print(name, func, ismethod(func))

        if ismethod(func):

            dispatcher = inspect.currentframe().f_back.f_locals.get(
                name,
                MethodDispatcher(name),
            )

            print(inspect.currentframe().f_back.f_locals, inspect.currentframe().f_back.f_code.co_name)
        else:
            if name not in namespace:
                namespace[name] = Dispatcher(name)
            dispatcher = namespace[name]

        dispatcher.add(types, func)
        return dispatcher
    return _

The code relies on f_locals which for functions and classes is only filled in case of an exception and it being attached to a traceback in Nuitka. Output differs:
# Python:
__init__ <function TestClass.__init__ at 0x000001E8BCCFE050> True
{'__module__': '__main__', '__qualname__': 'TestClass'} TestClass
__init__ <function TestClass.__init__ at 0x000001E8BCCFE0E0> True
{'__module__': '__main__', '__qualname__': 'TestClass', '__init__': <dispatched __init__>} TestClass

vs.
# Nuitka:
__init__ <compiled_function TestClass.__init__ at 0x000001ECFCED2E30> True
{'__module__': '__main__'} TestClass
__init__ <compiled_function TestClass.__init__ at 0x000001ECFCED2B60> True
{'__module__': '__main__'} TestClass

I believe Nuitka is delaying the assignment into the locals variable dictionary until the "return" to the metaclass usage, where it has to be updated for sure. But it seems to exist, and I am not sure if it is just not assigned to the frame yet.
So, what is happening, is that multidispatcher tries to find pre-existing assignments of functions in the dictionary, and work with that. For enum and friends, I am pretty sure, we are not delaying assignments in all cases, but recognize if the class dictionary might be something with side effects. The decorator from multidispatch is kind of similar in that.

So, the frame gets created after the mapping that the class uses, already exists, and actually in this instance, the storage could simply be put there. Instead it creates immediately for every class frame, one that contains __module__ with the module name, as it ought to be, despite on the outside having it put into the dictionary too.
It seems, this is a missing optimization, to create class frames with a passed and pre-existing f_locals (or in case of it being cached), assigning it to there after creating it.
This might actually be relatively easy then, even if of course tedious.

So, I got this to work, lets see how much CI likes this. It touches code that I changed for 3.11 already, so I am not going to make it a hotfix.

I have resisted the temptation to do the "once vs. full cached" frame optimization, that I feel like it ough to be done too, looking at the code. Right now, the class frame continues to exist even after a class is executed, which of course only makes sense inside loops or functions that can get called multiple times. That is memory and code wasting. For module frames, we already don't do it that way. Classes were so far not really considered, only generators/coroutines/asyncgen have their own thing.
There might be some scalability on the C level to be gained by removing this unnecessary bloat code. For performance the caching is irrelevant, but memory usage is slightly too high (a frame object is not huge), and code complexity is also high, but for that to be benefitted from, we also shouldn't cache (actually pre-create) their code objects at module load time.

Thanks for your report, this is fixed on the factory branch, which is a development version under rapid development. You can try it out by going here: https://nuitka.net/doc/factory.html
Feedback if this is working is very welcome, just please do not share plans of doing it, but rather confirmations or denials of it working.

This is part of the latest pre-release  I just made.

This is part of the stable release 1.3 that I just made.


Question about release process

Hello! Recently you just merged my contribution pr
May I know, when this change will be published to pypi, so we can update the lib?
Thanks!

@galynazholtkevych new releases are generated when relevant features/hot-fixes were added to the project.
I plan to do a new release at the end of the month.

@pitbulk cool, looking forward to it! Thanks

Hey, are there any updates?


Status(StatusCode="Internal", Detail="Failed to deserialize response message.")

What version of gRPC and what language are you using?
C#
    <PackageReference Include="Google.Protobuf" Version="3.21.12" />
    <PackageReference Include="Grpc.Net.Client" Version="2.51.0" />
    <PackageReference Include="Grpc.Tools" Version="2.51.0">

What operating system (Linux, Windows,...) and version?
Ubuntu 22.10
What runtime / compiler are you using (e.g. python version or version of gcc)
What did you do?
Please provide either 1) A unit test for reproducing the bug or 2) Specific steps for us to follow to reproduce the bug. If there’s not enough information to debug the problem, gRPC team may close the issue at their discretion. You’re welcome to re-open the issue once you have a reproduction.
What did you expect to see?
No error
What did you see instead?
Status(StatusCode="Internal", Detail="Failed to deserialize response message.")
Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs).
See TROUBLESHOOTING.md for how to diagnose problems better.
Anything else we should know about your project / environment?

@khteh This needs to be asked in the grpc-dotnet repository as you are using Grpc.Net.Client: https://github.com/grpc/grpc-dotnet/issues
Also there isn't enough information to diagnose you problem. Please provide more details such as example code that reproduces your problem.


Setting a new VideoTexture doesn't unload a previous VideoTexture's soundtrack

The issue is that if you pick a different movie and start it, the soundtracks from previously-played movies also play, and pausing the new movie doesn't pause the old soundtracks.
see https://gitlab.com/dcl-tech/bugdemo-multiplemovies for a scene that demonstrates this issue
----- Steps to reproduce
Load the demo scene in SDK preview
Select a movie
Leftmost cube selects previous movie from catalog
Rightmost cube selects next movie catalog
There are 3 movies in the catalog
Press the 2nd cube from the left (Play)
(Play resets any currently-playing movie, loads ths selected movie into the screen, and plays it)
// movie plays
Select a different movie
Press Play (2nd cube)
// expected: original movie stops, new movie plays
// actual: original movie visual stops, new movie plays, but so does the sound from the original movie.
(The 3rd button is a pause button.)
Press Pause when a subsequent mnovie is playing
// expected: vidoe and sound stop
// actual: sound from previously played movies continues playing

see https://gitlab.com/dcl-tech/bugdemo-multiplemovies fpr a scene that demonstrates this issue

I have determined that if I use videotexture.playing=false instead of videotexture.reset() both the video and audio are freed up to load and start a different video without hearing the old audio.
I recommend that whatever you are doing about audio during videotextureplaying=false be done in reset()
Perhaps your reset just needs to call playing=false.
In the meantime, i'm going to simply use playing=false instead of reset().
Not sure what reset() is supposed to do, but it seems it is missing something.


ARROW-9129: [Python][JPype] Remove JPype version check

Because we only run the test with the latest JPype.
Error details:
https://github.com/ursa-labs/crossbow/runs/769433714#step:6:7995
>       if jpype.__version_info__ >= (0, 7):
E       TypeError: '>=' not supported between instances of 'list' and 'tuple'

@github-actions crossbow submit test-conda-python-3.8-jpype

+1
The error has gone but new errors are appeared:
https://github.com/ursa-labs/crossbow/runs/770616977?check_suite_focus=true#step:6:10042
>       return om.readValue(jvm_spec, pojo_Field)
E       pyarrow.tests.test_jvm.com.fasterxml.jackson.databind.exc.MismatchedInputException: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize instance of `java.util.ArrayList` out of START_OBJECT token
E        at [Source: (String)"{"name": "field_name", "nullable": true, "type": {"name": "timestamp", "unit": "NANOSECOND", "timezone": "Europe/Paris"}, "children": [], "metadata": {"field meta": "field data"}}"; line: 1, column: 151] (through reference chain: org.apache.arrow.vector.types.pojo.Field["metadata"])

The new errors are out of scope of this pull request.

@xhochy Could you take a look the https://github.com/apache/arrow/pull/7433#issuecomment-643824491 error?


Port Mainnet Rosetta to dev

Port https://github.com/MinaProtocol/mina/pull/16156 to develop

!ci-build-me

!ci-build-me

!ci-nightly-me

!ci-build-me

!ci-build-me


Cannot configure OIDC with self-managed MongoDB

Bug description
My configuration file:
setParameter:
   authenticationMechanisms: MONGODB-OIDC
   oidcIdentityProviders:
     - issuer: "http://localhost:9000/.well-known/openid-configuration"
       clientId: "your-client-id"
       clientSecret: "your-client-secret"
       authorizationEndpoint: "https://your-identity-provider.com/authorize"
       tokenEndpoint: "https://your-identity-provider.com/token"
       userInfoEndpoint: "https://your-identity-provider.com/userinfo"

When I run mongod I get:
setParameter has a map with non scalar values, which is not allowed
Additionally, when I try to pass this via command line:
./mongod --config "../config/mongod.conf" --setParameter 'oidcIdentityProviders=[ {"issuer": "https://example.com", "clientId": "yourClientId", "clientSecret": "yourClientSecret"} ]'
I get:
{"t":{"$date":"2024-09-11T11:40:46.072Z"},"s":"F",  "c":"CONTROL",  "id":20574,   "ctx":"thread1","msg":"Error during global initialization","attr":{"error":{"code":2,"codeName":"BadValue","errmsg":"Unknown --setParameter 'oidcIdentityProviders'"}}}
Something's wrong at my end or it's a bug?
How to reproduce

Configure as in description
Run mongod

Expected behavior
Mongod starts successfully
Prisma information

// Add your schema.prisma

// Add your code using Prisma Client

Environment & setup

OS: macOS
Database: MongoDB Enterprise

Prisma Version

Hi, as far as I can see, nothing in what you posted above is linked to issues in Prisma.
Both errors pasted above, setParameter has a map with non scalar values, which is not allowed and Error during global initialization, are emitted by mongod.
Please open a new thread in https://www.mongodb.com/community/forums/, StackOverflow, or similar general-purpose platforms. Thanks!


CLDC-836: Don't make subsection labels links if they're not enabled

After:

This is great, but looking at the HTML, there are a few other issues we should address.
Here’s the current markup:

  About this log

    Not started

id should be on the tag
id should be kebab-case
a should be wrapped by a span with class app-task-list__task-name
a should have aria-describedby attribute with name of the id on the tag

    About this log

    Not started

See: https://govuk-prototype-kit.herokuapp.com/docs/templates/task-list


Communication does not work on EKS with AWS VPC CNI and Cilium deployed in chaining mode for the pods running on the same node when SecuirtyGroups For Pods is activated and applied to one of the communicating pods

Is there an existing issue for this?

[X] I have searched the existing issues

What happened?
On EKS cluster with Managed Node Groups,
Deployment

Activate Security Groups For Pods (SGFP)  following the tutorial described at at https://docs.aws.amazon.com/eks/latest/userguide/security-groups-for-pods.html#security-groups-pods-deployment.
AWS CNI Daemonset is configured as desribed in https://docs.cilium.io/en/v1.13/installation/cni-chaining-aws-cni/#enabling-security-groups-for-pods-eks with the following ENV variables relevant to SGFP  :

# for init  containers
DISABLE_TCP_EARLY_DEMUX=true

#for main container
ENABLE_POD_ENI = true
POD_SECURITY_GROUP_ENFORCING_MODE=standard
AWS_VPC_K8S_CNI_EXTERNALSNAT=true
ENABLE_PREFIX_DELEGATION=false

Install Cilium helm chart in VPC CNI chaining mode with the following values as decribed in documentation at https://docs.cilium.io/en/v1.13/installation/cni-chaining-aws-cni/#chaining-aws-cni:

cni:
  chainingMode: aws-cni
  exclusive: false

enableIPv4Masquerade: false
tunnel: disabled

endpointRoutes:
  enabled: true

Apply the taints configuration to the EKS Managed Nodegroups  to ensure applications pods are properly managed by Cilium:  tainted with node.cilium.io/agent-not-ready=true:NoExecute to ensure application pods will only be scheduled once Cilium is ready to manage them.

Run cilium connectivity test  - it passes successfully

Problem
Scenario:

Pods are running on the same EKS worker node.
Client application  is running as K8s pod with Security Group For Pods  (SGFP) attached based on the SecurityGroupPolicy definition. Attachment is applied via ServiceAccount selector matching the SA used for the pod. Problem is reproduced regardless of whether the Pod has only dedicated SecurityGroup attached and no EKS Worker Node SecurityGroup attached or both (e.g, it uses branch ENI)
Service application is also running as K8s pod on the same node and does not have matching SecurityGroupPolicy. As a result, it gets regular EKS Worker Node SecurityGroup applied (e.g, it shares the ENI from the node).
Cilium pod on the node is started before or approximately at the same time as AWS vpc-cni pod. This is mostly relevant to the newly created MNG worker nodes based on the cluster scaling events
The regular application workloads are assigned to this node later when cilium clears the taint from the node.
Problem is happening for the pods running on the same node when one of them is using regular ENI and other one is using branch ENI.
Pod with SGFP is able to communicate with the pods  running on other Worker nodes regardless of whether it has matching CiliumNetworkPolicy or not.
If Cilium network policy is applied to the pod with SGFP attached, connectivity to the pods on other nodes works with both L4 and L7 type of rules as well as for DNS based rules but does not work for the pod from the same deployment or workload running on the same node
Problem is fixed if the pods are restarted in order:  AWS VPC-CNI ==> cilium ==> all applicative pods

Observations:

If the Service pod gets the SecuirtyGroup attached (e.g, it also starts to use branch ENI) and if the rules on those SGPs both at client (egress) and service (ingress) allow the traffic, the connectivity works.
If we restart the aws vpc-cni and cilium pods in correct order (first aws, wait for it to be ready, than cilium) and later restart all pods that were scheduled  to that node and they are again runnning on it, the connectivity works.
Problem happens regardless of whether one or both of the pods have Cilium Network Policy applicable to them.
Setting the value endpointRoutes.enabled=false fixes problem when using L4-based policies but completely breaks L7 policies.

Cilium Version
1.13.1
Kernel Version
5.10.184-175.731.amzn2.x86_64
Kubernetes Version
Runtime details:

EKS  ControlPlane version: v1.24.15-eks-a5565ad
EKS WorkerNode AMI version: 1.24.10-20230304
Kubelet version: v1.24.10-eks-48e63af
Container runtime: containerd://1.6.6, containerd://1.6.19
AWS CNI Version:  1.11.4-eksbuild.1, v1.12.6-eksbuild.1

Sysdump
cilium-sysdump-20230729-234124.zip
Relevant log output
No response
Anything else?
The following Github issues looks somewhat relevant but not exactly the same:

https://github.com/cilium/cilium/issues/20677
https://github.com/cilium/cilium/issues/24943

Code of Conduct

[X] I agree to follow this project's Code of Conduct

From some investigation endpointRoutes.enabled=true isn't quite compatible with strict security group enforcing of the AWS VPC CNI. For SG attached Pods, traffic is supposed to leave the node and enter the VPC network. The VPC CNI will intentionally elide the creation of routes to such Pods in the main route table. In reloading endpoint datapaths, Cilium will note. This leads to asymmetric routing between SG attached pods and non-SG attached pods, and dropped traffic if strict reverse path filtering is enabled.
Could a potential solution be to allow a configuration option for DatapathConfiguration.InstallEndpointRoute? This could be explicitly set to false rather than the true implied by setting endpointRoutes.enabled=true.


docker gid can be different to host

Hi,
While I've had this working ok, a recent reboot of my host ended with the host having a different gid for the docker group (233) than that created in the container (999), so I had to resort to chgrp 999 /var/run/docker.sock on my host so that Jenkins has permission to access the socket.
I've tried using the 'group_add: docker' option in my docker-compose file, but this doesn't seem to change the gid of the docker group in the running container. Maybe because the group is already created in the container?
How do others cope with this?
Alex.

I was under the impression that groups and everything would already be setup inside the container. However, I have yet to successfully complete a docker job with this container. Maybe if we were able to pass the gid on the host to the container as an argument?

Hi guys,
Truly sorry for late response. I've had no time to investigate it at the start and then forget about it.
The fact that the GID of the docker group changed after the restart seems pretty weird to me. Once the group/user is created in the system, they are persistent and their GID/UID shall not change after rebooting.
Different docker group IDs shall be solvable either via the --group-add docker parameter when using the run command or adding group_add: \n - docker to the docker-compose. Just make sure your docker socket is set to docker group ls -la /var/run/docker.sock.
Please, take a look at the https://github.com/jenkinsci/docker/issues/263. This could add you some info.

I was having this issue on a BusyBox based VM. A BusyBox VM's docker GID is something like 99, but your Docker image's GID is 999. I haven't figured out a way to change this at build time or run time, and docker builds won't complete without access to the docker socket. My next step is to move from a BusyBox based VM to a Debian based VM. Maybe that will help? 🤷♂