Tentative fix for crash on host computer when socket cannot be connected.

While trying to compile and run for desktop (on Linux Mint), the following line crashed:
FD_SET(gPigNotify, &fds);

Looks like it is due to gPigNotify being -1 since this was on a host computer, not on the Raspi itself. Same crash occurs if the hostname or IP given isn't found:
PIGPIO_ADDR=the-pie-is-a-lie ./piscope

I don't understand how the code is getting that far.  Mine fails with

Do you have any idea why yours isn't doing the same?

I'm not too familiar with GTK, but I'd suspect thread or event loop race condition.
My laptop is running 4 cores, which "helps" :smile:  uncovering threading issues.
I could not quite make heads nor tails of the backtrace. When running it in GDB, this is what I got:
Reading symbols from ./piscope...(no debugging symbols found)...done.
(gdb) run
Starting program: /home/work/rbdc/rubendahl/Rubendahl/wdir/github/pigpio/piscope/piscope
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7fffef03f700 (LWP 21837)]
*** buffer overflow detected ***: /home/work/rbdc/rubendahl/Rubendahl/wdir/github/pigpio/piscope/piscope terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x7338f)[0x7ffff664e38f]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7ffff66e5c9c]
/lib/x86_64-linux-gnu/libc.so.6(+0x109b60)[0x7ffff66e4b60]
/lib/x86_64-linux-gnu/libc.so.6(+0x10abe7)[0x7ffff66e5be7]
/home/work/rbdc/rubendahl/Rubendahl/wdir/github/pigpio/piscope/piscope[0x4050ab]
/lib/x86_64-linux-gnu/libglib-2.0.so.0(+0x49703)[0x7ffff6c07703]
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_context_dispatch+0x135)[0x7ffff6c06ce5]
/lib/x86_64-linux-gnu/libglib-2.0.so.0(+0x49048)[0x7ffff6c07048]
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_loop_run+0x6a)[0x7ffff6c0730a]
/usr/lib/x86_64-linux-gnu/libgtk-3.so.0(gtk_dialog_run+0x160)[0x7ffff77fa8e0]
/home/work/rbdc/rubendahl/Rubendahl/wdir/github/pigpio/piscope/piscope[0x4043ce]
/home/work/rbdc/rubendahl/Rubendahl/wdir/github/pigpio/piscope/piscope[0x405b96]
/home/work/rbdc/rubendahl/Rubendahl/wdir/github/pigpio/piscope/piscope(main+0x8a5)[0x4040c5]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7ffff65fcec5]
/home/work/rbdc/rubendahl/Rubendahl/wdir/github/pigpio/piscope/piscope[0x404116]

[snip huge memory map output]
Program received signal SIGABRT, Aborted.
0x00007ffff6611cc9 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56	../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb)
(gdb) bt
#0  0x00007ffff6611cc9 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff66150d8 in __GI_abort () at abort.c:89
#2  0x00007ffff664e394 in __libc_message (do_abort=do_abort@entry=2,
    fmt=fmt@entry=0x7ffff675a52b "*** %s ***: %s terminated\n") at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007ffff66e5c9c in __GI___fortify_fail (msg=<optimized out>,
    msg@entry=0x7ffff675a4c2 "buffer overflow detected") at fortify_fail.c:37
#4  0x00007ffff66e4b60 in __GI___chk_fail () at chk_fail.c:28
#5  0x00007ffff66e5be7 in __fdelt_chk (d=<optimized out>) at fdelt_chk.c:25
#6  0x00000000004050ab in main_util_input ()
#7  0x00007ffff6c07703 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#8  0x00007ffff6c06ce5 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#9  0x00007ffff6c07048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00007ffff6c0730a in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 0x00007ffff77fa8e0 in gtk_dialog_run () from /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
#12 0x00000000004043ce in util_popupMessage ()
#13 0x0000000000405b96 in pigpioConnect.part.7 ()
#14 0x00000000004040c5 in main ()

Hardware info for my laptop for reference (from inxi -Fi output):
System:    Kernel: 3.19.0-32-generic x86_64 (64 bit gcc: 4.8.2)
           Desktop: Cinnamon 2.8.8 (Gtk 3.10.8) Distro: Linux Mint 17.3 Rosa
Machine:   System: Micro-Star product: GS70 2OD v: REV:1.0
           Mobo: Micro-Star model: MS-1771 v: REV:0.B Bios: American Megatrends v: E1771IMS.70D date: 11/20/2013
CPU:       Quad core Intel Core i7-4700HQ (-HT-MCP-) cache: 6144 KB
           flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 19156
           clock speeds: max: 3400 MHz 1: 2507 MHz 2: 2480 MHz 3: 2493 MHz 4: 2400 MHz 5: 2400 MHz 6: 2473 MHz
           7: 3058 MHz 8: 2400 MHz

I'd going to leave this pending for a while to see if anyone else has experienced the problem.  I've never seen the problem on my 2 core laptop.
If it is indeed a threading problem it will probably be best to synchronise the threads.
I'm afraid my knowledge of GTK is pretty limited as well.  When I use top and the H command the piscope process seems to be single threaded.

I had this happen to me today and this fixed the problem.
I did not have the pigpiod running on the remote pi and it gave me buffer overflow instead of erroring in gui.


🐛 fix OS and ARCH not being properly initialized

Summary
as per title.
Related issue(s)

/ok-to-test


Removal of email field has broken engine 1.11.0↔docker-compose 1.5 pulling private images

The removal of email prompt in docker login (and therefore the lack of "email" field in .docker/config.json) is causing docker-compose 1.5 (released on Dec 3, 2015) to fail pulling images from private repositories (incl. Docker Hub) with "image [...] not found" error during compose up against docker-engine v1.11.0.
Here is the error:
$ /usr/local/bin/docker-compose -p compose -f /etc/docker/compose/docker-compose.yml up -d
Pulling privaterepo (privateuser/privaterepo:v1.1)...
Error: image privateuser/privaterepo not found

More details at https://github.com/Azure/azure-docker-extension/issues/80#issuecomment-210176464
We fixed the issue by upgrading to docker-compose 1.6.2 however it is concerning that a product releases less than 5 months ago is not supported... Any comments? Perhaps the email field can be kept (as empty string) for backwards-compatibility reasons?

Sorry @ahmetalpbalkan, I discussed this with @dnephin (docker compose team), and he mentioned that this actually is a bug in docker compose 1.5, and should be fixed in in 1.6.2 and later. We decided to not make changes to the engine side


Update Float forwarding for MSRV 1.31

Core floats got more methods since these were originally written.

bors r+


Issue #2252. feature request: See who reacted with an emoji.

Issue #2252.
Discussion: https://chat.zulip.org/#narrow/stream/48-mobile/topic/reaction.20recipients

Referencing #3092.

That's a lot of 'WIP's ;)
I'll give you a quick first review then 👍

Thanks @jainkuniya for building this! And for the screenshots :smiley:
Quick feedback on the UI in the screenshots: let's sort by number of people reacting, most people first.
Also I think the list of names as text gets pretty hard to scan at even 3-4 people -- especially with the paragraph-style wrapping. I'd rather have something more structured, and with the users' avatars.
Two ideas for how to do that:

Facebook's UI for reactions on a post (FB proper, not Messenger, though maybe the latter is the same). I'm not immediately finding a handy screenshot of this on the web, but I'll take another couple of minutes shortly to look harder or make one. This UI works quite well in my experience.
An alternative (but here I'm making up something I haven't seen a concrete implementation of) would be a vertical list like this, but each item is a horizontal list of just avatars (like Facebook does in lots of other contexts, I guess.) And when you tap an item to expand, it turns into a vertical list of avatar + name. This idea would need more iterating on, I think.

I haven't looked closely through the code at this stage, since you have a number of WIP commits and also an early review from Boris :slightly_smiling_face:

Facebook's UI for reactions on a post

Ah here we are. This image looks to be desktop web, but the layout in the mobile app is very similar (to the window in the foreground):

Also it's from 2016, but looks familiar from when I've seen this UI recently.
Those are tabs at the top: "All", then each reaction in turn, sorted by number of people. I'd be happy even without the "All" tab. Each tab has a vertical list of people, with avatar + name.

Oh, and a question about the UI (that's hard to show in a screenshot): how does the user get to this screen?
I like the suggestion of long-pressing on a reaction.

I am seeing some WIP commits. Let me know when you think the work is ready for a review and I'll do both manual testing and code review. ;)

@jainkuniya ping me here and in chat when you think it is no longer WIP. 👍

Started testing this...
First issue: users' avatars are missing!

Apparently this line:
message: getMessageById(state, props.navigation.state.params.messageId),

might return undefined which throws an exception.

I had the app crash once on clicking the second emoji reaction tab!
Not sure how that happened.

ExceptionsManager.js:84 Warning: Encountered two children with the same key, -1. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version

This happens in ReactionUserList and might be related to the missing avatars.

I think the action to show it "long-press on empty space" is very undiscoverable.
A better alternative to that would be adding a circle, at the end of the list of reactions with an i inside of them (the same styling as #scroll-bottom but smaller radius). On pressing of it, you show the details.

While I did a lot of comments on this PR, I think we are getting close.
If you manage to address the issues before the meeting, ping me so I can have another look.

Thanks @jainkuniya !
I'm going to focus in this review on just the first few commits, so we can get those merged and then focus on the rest. Specifically these four:
d9d86cf3e flow: Fix type of reaction in Messsage.
235ffaebd flow, groupItemsById: Make function param type more precise.
026e0ad9c messagesReducer: Make sure reaction in msg always have `user.user_id`.
# f3e1d6cd3 webView: Use `.closest` instead of `.matches` in `getTouchTargetType`.
4c3453179 aggregatedReactions: Include reacted users list.

(Ignoring the webview .closest one for now, just because it isn't related to the other four.)
d9d86cf3e flow: Fix type of reaction in Messsage.

Thanks for the revisions on this!

One thing I mentioned last time still applies:

Another thought I have looking at this commit is: surely either the version just before this change or just after it is a type error. If they both pass Flow, then our types somewhere aren't being effective, and we should figure out why. :slightly_smiling_face: If one of them doesn't, then either the previous commit or this one broke things.

Can you identify where the type error is? That'd help us make sure we take care of whatever the problem was that caused our types not to be effective in catching the issue.

235ffaebd flow, groupItemsById: Make function param type more precise.

Huh interesting. It kind of confuses me that this is necessary (Flow is already inferring what type T is -- why doesn't it infer this detail for itself?), but so be it.

commit 026e0ad9c

    messagesReducer: Make sure reaction in msg always have `user.user_id`.

    User object in reaction of message has `id` key in it, but reaction
    event has `user_id` in it. So copy `user_id` to `id`, and then we can
    always use `id`.

    In future we will be migrating to `user_id`, so on receiving messages
    from server migrate to new type that is `user_id`.

    Also added new migration (`5`). So that old stored messages also get
    migrated.

Huh, I'm a bit confused -- we're going to copy to id, but also migrate to the type that has user_id?
Those sound like going in two opposite directions. The latter is the one I'm hoping to see, though.

Reading through the code: this has the effect of making a lot of our application code need to worry about legacy vs non-legacy messages, and call migrateMessages in various spots.
The way I'd like to arrange this would be: none of the code outside of api/, and I guess a store migration, knows that "legacy" messages ever existed. In other words: everything on the inside, everything that isn't directly interacting with the messy outside world of version skew, deals with data in nice consistent types.
It's OK that the commit that introduces the legacy types causes the word "legacy" to appear in some of the app code (outside of api/), because that's just putting a more accurate name on what was already the truth. But what I'd like to avoid, even in the middle of the branch, is having actual conversion logic show up in the app code.

Specifically, this line worries me:

       this.setState({
-        messages,
+        messages: migrateMessages(messages),

Can you arrange things so that migrateMessages only appears in src/api/, and the store migration?

Also: looking at migrateMessages, it looks like it won't work right when the server does start sending the preferred format. Let's fix that.

Also, migrateMessages is the kind of function that would benefit greatly from having its own unit tests. :wink:

4c3453179 aggregatedReactions: Include reacted users list.

This previous comment applies to this code:

+        const reactedUser: User = (users && users.get(x.user.user_id)) || {
+          ...NULL_USER,
+          ...x.user,
+        };

Yikes -- what's all this doing?
The baseline here is to simply use x.user. Anything more complex than that, there needs to be a clear reason for.
[...]
 Pretty much by definition, if we ever actually touch any of that data, we have a bug -- so if there's some other code that complains or fails when we leave it out, we should fix that code.

Can you identify where the type error is? That'd help us make sure we take care of whatever the problem was that caused our types not to be effective in catching the issue.

We have pre assumed that it will always be user_id, in both the cases. And we are using same Reaction in both the places i.e in Message and in reaction action. So there is no flow error.

Huh interesting. It kind of confuses me that this is necessary (Flow is already inferring what type T is -- why doesn't it infer this detail for itself?), but so be it.

Without this commit, flow gives error:
Running flow...
Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/boot/store.js:99:32

Cannot call groupItemsById with migrateMessages(...) bound to items because property id is read-only in
object type [1] but writable in object type [2] in array element.

     src/boot/store.js
      96│     const { messages } = state; // migrate message from LegacyMessage to Message (LegacyReaction -> Reaction)
      97│     return {
      98│       ...state,
      99│       messages: groupItemsById(migrateMessages(Object.keys(messages).map(id => messages[id]))),
     100│     };
     101│   },
     102│ };

     src/message/messagesReducers.js
 [1]  20│ export const migrateMessages = (messages: LegacyMessage[]): Message[] =>

     src/utils/misc.js
 [2]  24│ export function groupItemsById<T: { id: number }>(items: T[]): { [id: number]: T } {

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/message/messagesReducers.js:136:27

Cannot call groupItemsById with migrateMessages(...) bound to items because property id is read-only in
object type [1] but writable in object type [2] in array element.

     src/message/messagesReducers.js
 [1]  20│ export const migrateMessages = (messages: LegacyMessage[]): Message[] =>
        :
     133│     case MESSAGE_FETCH_COMPLETE:
     134│       return {
     135│         ...state,
     136│         ...groupItemsById(migrateMessages(action.messages)),
     137│       };
     138│
     139│     case EVENT_REACTION_ADD:

     src/utils/misc.js
 [2]  24│ export function groupItemsById<T: { id: number }>(items: T[]): { [id: number]: T } {

Huh, I'm a bit confused -- we're going to copy to id, but also migrate to the type that has user_id?

Sorry my mistake. It was supposed to be:
So copy `id` to `User_id`, and then we can
    always use `user_id`.

Now migrateMessages is only limited to src/api.

Also: looking at migrateMessages, it looks like it won't work right when the server does start sending the preferred format. Let's fix that.

Now I have added a condition to handle it :)

Added tests for migrateMessages

Removed the usage of NULL_USER

Thanks @jainkuniya !
Again focusing on those 4 early commits.
f49bf1fee flow: Fix type of reaction in Messsage.

You mention a migration, and link to a chat thread. But that conversation doesn't seem to agree with the new Reaction type in this diff. Here's what Steve said there:

In this specific case I would just want to send user_id and no email or full_name, ideally.

Did any version of this migration end up happening on the server?

I asked:

Another thought I have looking at this commit is: surely either the version just before this change or just after it is a type error. If they both pass Flow, then our types somewhere aren't being effective, and we should figure out why. :slightly_smiling_face: If one of them doesn't, then either the previous commit or this one broke things.
Can you identify where the type error is?

You offered an answer (thanks!):

We have pre assumed that it will always be user_id, in both the cases. And we are using same Reaction in both the places i.e in Message and in reaction action. So there is no flow error.

But it didn't really address what I was wondering about, so I guess the question wasn't clear.
So, I checked for myself. The answer seems to be:

First, Flow does indeed pass both just before this commit (good, because that's master!), and just after.
OK, so apparently our types before this commit are just wrong; and Flow says our code agrees with the types. So, why hasn't that caused problems already? E.g., crashing because we look for a property that isn't there.
It turns out we weren't using this property at all, anywhere. So we never bumped into the fact that we'd written down the wrong (in some contexts) name for it.
Two ways to see that efficiently, using Flow:

Comment out the property, so the type says it isn't there at all. Run Flow -- no error. If we'd had any code looking for this user_id, it would have shown up here as an error.
Make the entire user property have the type mixed, which means "could be anything, can't make any assumptions".

Run Flow, and there are 3 errors. These are all the places we try to look inside the user property on a reaction.
Read the code shown for each error. In each case, the only part we look at is the user.email.

759d8f102 flow, groupItemsById: Make function param type more precise.

Ah, I see now. I think the point is that this isn't really more precise -- it's actually more general. In particular, we're allowing it to have a property +id: number, not only a property id: number.

First off: this notation $ReadOnly<{ id: number }> is exactly equivalent to { +id: number }. Let's prefer the latter here -- I think it deflates some of the mystery of what $ReadOnly means.

Then: I say { id: number } is a subtype of { +id: number }. Why? If you have a { id: number }, you can pass it to something that takes a { +id: number } -- after all, the thing is just giving you an extra promise that it won't write to the object's id. But if you have a { +id: number }, then you made a promise not to write to the object's id -- so you can't pass it to something that takes { id: number }, because that other code might write to it and break the promise you made.

So, we're changing the bound on this parameter from { id: number } to a supertype, { +id: number }.

That makes it no longer mysterious why we need the change -- it's because we're going to use the function in a new place that needs the more general interface.

b234cfacd messagesReducer: Make sure reaction in msg always have user.user_id.

Thanks for the updates!

Can we avoid splitting up the definition of Message? (Splitting it makes it more complicated to read.) And instead write

export type LegacyMessage = $ReadOnly<{|
  ...$Exact<Message>,
  reactions: $ReadOnlyArray<LegacyReaction>,
|}>;

just overriding the property from plain Message.
192463070 aggregatedReactions: Include reacted users list.

Much improved, thanks! :smile:

This code can be simplified:

+        const reactedUser: User | ReactionUser = (users && users.get(x.user.user_id)) || {
+          ...x.user,
+        };

to just:
        const reactedUser: ReactionUser = x.user;

(And then, just inline that where reactedUser is used.)
That'd let some types be simplified downstream, too.

Did any version of this migration end up happening on the server?

This is how we get in message fetch call:

reactions: Array(33)
0:
  emoji_code: "1f476"
  emoji_name: "baby"
  reaction_type: "unicode_emoji"
  user:
    email<EMAIL_ADDRESS>    full_name: "****"
    id: 1057
    __proto__: Object
__proto__: Object

And this is how we get in reaction add event:
user:
  email<EMAIL_ADDRESS>  full_name: "****"
  user_id: 161
  __proto__: Object

So, seems none of the migration happened.

But that conversation doesn't seem to agree with the new Reaction type in this diff.

In the conversation, focus was on discussing between user_id and id. So I migrated considering that only. BTW if we just have user_id, then we should have complete list of users all the time. I mean its size is too large in large realms and and we cannot show reactionList screen until we have full list.
So should we migrate to only user_id? I don't think we should, your thoughts?

it's because we're going to use the function in a new place that needs the more general interface.

Yup, making it generic so that it can be used in other places a well.

just overriding the property from plain Message.

Done 👍

(And then, just inline that where reactedUser is used.)

Done 👍

b0f1f69c1 flow: Fix type of reaction in Messsage.

Hmm. Your comments are predicting that a certain migration will happen. But nobody in that thread suggested that we might do that particular migration :wink:, and there's no sign of it happening. So I don't think that prediction is a good one -- certainly not as it stands.
Steve's first suggestion there was that we just handle it as it currently is:

Unfortunately, I think it's a bit difficult to fix this in a backward compatible way, so I recommend for the proximal issue that you just deal with it (and perhaps try to encapsulate it with a helper function of some sort, or just immediately mutate it at the "edge").

Tim then suggested that we (1) choose something to migrate to, then (2) do that:

decide which we prefer, and then start sending both; this would set us up to migrate mobile + deprecate sending both in several months.

To which Steve replied that his preferred answer to what to migrate to was plain user_id:

In this specific case I would just want to send user_id and no email or full_name, ideally.

So if I'd predict any migration based on this thread, it would be to that.

I think it's best if we try to get this PR moving without worrying about a migration -- I don't think a migration of this is a priority for either us or the backend folks. So let's take Steve's original suggestion: let's just convert the .user.id form to the .user.user_id form at the very edge. Which means as we get each message from the server.

163963049 flow, groupItemsById: Make function param type more generic.

Cool, I'll go and merge this. I'll want to add a bit to the commit message, because it's pretty mysterious as it stands.

cb8763aa9 messagesReducer: Make sure reaction in msg always have user.user_id.

This appeared in your newest revision:

+++ src/reduxTypes.js
+export type LegacyMessagesState = {|
+  [id: number]: $Exact<LegacyMessage>,
+|};

That would mean "legacy" types spreading within our internal application code. We don't want to do that.
Fortunately it doesn't look like you use this, so can just take it back out.
e08a867a0 aggregatedReactions: Include reacted users list.

Hmm, I think you misunderstood me :wink:
The point was that we don't need all that multi-conditional goo with the users &&! We can just write users: [...prevReaction.users, x.user].

(And merged that one commit, as 0b8920aea .)

just convert the .user.id form to the .user.user_id form at the very edge.

That is what we doing right now, right? I have written migration, so that older message will also have user_id instead of id. I guess I am not getting what you mean to say here. :)

Fortunately it doesn't look like you use this, so can just take it back out.

Done 👍

Hmm, I think you misunderstood me 😉

Yup! :)
Do you mean to always use x.user, means always take user from reaction object?
export type AggregatedReaction = {|
  code: string,
  count: number,
  name: string,
  selfReacted: boolean,
  users: $ReadOnlyArray<User | ReactionUser>,
  type: string,
|};

I have made it User | ReactionUser. So if user is deactivated, we can get his name from reaction object user and show it. By looking up first in users will make sure that if someone has changed his name, we show his updated name.

So if user is deactivated, we can get his name from reaction object user and show it. By looking up first in users will make sure that if someone has changed his name, we show his updated name.

Pretty sure the info in the reaction object reflects the server just looking up that user's details at the time it's giving us the data -- so if they've changed their name etc., it's already reflected there.
Or do you mean if they've changed it since we got the reaction from the server? That's a good point... and really it highlights the problem with keeping all this data denormalized, scattered in copies across lots of miscellaneous objects. :stuck_out_tongue: We shouldn't keep around data that we're not going to keep up to date.
(The principle is very much like with the bug where we used to keep a flags property on the messages under state.messages but let it get stale didn't update that, and so flags.starred would be stale -- the cause of #2676 and #2677, and explained in https://github.com/zulip/zulip-mobile/pull/2843#issuecomment-409509922 .)
Hmm. I think in fact what that really means is that (a) we should always use the user ID here, and look it up in the users data -- never get the other details from the reaction object; (b) as a further cleanup, when we get a reaction object from the server we should completely throw away the user object inside it, and just keep a user_id property!
(Earlier you alluded to a possible future where we don't keep a comprehensive user list all the time. In that future, we might add some logic like: (b') when we get a reaction from the server containing a user object, we go and store that in our user list... as part of maintaining an invariant that we have a comprehensive list of users that we have other data referring to, like they sent a message. But we don't need to worry about anything like that now, because at present we do just keep a comprehensive list of users in the realm.)

(continuing after the call here, conclusion of the call was to find what webapp does for deactivated users and do same in the mobile app )
Just tested on webapp, others users can see deactivated users messages, reactions as it  is without any difference.
Deactivated status is visible in user profile and at the time of sending PM.

Thanks! That's a helpful finding on deactivated users.
I just filed #3551 with the things you found we don't do that we probably should. Not a priority, though, and we have plenty of other things to focus on -- so let's focus for now on the things we're already doing.
I also just added some jsdoc, in eeb0c3859 (and its parent). I hope that helps clarify when to include deactivated users and when not to -- please take a look and ask any follow-up questions.
Coming back to emoji reactions: I think the answer this gives us is that we simply don't have to worry or care about whether a user is deactivated or not. Just be sure to use getAllUsers and friends, not getUsers* or getActiveUsers*.
And then that gets us back to this:

I think in fact what that really means is that (a) we should always use the user ID here, and look it up in the users data -- never get the other details from the reaction object; (b) as a further cleanup, when we get a reaction object from the server we should completely throw away the user object inside it, and just keep a user_id property!

So I think that's the next step for this PR.

Got an error playing with this in dev:

That was while looking at the reaction screen, and adding and removing reactions from another user (from the webapp)

That was while looking at the reaction screen, and adding and removing reactions from another user (from the webapp)

Mmm, I think specifically I'd removed the reaction that was the one I'd navigated to. I.e. the other user had reacted with slight_smile; I'd long-pressed that reaction to get to this screen; then the other user removed that reaction.

Hmm also this is puzzling:

They aren't sorted there by count. That's while they are indeed ordered that way in the message list:

Mmm, I think specifically I'd removed the reaction that was the one I'd navigated to. I.e. the other user had reacted with slight_smile; I'd long-pressed that reaction to get to this screen; then the other user removed that reaction.

Interesting situation 😅

I tried with multiple users, reacting with reactions even when user is on reactions list. It is getting re rerendered correctly.

I tried with multiple users, reacting with reactions even when user is on reactions list. It is getting rerendered correctly.

OK, thanks. It seems like whatever I saw is uncommon, and it isn't critical anyway; so I'll merge without worrying about it. If we see it later, we can handle it as a bug.
I'm about to merge most of this branch. We're not yet quite done, because as I prepared to merge I noticed a new issue. I looked into the relevant react-navigation API, and found:

The use of TabNavigator is deprecated. See the 2.x release notes, and more generally the discussion in our commits 98836b84d, 845834c1d, 0d2066f41.
Using TabNavigator gives different behavior on iOS vs. Android! I think both you and I have been mostly looking at it on Android. The iOS behavior seems like not what we want.

So, let's use createMaterialTopTabNavigator.

In the API doc: https://reactnavigation.org/docs/en/2.x/material-top-tab-navigator.html#initialroutename for the initialRouteName option, there's no mention of '' as a special value. Rather, it's supposed to be the name of some route. (This is the question that originally caused me to look the API up.)

When we don't have a useful reactionName, let's just not pass this option.

I've just pushed :tada: the following commits:
e438f82b2 reactions: Sort aggregated reactions by most numerous.
943c31ba1 styles/tabs [nfc]: Add catch-all option style.
595f1dcb3 reactions: Add new component ReactionUserList.
a49b1f1f8 reactions: Add new screen MessageReactionList.
0feabcc03 reactions [nfc]: Assimilate style of props type definitions.
327ace399 reactions [nfc]: Rename some props to explicit allUsersById.
The first four are from this branch; the last two are commits I added on top.
I've also pushed back to this branch some revisions, so it's now the following commits:
596965dd3 webview: On long-press on reactions, navigate to reactions list.
bb8c05c52 MessageReactionList: Focus tab for reaction that user long-pressed on.
c7ea86008 message action sheet: Add option to show reaction list.
9f265ecfd reactions [nfc]: Add some jsdoc and revise a comment.
That includes one commit of mine at the end, some adjustments to commit messages, and one substantive change described in a commit message:
commit bb8c05c52e292839dbf8b3d734ee3c40fdcbf7a5
Author: Vishwesh Jainkuniya<EMAIL_ADDRESS>Date:   Thu Jan 10 16:13:31 2019 +0530

    MessageReactionList: Focus tab for reaction that user long-pressed on.

    [greg: fixed `reactionName` type in nav prop and inward; type error
     was hidden by lack of type-checking on `StackActions.push`]

The net effect is:

The new screen is merged!
But it isn't actually reachable in the UI; the commits that do that are in the remaining branch.
That's because of the issues I mentioned in the previous comment in the use of the react-navigation API.

Please fix that, and then I think we'll be able to merge the whole thing and close this out for real :smile:

OK -- this is now merged! :checkered_flag: :tada:
I made a couple more edits to the branch before merge:

Moved the createMaterialTopTabNavigator commit to the start. That fixes an issue I wanted to fix before adding UI to get to this screen, so it goes before the commits that add that UI.
Added a new commit refactoring the conditional buildup of the tabs options object -- demonstrates a bit of a trick for keeping those reasonably clean.

Glad to have this finally complete. :sweat_smile:


Feature request load from URL

Is it possible to provide a way to load CSAF data from a URL instead of a local file? The use case is more of a presentation use case where a published CSAF document can be loaded for updates, reviews and changes. For example a CSAF feed or CSAF individual document location https://kb.cert.org/vuls/api/309662/csaf/ can be loaded into secvisogram.
Vijay

I'm not sure yet - as we are working on the backend there might be a way to do that. I guess, we will defer that until we have Secvisogram 2.0 published.

I suggest to add an additional option "Load from URL" into the opening dialogue. Please also check if the security considerations need to be updated when adding this change.

This could create problems with cors-policies.
Loading from URL would therefore only work with servers, which listed the secvisogram url in its cors-policy.
One easy solution would be to just display an error message for cors-errors. Otherwise it would also be possible to use a backend proxy.
Which one would you prefer?

yes, an error message about the reason is necessary. It would also be easier if no backend was necessary.
We can try it out.
I would suggest an additional option in the "New" dialog.

Sounds good!

Released in new version 2.2.0


Tcp proxy: adding a test

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]
trying to repro https://github.com/envoyproxy/envoy/issues/34055

/retest


change the alignment, which might potentially casue problem.

Using __alignment(4) guarantees wasm_test_file[] will be aligned with the memory paging. Otherwise, it might cause the issues in
https://github.com/bytecodealliance/wasm-micro-runtime/issues/257
Root cause:
https://github.com/bytecodealliance/wasm-micro-runtime/blob/master/core/iwasm/interpreter/wasm_loader.c#L1909
This line will trigger the crash of the system because of the alignment problem.

@liurenju  thanks for the finding and the fix.


postgres connection error: sslv3 alert handshake failure

Running kong from docker, trying to connect to Postgres thru SSL with those settings:
KONG_PG_SSL: 'on'
KONG_PG_SSL_VERIFY: 'on'
KONG_LUA_SSL_TRUSTED_CERTIFICATE: /path/to/ca/
KONG_LUA_SSL_VERIFY_DEPTH: 10
gettings the following error:
Error: [PostgreSQL error] failed to retrieve server_version_num: sslv3 alert handshake failure
I read that it's maybe related to the TLS version?
Additional Details & Logs

Kong version 1.21

Hi @melkayam92,
Does it work correctly if you try to connect to PostgreSQL without an SSL certificate?

Yeah, it does works correctly without.
The issue only occurs when connecting with SSL certificate.

Able to reproduce with kong version 1.1.2 and postgresql 9.6
kong.conf
pg_ssl = on
pg_ssl_verify = off

postgresql.conf
ssl = on
ssl_ciphers = 'TLSv1.2:!aNULL'
ssl_prefer_server_ciphers = on

I suspect this is due to postgresql not allowing anything other than TLSv1.2 and the error mentions SSLv3.
SSLv3 is really old and is considered insecure.
Does kong support specifying the TLS protocol to use?

There are two issues here; I will attempt to address both.
@melkayam92 KONG_LUA_SSL_TRUSTED_CERTIFICATE needs to reference a specific CA file, not the path to where CA certificates are located.
@jpuris no this is not the case, the error you are seeing here is a general OpenSSL response. Without seeing the actual error it is impossible to know.
Given the age of this issue and lack of action, I will close this now.please let us know if you have any other questions. Thanks!

Hi there! I am getting the same issue as the OP. I am trying to connect Kong running on Docker with my PostgreSQL database cloud server.
The following environment variables are set when running the Docker container:
KONG_ADMIN_LISTEN="<IP_ADDRESS>:8001"
KONG_ADMIN_LISTEN_SSL="<IP_ADDRESS>:8444"
KONG_DATABASE="postgres"
KONG_PG_HOST="host.to.database.cloud"
KONG_PG_PORT="1337"
KONG_PG_USER="kong"
KONG_PG_PASSWORD="kong_password"
KONG_PG_DATABASE="kong"
KONG_PG_SSL="on"
KONG_PG_SSL_VERIFY="on"
KONG_LUA_SSL_TRUSTED_CERTIFICATE="/etc/ssl/certs/cert.pem"

Additional Details & Logs

Operating system: Docker with image kong:2.1-ubuntu

Here are my output logs when I run docker logs:
nginx: [warn] load balancing method redefined in /usr/local/kong/nginx-kong.conf:57
nginx: [error] init_by_lua error: /usr/local/share/lua/5.1/kong/init.lua:447: [PostgreSQL error] failed to retrieve PostgreSQL server_version_num: sslv3 alert handshake failure
stack traceback:
 [C]: in function 'assert'
 /usr/local/share/lua/5.1/kong/init.lua:447: in function 'init'
 init_by_lua:3: in main chunk

Any guidance is much appreciated! Thank you :)

Hi @jennuineness,
did you get any info to solve you issue?
I got a similar issue with cassandra. Maybe I missed some config in kong.conf (i'm using kong 2.4.0 and cassandra 3.11.11).
Thanks for any help.


Added duplicate button on group

@ivanslabbert i have added a duplicate button on groups.

Unrelated to your PR, but as we noticed when saving with a duplicated group it sometimes displays the group twice. Can you add this to the saveChanges method  just after updateGroup where we reset the other properties as well (line 340 for me):
    addedGroups.value = [];


[ci] Add new python runtime

Add new python runtime (3.9)
Fixes #120

Coverage remained the same at 99.129% when pulling cb6bf3da223fedc76795c655b1cd7da61b69f231 on mohzulfikar:120-add-new-python-runtime into e8995434d3d7141cc03bb27e1b4c2333cd9043db on openwisp:master.

All checks have passed, any additional request for this? @codesankalp


Cosmetic rules content script is usually isn't injected

Known issue, need to recheck it after merge of #85

Cosmetic rules script is still not injected.

Currently it is due to lack of HTTP/2 flow control handling, need to recheck it after merge of #96

Now it injected, but compiled incorrectly, but it is another issue.


hlsdl error: cannot find module "hlsdownloader"

Hi,
I tried hlsdownloader-cli but get errors.
Command is : hlsdl http://url .m3u8 (link to m3u8 is valide, i can read the playlist)
I receive error :
module.js:549
throw err;
^
Error: Cannot find module 'hlsdownloader'
at Function.Module._resolveFilename (module.js:547:15)
at Function.Module._load (module.js:474:25)
at Module.require (module.js:596:17)
at require (internal/module.js:11:18)
at Object. (/usr/local/lib/node_modules/hlsdownloader-cli/bin/lib.js:1:542)
at Module._compile (module.js:652:30)
at Object.Module._extensions..js (module.js:663:10)
at Module.load (module.js:565:32)
at tryModuleLoad (module.js:505:12)
at Function.Module._load (module.js:497:3)
I tried to fix, by reinstalling node_modules, hlsdownloader is installet too, I asked on forums, I updates npm... OS : ubuntu 18 (i tried on Mac OS idem).
Can you help ?
Thks, Yan

@yannLYS Thanks a lot for opening the issue. Extremely sorry for late reply. The problem is not with hlsdl.  It is in hlsdownloader. please run  the following commands. Hope this will resolve the issue.
# you may need to add sudo in front these command
$ npm uninstall --global hlsdownloader-cli
$ npm cache clean --force
$ npm install --global hlsdownloader-cli

hlsdownloader-cli is still in active development. I will be grateful to you if you report issues or send feature request.  Feel free to reopen it if you face any issue related to this. Closing it for now.


Backport 1.3: Check rc of mbedtls_mpi_fill_random

Add MPI_CHK to check for error value of mpi_fill_random.
Backport from Report and fix suggestion by guidovranken in #740

Back ported from https://github.com/ARMmbed/mbedtls/pull/749

Reviewed, approved, tested and merged into mbedtls-1.3.


[DDW-570] Update "View wallet's public key" feature

This PR updates the "View wallet's public key" feature.
Todos

[x] Re-enable functionality in the constant variable
[x] Implement the password dialog
[x] Change the field logic to trigger the dialog
[x] Update the API request
[x] Enable "view wallet's public key" option for HW

Screenshots

Testing Checklist

Slack QA thread
[ ] Test

Review Checklist
Basics

[ ] PR has been assigned and has appropriate labels (feature/bug/chore, release-x.x.x)
[ ] PR is updated to the most recent version of the target branch (and there are no conflicts)
[ ] PR has a good description that summarizes all changes
[ ] PR has default-sized Daedalus window screenshots or animated GIFs of important UI changes:

[ ] In English
[ ] In Japanese

[ ] CHANGELOG entry has been added to the top of the appropriate section (Features, Fixes, Chores) and is linked to the correct PR on GitHub
[ ] Automated tests: All acceptance and unit tests are passing (yarn test)
[ ] Manual tests (minimum tests should cover newly added feature/fix): App works correctly in development build (yarn dev)
[ ] Manual tests (minimum tests should cover newly added feature/fix): App works correctly in production build (yarn package / CI builds)
[ ] There are no flow errors or warnings (yarn flow:test)
[ ] There are no lint errors or warnings (yarn lint)
[ ] There are no prettier errors or warnings (yarn prettier:check)
[ ] There are no missing translations (running yarn manage:translations produces no changes)
[ ] Text changes are proofread and approved (Jane Wild / Amy Reeve)
[ ] Japanese text changes are proofread and approved (Junko Oda)
[ ] UI changes look good in all themes (Alexander Rukin)
[ ] Storybook works and no stories are broken (yarn storybook)
[ ] In case of dependency changes yarn.lock file is updated

Code Quality

[ ] Important parts of the code are properly commented and documented
[ ] Code is properly typed with flow
[ ] React components are split-up enough to avoid unnecessary re-renderings
[ ] Any code that only works in main process is neatly separated from components

Testing

[ ] New feature/change is covered by acceptance tests
[ ] New feature/change is manually tested and approved by QA team
[ ] All existing acceptance tests are still up-to-date
[ ] New feature/change is covered by Daedalus Testing scenario
[ ] All existing Daedalus Testing scenarios are still up-to-date

After Review

[ ] Merge the PR
[ ] Delete the source branch
[ ] Move the ticket to done column on the YouTrack board
[ ] Update Slack QA thread by marking it with a green checkmark

@aleksandardjordjeviciohk please check for which types of wallets do we show this "View wallet's public key" feature.
Is it Shelley only? Or do we show it for Byron wallets and Hardware wallets too?

It would make a lot of sense for the dialog to open after we enter a spending password. @daniloprates @nikolaglumac @alexander-rukin?

Reveal is not asking for spending password after the key was shown once. Not saying this is wrong, but something we should discuss.

Reveal is not asking for spending password after the key was shown once. Not saying this is wrong, but something we should discuss.

I believe that is fine. We can easily change it if you want.

It would make a lot of sense for the dialog to open after we enter a spending password. @daniloprates @nikolaglumac @alexander-rukin?

Agree

It would make a lot of sense for the dialog to open after we enter a spending password. @daniloprates @nikolaglumac @alexander-rukin?

Agree

Added

button should be disabled I suppose at this case (while enetered password at least matches our password rules)

copy action wrong styles:
now it's always 0.5 opacity for text and 1 for icon
->
it should be 0.7 for both in normal state and 1 for both in hovered

Reveal is not asking for spending password after the key was shown once. Not saying this is wrong, but something we should discuss.

It's probably OK for one wallet, but now it doesn't ask for password in other wallets too!!!
MOREOVER
Is this OK that all my wallets have the same public key?

I am not sure why the buttons 'REVEAL' and 'HIDE' are both in capital letters which seem to be inconsistent with the other buttons.

I am not sure why the buttons 'REVEAL' and 'HIDE' are both in capital letters which seem to be inconsistent with the other buttons.

this is by design for special "input buttons"

I am not sure why the buttons 'REVEAL' and 'HIDE' are both in capital letters which seem to be inconsistent with the other buttons.

this is by design for special "input buttons"

@nikolaglumac @darko-mijic  is this really what we want, a mixture of buttons in entirely uppercase and some entirely lowercase but the first letter? From user point of view, I don't see anything special with 'REVEAL' or 'HIDE' buttons.

There's a pipe symbol, on the very left hand side of the buttons when 'Reward view' is chosen which is inconsistent with other views. Tested on 17020
List view

Reward view

Tile view

@alexander-rukin @miorsufianiohk can we perhaps use the 👁   icon here instead of "REVEAL"?

@miorsufianiohk @alexander-rukin @darko-mijic all issues have been resolved. Please re-test 🙏
@miorsufianiohk we won't be changing the "REVEAL/HIDE" designs for now so let's take that discussion out of this PR. Thanks!

@darko-mijic one thing to be aware of - the public key returned from the API is in the bech32 format so it looks something like this:

acct_xvk1cm454leclzk352njpt77qevyuw0ac5k90gzqrztlce8m0a5spg3sq7rd7fsynpe3lkn74a6l5dnmjaf9le5wam88lxcmakctag5hs8gq78hah

...while the one we have for hardware wallets is in hex and looks like:

45cc0e2b463831023afcd441b212cdca539ef202b612b59eb1944fbfc8e75b3g

@tomislavhoracek @KtorZ  can we convert those hex values to bech32 format?


Set default encoding and collation to UTF-8

Feature request
Consider something along the lines of
update pg_database set encoding = 6, datcollate = 'en_US.UTF8', datctype = 'en_US.UTF8' where datname = 'template0';
update pg_database set encoding = 6, datcollate = 'en_US.UTF8', datctype = 'en_US.UTF8' where datname = 'template1';

from https://www.postgresql.org/docs/9.5/manage-ag-templatedbs.html
Describe the solution you'd like
This makes some sensible defaults for the create database command. For some reason this fixes acute problems on Docker for Windows Desktop hosts along with adding this to many different containerized applications, including probably yours:
ENV LANGUAGE=en_US.UTF-8
ENV LANG=en_US.UTF-8
ENV LC_ALL=en_US.UTF-8

Describe alternatives you've considered
FROM supabase/postgres:0.13.0
COPY 99-database-customizations.sql /docker-entrypoint-initdb.d/

99-database-customizations.sql:
update pg_database set encoding = 6, datcollate = 'en_US.UTF8', datctype = 'en_US.UTF8' where datname = 'template0';
update pg_database set encoding = 6, datcollate = 'en_US.UTF8', datctype = 'en_US.UTF8' where datname = 'template1';

Just pushed the changes to the branch develop. This will eventually be part of the upcoming docker version supabase/postgres:0.14.0

acute problems on Docker for Windows Desktop hosts

Not entirely sure but this may have been a plausible reason why.


Add import nltk lib music sample

This download does not need to happen every time the code is run, just the first instance, so I have updated the setup section of the repository's README.md file to reflect the need for this to be run before users try to run the samples themselves


upmerge from v1.14.x

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++
  +0.0%     +16 [None]                                                          +32  +0.0%
  +0.1%     +16 src/core/ext/transport/chttp2/transport/chttp2_transport.cc     +16  +0.1%
      +2.6%     +16 grpc_chttp2_complete_closure_step                               +16  +2.6%

  +0.0%     +32 TOTAL                                                           +48  +0.0%

****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

[trickle] No significant performance differences

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
   722,941  BoringSSL (>)        718,381

 1,981,190      Total (>)      1,976,654

***************FRAMEWORKS****************
  New size                      Old size
 3,326,168  BoringSSL (>)      3,290,048

10,811,859      Total (>)     10,775,731

[microbenchmarks] No significant performance differences

https://github.com/grpc/grpc/issues/16211
https://github.com/grpc/grpc/issues/16212
Both passed in previous runs so likely flakes.


TO JS undefined

When i run on ionic6, it doesn't work. The xcode output shows "TO JS undefined", no matter use BarcodeScanner.hideBackground() or BarcodeScanner.startScan().
Is anyone also got this error?

I have no issues in with my ionic 6 test setup, the plugin works as expected in my case so it might be an issues with your setup and not the plugin itself.
Can you share a repo where I can reproduce your issue?

I'm getting this too. When looking at Xcode I can see a warning:
discoverCaptureDevices()' is deprecated: New Xcode? Check if AVCaptureDevice.DeviceType has new types and add them accordingly.

Same here

For me this was the issue.
In @capacitor-community/barcode-scanner/ios/Plugin/Plugin.swift the UIApplication.shared.statusBarOrientation was not just deprecated, but is obsolete. This line:
self.videoPreviewLayer?.connection?.videoOrientation = interfaceOrientationToVideoOrientation(UIApplication.shared.statusBarOrientation)

should be replaced to something like this
if let interfaceOrientation = UIApplication.shared.windows.first(where: { $0.isKeyWindow })?.windowScene?.interfaceOrientation {
    self.videoPreviewLayer?.connection?.videoOrientation = interfaceOrientationToVideoOrientation(interfaceOrientation)
}

The TO JS undefined log remains, but the barcode scanner seems to work.

I have gotten the same problem

Me too


started some tests for the publication page

The publications page seems to doing a lot and doesn't have much test coverage... so I started some.

Great idea, thanks for starting this


Using DropKick with form validation.

Is it possible to use DropKick with form validation. See the form on my website where I have two select elements. Without using dropkick they would normally get validated. I am using jqueryvalidation to do my validation.

Hey Ben,
By default, Dropkick hides original select elements with display: none. There's an option for .validate() called ignore which defaults to ":hidden". This means that it is ignoring the original select elements, and since Dropkick isn't a normal form control, no validation is being performed on them.
You have 2 possible solutions:

This what I ended up doing and it works as should!
ignore: {
hidden:"not(.selectoptions)",
hidden:"not(textarea.tinymce)"
},
I added tinymce so that is why you see I added textarea.tinymce to the do not ignore option.
Thanks!


CLI implementation for AutoDR (Failover Groups)

This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines

[ ] The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

Command Guidelines

[ ] Each command and parameter has a meaningful description.
[ ] Each new command has a test.

(see Authoring Command Modules)

View a preview at https://prompt.ws/r/Azure/azure-cli/5710
This is an experimental preview for @microsoft users.

(env) C:\azure-cli>az sql failover-group -h
Group
az sql failover-group: Manage SQL Failover Groups.
Commands:
add-databases   : Adds provided list of databases to the failover group.
create          : Creates a failover group.
delete          : Deletes a failover group.
failover        : Executes a failover of a failover group.
list            : Lists the failover groups in a server.
remove-databases: Removes provided list of databases from failover group.
show            : Gets a failover group.
update          : Updates the parameters of the failover group.
(env) C:\azure-cli>az sql failover-group create -h
Command
az sql failover-group create: Creates a failover group.
Arguments
--name -n                  [Required]: The name of the Failover Group.
--partner-server-name      [Required]: The name of the partner server of a Failover Group.
--resource-group -g        [Required]: Name of resource group. You can configure the default
group using az configure --defaults group=<name>.
--server -s                [Required]: Name of the Azure SQL server.
--allow-read-only-failover-to-primary: This value indicates whether an outage on the secondary
server should trigger automatic failover of the read-only
endpoint. This feature is not yet supported.
--failover-policy                    : The failover policy of the Failover Group.  Default:
Automatic.
--grace-period-with-data-loss        : Interval before automatic failover is initiated if an
outage occurs on the primary server. This indicates that
Azure SQL Database will not initiate automatic failover
before the grace period expires. Please note that
failover operation with AllowDataLoss option might cause
data loss due to the nature of asynchronous
synchronization.  Default: 60.
--partner-resource-group-name        : The name of the resource group of the partner server.
Global Arguments
--debug                              : Increase logging verbosity to show all debug logs.
--help -h                            : Show this help message and exit.
--output -o                          : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                              : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                            : Increase logging verbosity. Use --debug for full debug
logs.
(env) C:\azure-cli>az sql failover-group update -h
Command
az sql failover-group update: Updates the parameters of the failover group.
Arguments
--name -n                  [Required]: The name of the Failover Group.
--resource-group -g        [Required]: Name of resource group. You can configure the default
group using az configure --defaults group=<name>.
--server -s                [Required]: Name of the Azure SQL server.
--allow-read-only-failover-to-primary: This value indicates whether an outage on the secondary
server should trigger automatic failover of the read-only
endpoint. This feature is not yet supported.
--failover-policy                    : The failover policy of the Failover Group.
--grace-period-with-data-loss        : Interval before automatic failover is initiated if an
outage occurs on the primary server. This indicates that
Azure SQL Database will not initiate automatic failover
before the grace period expires. Please note that
failover operation with AllowDataLoss option might cause
data loss due to the nature of asynchronous
synchronization.
Global Arguments
--debug                              : Increase logging verbosity to show all debug logs.
--help -h                            : Show this help message and exit.
--output -o                          : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                              : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                            : Increase logging verbosity. Use --debug for full debug
logs.
(env) C:\azure-cli>az sql failover-group add-databases -h
Command
az sql failover-group add-databases: Adds provided list of databases to the failover group.
Arguments
--databases         [Required]: List of databases to add (for add-databases command)or remove
(for remove-databases command) from Failover Group.
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
Global Arguments
--debug                       : Increase logging verbosity to show all debug logs.
--help -h                     : Show this help message and exit.
--output -o                   : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                       : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                     : Increase logging verbosity. Use --debug for full debug logs.
(env) C:\azure-cli>az sql failover-group remove-databases -h
Command
az sql failover-group remove-databases: Removes provided list of databases from failover group.
Arguments
--databases         [Required]: List of databases to add (for add-databases command)or remove
(for remove-databases command) from Failover Group.
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
Global Arguments
--debug                       : Increase logging verbosity to show all debug logs.
--help -h                     : Show this help message and exit.
--output -o                   : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                       : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                     : Increase logging verbosity. Use --debug for full debug logs.
(env) C:\azure-cli>az sql failover-group failover -h
Command
az sql failover-group failover: Executes a failover of a failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
--allow-data-loss             : Complete the failover even if doing so may result in data loss.
This will allow the failover to proceed even if a primary
database is unavailable.
Global Arguments
--debug                       : Increase logging verbosity to show all debug logs.
--help -h                     : Show this help message and exit.
--output -o                   : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                       : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                     : Increase logging verbosity. Use --debug for full debug logs.
(env) C:\azure-cli>az sql failover-group delete -h
Command
az sql failover-group delete: Deletes a failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
Global Arguments
--debug                       : Increase logging verbosity to show all debug logs.
--help -h                     : Show this help message and exit.
--output -o                   : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                       : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                     : Increase logging verbosity. Use --debug for full debug logs.
(env) C:\azure-cli>az sql failover-group show -h
Command
az sql failover-group show: Gets a failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
Global Arguments
--debug                       : Increase logging verbosity to show all debug logs.
--help -h                     : Show this help message and exit.
--output -o                   : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                       : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                     : Increase logging verbosity. Use --debug for full debug logs.
(env) C:\azure-cli>az sql failover-group list -h
Command
az sql failover-group list: Lists the failover groups in a server.
Arguments
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
Global Arguments
--debug                       : Increase logging verbosity to show all debug logs.
--help -h                     : Show this help message and exit.
--output -o                   : Output format.  Allowed values: json, jsonc, table, tsv.
Default: json.
--query                       : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose                     : Increase logging verbosity. Use --debug for full debug logs.

@jaredmoo  @btasdoven  Can you please take a look?

I want to make sure that the design works nicely with future managed failover group and job target group commands. Specifically

What will be the planned syntax for managed failover group? (Managed failover group will be able to contain both MI and servers/db, right?)
failover-group add-databases --databases x y z a few comments:

I think adding/removing should be part of failover-group update. e.g. failover-group update --add-db  x y z. This allows multiple properties to be set in one go.
failover-group create also should have the ability to add db's

--partner-server-name -> --partner-server
--partner-resource-group-name -> --partner-resource-group
Please comment what happens if --partner-resource-group is not specified
--allow-read-only-failover-to-primary: (...) This feature is not yet supported. - so why is there a param for it? :)
--failover-policy - what are allowed values? Can you put enum in params.py?
failover

I know "failover" is in the name of the resource, but please consider set-primary in order to match same as az sql db replica set-primary command (single db geodr).
please make sure it is idempotent.

--grace-period-with-data-loss this is very long param name. We are not implementing powershell here ;) can it just be --grace-period ?

@jaredmoo I have addressed issues you mentioned in your comment. I don't know however how CLI for managed instances would look like and the team working on it does not know either. So imho we shouldn't take a dependency on it.

Managed failover group is not that far away, it would be bad to add failover group now and then realise that managed failover group needs to be totally different. Can you past help output so I can review updated design?

I talked with manged failover group folks and they are ok with following the design for sterling.
@btasdoven @dlemMSFT Can you look at this PR from GeoDR team point of view?
Here is updated help output (without global arguments which are staying default):
(env) C:\azure-cli>az sql failover-group -h
Group
az sql failover-group: Manage SQL Failover Groups.
Commands:
create     : Creates a failover group.
delete     : Deletes a failover group.
list       : Lists the failover groups in a server.
set-primary: Executes a failover of a failover group.
show       : Gets a failover group.
update     : Updates the parameters of the failover group.
(env) C:\azure-cli>az sql failover-group create -h
Command
az sql failover-group create: Creates a failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--partner-server    [Required]: The name of the partner server of a Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
--add-db                      : List of databases to add to Failover Group.
--failover-policy             : The failover policy of the Failover Group.  Allowed values:
Automatic, Manual.  Default: Automatic.
--grace-period                : Interval before automatic failover is initiated if an outage
occurs on the primary server. This indicates that Azure SQL
Database will not initiate automatic failover before the grace
period expires. Please note that failover operation with
AllowDataLoss option might cause data loss due to the nature of
asynchronous synchronization.  Default: 60.
--partner-resource-group      : The name of the resource group of the partner server.
(env) C:\azure-cli>az sql failover-group update -h
Command
az sql failover-group update: Updates the parameters of the failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
--add-db                      : List of databases to add to Failover Group.
--failover-policy             : The failover policy of the Failover Group.  Allowed values:
Automatic, Manual.
--grace-period                : Interval before automatic failover is initiated if an outage
occurs on the primary server. This indicates that Azure SQL
Database will not initiate automatic failover before the grace
period expires. Please note that failover operation with
AllowDataLoss option might cause data loss due to the nature of
asynchronous synchronization.
--remove-db                   : List of databases to remove from Failover Group.
(env) C:\azure-cli>az sql failover-group set-primary -h
Command
az sql failover-group set-primary: Executes a failover of a failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
--allow-data-loss
(env) C:\azure-cli>az sql failover-group delete -h
Command
az sql failover-group delete: Deletes a failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.
(env) C:\azure-cli>az sql failover-group show -h
Command
az sql failover-group show: Gets a failover group.
Arguments
--name -n           [Required]: The name of the Failover Group.
--resource-group -g [Required]: Name of resource group. You can configure the default group
using az configure --defaults group=<name>.
--server -s         [Required]: Name of the Azure SQL server.

Design looks good!

--grace-period: please comment in help that it is in seconds
--allow-data-loss: missing help text

This PR looks stale. Closing soon...


Project is not open source without a license file

Without a license, this project isn't open source and no one can use the code.
Please use something like http://choosealicense.com/ to decide what license to use. I recommend MIT or GPL.

A novel effort by Add a License Please

It was called MIT-LICENSE.
I changed the to avoid the false-positives on your search ;)


An UTXO from a previous tx is still selected

doing unselect txid-n or unselect all display the confirmation
>>> unselect all
info: Main:           Unselected 2 UTXOs

but list still shows those UTXOs selected

fix it, indeed forgot to implement it :D

this is still happening
i select then dump then list and the last UTXO is still selected
i can unselect here to delete the UTXO's - thank you for doing that :+1:
i guess dump should trigger unselect all

indeed, updated, now it will trigger unselect all.


Verify min(date) and max(date) aggregate functions end-to-end

Currently, min and max aggregate functions with DATE input types produce DATE intermediate results. We need to double check whether this matches intermediate types expected by the Presto planner and if not update the implementation. We would also like to add an e2e test to ensure min(date) and max(date) work properly.
    @Test
    public void testMinMax()
    {
        // tinyint
        assertQuery("SELECT min(cast(linenumber as tinyint)), max(cast(linenumber as tinyint)) FROM lineitem");
        // smallint
        assertQuery("SELECT min(cast(linenumber as smallint)), max(cast(linenumber as smallint)) FROM lineitem");
        // integer
        assertQuery("SELECT min(linenumber), max(linenumber) FROM lineitem");
        // bigint
        assertQuery("SELECT min(orderkey), max(orderkey) FROM lineitem");
        // real
        assertQuery("SELECT min(cast(quantity as real)), max(cast(quantity as real)) FROM lineitem");
        // double
        assertQuery("SELECT min(quantity), max(quantity) FROM lineitem");
        // timestamp
        assertQuery("SELECT min(from_unixtime(orderkey)), max(from_unixtime(orderkey)) FROM lineitem");
    }

CC: @spershin @pramodsatya

also needed for min_by/max_by aggregate functions :)

#2189 added in Velox to address the initialization issue for min(date) aggregate, https://github.com/facebookexternal/presto_cpp/pull/827 adds e2e test in presto_cpp


[BUG]No luckysheet.umd.js in dist

I have built the sources of the current snapshot with npm run build, but there is no luckysheet.umd.js required by the following line from README: <script src="https://cdn.jsdelivr.net/npm/luckysheet/dist/luckysheet.umd.js"></script>.
All scripts:
$ find . -name "*.js"
./assets/iconfont/iconfont.js
./plugins/js/plugin.js
./expendPlugins/chart/chartmix.umd.min.js
./demoData/sheetChart.js
./demoData/sheetCell.js
./demoData/sheetTable.js
./demoData/sheetSparkline.js
./demoData/sheetDataVerification.js
./demoData/sheetConditionFormat.js
./demoData/sheetPivotTable.js
./demoData/sheetComment.js
./demoData/sheetFormula.js
./demoData/sheetPivotTableData.js
./demoData/sheetPicture.js

Make sure?

Possibly I made a mistake before, because now I recompiled and the files are there.


where is the history option?

the new version v1.3.2 can't  find the history option

The plugin is made of three windows.

The first window is the main plugin window.
is a secondary window. It’s designed for controlnet, previewing and selecting the generated images, and other features that used to require tab switching.
The third is a toolbar. It provides quick access to all the important buttons.

This setup is all about making your life a bit easier. No more tab switching or endless scrolling. Just a smoother, more efficient workflow.


x/tools/gopls: regtest flakes due to hanging go commands

#!watchflakes
post <- goos != "netbsd" && (pkg ~ `^golang.org/x/tools/gopls` || (repo == "tools" && pkg == "")) &&
        `detected hanging go command`

greplogs --dashboard -md -l -e 'runCmdContext' --since=2022-07-14

2022-08-15T17:42:12-987de34-1f833e4/darwin-amd64-12_0
2022-08-14T00:06:23-35f806b-59865f1/netbsd-amd64-9_0
2022-08-12T20:40:05-bebd890-2f6783c/netbsd-386-9_0
2022-08-12T18:15:28-bebd890-b6f87b0/netbsd-amd64-9_0
2022-08-12T12:39:26-88d981e-f67c766/netbsd-amd64-9_0
2022-08-12T00:04:29-c4ec74a-a5cd894/netbsd-386-9_0
2022-08-11T19:05:54-c4ec74a-62654df/netbsd-amd64-9_0
2022-08-11T17:53:50-37a81b6-a526ec1/netbsd-amd64-9_0
2022-08-11T16:19:14-37a81b6-2340d37/netbsd-amd64-9_0
2022-08-11T12:53:59-b2156b5-3c200d6/netbsd-386-9_0
2022-08-10T22:22:48-b2156b5-6b80b62/netbsd-amd64-9_0
2022-08-10T17:41:25-0ad49fd-f19f6c7/netbsd-amd64-9_0
2022-08-10T15:08:24-3950865-c81dfdd/netbsd-amd64-9_0
2022-08-10T02:14:09-6fa767d-5531838/plan9-386-0intro
2022-08-09T14:33:24-92d58ea-0981d9f/openbsd-386-70
2022-08-09T14:12:01-92d58ea-662a729/netbsd-amd64-9_0
2022-08-09T13:39:27-92d58ea-9e8020b/netbsd-386-9_0
2022-08-09T11:28:56-92d58ea-0f8dffd/netbsd-amd64-9_0
2022-08-08T18:10:56-fff6d6d-4bcc138/netbsd-amd64-9_0
2022-08-08T15:33:45-06d96ee-0581d69/netbsd-amd64-9_0
2022-08-08T15:07:46-06d96ee-cd54ef1/netbsd-amd64-9_0
2022-08-08T14:12:21-06d96ee-e761556/netbsd-amd64-9_0
2022-08-08T06:16:59-06d96ee-0f6ee42/darwin-amd64-11_0
2022-08-08T06:16:59-06d96ee-0f6ee42/netbsd-386-9_0
2022-08-06T15:20:00-06d96ee-0c4db1e/plan9-386-0intro
2022-08-05T19:51:08-06d96ee-4fb7e22/plan9-386-0intro
2022-08-04T20:05:03-81c7dc4-39728f4/netbsd-386-9_0
2022-08-04T20:05:03-81c7dc4-39728f4/netbsd-amd64-9_0
2022-08-04T20:04:16-3519aa2-39728f4/netbsd-386-9_0
2022-08-04T19:57:25-763f65c-39728f4/netbsd-386-9_0
2022-08-04T18:51:46-99fd76f-39728f4/openbsd-386-70
2022-08-04T17:05:18-3e0a503-fb1bfd4/netbsd-amd64-9_0
2022-08-04T15:50:11-3e0a503-fcdd099/netbsd-386-9_0
2022-08-04T15:50:11-3e0a503-44ff9bf/netbsd-amd64-9_0
2022-08-04T15:31:49-87f47bb-44ff9bf/plan9-386-0intro
2022-08-04T14:58:59-87f47bb-4345620/netbsd-386-9_0
2022-08-04T10:32:51-3e0a503-a10afb1/linux-386-buster
2022-08-03T21:02:27-8b9a1fb-f28fa95/plan9-386-0intro
2022-08-03T21:02:27-8b9a1fb-4345620/netbsd-386-9_0
2022-08-03T18:07:40-d08f5dc-fcdd099/netbsd-386-9_0
2022-08-03T13:50:38-ddb90ec-c6a2dad/dragonfly-amd64-622
2022-08-03T13:50:38-ddb90ec-c6a2dad/plan9-386-0intro
2022-08-03T12:09:24-ddb90ec-29b9a32/plan9-386-0intro
2022-08-02T18:52:36-0d04f65-29b9a32/plan9-386-0intro
2022-08-02T18:19:01-d025cce-be59153/netbsd-amd64-9_0
2022-08-02T18:16:22-10cb435-d723df7/netbsd-amd64-9_0
2022-08-02T18:07:14-4d0b383-d723df7/netbsd-386-9_0
2022-08-02T18:07:14-4d0b383-d723df7/netbsd-amd64-9_0
2022-08-02T17:23:42-4d0b383-1b7e71e/darwin-amd64-nocgo
2022-08-02T16:05:48-4d0b383-f2a9f3e/netbsd-amd64-9_0
2022-07-29T20:19:23-9580c84-9240558/windows-arm64-11
2022-07-28T20:06:00-8ea5687-d9242f7/darwin-amd64-nocgo
2022-07-27T15:04:58-39a4e36-4248146/freebsd-386-13_0
2022-07-26T18:43:08-6c8a6c4-d9242f7/aix-ppc64
2022-07-25T20:44:49-2a6393f-24dc27a/darwin-amd64-10_14
2022-07-25T18:11:01-4375b29-795a88d/plan9-386-0intro
2022-07-25T14:16:17-178fdf9-64f2829/plan9-386-0intro
2022-07-22T20:12:19-7b605f4-c5da4fb/plan9-386-0intro
2022-07-21T20:11:06-ec1f924-c4a6d30/plan9-386-0intro
2022-07-15T15:11:26-22d1494-2aa473c/windows-386-2008-newcc
2022-07-15T14:27:36-1a4e02f-4651ebf/windows-arm64-10
2022-07-15T14:20:24-db8f89b-4651ebf/windows-arm64-10
2022-07-14T21:03:14-db8f89b-783ff7d/windows-arm64-11
2022-07-14T21:01:58-db8f89b-aa80228/darwin-arm64-11
2022-07-14T19:05:09-db8f89b-a906d3d/windows-arm64-10
2022-07-14T15:54:36-db8f89b-266c70c/windows-arm64-10
2022-07-14T01:47:39-db8f89b-558785a/windows-arm64-11

We recently started waiting for all go command invocations when shutting down gopls regtests.  It appears that sometimes we kill the go command and still don't get a result from cmd.Wait(). For example, here:
https://build.golang.org/log/00046e0b005c7660d676a3a415561950048f756a
In that failure, the test runner looks otherwise healthy (other tests ran fast), and yet the goroutine stack clearly shows a go command hanging for 9 minutes here:
https://cs.opensource.google/go/x/tools/+/master:internal/gocommand/invoke.go;l=260;drc=f38573358cbedf46d64c5759ef41b72afcf0c5c0
@bcmills do you happen to have any idea of what might cause this?

Two observations:

We're not checking the error from Kill. If killing failed, I don't think we can expect Wait to return.
 More likely kill terminated the go process itself, but not the tree of processes rooted at it. If one of them (a test?) retains an open file descriptor to the stdout pipe created by os/exec then the cmd.Run operation will hang indefinitely.

Please do check the error from Kill — that would at least give a useful data point for diagnostics. (This may be closely related to #53400.)

Interesting, I was debugging this in https://go.dev/cl/424075.

After reading the source a bit more: this is EINVAL, which appears to mean that the Process.wait() has exited and the handle released, so this is a race, although it is surprising that we hit it so reliably.

any advice for how to properly kill the go command on windows?

Can't be done without creating a whole extra process group, unfortunately. (Probably we should add a side-channel — perhaps an open file descriptor or a pidfile? — to request clean shutdown on Windows.)

A hit!
https://build.golang.org/log/4eea0e5bfe425fcd097a49fcb92fd015fa88f383

Nice. Well, that test process seems very much alive, falsifying my hypothesis.

13032 12910 (compile)
 9802 13032 go list -modfile=/tmp/workdir/tmp/go.8f5d4cc245da9790bda529e64ae3e7

Looks like the hanging go command is in the middle of a compile. Wish we had the full subprocesss command line -- I'll look into that.
Not sure how to interpret the fstat output.

That's a dead cmd/compile process: there's no command because argv has been destroyed along with the rest of the address space. Perhaps the go list parent simply hasn't called waitpid yet, so the process table entry has to be retained. I suspect the problem is in go list.

Aha, thanks (excuse my ps noobness).
Note that we instrumented this panic in two places: once before Kill() and once after. This is before, so I don't think it falsifies your hypothesis, if I'm understanding correctly.

greplogs --dashboard -md -l -e 'HANGING GO' --since=2022-09-15
2022-09-17T02:56:51-4d18923-cc1b20e/netbsd-amd64-9_0
2022-09-16T17:59:31-a61f20e-a84f46a/netbsd-amd64-9_0
2022-09-16T16:33:11-a61f20e-8451529/netbsd-386-9_0
2022-09-16T14:49:13-a61f20e-b35b4a3/netbsd-amd64-9_0
Still only netbsd. Posting the greplogs query here for future reference.

Ooh, nice! https://go.dev/issue/55323#issuecomment-1254107802 has a darwin/arm64 hang. 😃

As expected, cmd/go itself has already exited at the point of the hang:
2022/09/20 08:42:37 error killing the Go command: os: process already finished

But there are an awful lot of processes in that dump, and nothing that obviously stands out as a subprocess of cmd/go that would have access to stdout or stderr. 😕

Here's an off-the-wall theory: maybe the file descriptors actually are closed, and something in the parent Go process (i.e. gopls) isn't waking up from the end-of-stream event..?
#53434 could be related.

To check that hypothesis, two suggestions:

gopls should log exactly which PID was the stuck go command, just in case it left a remnant in the ps or lsof output that we can track down.
gopls should invoke debug.SetTraceback("system") before the terminal panic here.

A simpler explanation is that gopls sent SIGINT, waited for 1s, then sent SIGKILL, and the child (go) exited during the 1s grace period, causing kill(2) to fail with ESRCH (-> ErrProcessDone = "process already finished"). Seems like that's to be expected if a busy machine causes a delay in responding to SIGINT. Perhaps the debugging logic should treat ErrProcessDone as success, not a reason to debug?

@adonovan but in this case the panic indicates that we waited up to 10 seconds after that kill. The error from Kill is merely logged.
Per @bcmills comment on the CL, we should expect that cmd.Wait returns quickly once the process is done.

the panic indicates that we waited up to 10 seconds after that kill. The error from Kill is merely logged.

Good point.
So the sequence of events that we know must have happened from the log is:

ctx cancelled
SIGINT
wait 1s
SIGKILL => ErrProcessDone
wait 10s
dump shows a go process still alive

ErrProcessDone means that either waitpid returned ESRCH or that this was the second call to Process.signal after a first one set the 'done' event at the end of a successful call to Process.wait. In either case, the process must already have terminated.
Is it possible that the dump shows a different go process? Seems improbable, but I feel like we've eliminated the impossible. We should log the pid of cmd.Process, like @bcmills suggested.

Is it possible that the dump shows a different go process? Seems improbable, but I feel like we've eliminated the impossible. We should log the pid of cmd.Process, like @bcmills suggested.

The pid in the last log above clearly shows that that the go list process is the child of the test, not some other process in the container. Huh...

We still haven't seen a single log with both (a) the "error killing the Go command" message and (b) a pid showing that 'go' is the child of the test. So it's possible that the log with just (a) was merely a very slow (netbsd) machine and the log with just (b) was showing a different go process in the ps output. If both were to occur in the same log it would indicate a bug in the system call.

merely a very slow (netbsd) machine

The NetBSD machines aren't particularly slow; they're buggy (#50138 up until yesterday, and #56180 after that).

From the last log, we're seeing that go list (pid 11323) is slow to complete, more than 1 minute, on the netbsd builder. We don't have the complete go list command, but we can tell from the output of ps that it is definitely running the compiler, which is surprising, as that should only be needed in -export mode, which gopls has no need for. Notice the three "(compile)" child processes in ps, plus all gopls' in/out/err pipes in the lsof dump.

Yes, that does look like the explanation. (Nice work tracking that one down, BTW.)
I'll limit my attention to non-netbsd platforms going forward.

Note: some recent flakes have been in darwin (rather than netbsd), and may contain interesting results. (Haven't dug in yet)

The netbsd-386-9_3 hangs look like a platform bug to me (filed as #57999).

Looking at the recent darwin failures, they all appear to have occurred with the go command already exited.
I wonder if the check is too sensitive at this point? The examples in the original post were hanging for multiple minutes, not just a few seconds after the test. 🤔

There have been a lot of reports of this lately, and we shouldn't even be running gopls tests on a 386 (or any 32-bit machine).

I don't think we've seen any failures on a 386 platform lately?
They're mostly on the darwin/amd64 platform, and with the failure mode of error killing the Go command: os: process already finished, which suggests to me that the 1s timeout after cmd.Process.Kill() is probably just too short:
https://cs.opensource.google/go/x/tools/+/master:internal/gocommand/invoke.go;l=277;drc=5214f412aecffbca887fc0c26f8d59e3a2342d44

I don't think we've seen any failures on a 386 platform lately?

Sorry, ignore me. Scanning too many watchflakes posts in haste, I misread the exclusion of 386 as a positive report.

1s ... is probably just too short

Yeah, that seems plausible. Shall I make it 2? ;-)

Thinking about this some more: if the process is already finished but Wait is taking longer than a second, it's probably spending that time ingesting a lot of (presumably buffered) output from the command's stdout and/or stderr.
Perhaps we should be redirecting the output through an os.Pipe or temporary os.File so that we can stop reading it as soon as the command is canceled. 🤔
(In theory we could use the new WaitDelay field to bound the time spent waiting, but WaitDelay is not intended for cases where draining the buffer after a successful command might take longer than we're willing to wait upon cancellation.)

Thinking about this some more: if the process is already finished but Wait is taking longer than a second, it's probably spending that time ingesting a lot of (presumably buffered) output from the command's stdout and/or stderr.

That's an interesting idea. 10s is an absurdly long time for this to take though.
But your CL should be an improvement anyway.

At present I believe that the darwin bug comprises this cluster of watchflakes issues:

#60108
#59995
#59892
#60099
#54461

Here's an interesting hang that wasn't on Darwin (linux-s390x-ibm):
https://build.golang.org/log/760ddd8386c57e95d80a543849ac2f344521dd07

linux-s390x-ibm is a little suspect at the moment because of other timeout problems (#60413, #60109). I don't plan to look too much into that one.

I have filed a new issue for the darwin hangs, since they seem to be much more general than either gopls or cmd/go: #63937.

@pjweinb, I would suggest filing a separate issue for that — on a Google cloudtop in particular you may be running up against port exhaustion (see https://github.com/golang/go/issues/52545#issuecomment-1454157883).

@golang/release, this issue has gotten very noisy because a bunch of these builders are just plain timing out. Could you please either prioritize a fix for those timeouts (maybe just set GO_TEST_TIMEOUT_SCALE really high for now?) or update the watchflakes pattern on this issue to exclude the misconfigured builders?
See:

#65311
#64697
#65040

The recent spurious failures were #65845; marking as off-topic.

This seems to be occurring more frequently now, perhaps due to telemetry? @matloob we should investigate.


Update app generator to include ember-cli-typescript

Can we update instructions to make it nicer too?

Allows people to create a TS ember project from scratch.


exit logspout on fatal error

rather than continuing on a connection error (which will never recover) exit the process so that the process manager can restart logspout
This should fix https://github.com/looplab/logspout-logstash/issues/7# and https://github.com/gliderlabs/logspout/issues/125

Why does this change so much more than the title/desc?

I'm closing this for now, it changes too much. You are welcome to reopen with only the closing fix if you wish.


Add authentication arguments

If someone else finds this issue, see https://github.com/afuetterer/oaipmh-scythe/discussions/277#discussioncomment-8555095


Fix #1205

fixes #1205

Checklist:

[x] Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
[x] Add or update relevant docs, in the docs folder and in code.
[x] Add an entry in CHANGES.rst summarizing the change and linking to the issue.
[x] Add .. versionchanged:: entries in any relevant code docs.
[x] Run pre-commit hooks and fix any issues.
[x] Run pytest and tox, no tests failed.

Delete line means I think there is no need to do that.

Thanks, but this has a lot of changes that aren't related to the issue, and I'd rather use import sqlalchemy.orm as sa_orm.

Also, when opening a pull request, make sure to create a new branch for your work, and give the PR a descriptive title.

@davidism The changes that not related to the issue, means the test and ?

The change to cli.py, just leave that as it currently is.

The change to cli.py, just leave that as it currently is.

OK. I will do it now.


Set color filter on a loaded svg image

Hi,
Thanks so much for this easy and helpful library :)
I successfully load an image from server, but then i want to apply a color tint to it. setColorFilter is not working thought !
Thanks

Hi! Can you please share a snippet of code you're using?


docs: rename examples to notebooks

This pull request renames the /examples directory, which contains Livebook notebooks, to /notebooks, following common convention in Elixir community. One example is Axon's repo.
Changes

rename /examples to /notebooks
correct Livebook links in /notebooks/README.md

Thanks @mnishiguchi! :D


Compiler support for ISSR

I have noticed that ISSR instructions have been added to the banshee simulator. However, the tests now need compiler support (the scfgwi instruction is not supported). Is there a link to the new compiler?

We are working on compiler support for the SSR instruction and will release it in the next weeks. Until then, you can use the runtime library to configure the SSRs

Is there a way to compile tests that use ISSR at the moment?
For example, I would like to compile the following test

Unfortunately that is not possible yet, sorry.

Thank you for the information!
I would be very interested to use it once it is available.

Ok, w've now release the LLVM compiler for Snitch. Closing this for now, let me know if you encounter any issues


[DS-3158] 'dspace dsprop' does not do property substitution

https://jira.duraspace.org/browse/DS-3158

+1 Tested & works. I was able to verify the existing problem, and verify this fixes it. Code looks great too.

pbecker spotted a problem:  when -m is given, the property name is composed in reverse (property.module, not module.property).

+1 by review. I trust mhwood and tdonohue that it works and the option -m is tested now. ;-)
I'll merge this.


best_score.sh file is missing

model is training properly.
But its ending with
inference at exp/diarize/infer/train_clean_5_ns2_beta2_500.dev_clean_2_ns2_beta2_500.train.avg8-10.infer
scoring at exp/diarize/scoring/train_clean_5_ns2_beta2_500.dev_clean_2_ns2_beta2_500.train.avg8-10.infer
./run.sh: line 147: best_score.sh: command not found
Could you plz tell where to find best_score.sh

You can find the script at ../../../utils/best_score.sh.
I believe the run.sh could find the path to ../../../utils through ./path.sh and tools/env.sh if tools made correctly.


eventually: new 0.5.0 API

Codecov Report

Merging #140 (19d20e0) into master (726250e) will decrease coverage by 2.18%.
The diff coverage is 48.66%.

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
- Coverage   69.37%   67.18%   -2.19%
==========================================
  Files          26       32       +6
  Lines        1270     1420     +150
==========================================
+ Hits          881      954      +73
- Misses        389      466      +77

Flag
Coverage Δ

unit
67.18% <48.66%> (-2.19%)
:arrow_down:

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

Impacted Files
Coverage Δ

eventually-new/src/eventstore.rs
0.00% <0.00%> (ø)

eventually-new/src/inmemory.rs
31.03% <31.03%> (ø)

eventually-new/src/lib.rs
42.85% <42.85%> (ø)

eventually-new/src/scenario.rs
46.15% <46.15%> (ø)

eventually-new/src/aggregate.rs
57.14% <57.14%> (ø)

eventually-new/src/test.rs
70.83% <70.83%> (ø)

... and 2 more

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 726250e...19d20e0. Read the comment docs.

Do you have a rough timeline for releasing this?
Is there anything that you would welcome collaboration on?

Hey @adtennant, don't worry about the tone of the message (which was perfectly fine) 😄
My bad for not updating the PR or linking it to a tracking issue, lately I've been more focused on the Go version than the Rust one.
Initially, my idea for v0.5.0 was to have:

A complete redesign of the AggregateRoot trait, using it more similarly to a simple Entity with methods instead of using the Handle method with a Command enum. Essentially, hiding the Event Sourcing part away from the user.
Redesign the EventStore trait, splitting it in multiple sub-traits (Interface Segregation anyone? 😅) and use async-trait for the time being.
Remove the multi-crate approach for the main eventually crate, and just have a monorepo with the core library and the extension implementations (event stores, telemetry layer, etc.)

While 2. and 3. are mostly done (although not in this PR, I've admittedly rewritten eventually from scratch at least 3 times, lol), I've actually changed my mind on 1. (which was the most "controversial" -- for me, at least -- decision on the new version): I like the "functional" aspect of this implementation, so I'd like to keep it as such.
Next steps for the new version:

Definitely create a project/milestone to track the progress
Scope the requirements for the new API
Create the issues
Get to work

If you're interested in help, I would suggest to move the conversation in our Discord server (which looks a bit abandoned now) to keep the feedback cycle short 😄

would love to contribute to this! how can i help?

Hey @danieleades, as I already said in https://github.com/get-eventually/eventually-rs/issues/133#issuecomment-912804263 I think I'll need to start planning some work for this new version, starting from the issues that require some modeling (e.g. Aggregate Roots).
I would really need implementing some of these things, would you be up for helping? 😄

Hey @danieleades, as I already said in https://github.com/get-eventually/eventually-rs/issues/133#issuecomment-912804263 I think I'll need to start planning some work for this new version, starting from the issues that require some modeling (e.g. Aggregate Roots).
I would really need implementing some of these things, would you be up for helping? 😄

Of course! I'm having a ball playing around with this code base. If you could tell me what problems you're trying to solve, I'd love to help (if I can!).

Closing this PR in favor of smaller, targeted PRs.
You can follow the progress here: https://github.com/get-eventually/eventually-rs/projects/1
I'll convert some of the cards to issues for contributors help soon 👍🏻
I'm also looking for reviewers, if that interests you!
Thanks for now ❤


Update visualisation for Unemployment / Economic inactivity for Labour Market dashboard

[x] Extract data for unemployment / economically inactive rate for all Young people 16-24 (regardless of education status)
[x] Update headlines visualisation with appropriate data

Should be building now


🐛 Bug Report: unable to upload files larger than around 1 mb

👟 Reproduction steps
when i try to upload a file larger than around 1mb it fails to upload and gets stuck retrying to upload it.

I get the same thing if i do it from my phone on firefox or bromite.
pingvin.log
here are the logs for the docker container
docker-comepose.txt
and my docker compose yml file
is there any other logs i could give here?
👍 Expected behavior
it should upload files larger than 1mb
👎 Actual Behavior
fails to upload and gets stuck retrying
🌐 Browser
firefox

yes, that could be the issue since i have no idea what im doing when it comes to nginx.

here is the nginx config im using (got it off someone on discord) what way would you recommend setting up?

this is what i get in /var/log/nginx/error.log (many lines of that)
the red boxes are all just my domain

and that is what i get in access.log

i just realised this was an nginx issue and i needed to add a client_max_body_size line to it. could you close this issue? tho i am having some very weird upload issues with speed and it failing over the internet and lan, so i might test it a little more and make a bug report or something later.

Nice! Yes sure, feel free to create other issues if you encounter other problems :)


Update joda-time to 2.10.3

Updates joda-time:joda-time from 2.10.2 to 2.10.3.
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention @scala-steward in the comments below.
Have a fantastic day writing Scala!

Ignore future updates
Add this to your .scala-steward.conf file to ignore future updates of this dependency:
updates.ignore = [{ groupId = "joda-time", artifactId = "joda-time" }]

labels: semver-patch

Black Duck Security Report
Merging #182 into master will not change security risk.
Added Components
Clean:       4
Removed Components
Clean:       4
Click here to see full report


Direct scan support

Some keyboards support direct scan. e.g. https://www.sho-k.co.uk/tech/1246.html
These keyboards do not use a matrix, but use one pin per key.
This PR is working on those keyboards as well.
When defining these keyboards, the following writing style was supported.
kbd.set_scan_mode = :direct
kbd.init_pins(
  [],
  [ 8, 27, 28, 29, 9, 26 ]
)

or
kbd.init_direct_pins(
  [ 8, 27, 28, 29, 9, 26 ]
)

Thanks!
I don't have a keyboard that requires direct scanning though, it's working well on my keyboards which do matrix scanning.