Fixed app crashing on MacOS a few seconds after opening

It was an issue with the tray icon, the ico format file provided only works on windows.
This might address #57
Changelog:

Fixed mac tray icon from attempting to load an ico format file
Prevent attempting to create the tray on untested operative systems
Fixed macOS application icon
Updated the MacOS build instructions
Added important file paths to the README.md

lgtm, ty magic mac haver. I am a poor doge and cannot afford shiny apple products :)


chore: Migrate phishingprotection synth.py to bazel

The changes in grpc stubs are caused by the gRPC upgrade from 1.10 (more than a year old) to 1.27 (same version which is used as runtime dependency)

@chingor13 PTAL


[MM-50229] Fix right-click paste for user invites

Summary
The input was so tiny that you could not right-click into it. It's not perfect, as it's not expanded vertically, but it's better now.
Ticket Link
https://mattermost.atlassian.net/browse/MM-50229
Related Pull Requests
N/A
Screenshots
Before
https://user-images.githubusercontent.com/684680/217881062-390a52af-bcf5-45fb-b319-e185fdbcce5a.mp4
After
https://user-images.githubusercontent.com/684680/217881093-08c9f307-e76b-4f54-9af2-5476b83503bc.mp4
Release Note
NONE

E2E tests not automatically triggered, because PR has no approval yet. Please ask a developer to review and then try again to attach the QA label.

/e2e-test

Successfully triggered E2E testing!
GitLab pipeline | Test dashboard

@furqanmlk Yes, that's the drawback of this solution. Our third-party library has an <input> under the hood. This means that this won't be able to accept multi-line inputs, so the "clickable" area is the first row of the textarea.
As an illustration:

Not sure if we can achieve any better results without heavily manipulating the Input component of the library, which is a much more involved change.


Desktop Logs

For windows client, below mentioned location will not work.
Client	Location
Windows	%appdata%\Roaming\Microsoft\Teams\logs.txt
It must be
Windows: 	%appdata%\Microsoft\Teams\logs.txt

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

ID: 97d5e6d3-b7e0-d79f-0dea-ed423186a157
Version Independent ID: ec6e68aa-552f-ab45-3c8e-92eff526b173
Content: Use log files in troubleshooting Microsoft Teams
Content Source: Teams/log-files.md
Service: msteams
GitHub Login: @LolaJacobsen
Microsoft Alias: lolaj

@baluilag - I checked my installation on Windows 10 and it is C:\Users<user>\AppData\Roaming\Teams\logs

I'm with @baluilag. While @tonysmit has the path correct, %appdata% resolves to C:\Users\Username\AppData\Roaming so the instructions ask for Roaming to be in the folder path twice


IDA: fix userA syncing userBs struct showing userA made new struct changes

Take a look at the api_lock usage in the controller code of IDA to get an idea of how this is fixed with other reversing structures.

closed in #43


DRAFT: Add support for posix_madvise to Java 21 MMapDirectory

This is a first idea how we can use Panama Foreign to pass madvise() hints to the kernel when mapping memory segments.
The code looks up the function pointer from stdlib (libc) on Linux and Macos (untested, but should work) and then invokes madvise() for all MemorySegments we have mmapped when the following is true:

IOContext#readOnce is set
The chunk size is large enough (at least 8192 means chunkSizePower>=13) - this prevents TestMultiMMap from failing because for very small mappings (as done by this test), the FileChannel#map call will produce unaligned memory segments (it uses some tricks and maps larger segments and returns slices - which are no longer pageSize aligned)
There is a noop implementation doing nothing which is choosen if you disable native access

Interestingly it works without any extra parameters to command line (at least in Java 21).
This is a draft only to do some performance tests and extend the IOContext interpretation to try out more possibilities. The current "readOnce => MADV_SEQUENTIAL" is just an example as this is the main issue: We merge segments and don't want the soon to be trashed segments be sticky in RAM. MADV_SEQUENTIAL instructs kernel to forget about the mappings and also do readahead which helps during merging.

I remember this kind of things being discussed more than 10 years ago, it's extremely exciting to see it close to being included in the default Directory!

current "readOnce => MADV_SEQUENTIAL" is just an example as this is the main issue: We merge segments and don't want the soon to be trashed segments be sticky in

I believe that we don't use readOnce for merging, but for tiny metadata files that we load fully in memory. That said, I don't think that this should block this change, we can later look into the proper way to detect whether to pass MADV_SEQUENTIAL based on the IOContext.

I refactored the code a bit:

@jpountz I added the merge context (I checked DirectIODirectory).
I retrieve the pageSize to check if a segment is correctly aligned. Unfortunately this is mega-stupid. I tend to remove it again (the constant _SC_PAGESIZE is part of a C enum and its value is different in various platforms as no fixed value is assigned) and maybe keep a hardcoded page size to handle the case. Maybe just check that it is a multiple of 64K (native page size is 8K, but may be different), because it is normally only an issue in this test....

I removed the page size retrieval again. No need for it, we just be safe and only apply the advice, when the chunk size is large enough.

Unfortunately after the problems with other constants I tend to hide the advice constants in the class any use some abstraction, so we can call
We should maybe also simply remove the NoopNativeAccess and do a null check.

I refactored the code a bit:

The POSIX constants are now part of PosixNativeAcess class
The abstract method madvise() now only takes a MemorySegment and the IOContext. So it is now the hook for native implementations to implement the "advise" given by the IOContext for the platform (POSIX, Windows, Maybe special case for Linux which has more constants if you use plain low-level non-posix MADVISE
I removed OptionalInt: The values are all small and are in the valueOf cache, so we just return null when no madvice is needed

One thing: Should we remove the NoopNativeAccess class and just add a null check?

Thanks for the PR contributions. You may also commit directly to this branch, unless the change is a complete rewrite :-)

I took another run at static final ;-). https://github.com/uschindler/lucene/pull/5 (if we still don't want it, then I'll drop it)

One thing: Should we remove the NoopNativeAccess class and just add a null check?

Null is a little easier to reason about in the code at the use-site, since the reader is singled that the action may not happen (if null), rather than assuming the advise be given unconditionally.

It would be good if you always run tests with ./gradlew :lucene:core:test -Ptests.directory=MMapDirectory
This will use MMapDir for all tests. I have no macos machine, if somebody else could test the whole thing with macos (x86 and arm), it would be cool.
It would also be good to get a printout of the haeder file containing those POSIX_MADV_* constants. Just grep on POSIX_MADV_ through the include files (they are a bit hidden on macos, not in /usr/include, best is to search for unistd.h to find their location):
serv1-vm2:include jenkins$ xcrun --show-sdk-path
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk
serv1-vm2:~ jenkins$ cd /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include
serv1-vm2:include jenkins$ fgrep -R POSIX_MADV *
sys/mman.h:#define POSIX_MADV_NORMAL       0       /* [MC1] no further special treatment */
sys/mman.h:#define POSIX_MADV_RANDOM       1       /* [MC1] expect random page refs */
sys/mman.h:#define POSIX_MADV_SEQUENTIAL   2       /* [MC1] expect sequential page refs */
sys/mman.h:#define POSIX_MADV_WILLNEED     3       /* [MC1] will need these pages */
sys/mman.h:#define POSIX_MADV_DONTNEED     4       /* [MC1] dont need these pages */
sys/mman.h:#define MADV_NORMAL             POSIX_MADV_NORMAL
sys/mman.h:#define MADV_RANDOM             POSIX_MADV_RANDOM
sys/mman.h:#define MADV_SEQUENTIAL         POSIX_MADV_SEQUENTIAL
sys/mman.h:#define MADV_WILLNEED           POSIX_MADV_WILLNEED
sys/mman.h:#define MADV_DONTNEED           POSIX_MADV_DONTNEED
serv1-vm2:include jenkins$

One thing: Should we remove the NoopNativeAccess class and just add a null check?

Null is a little easier to reason about in the code at the use-site, since the reader is singled that the action may not happen (if null), rather than assuming the advise be given unconditionally.

So your recommendation is also to remove it. Will do later.

One thing: Should we remove the NoopNativeAccess class and just add a null check?

Null is a little easier to reason about in the code at the use-site, since the reader is singled that the action may not happen (if null), rather than assuming the advise be given unconditionally.

So your recommendation is also to remove it. Will do later.

I removed it and used an Optional (looks better than null check).

./gradlew :lucene:core:test -Ptests.directory=MMapDirectory

All tests pass on my Mac M2.

It is always a good idea to run one of those tests with -Ptests.verbose=true to see if the INFO mesage about madvise is enabled is printed on console:
thetaphi@serv1:~/repro/lucene$ ./gradlew :lucene:core:test --tests TestDirectoryReader -Ptests.directory=MMapDirectory -Ptests.verbose=true | grep madvise
  2> 信息: posix_madvise() available on this platform
WARNING: Using incubator modules: jdk.incubator.vector
WARNING: A command line option has enabled the Security Manager
WARNING: The Security Manager is deprecated and will be removed in a future release

..
It is always a good idea to run one of those tests with -Ptests.verbose=true to see if the INFO mesage about madvise is enabled is printed on console:
thetaphi@serv1:~/repro/lucene$ ./gradlew :lucene:core:test --tests TestDirectoryReader -Ptests.directory=MMapDirectory -Ptests.verbose=true | grep madvise
  ...

Yes that's better. I been just running a modified test locally where I eyeball the output. Here is the output on my machine (same was your output)
$ ./gradlew :lucene:core:test --tests TestDirectoryReader -Ptests.directory=MMapDirectory -Ptests.verbose=true | grep madvise
  2> INFO: posix_madvise() available on this platform
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
WARNING: Using incubator modules: jdk.incubator.vector
WARNING: A command line option has enabled the Security Manager
WARNING: The Security Manager is deprecated and will be removed in a future release

It looks like we are close for making this ready to be added to main branch. We can decide if we want to backport it (should not be too hard). If yes, I will only backport to Java 21 (not 20 or 19).
I will add some getter to MMapDirectory so tests (we should add a test that it is really enabled for Linux and Mac) or users can figure out if madvise is used. I would add public static boolean MMapDirectory#supportsMadvise() and use it in tests around MMapDir. Thoughts?

We can decide if we want to backport it (should not be too hard). If yes, I will only backport to Java 21 (not 20 or 19).

++  Java 21 is fine.

I will add some getter to MMapDirectory so tests (we should add a test that it is really enabled for Linux and Mac) or users can figure out if madvise is used. I would add public static boolean MMapDirectory#supportsMadvise() and use it in tests around MMapDir. Thoughts?

The platform may support madvise, but it is up to the calling code to set the IOContext appropriately for it to have any affect. I'm not sure what the user can do about any of this, other than log it? I wonder if it makes sense to add some short narrative to the IOContext javadoc - which would reference the new MMapDirectory#supportsMadvise.
We kinda need MMapDirectory#supportsMadvise to assert the output of some of the tests - I can only think of checking the log output? Then we just need a handful of new tests to check the various combinations of IOContext. Additionally, I assume we can have unit tests for NativeAccess directly. Lemme take a quick look at this.

... Additionally, I assume we can have unit tests for NativeAccess directly. Lemme take a quick look at this.

Dammit! I forgot about how our code is organised. Over in Elasticsearch we added version specific test sets for exactly this reason, so that test21 could depend upon java21 (which we call main21). For unit tests of NativeAccess, we would need similar.

... Additionally, I assume we can have unit tests for NativeAccess directly. Lemme take a quick look at this.

Dammit! I forgot about how our code is organised. Over in Elasticsearch we added version specific test sets for exactly this reason, so that test21 could depend upon java21 (which we call main21). For unit tests of NativeAccess, we would need similar.

When we implemented the MRJAR part for MMapDirectory and Vectorization, I said: Let's have no extra complexity with tests and just indirectly test the code. For MMapDirectory this worked fine, because when the test passes, the implementation of the IndexInput is fine.
For vectorization its similar, we have a scalar and a vectorized impl that is compared by tests => also fine.
I agree, here it triggers platform specific madvise(). We may add mocks for testing, but I am not sure if it is worth. Maybe the easiest would be to extend IOContext at a later time to return some madvise-like enum as additional abstraction., so the native code just gets a MAdvice enum constant and maps it straight to integers in a simple Map<MAdvice,Integer>.

I'm curious if you've already thought of how we could update the IOContext on our merge instances. If I'm not mistaken, given how reader pooling works in IndexWriter, if you use a SegmentReader for search and then the same segment is included in a merge, then this SegmentReader will not use an IOContext.Context.MERGE context but a IOContext.Context.READ context (because it's reusing the first reader that was created). It would be nice if our file format merge instances (e.g. StoredFieldsReader#getMergeInstance) could somehow use a IOContext.Context.MERGE even though they might be cloned from a reader that was created with a IOContext.Context.READ context. I don't think it should block this change, it's a good step in the right direction, this other problem is quite orthogonal.

I added a basic test that verifies that we have madvise support enabled on Linux and Macos. This also added
One question: In Lucene 9.x we had some system properties to disable MemorySegments. I think in main barnch we can have all of this by default enabled.
But when we backport this to 9.x, I tend to add another system property to disable this feature.

I agree, here it triggers platform specific madvise(). We may add mocks for testing, but I am not sure if it is worth. Maybe the easiest would be to extend IOContext at a later time to return some madvise-like enum as additional abstraction., so the native code just gets a MAdvice enum constant and maps it straight to integers in a simple Map<MAdvice,Integer>.

We might be saying the same thing, but I really just wanna get a the Optional<NativeAccess> in a unit test and, where present, so as to exercise it's functionalist directly. For now, and it's perfectly fine here, we can test indirectly once we know the various different ways to exercise the code paths. I pushed a trivial test that at least exercises POSIX_MADV_RANDOM.

But when we backport this to 9.x, I tend to add another system property to disable this feature.

Why is this extra property needed? And not just enable madvise for the MemorySegmentIndexInput on Java 21+ unconditionally? Maybe I'm missing something obvious?

Do we have any benchmarks by Mike? Some on memory restricted devices. An idea would be a limited VM or by using the memory hog in Mike's benchmark.

It's not the fault of the PR, but it's almost impossible for me to follow what-happens-when thru the iocontext. Maybe we can overhaul it in trunk... I don't think we should add our own layer of abstraction here (I know that's "java"-style to do but it's wrong). It would be clearer if we just used madvise semantics directly: then you could see MADV_RANDOM from the code opening the file and understand what happens.

It's not the fault of the PR, but it's almost impossible for me to follow what-happens-when thru the iocontext. Maybe we can overhaul it in trunk... I don't think we should add our own layer of abstraction here (I know that's "java"-style to do but it's wrong). It would be clearer if we just used madvise semantics directly: then you could see MADV_RANDOM from the code opening the file and understand what happens.

Please do the change to IOContext separately. We need an abstraction anyways, because not all platforms have same constants. We may add another layer between IOContext and madvise, but that's out of the scope of this issue.

We need an abstraction anyways, because not all platforms have same constants.

Things can have the same names at least.

Let's meet in the center:

I will add a public enum PosixMFAdvise to store package with enum constants named as the POSIX_MADV / POSIX_FADV constants (they are same, just different numbers)
IOContext becomes a toPosixMFAdvise() method that converts it into that enum (in a similar way like the previously mentioned private/internal method).
MMapDirectory by defaults uses this method to interpret the IOContext. As it's a Function<IOContext,PosixMFAdvise> you can configure it like the other MMapDirectoy. This allows us to play around in benchmarks and also allows to disable it completely.
Inside the private PosixNativeAccess class we just convert the enum to the low-level integer value.

I can do this later. OK?

At a later stage we can ...
I can do this later. OK?

This seems quite reasonable to me. Native access should be as close to a wrapper around the native functions as possible, so should look and feel familiar to the reader of the code.
As it stands this PR is a significant step forward, we should merge it as is and do further refactoring incrementally.

I agree, wasn't trying to hold up the change, was just thinking out loud!

Maybe it is easier to see results on benchmarking when it is in main branch. I am waiting for final review by @jpountz and then merge this. Backporting to 9.x is also planned and should be done before #13205 is applied.
I will merge this tomorrow afternoon CET.

@jpountz Are you fine with merging?

The test added by @ChrisHegarty sometimes fails on windows: It does not close the file it opened for random access testing, so the directory can't be deleted. Will fix this in a separate commit.

I fixed the test in https://github.com/apache/lucene/commit/ae5d3534e3ef44ff3336dea0308d8a82f0672ff2

I also removed the extra logging included while development from the main branch. In 9.x the log message was adapted to list both features together with the sysprop to disable).

The test added by @ChrisHegarty sometimes fails on windows: It does not close the file it opened for random access testing, so the directory can't be deleted. Will fix this in a separate commit.

Oops, sorry about this. Thankfully the fix was straightforward.

The test added by @ChrisHegarty sometimes fails on windows: It does not close the file it opened for random access testing, so the directory can't be deleted. Will fix this in a separate commit.

Oops, sorry about this. Thankfully the fix was straightforward.

About this test and the quite large 8 MB buffer (see https://github.com/apache/lucene/pull/13196/files#diff-ebee319f3691cdb1627e5e9b1dfbdd0c266b1da28ccf0b2a9218dee1d34ff2b7R103):
Is this some limit inside the kernel to trigger something? For this test, we call posix_madvise() always.
Maybe there was a misunderstanding: We only do not call madvise, if the chunk size is too small (< 2 MiB), but by default the chunk size is 16 Gigabytes, except for tests we always call madvise. Theres only one special case: If the file is zero length, then we can't call it.

I dunno what I was thinking, this is clearly not correct. I opened #13214 to fix the test. ( apologies for the stupid test issues! )

@uschindler Should we open a separate issue for adding fadvise support to NIOFSDirectory?

Unfortunately fadvise is at moment close to impossible. Reason: we have no file handle!
Chances are good that we also get a Java-based fadvise some time in the future (e.g., through an OpenOption like with O_DIRECT).

As disussed before, for implementing fadvise for reading/writing files, we would need to write a full stack of IO layer natively (OutputStream for writing and FileChannel for NIOFSDir). See https://bugs.openjdk.org/browse/JDK-8292771

Anyways we can open an issue to track what's going on on the JDK (listing all relevant issue numbers like the above one).

As disussed before, for implementing fadvise for reading/writing files, we would need to write a full stack of IO layer natively (OutputStream for writing and FileChannel for NIOFSDir). See https://bugs.openjdk.org/browse/JDK-8292771

Here is the posix_fadvise specific issue.


Style Bookstore

Install react icons
Style all pages
Add validation to form
Style form
fix errors

Thanks for the review.


Use webhooks to reflect irc usernames + avatars

This fixes #128
The discord bot gets renamed with the irc nickname, and use the account avatar if there's a matching username in the room.

Coverage decreased (-3.3%) to 95.076% when pulling 2c05038c373df3704e5c3748d99ba51459a343e9 on Fiaxhs:master into fe37ec48a4129ea215c74fac7e2dfe0b7a0353c3 on reactiflux:master.

Coverage decreased (-3.3%) to 95.057% when pulling b4452dd4874e99d4ef60935cd9b9d32e0b5b7998 on Fiaxhs:master into fe37ec48a4129ea215c74fac7e2dfe0b7a0353c3 on reactiflux:master.

Could you add tests for getDiscordAvatar and findWebhook, as well as ensuring that it uses a webhook in sendToDiscord when one is available (and then does not go on to send a regular message)? I can probably advise on how to do this if you're unsure.

Sure! I'll do this soon.

Yes, I'm a bit short on free time these days. I'll try to produce something
this week-end :)
On 16 May 2017 at 20:01, Edward Jones<EMAIL_ADDRESS>wrote:

@Throne3d requested changes on this pull request.
Bump on tests?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/reactiflux/discord-irc/pull/230#pullrequestreview-38466086,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AArNkT1dnEipdy1wkc2PF_PymWZs83xFks5r6eRngaJpZM4NN89p
.

Coverage increased (+0.1%) to 98.582% when pulling 2bf761b16318e37c90b87cde10ca516b3a6181c5 on Fiaxhs:master into 8c4d83a37dccc4e5794ef1a8a62f7e80648cc89b on reactiflux:master.

Finally got some time to write tests, sorry for the delay :)

I would like to add this as a warning if this gets pushed to master.
https://trello.com/c/6ZZTEBJw/278-webhook-incorrectly-stacks-message-authors-on-android-client
So basically currently this wont be really readable on android client because of message merging under one author incorrectly.

Aw man, you already made it work with latest commit. Oh well. I'll delete my fork now.

Testing this locally, it seems to work well, and the tests added should help to ensure it keeps this functionality.
Assuming @ekmartin is okay with it, I think this is good to go.
(I'm slightly worried about IRC users masquerading as authenticated Discord users by changing to their usernames, but I don't think this is readily fixable, and the messages have 'BOT' marked on them, and it presents a problem on the IRC end as is anyway, so this shouldn't actually make it much if at all worse.)

Just added a note on webhooks. However feel free to change it, even the image. I feel quite uncomfortable writing the readme for a project that isn't mine!

Coverage increased (+0.1%) to 98.582% when pulling 8c0c11a2b1a5ed532de2a27b50d4ffb340a93160 on Fiaxhs:master into 8c4d83a37dccc4e5794ef1a8a62f7e80648cc89b on reactiflux:master.

That looks like an accurate and useful summary, so it should be fine! Thanks.

@Fiaxhs Can you make this case-insensitive?

(Sorry I didn't notice this before.)

Hey @Fiaxhs, can you update this to the latest commit?

Yes, I'll update my PR.
Also, I'm changing the way I match IRC nicknames and users nicknames, didn't fully understood the concept of guilds at the time.

Coverage decreased (-0.6%) to 98.667% when pulling 4facb56ebb5720b8f28cfab9d0d808bcf32339ff on Fiaxhs:master into ddd9c54645dbf6d85b3bb1893cc86786135662cd on reactiflux:master.

Coverage increased (+0.08%) to 99.333% when pulling 8a23a9193159b6167185af892e8e25ddfeab38cf on Fiaxhs:master into ddd9c54645dbf6d85b3bb1893cc86786135662cd on reactiflux:master.

The changes look good (the code, that is) but I can't test it on an actual Discord server right now. @ekmartin, opinions?

Oh right, I forgot to test it on my server, was relying on tests solely for
today's commits. I'll check asap (this weekend).
On Fri, 28 Jul 2017, 18:35 Edward Jones<EMAIL_ADDRESS>wrote:

The changes look good (the code, that is) but I can't test it on an actual
Discord server right now. @ekmartin https://github.com/ekmartin,
opinions?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/reactiflux/discord-irc/pull/230#issuecomment-318701740,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AArNkZeG19EhA-lTsbzq1Q5XrG48VP6aks5sSg2zgaJpZM4NN89p
.

Coverage increased (+0.08%) to 99.333% when pulling e8c96e2a4477551659b8e9ef23da61726547b9ce on Fiaxhs:master into ddd9c54645dbf6d85b3bb1893cc86786135662cd on reactiflux:master.

[Aside: as a note for future, this is going to behave weirdly with #277, probably by sending actions without any formatting if they don't conflict. When they're both merged I think the code should be modified – probably in whichever PR is outstanding last – to wrap text like _${text}_ if webhooks are in use; that's how Discord handles /me actions from Discord users and the webhook message functionality seems designed to make it look as much like a regular user as possible.]
Could you edit the tests to use addUser instead of setting the guild members directly like you have? This will fix the weird <@undefined> thing by converting it to an ID, as well as (hopefully) ensuring if we modify the code later to retrieve the Discord user elsewhere it will be less likely to break tests. (The reason I use different IDs is so they don't overlap in these tests, in case there's some erroneous thing relating to a particular ID.)
I can't seem to figure out how to push to the PR myself, if that's in fact enabled, which is why I haven't added the commit already. Here's the patch for how I'd do it, which seems to pass tests:
diff --git a/test/bot.test.js b/test/bot.test.js
index 9da4d54..b607a2f 100644
--- a/test/bot.test.js
+++ b/test/bot.test.js
@@ -836,37 +836,42 @@ describe('Bot', function () {
     const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
     const bot = new Bot(newConfig);
     bot.connect();
-    const user = new discord.User(this.bot.discord, { username: 'Nick', avatar: 'avatarURL' });
-    this.guild.members.set(1, { user, nickname: 'Different' });
-    this.bot.getDiscordAvatar('Nick', '#irc').should.equal('/avatars/<@undefined>/avatarURL.png?size=2048');
+    const userObj = { id: 123, username: 'Nick', avatar: 'avatarURL' };
+    const memberObj = { nickname: 'Different' };
+    this.addUser(userObj, memberObj);
+    this.bot.getDiscordAvatar('Nick', '#irc').should.equal('/avatars/123/avatarURL.png?size=2048');
   });

   it('should find a matching username, case insensitive, when looking for an avatar', function () {
     const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
     const bot = new Bot(newConfig);
     bot.connect();
-    const user = new discord.User(this.bot.discord, { username: 'nick', avatar: 'avatarURL' });
-    this.guild.members.set(1, { user, nickname: 'Different' });
-    this.bot.getDiscordAvatar('Nick', '#irc').should.equal('/avatars/<@undefined>/avatarURL.png?size=2048');
+    const userObj = { id: 124, username: 'nick', avatar: 'avatarURL' };
+    const memberObj = { nickname: 'Different' };
+    this.addUser(userObj, memberObj);
+    this.bot.getDiscordAvatar('Nick', '#irc').should.equal('/avatars/124/avatarURL.png?size=2048');
   });

   it('should find a matching nickname, case sensitive, when looking for an avatar', function () {
     const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
     const bot = new Bot(newConfig);
     bot.connect();
-    const user = new discord.User(this.bot.discord, { username: 'Nick', avatar: 'avatarURL' });
-    this.guild.members.set(1, { user, nickname: 'Different' });
-    this.bot.getDiscordAvatar('Different', '#irc').should.equal('/avatars/<@undefined>/avatarURL.png?size=2048');
+    const userObj = { id: 125, username: 'Nick', avatar: 'avatarURL' };
+    const memberObj = { nickname: 'Different' };
+    this.addUser(userObj, memberObj);
+    this.bot.getDiscordAvatar('Different', '#irc').should.equal('/avatars/125/avatarURL.png?size=2048');
   });

   it('should not return an avatar with two matching usernames when looking for an avatar', function () {
     const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
     const bot = new Bot(newConfig);
     bot.connect();
-    const user1 = new discord.User(this.bot.discord, { username: 'common', avatar: 'avatarURL' });
-    const user2 = new discord.User(this.bot.discord, { username: 'Nick', avatar: 'avatarURL' });
-    this.guild.members.set(1, { user: user1, nickname: 'Different' });
-    this.guild.members.set(2, { user: user2, nickname: 'common' });
+    const userObj1 = { id: 126, username: 'common', avatar: 'avatarURL' };
+    const userObj2 = { id: 127, username: 'Nick', avatar: 'avatarURL' };
+    const memberObj1 = { nickname: 'Different' };
+    const memberObj2 = { nickname: 'common' };
+    this.addUser(userObj1, memberObj1);
+    this.addUser(userObj2, memberObj2);
     chai.should().equal(this.bot.getDiscordAvatar('common', '#irc'), null);
   });
 });

As well as this, could you make sure that it doesn't crash when a user doesn't have a nickname (I just found it to do that when I tested it and there was no match for an IRC user):
diff --git a/lib/bot.js b/lib/bot.js
index 1388665..233d9a7 100644
--- a/lib/bot.js
+++ b/lib/bot.js
@@ -367,7 +367,7 @@ class Bot {
         }
         const nickLowerCase = nick.toLowerCase();
         return member.user.username.toLowerCase() === nickLowerCase
-          || member.nickname.toLowerCase() === nickLowerCase;
+          || (member.nickname && member.nickname.toLowerCase() === nickLowerCase);
       };

     // Try to find exact matching case
diff --git a/test/bot.test.js b/test/bot.test.js
index b607a2f..f7c5351 100644
--- a/test/bot.test.js
+++ b/test/bot.test.js
@@ -874,4 +874,17 @@ describe('Bot', function () {
     this.addUser(userObj2, memberObj2);
     chai.should().equal(this.bot.getDiscordAvatar('common', '#irc'), null);
   });
+
+  it('should not return an avatar when no users match and should handle lack of nickname, when looking for an avatar', function () {
+    const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
+    const bot = new Bot(newConfig);
+    bot.connect();
+    const userObj1 = { id: 128, username: 'common', avatar: 'avatarURL' };
+    const userObj2 = { id: 129, username: 'Nick', avatar: 'avatarURL' };
+    const memberObj1 = {};
+    const memberObj2 = { nickname: 'common' };
+    this.addUser(userObj1, memberObj1);
+    this.addUser(userObj2, memberObj2);
+    chai.should().equal(this.bot.getDiscordAvatar('nonexistent', '#irc'), null);
+  });
 });

After applying these two (well, mainly the latter), it seems to work quite nicely. Thanks for your contribution!

Coverage increased (+0.08%) to 99.333% when pulling ff788083d1cdaf7030aaf5ee9f89b2b442e1019e on Fiaxhs:master into ddd9c54645dbf6d85b3bb1893cc86786135662cd on reactiflux:master.

This looks good to go now. @ekmartin?

Nice! I didn't see the addUser function, this would have save me some time.

I keep finding things to be picky about

Fine by me :)

Thanks for your contribution!

 I use it every day, for once I could bring some value to a project I use.

Coverage increased (+0.07%) to 99.327% when pulling 549b8fb7679fea11f96d23eb82645eb741c0d620 on Fiaxhs:master into ddd9c54645dbf6d85b3bb1893cc86786135662cd on reactiflux:master.

What if we fall back to adorable.io avatars?
See https://github.com/ekmartin/slack-irc/ and https://github.com/qaisjp/go-discord-irc/commit/859348452f1bb5a47f6189064b5c79ef782e9776

Also, please send feedback to Discord asking them to put some focus on this! https://trello.com/c/6ZZTEBJw
@Kakkela's https://github.com/reactiflux/discord-irc/pull/230#issuecomment-304867667 shows a major block

@qiasjp
I'm not sure there's a need to fallback to specific avatars – if the users haven't got Discord accounts with avatars, then they haven't chosen an avatar, so it seems sensible to stick to what Discord does by default? (That is, the standard game controller picture.)
I agree that there should be a warning about the bug when this gets merged. That's a pretty big issue on Discord's end. I don't think it should be a block to us merging this, but instead added as a clear warning in the relevant config area, so users of the individual servers can choose whether to enable it despite this in the meantime (or not).

@Throne3d yes but the Discord default avatars have a bit of variety. the webhook currently has the same default avatar across usernames.
fyi the only reason I suggested it is because @ekmartin's slack bridge supports it

Also, something to mention to people reading this issue:
A nice workaround for the Android/web bug is to alternate the webhook per user
(This wouldn't be suitable for reactiflux/discord-irc, but my purpose build discord-irc bridge bot will do this)

Hey, I have a recommendation for this.
Basically, make the bot have a little command where if you type !ban {user} on discord, you can ban them on IRC.
And how this works? Well, it basically detects the rank that the user who sent it has and checks if he has the "Ban users" perm. If he has it, and the bot on IRC has the perm to do so as well, it'll ban the user off IRC from Discord.

Webhook merging bug on Android is now moved to "Claimed fixed, needs verification"
I'm going to test on my bridge today or tomorrow. Not sure if it's fixed on just Alpha channel on Android or what.
https://trello.com/c/6ZZTEBJw/278-webhook-incorrectly-stacks-message-authors-on-android-client

Omg excited. Please keep us updated :)
On Wed, 21 Mar 2018, 10:29 pm Iku<EMAIL_ADDRESS>wrote:

Webhook merging bug on Android is now moved to "Claimed fixed, needs
verification"
I'm going to test on my bridge today or tomorrow. Not sure if it's fixed
on just Alpha channel on Android or what.
https://trello.com/c/6ZZTEBJw/278-webhook-incorrectly-stacks-message-authors-on-android-client
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/reactiflux/discord-irc/pull/230#issuecomment-375116840,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4WasiBpdYzaXZsPzjqPNtLG0Lt89sUks5tgtQwgaJpZM4NN89p
.

Yes this does now indeed work on Android (all channels) as intended and no unnecessary merging happens.

And it's fixed on web too!!
On Thu, 22 Mar 2018, 9:36 am Iku<EMAIL_ADDRESS>wrote:

Yes this does now indeed work on Android (all channels) as intended and no
unnecessary merging happens.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/reactiflux/discord-irc/pull/230#issuecomment-375234121,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4Wahd1WFQzo45R210JnMnMbLjBiy6Zks5tg3CNgaJpZM4NN89p
.

Published in version 2.6.0 of discord-irc - thank you for your patience!
I wonder if the usernames on the Discord side should be suffixed with (IRC) to make what's happening clearer though? If you think that's a good idea we can publish a new version later.

Side note: discord webhooks have a minimum name length of 2 characters, and
having a suffix of (IRC) would be an easy countermeasure.
On Thu, 22 Mar 2018 at 14:34 Martin Ek<EMAIL_ADDRESS>wrote:

Published in version 2.6.0
https://github.com/reactiflux/discord-irc/blob/master/CHANGELOG.md#260---2018-03-22
of discord-irc - thank you for your patience!
I wonder if the usernames on the Discord side should be suffixed with
(IRC) to make what's happening clearer though? If you think that's a good
idea we can publish a new version later.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/reactiflux/discord-irc/pull/230#issuecomment-375328212,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4WalOpkW_cP004I-c5nKEfn50ToSWgks5tg7aOgaJpZM4NN89p
.

Opened an issue for it: #377 - PRs welcome!

Thank you for this update!


Add mentions auto-completion

As part of the mentions project, we will be adding auto-completion for mentions by using the @ symbol. We should break this up into at least two PRs:

Adding the generic AutoCompleteSuggestions component and refactoring the existing EmojiSuggestions to use the new AutoCompleteSuggestions component
Adding MentionAutoSuggestions which will also use the AutoCompleteSuggestions component

The details of the implementation are described in the doc here.

Current assignee @puneetlath is eligible for the NewFeature assigner, not assigning anyone new.

📣 @szebniok! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

PR 1 is up!

@0xmiroslav sent you an offer in Upwork: https://www.upwork.com/nx/wm/pre-hire/c/8577561/offer/24557708

All paid. Thanks again everyone!

Ahhh sorry. I see the mistake. Sent you another contract.


Introduce types.raw and types.unconditional

Motivation for this change
This PR contains a number of changes which are related or weakly dependent on each other:

Fixes the emptyValue of types.listOf, which should be [] not {}
For options that don't have any definitions, but whose type has an emptyValue, the emptyValue is used instead of throwing an error. Types who have an emptyValue:

nullOr: null
attrsOf: {}
listOf: []
submodule: {}
And wrapper types of the above
This means that it's not necessary anymore to define default = {} for submodules or attrsOf to make it work without any definitions.

Introduce types.raw as a type that doesn't do any nested processing for mkIf, mkForce, etc. on its values. This is useful when this processing would throw errors or where it would be too expensive (such as with package sets like nixpkgs).
Use types.raw for _module.args's type. This finally fixes https://github.com/NixOS/nixpkgs/issues/53458
Introduce types.unconditional <elemType> as a type, which when used as e.g. attrsOf (unconditional str) can replace lazyAttrsOf str. This type ensures that conditional mkIf definitions can't influence whether the option is defined or not, and therefore make the attribute set values be evaluated lazily. This notably also works with listOf (unconditional str), making the list elements evaluated lazily, which was not possible before.

This is probably the last piece needed before we can finally deprecate types.attrs and types.unspecified.
Ping @roberth @nbp @danbst @sphaugh
Things done

[x] Write tests

[x] For emptyValue's being used
[x] For types.raw
[x] For types.unconditional <elemType>

[x] Write docs

[x] For types.raw
[x] For types.unconditional <elemType>

unconditional is neat!
However, I don't think pkgs is a valid use case for raw and I don't know what would be.
The crux of it is that "attrsets and lists can be big" is not a good intuition. Let me explain.
The thing is, you can always insert a minimal check. For an option to make sense it must be used in some way. The minimal way of using a thunk is to evaluate it to weak head normal form (whnf)*. This means that we always have the opportunity to put a check in front of the config's thunk, so that the check evaluates it to whnf and shallowly checks the type before returning it to whatever needed the value. The overhead of this is absolutely tiny compared to the rest of the module system and does not increase with value size, no more of the value is evaluated than would be without the check.
Of course this is only possible because of the fact that definitions are only checked when used. That doesn't usually feel like a relevant property and there's something unpleasant about it, but it works out rather well.
Another thing to note is that whnf doesn't always give you a lot of information, but discerning between types like int, list, attrset, string, bool is possible and useful.
I guess another way to put it is that checking whnf only doesn't increase strictness and is therefore of constant time (and small, because what can you really do).

I don't think whnf has been defined anywhere for Nix, but the concept applies. It's essentially Expr::forceValue; the minimal amount of work before you can get any information out of it that is not an implementation detail.

@roberth Good point. But doing such an extra check can be done by types.addCheck, and types.raw can be the used as its base type.
However, after thinking about it, I think you're right, we don't need types.raw. The real motivation was to be able to replace all those types.attrs and types.unspecified with something better (since those two have weird merging behavior). However as you say, we need to evaluate values at least to WNHF, which for atomic values like ints/strings/bools/etc means evaluating the whole thing.
The only place we could therefore use such a types.raw is for attrsOf raw and listOf raw. However, instead of using types.raw for this, we should instead implement it as an unchecked types.attrs/types.list, which explicitly only checks for isAttrs/isList, without making any attempts at merging multiple definitions of the same attribute. This then doesn't need to involve the module system really and can be implemented via the type interface.
Or maybe we should still just have types.raw, but we don't need to do the whole "not processing any mkIf's, etc." involving the module system. Instead we can simply implement it defining a new type with merge = mergeOneOption. Additional checks, e.g. for it being an attribute set, can be added with types.addCheck. Or alternatively types.attrsOf types.raw works too.

I guess there's one potential use case of a types.raw that doesn't do any processing at all: If the value of an option has the _type attribute set to something that messes up the module system. Because it uses _type = "if" for mkIf's, and so on. But that's really very niche, I don't expect anybody to run into this.

Updated to resolve merge conflicts after https://github.com/NixOS/nixpkgs/pull/148315

This PR is a bit too convoluted with too many separate changes. I split off the emptyValue fixes into https://github.com/NixOS/nixpkgs/pull/160487
I also thought about it some more: Making options default to the emptyValue of their type is a bit weird and might break some assumptions. Specifically currently there's the assumption that users get an error when a type is not set but it's used, which would kind of not be a thing anymore for values with such a default. So I'm not doing that in the above PR

Regarding types.raw, this is now split off into https://github.com/NixOS/nixpkgs/pull/160489

Finally, the types.unconditional has been split off into https://github.com/NixOS/nixpkgs/pull/160491


CAMEL-7672 copy message headers that do not begin with 'camel' into STOMP frame headers

CAMEL-7672 copy message headers that do not begin with 'camel' into STOMP frame headers

Any chance you got time to work on this again? If not you are welcome to close this, so others may pickup the work.


Add LRU cache to web3 provider

Where does this come from and why do we trust it to work without unit tests?

The cache is already used within Gashawk and tested there

Where does this come from and why do we trust it to work without unit tests?

The cache is already used within Gashawk and tested there

Why not take the tests from there and using them here, too? Also, adding a reference to the gashawk repo in comments would be a good idea.


[incubator/zookeeper] Add Prometheus Operator PrometheusRule support

What this PR does / why we need it:
Adds Prometheus Operator PrometheusRule support for deploying of rules/alerts alongside Zookeeper itself.
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

[x] DCO signed
[x] Chart Version bumped
[x] Title of the PR starts with chart name (e.g. [stable/mychartname])

/assign @lachie83

/remove-lifecycle stale

/remove-lifecycle stale


Limit plugin

Motivation for changes:
We needed a way to limit entries created by trakt_list plugin #2386, #2388. We added a limit option to that plugin. This is an experiment to see if we could use a more generic approach to limit any input plugin, while still allowing the optimization of not having the input plugin request more than is necessary from its source. (i.e. limiting HTTP or database requests)
Detailed changes:

Allows input plugins to be generators
Adds 'limit' plugin, which takes a limit amount, as well as the configuration for another input plugin

Config usage if relevant (new plugin or updated schema):
limit:
  amount: 5
  from:
    rss: http://blah.com

To Do:

[x] Verify whether we like this idea
[x] Convert input plugins which might take a long time to get many entries to generators
[x] Deprecate limit options from plugins that implemented themselves (T411, Letterboxd)
[ ] Figure out how to best handle @cached decorator with generator inputs

Looks like we like this, so I fixed up a few more things:

Fixed the key for the subplugin to be called 'from'
Switched over any input plugins I saw that were fetching successive pages to be generators
Converted 'inputs' and 'sequence' plugins to both pass through generators

@paranoidi @andrey-yantsen So, now the question is what to do with trakt_list plugin. Do we keep the limit option in there, or remove that again in favor of this? Do we keep the default limit for trakt_list, or maybe have a log message that recommends using this plugin if the number of entries is bigger than a certain amount?

I suggeest removing the limit from trakt_list, and adding a check to an "upper" level, and if any input plugin except limit produces more than, let's say, 1000 items — send a notification to the log. Therefore we'll get the code with less cohesion level, where input plugins just produce items and don't care about amounts.

BTW, feel free to steal my modified trakt_list from #2388, and convert it to the generator :)

Shouldn't we deprecate max_results from Letterboxd and T411 as well?

@liiight limit_new is a bit different, that just limits the amount of accepted entries per run, and saves the rest for later runs
@andrey-yantsen You are right, those should probably be deprecated with this

Hmm, things to figure out:

What the hell changed about trakt_list_add test that it doesn't work anymore.
How to handle our @cached input plugin decorator with generator style inputs. In memory persistence should be really easy, we can just use itertools.tee and store one copy of the resultant generator in cache. DB persistence on the other hand....

What the hell changed about trakt_list_add test that it doesn't work anymore.

Basically, you've fixed TraktSet.clear() method and now tests are making one request less :) previously it was populating items as the first step of the test, when the object is empty. Check out my latest commit to #2388.

What the hell changed about trakt_list_add test that it doesn't work anymore.

Basically, you've fixed TraktSet.clear() method and now tests are making one request less :) previously it was populating items as the first step of the test, when the object is empty. Check out my latest commit to #2388.

Oh awesome, thanks for the investigation. :)

@andrey-yantsen Is t411 a plugin anymore? I don't see it anywhere.

Probably no - by some reason I’ve searched in wiki, not in github.

t411 as a site does not exist anymore

from_digest plugin has a limit as well: https://github.com/Flexget/Flexget/blob/a4037da07f7d7707650596670c467ab3b7e748c9/flexget/plugins/operate/digest.py#L100

Okay, I've modified the input caching to work with these new generator inputs. I think it's probably fine, but some feedback would be appreciated. Basic gist of how it works:

Entries don't get cached to memory until they are iterated over. The memory cache also stores the generator though, so another task can iterate farther if the first task limited the input.
Entries don't get cached to database at all, unless every entry is iterated over, since we can't cache the partially exhausted generator to db along with the partial entry list.

In practice, none of this probably matters, it's only protecting against the case of 2 tasks using the same input, but limited to a different quantity of entries (or increasing the limit number between runs while entries are cached to db.)

Well, I haven't thought of any better ways to handle @cached decorator, so I've become a bit more confident on the solution I implemented. I think we are probably good to go on this PR unless anyone else has something.

Going with it.


Training loss not going down at all!

I am training the following model (code is here),
I have a input sequence (X) and its corresponding sequence (Y). You can think of X being a question and Y being its correct answer. I am trying to learn a similarity function F such that F(X,Y)=1 and F(X,Y')=0.
My training input is X concatenated with Y and my target output is 1/0 (0 is for negative examples). I generate k negative examples by picking a random Y' to concat with X (with a delimiter separating them). So my input looks something like (I pre-pad X with 0s and post-pad Y with 0s to ensure length = xmaxlen + ymaxlen + 1 for all examples),
[...0 0 0 0 634.  1759.   890.  1289.   255.  ... 1.  2164.  1746. 0 0 0 ..]--> [1.0]
[...0 0 0 0 634.  1759.   890.  1289.   255.  ... 1.  2141.  1733. 0 0 0 ..]--> [0.0]
[...0 0 0 0 634.  1759.   890.  1289.   255.  ... 1.  2111.  1033. 0 0 0 ..]--> [0.0]
...
[...0 0 0 64.  179.   89.  128.   25.  ... 1.  216.  174....0 0 0 ]--> [1.0]
[...0 0 0 64.  179.   89.  128.   25.  ... 1.  210.  118....0 0 0 ]--> [0.0]
...

I plug in both sequences into a bidirectional LSTM and get its last state's output to a softmax which predicts the label.
    def get_H_n(X):
        ans=X[:, -1, :]  # get last element from time dim
        return ans

    model.add_input(name='input', input_shape=(N,), dtype=int)
    model.add_node(Embedding(options.max_features, options.wx_emb, input_length=N), name='emb',
                   input='input')
    model.add_node(LSTM(options.lstm_units, return_sequences=True), name='forward', input='emb')
    model.add_node(LSTM(options.lstm_units, return_sequences=True, go_backwards=True), name='backward', input='emb')

    model.add_node(Dropout(0.5), name='dropout', inputs=['forward','backward'])
    model.add_node(Lambda(get_H_n, output_shape=(k,)), name='h_n', input='dropout')
    model.add_node(Dense(1, activation='softmax'), name='out', input='h_n')
    model.add_output(name='output', input='out')

The model compiles and trains, but nothing changes at all!
Epoch 1/10
1094/1094 [==============================] - 3s - loss: 14.4560 - val_loss: 14.4409
Epoch 2/10
1094/1094 [==============================] - 3s - loss: 14.4560 - val_loss: 14.4409
Epoch 3/10
1094/1094 [==============================] - 4s - loss: 14.4560 - val_loss: 14.4409
Epoch 4/10
1094/1094 [==============================] - 3s - loss: 14.4560 - val_loss: 14.4409
Epoch 5/10

Things I have tried to debug,

Lower learning rate.
Check if data manipulation is sane.
Chop input to avg xlen and avg ylen
Print the output of embedding layers (look alright).
Tried custom embedding layer which drives 0 to 0-vector (from #1800).
Print softmax weights (bias is always 0, but the W is non-zero).

Why is this happening? Any suggestion is welcome!
Please make sure that the boxes below are checked before you submit your issue. Thank you!

[x] Check that you are up-to-date with the master branch of Keras. You can update with:
pip install git+git://github.com/fchollet/keras.git --upgrade --no-deps

[x] If running on Theano, check that you are up-to-date with the master branch of Theano. You can update with:
pip install git+git://github.com/Theano/Theano.git --upgrade --no-deps

[x] Provide a link to a GitHub Gist of a Python script that can reproduce your issue (or just copy the script here if it is short).

 I introduced this bug when changing from sigmoid to softmax, but forgot to change the output layer size.

@shyamupa When i collect 1505 numbers pics to classify, the valid_acc doesn't change.  Thanks.


GCM or APNS Support

If I recall, GCM and APNS (Apple and Android Push) can be sent in a similar fashion as an email. Simply requires additional certificates to be generated. Does it make sense for such a feature to exist in this app? and possibly even TXT message sending support too?

Re push notifications, Postal is designed to send emails so if the devices that receive the messages are capable of understanding an email format then it might work but I'd imagine there are some specifics that might require you to send messages with the raw API endpoint, assuming the messages complied with RFC2822 or make significant changes to how Postal handles messages.
I knew that some providers let you send SMS messages via email so in theory this would still work with Postal but I'm not sure how many of them still do.
In both cases, you would be infinitely better off with a software solution that was designed to carry out these functions from the ground up.

We don't have any plans to add this to Postal. We do actually have proxy apps for APNS & GCM which we use internally.

https://github.com/adamcooke/apns-proxy
https://github.com/atech/gcm-proxy


[Request] Export video with overlay already applied?

As there isn't a request section I guess it has to go here. I'm wondering if there are any plans to make it so you can export a video with the FPS analysis overlayed to the video. Otherwise the only way to do things is the long way either through video editing or capturing the capture as the software exports... both of which are kinda a pain in the arse.

you can already do this, export as jpeg and make sure "Export as overlay" is not enabled, use your editing software or ffmpeg to create a video out of the jpegs

Thanks but that's not quite what I'm asking, having to run a video though several pieces of software kinda defeats the purpose of TrDrop as a tool. I'm asking if there's an option I've missed or a planned option to enable TrDrop to directly export videos with the framerate analysis already overlayed ;) Having to individually generate over 9000 images (in the case of my test video of 6 mins) then having to use another piece of software to stitch everything together is a very long winded and messy way to do things :(

Yes I understand, I have it scripted. When te output is ready I just run the script (ffmpeg) and make a prores with alpha video that I can just use as overlay (I export it as .tiff). You could do the same with the jpegs and have it stitched together as mp4. Still not what you really want but something like this might sooth the pain.

Thanks for the follow-up, sadly, it looks like I'm going to have to keep searching which is a real shame I like how TrDrop works being a quick and straightforward UI but not being able to directly export a video is painful, particularly as I'm literally sitting there watching the software analyse the framerate of the video in realtime when creating all the images.


Support torch>=2.3.0

The decorator syntax added for quantize_symmetric and quantize_affine are only supported on 2.4+.
I'm trying to use the from_pretrained feature for pulling the quantized models from HF, specifically for FLUX.
But the problem is, FLUX currently has issues with Torch2.4, generating grainy images. So if we want to be able to use this from_pretrained feature to pull quantized FLUX models from the hub, we need to use torch 2.3.1, which means using the old syntax instead of the decorator syntax.
More on this issue here: https://github.com/huggingface/diffusers/issues/9047

@cocktailpeanut I understand why you would like to use a lower pytorch version, but optimum-quanto relies on some very recent pytorch features, for which fixes will only be provided on the latest version. This is why until 1.0 optimum-quanto will almost certainly require the latest pytorch version.


voucher generation freeze

Important notices
Before you add a new report, we ask you kindly to acknowledge the following:

[x] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
[x] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Describe the bug
Voucher generation freeze and causes any call to the voucher api and voucher ui web to timeout as well.

Fatal error: Maximum execution time of 900 seconds exceeded in
/usr/local/opnsense/mvc/app/library/OPNsense/Auth/Voucher.php on line 212

To Reproduce
Steps to reproduce the behavior:

Go to 'System > Access > Servers'
Create a New Voucher Server
Set "less secure passwords" and "username length: 0"'
Goto 'Services > Captive Portal > Voucher'
Generate Vouchers
Now every generation will cause Firewall to freeze and any call to voucers will timeoout

Describe alternatives you considered
there should be a smart choice when generating passwords or using voucher system that will not consider the susername if it's 0 and hide the username field on the captive portal
Additional context
that same problem exists on 23.1 up to 23.7.10_1
Environment
Software version used and hardware type if relevant, e.g.:
OPNsense 23.7.10_1 (amd64).
Intel(R) Xeon(R) CPU E5-2680 v4 @ 2.40GHz x 28

validation added in https://github.com/opnsense/core/commit/d7a858de3449ae3aecf60e7d074d3adb0142308d ,  if you want a single token you can concat user+pass in the template.


Removed all instances of Funcaptcha and Recaptcha

Removed all instances of Funcaptcha and Recaptcha since they are no longer usable (due to increase of costs of Funcaptcha service).

@veronicaz41 @smf11 please check when you have time

Made changes - please review when you can!


Prettify tables

Use the Box styles and tooltips to make things more readable and easier-to-understand.

Coverage remained the same at 81.625% when pulling c4f2bae5480c24bac92e1290bfa758c23ba2018d on prettify-tables into 4683b06513be1b77d286bb16d944a39f6a58be9c on master.


Multiple storage IDs are not supported

Currently we don't handle https://mavlink.io/en/messages/common.html#STORAGE_INFORMATION properly as we are not checking the storage ID. So whichever message arrives latest will be the one saved.

Implemented with https://github.com/mavlink/MAVSDK-Proto/pull/320.


Rename default server configuration

There is an inconsistency between server configuration default value being poule.yaml and the default repository-local configuration value being poule.yml.
Resolve the discrepancy by renaming the default server configuration to poule-server.yml.
Signed-off-by: Arnaud Porterie (icecrime<EMAIL_ADDRESS>

Cc @vieux

:+1:


Optionally link against c++ std library

Users have to link against c++ std library because of the usage of std::string and virtual methods.
It is better if the requirement is optional.

It might be reasonable to provide that option for users who wish to compile their C++ without access to the standard library. This would mean, in our case, limiting some functionality, which we can do with macros. Of course, we would need additional tests where we check that it is possible to compile and run our code without the C++ library in some cases.
Pull request invited.

I am bumping up the priority of this issue.

Ah. It is significantly more challenging than I expected.

It is not sufficient to do away with the C++ headers. And there is no clean/portable way to compile in C++ without dependencies on the C++ standard library.

I am closing this issue. We will not proceed in this direction.


Producing measurements like those in the paper?

I ve played a bit around with the tests that you have but I cannot find anywhere a log that would be giving me figures for the type of measurements you have in the paper (CPU utilisation, Cache utilisation, Disk IO and network IO).
Which part of the code/scripts generates such reports ?
tia
m.

We use ganglia to collect resource utilization information. http://ganglia.info/


Add quiet option

This option will suppress success messages. A new Logger struct is added to the XcodeGen target that encapsulates printing, exiting, and the Rainbow logic.
I only added a Commander flag for --quiet. If we added a Commander interface, it would be trivial to remove colored output too if that is desired. I didn't do this since I couldn't decide on a name. Also, I figured if you were in a situation where you didn't want colored output, you might also want to suppress emojis.

Could you add documentation to Readme as well?

Made the changes your requested! I tested this locally since there aren't tests for the output of the command line tool (yet!).
I was thinking about --verbose too. Maybe this option down the road could list all of the targets, groups, files etc. This may be useful for debugging at some point.

@yonaskolb thanks for your time on this! Sorry for all of the back and forth!

Thank YOU :)


Center loss being ignored

I have noticed that you do not use center loss, despite setting up optimizer for it and adding centroid scaling.
https://github.com/Syliz517/CLIP-ReID/blob/5b92124ec041c1c7a9fa1ac7ee9b86a5d45ac4ad/processor/processor_clipreid_stage2.py#L13
It's just not being called with loss function.
Can I ask why? I have implemented it myself, but I am yet to see how it works

This part of the code follows Transreid, I didn't change it.


Experimental Soft Navigation support

Warning: This branch is likey to stay in draft for some time, and not to be merged into main until the Soft Navs experiment completes
Fixes #119
Implements experimental support of Soft Navigations to the library:

[x] - Updated README
[x] - TTFB
[x] - FCP
[x] - LCP
[x] - CLS
[x] - FID (dependent on 1407656)
[x] - INP
[x] - Attribution (particularly navigationTiming)
[ ] - Test Cases (needs to wait for a supporting version of Chrome to be available in WDIO)

That’s for the review @mmocny ! I’ve cleaned it up quite a bit further so it’s a bit smaller now. Have another look if you get a chance and see what you think now.

Hey! I'm super excited about this work. Could you republish the branch?
The current 3.1.1-soft-navs doesn't report LCP  and instead prints BARRY.

Well that's embarrassing! Republished to the soft-navs tag (which is an alias to 3.1.1-soft-navs-1 now).

Thanks, that was fast.
Maybe, I'm doing something wrong, but there's still no LCP report, despite actual values are available (Chrome 110):

Can you host the site somewhere so I can have a look?
Here's a demo site where it does work: https://www.tunetheweb.com/experiments/softnavsdemo/

Thanks for the example.
Found the issue: when onLCP is called twice (like in the example), it doesn't report soft-navs. Other metrics work.
// works (your example)
onLCP(doSoftNavProcessing, { reportSoftNavs: true })

// !! doesn't report soft navs
onLCP(doTraditionalProcessing)
onLCP(doSoftNavProcessing, { reportSoftNavs: true })

// works as expected
onCLS(doTraditionalProcessing)
onCLS(doSoftNavProcessing, { reportSoftNavs: true })

Oh that's interesting. Will dig into it...

That's fixed now if you want to reinstall the soft-navs tag (or 3.1.1-soft-navs-2 if you prefer to be explicit).

Thanks, Barry. That was faster than I finished my coffee.
Another issue:  the navigationType for FCP & FID is always soft-navigation. So the way to distinguish the hard nav and softs is to use navigationId !== 1.
You can replicate the issue in your demo example.

Fixed!
Hoping to be able to add automated tests to this branch soon now this has landed in stable and so will be available in our test automation suite. But in the meantime greatly appreciate you finding them!

Hi @tunetheweb,
Thanks a lot for introducing support for Soft Navigations in the library!
While exploring the new features, I noticed something interesting. It seems that not all changes are being reported when reportAllChanges is activated, as the onINP module is only tracking the longest interactions.
To handle that scenario, I've slightly updated the onINP module (https://github.com/GoogleChrome/web-vitals/pull/411) to ensure that all changes are reported when reportAllChanges is turned on (demo). I've directed my PR to your branch, as my changes are somewhat related to soft navs. I'd be glad to hear your thoughts on this.

As discussed in #411 that is working as intended. But I raised #413 to make it clearer what exactly that intention is.

Updated this branch to v4 code.


Missing early error for duplicate lexically-bound variable declarations

Using Acorn 4.0.3:
let foo = 1;
let foo = 2;

I expected a syntax error to get thrown, because this code is invalid syntax according to the spec. However, acorn parsed it without any errors.
Relevant portion of the spec:

It is a Syntax Error if the LexicallyDeclaredNames of StatementList contains any duplicate entries.

Bump

This needs to be fixed in Acorn for it to flow through here, although, I'm not sure this check is feasible implementation-wise.
@oliverwoodings please don't do that. We are all very busy and sometimes it takes longer for us to get around to less critical issues.

This needs to be fixed in Acorn for it to flow through here, although, I'm not sure this check is feasible implementation-wise.

I'm confused -- this is Acorn.

@nzakas apologies, I wouldn't normally, but given this was posted a month ago and has had zero responses, what else should I do? Whilst it may not seem critical, it is causing myself and others issues downstream.

@not-an-aardvark sorry, some reason I thought this was in Espree.
@oliverwoodings something like, "I'm affected by this, is there anything I can do to help address it?" is much nicer. "Bump" is a bit rude as it triggers notifications without providing any additional information.

Will do next time :+1:


String concatenation performance hit

Steps to reproduce:

Crate a database with some entries (tested with 250)
Perform the following query to get all measurements of custom_Performance: `
from(bucket: "mybucket")
|> range(start: 2021-02-05T02:51:27Z, stop: 2021-02-05T08:51:27Z)
|> filter(fn: (r) => r["_measurement"] == "custom_Performance")
Perform the following query to get all measurements of custom_Performance: `
from(bucket: "mybucket")
|> range(start: 2021-02-05T02:51:27Z, stop: 2021-02-05T08:51:27Z)
|> filter(fn: (r) => r["_measurement"] == "custom_"+"Performance")

Expected behavior:
The execution time should be similar.
Actual behavior:
The first query is ~4s faster than the second one for the last 6h. For the last 30d the difference is ~40s
Environment info:
InfluxDB version: InfluxDB Cloud
Performance:
Generate profiles with the following commands for bugs related to performance, locking, out of memory (OOM), etc.
I can not create the required logs, because I am on Windows.
I hope some can test and validate this issue.

@FinnHuelsbusch Thanks for reporting this. This issue is more related to Flux than InfluxDB, so I'm going to transfer it to the Flux project.
I have tested and can reproduce the performance hit. I think what's happening here is, that because string concatenation is part of the filter fn, the operation can't be pushed down to the storage tier (more information on pushdowns here), so filter has to pull all the data returned by from() and range() into memory and operate on it there. Filter iterates through every input row and evaluates it against the fn predicate function. Typically filter() would push this operation down to the storage tier where it would happen much faster, but with concatenation, there's the potential that the predicate function could be different for each an every row, so it can't be pushed down.
I'm curious about your use case. Why are concatenating strings in your filter predicate function? If it's because you want the filter logic to be different for each row/record, then I don't think much can be done to improve the performance other than using a filter call before this to decrease the size of the dataset that gets filtered with concatenation. If you don't need the filter logic to be different for each row, you can store the concatenation in a variable outside of the filter function:
filterMeasurement = "custom_"+"Performance"

from(bucket: "mybucket")
  |> range(start: 2021-02-05T02:51:27Z, stop: 2021-02-05T08:51:27Z)
  |> filter(fn: (r) => r["_measurement"] == filterMeasurement)

Generating profiles
The Flux query engine actually includes profilers that you can use to measure query and operation performance. More information is available in the profiler package documentation.


Added way to send url previews

send_uri(link, message) will send the preview for link below the message in chat

I rebased fbchat-asyncio to the v2.0 upstream, so this no longer applies directly. In general it's better to get things merged into the upstream and I'll later merge that here.


Require GHC 8.8

The latest release of wreq doesn't work on GHC 8.6. The bumps the bounds to make sure future releases don't claim to be compatible while they're not.

@ondrap Have you seen this?

Hopefully fixed, I have deprecated the <IP_ADDRESS>.


Add Message Serialization for Transport

When a message is sent or received it does not have an explicit form. While TextMessage may default to a string, more complicated messages will have to use a marshaling scheme to get data into a transmittable form. Metadata including agent and channel attribution to a message will also also need to be sent.
As a transport consumer I should be able to implement a marshaling contract that will allow the data of the message to be marshaled and marshaled.

Resolved by #15


Board search labels endpoint has changed

board.searchLabels method I think calls an incorrect endpoint
searchLabels: trelloMethod({         method: 'GET',         path: '/{id}/customLabels',         urlParams: [ 'id' ]     }),
path should be '/{id}/labels' base on API documentation
https://developer.atlassian.com/cloud/trello/rest/api-group-boards/#api-boards-id-labels-get

For anyone coming here, while the merge request is not merged, you can do this "ugly solution"
const Trello = require('trello-node-api');
const trelloMethod = require('trello-node-api/lib/TrelloMethod');

const trello = new Trello('abc', 'def');

// This overrides the trello library as the searchLabels is outdated.
trello.board.searchLabels = trelloMethod({
  method: 'GET',
  path: '/{id}/labels',
  urlParams: ['id'],
});

For anyone coming here, while the merge request is not merged, you can do this "ugly solution"
const Trello = require('trello-node-api');
const trelloMethod = require('trello-node-api/lib/TrelloMethod');

const trello = new Trello('abc', 'def');

// This overrides the trello library as the searchLabels is outdated.
trello.board.searchLabels = trelloMethod({
  method: 'GET',
  path: '/{id}/labels',
  urlParams: ['id'],
});


cdlUtil object does not Initialized

Environment
[ ] Office 365 / SharePoint Online
`$(function () {
//create instance of library
cdlUtil = new CrossDomainUtil();

//initialize library
cdlUtil.Init(ServerUrl + "/SharePoint/proxy?SPHostUrl=" + SPHostUrl);

//execute code when the library is initialized
cdlUtil.OnInitialized(function () {

 // My Code

 });
});

`
I have two button on Index view Add and Remove, the is wrapped in fn call from index im able to make call once after the App page has loaded, however when in click on Remove button the library object cdlUtil is never initialized. It is always false. This may be bug!

thx mrajpurohit, could you also share the exact sample / solution this is referring to. If it's on native SharePoint APIs, then we can't really fix that through PnP initiative.

Since there's no response on requested details, will close up the issue.


Flutter Element Embedding is not working properly between different origins

Is there an existing issue for this?

[X] I have searched the existing issues
[X] I have read the guide to filing a bug

Steps to reproduce
First of all, thanks Flutter community, specially to @ditman, @kevmoo and @malloc-error, by provide this feature. I belive this feature breaks any possible roadblocks to using Flutter on the web, allowing us to easily integrate and even migrate just parts of an existing website. It's really amazing!
Note: the issue may be better explained at flutter/samples/issues/1742.
Use case
I wanna to use this feature to embed a Flutter Web app inside another website. Like a chat bot made in Flutter to embed in different e-commerce platforms as customers of the chat bot (B2B).
Steps to reproduce

I cloned the Flutter Element Embedding demo.
I pushed just element_embedding_demo folder to my repository instead of fork all the samples repository.
I built the app using the version below and published as the original at demo-embedding.

Flutter 3.10.0-7.0.pre.2 • channel master • https://github.com/flutter/flutter.git
Framework • revision c368d64d88 (4 days ago) • 2023-04-17 11:51:25 -0400
Engine • revision 30e1c43082
Tools • Dart 3.1.0 (build 3.1.0-11.0.dev) • DevTools 2.23.1

I separated the Flutter Element Embedding demo into Flutter app and the 3rd party website at new branch.
I created the flutter_embed.js at Flutter app to change the getBaseURI (explained at workaround section below).
I changed the index.html at 3rd party website to point to the flutter_embed.js.
Thus the Flutter app was loaded inside the 3rd party website and then I published the flutter app and 3rd party website.

Expected results
I believe it can be fixed considering the currentScript.src instead of base.getAttribute("href") in flutter.js and main.dart.js. I'd love the community help to solve that or I'd like to know how can I change the Flutter engine to adapt main.dart.js file generation to fix by myself.
Actual results
The flutter.js and main.dart.js doesn't using the correct path reference when the Flutter app come from other origin. This is needed to get other files correctly through main.dart.js and when it isn't happen, the app not works properly.
Code sample
I changed the `flutter.js` by modifying the name to `flutter_embed.js` to use the origin dynamically. However it should be done similarly at `main.dart.js` too.
Before at flutter.js link
function getBaseURI() {
    const base = document.querySelector("base");
    return (base && base.getAttribute("href")) || "";
 }

After at flutter_embed.js link
function getBaseURI() {
    return new URL(document.currentScript.src).origin;
}

Screenshots or Video

The workaround using `flutter_embed.js` managed to load the `main.dart.js` correctly and then the app partially, however the `main.dart.js` doesn't have the right path to get files correctly and the app is not working properly.

Logs
No response
Flutter Doctor output
Doctor output
[✓] Flutter (Channel master, 3.10.0-7.0.pre.2, on macOS 13.3.1 22E261 darwin-arm64,
    locale en-BR)
    • Flutter version 3.10.0-7.0.pre.2 on channel master at
      /Users/diegomoraes/fvm/versions/master
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision c368d64d88 (4 days ago), 2023-04-17 11:51:25 -0400
    • Engine revision 30e1c43082
    • Dart version 3.1.0 (build 3.1.0-11.0.dev)
    • DevTools version 2.23.1

[✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0)
    • Android SDK at /Users/diegomoraes/Library/Android/sdk
    • Platform android-33, build-tools 32.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E222b
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] VS Code (version 1.77.3)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.62.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 13.3.1 22E261 darwin-arm64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 112.0.5615.137

[✓] Network resources
    • All expected network resources are available.

• No issues found!

@diegomors there's an (undocumented for now) parameter called assetBase that does what you need. Please, try to set it to the root location of your main.dart.js (and other assets) so flutter knows what's the base of all its assets.
For this you need at least Flutter beta and flutter.js for initialization. (You should be able to undo your changes to flutter.js file, actually).
      onEntrypointLoaded: async (engineInitializer: any) => {
        let appRunner = await engineInitializer.initializeEngine({
          hostElement: target,
          assetBase: "https://demo-app.flutterhub.dev/",
        });
        await appRunner.runApp();
      }

I've used this parameter for a future embedding demo that I'm open-sourcing in the next few weeks, but since it's still not available in stable, it's not properly documented on the flutter.dev site.


Stories

Hi,
Would you consider adding the possibility to upload stories?
Thanks

is posting story there now ?


Minimal freesurfer image

Hello together!
Some time ago I've created a minimal freesurfer image that is based on freesurfer-Linux-centos6_x86_64-stable-pub-v6.0.0 and is able to run recon-all.
It is only 732.6 MB compressed and 1.93GB when pulled.
I've figured you might be interested using it as a base in order to make your image a little smaller.
Here is it: https://hub.docker.com/r/alerokhin/freesurfer6

Ahoi hoi @commandercool,
thanks so much for the pointer, that sounds fantastic! Did you test any other FreeSurfer functions/pipelines as well?
Tagging @Shotgunosine to maybe discuss if we can/should incorporate this somehow!?


Pt 160732539 implement blockhash

https://www.pivotaltracker.com/story/show/160732539

Protocol PR https://github.com/aeternity/protocol/pull/211

Does that not test the right thing?

Actually, it is correct, since BLOCKHEIGHT should return 0 for the current height. I will look at adapting the test to test some more.

Rebased to resolve conflicts, and modified test that was pointed out by @UlfNorell


Paging support?

This looks like a great new library that I'd love to use in place of my own hacked together wrapper. Are you expecting the caller to do the paging, or should that be included when returning a query result? I'm not sure if ofAsyncEnum does that or not, but the code doesn't seem to check .HasMoreResults.

I'm going to admit that I'm still wrapping my head around how the IAsyncEnumerable and AsyncSeq work, given we don't have the await foreach in F# that C# would tend to unpack.
Because of that, I'm not completely sure that AsyncSeq understands pagination the same way.
Do you have an example of what you're trying to do?

What I'm doing right now (with the v3 SDK) is this:
let public fetchAllItems(feedIterator: FeedIterator<'a>) =
    asyncSeq {
        while feedIterator.HasMoreResults do
            let! response = feedIterator.ReadNextAsync() |> Async.AwaitTask
            yield! response |> AsyncSeq.ofSeq
    }

This returns an AsyncSeq<'a> and seems to work, but I have my doubts if it is the "right" way. I believe someone helped me with the final yeild! line.

For a bit more explanation, response is a FeedResponse<'a> which implements IEnumerable<'a>.  response |> AsyncSeq.ofSeq then gives AsyncSeq<'a> which is then merged into the parent sequence via yield!.

Looking into the v3 SDK source and comparing it to v4, it looks like it works a bit differently. In v4 there isn't the FeedIterator<'T> that v3 used, instead it's AsyncPagable<'T>.
Now, digging through this a bit further it turns out that this is really a wrapper over FeedIterator and hides away the paging unless you call AsyncPagable.AsPages(), in which you provide the size of the pages you need.
So, I probably would have to have execPagedAsync where you can provide the right info and that could return it as an AsyncSeq then, but I'll have to play (trying to work out how to handle the Insert API presently).

Could you not just use OFFSET and LIMIT in your query "SQL" and remember what page index you're on?  Or have I missed something here?

@seankearon I don't think these two types of paging are the same thing, though I may be wrong. The type I'm referring to is more akin to batching. The Cosmos SDK client won't return everything all at once. You have to continually call it to get the next batch of data. I'll take a look at what's in V4 when I get a chance.

Yeah, I'm wondering where the difference is between those two.
If an async stream is like using a drinking straw to drink from a lake.  Then paging/batching is like using a bucket to drink from a lake.
If I'm passing you the bucket to drink from, do you care whether I fill it all at once or in little steps using my drinking straw?  Probably not - you just want the next bucket of water.
I'm thinking that the way to fill up bucket n using the straw would be something like this:
let usersByPage(page: Int32) =
    host
    |> Cosmos.host
    |> Cosmos.connect key
    |> Cosmos.database "UserDb"
    |> Cosmos.container |> "UserContainer"
    |> Cosmos.query "SELECT u.FirstName, u.LastName FROM u WHERE u.LastName = @name OFFSET xyz LIMIT pqr"
    |> etc. etc. etc.
    |> Cosmos.execAsync<User>

But then, it's been a loooong day! :)

I've finally had some time to come back and revisit this issue and work out if it's possible/viable to do pagination support.
TL;DR: Use the OFFSET and LIMIT as @seankearon has suggested, I don't think I can put anything into the API to do it for you. Best I can do it have a way to get batched results per iteration of the AsyncSeq.
We're going to dive through a rabbit hole now, so choose if you want to read on as I'm partially writing this down for my own sake. I'm going to trace through a bunch of the Azure.Cosmos code as it currently stands.
When you execute a GET query you call the method GetItemQueryResultsAsync (_Note: The Async suffix is added after -preview4, so my code doesn't use it, but it will eventually) and this creates a FeedIteratorCore which is what handles the ReadNextAsync operation to fetch records from CosmosDB.
This type is then wrapped in FuncAsyncPagable to return the AsyncPagable<Page<T>> response that is consumed by AsyncSeq in F# to make our nice API.
AsyncPagable, the base class of FuncAsyncPagable has the AsPages and MoveNext methods defined on it, MoveNext being what is ultimately called by the state machine to go over the iterator (it bubbles through a few other types, but it's ultimately where we land). What's interesting about the implementation is that it actually called AsPages anyway, so the AsPages method is our important one.
Our AsPages calls the func passed in here which is a call to GetPagesAsync on PageIteratorCore.
Now, if we trace through here, AsPages takes a continuationToken and pageHitSize, but GetPagesAsync on our iterator only takes the continuationToken, the pageHitSize is dropped along the way. My guess is that the pageHitSize doesn't map to anything that is available on the CosmosDB REST API, so it can't be used and is discarded in turn.
So, what's the difference between iterating over the AsyncPageable<T> and IAsyncEnumerator<Page<T>> (how it currently works vs calling .AsPages)? Whether or not you get a single result or a batch of results. Page<T> has a Values property which will contain 100 T items that you need to unpack. This means it comes down to "do you want to work with a single result each iteration, or with a batch of items?" (no where can I find exposed a "HasMore" property, that's just determined by whether you keep iterating).
I tested this against a large Cosmos store I have with the following code:
async Task Main()
{
	var client = new CosmosClient("...");

	var container = client.GetDatabase("...").GetContainer("...");

	var qd = new QueryDefinition("SELECT c.id FROM c");

	var nonCount = 0;
	"Non-paged query".Dump();
	await foreach (var response in container.GetItemQueryIterator<Dictionary<string, string>>(qd))
	{
		nonCount++;
	}

	"Paged query".Dump();
	var pageCount = 0;
	await foreach (var response in container.GetItemQueryIterator<Dictionary<string, string>>(qd).AsPages())
	{
		pageCount++;
	}

	$"Non-Paged ran {nonCount} times to Paged {pageCount}".Dump();
}

And here's the response:
Non-paged query
Paged query
Non-Paged ran 3811 times to Paged 39

The iteration count dropped and I ran a network trace on it, which saw the same number of network requests happening.
I might look at putting in a queryBatch or something like that which returns AsyncSeq<Page<T>> to give feature parity with the underlying API.

Thanks for the detailed write-up. I'm waiting to use this on my project until the v4 Cosmos API SDK is fully released, but this looks great.

I've added a "pagination" option in a new branch: https://github.com/aaronpowell/FSharp.CosmosDb/tree/pagination
Basically all it does is adds a new method Cosmos.execBatchAsync which returns AsyncSeq<Page<T>> so you can get the some paged results but it's not really paged due to what I mentioned above.

This will be coming in the next release.

If anyone wants to test, grab the 0.3.0 pre-release packages from https://github.com/aaronpowell?tab=packages&repo_name=FSharp.CosmosDb

Available on NuGet now.

Available on NuGet now.


Release as a 'Community Plugin'?

Thank you so much for making this plugin! I love pandoc, and was considering writing a plugin for Obsidian with this functionality until I found this one. Are there plans to have it added to the 'Community Plugins' in Obsidian?

Thanks for the kind words!
I have been meaning to add this to the community plugins list for over a month, I've just been bombarded with assignments at university which has throttled development. I intend to pick up development again after the semester ends in a month or so.
I might add this plugin to the community plugins list in the meantime though if I get the chance before then - stay tuned!

Turned out to be much easier than expected. I've submitted it for review, should be easy to install in the app in a week or two.
https://github.com/obsidianmd/obsidian-releases/pull/294


add WritePureJson for ghttp.Response

Codecov Report

Merging #2030 (d3cbe61) into master (6f7cd96) will increase coverage by 0.00%.
The diff coverage is 42.85%.

@@           Coverage Diff           @@
##           master    #2030   +/-   ##
=======================================
  Coverage   76.50%   76.50%
=======================================
  Files         516      516
  Lines       46712    46726   +14
=======================================
+ Hits        35735    35747   +12
- Misses       9005     9008    +3
+ Partials     1972     1971    -1

Flag
Coverage Δ

go-1.15-386
76.41% <42.85%> (+<0.01%)
:arrow_up:

go-1.15-amd64
76.43% <42.85%> (+0.01%)
:arrow_up:

go-1.16-386
76.40% <42.85%> (-0.02%)
:arrow_down:

go-1.16-amd64
76.42% <42.85%> (+<0.01%)
:arrow_up:

go-1.17-386
76.46% <42.85%> (+<0.01%)
:arrow_up:

go-1.17-amd64
76.44% <42.85%> (-0.01%)
:arrow_down:

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

Impacted Files
Coverage Δ

net/ghttp/ghttp_response_write.go
37.50% <42.85%> (+0.70%)
:arrow_up:

os/gcron/gcron_schedule_fix.go
87.50% <0.00%> (-12.50%)
:arrow_down:

net/ghttp/ghttp_server.go
65.04% <0.00%> (+0.66%)
:arrow_up:

internal/json/json.go
81.25% <0.00%> (+6.25%)
:arrow_up:

os/gcache/gcache_adapter_memory_lru.go
97.82% <0.00%> (+8.69%)
:arrow_up:

Continue to review full report at Codecov.

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

@felix-Lu Thanks for your PR. You can use Response.Write along with Response.Header().Set("Content-Type", contentTypeJson) to do so, instead of adding new function, which is unnecessary and making it more complicated for JSON response.


Fix issues

Hey,
This PR solves a few issues:

It's currently impossible to specify dimensions for string type datasets. It is made possible with this PR.
Whenever there is an HDF5 library error, SyntaxErrors are being thrown. SyntaxError in javascript is specifically for javascript parsing errors, when in doubt Error should be used.
Fixed a few deprecations for newer versions of nodejs. Still doesnt compile with the latest version but there will be less work to do with these changes.

What do you think would be the timeline for merging and deploying this PR? I'm working on a loompy module for node that depends on this, for an implementation of ga4gh-rnaseq, and we hope to deploy it as soon as possible, so any diligence in approving this would be greatly appreciated.
Thanks,
Romain

There are a few more issues that I'd like to fix with this PR:

Make it possible to create datasets of fixed-size string
Make it possible to specify the string-padding for string datatypes
Make it possible to specify the string-padding for string attributes

I've finished working on those issues.
A new method was implemented on File and Group, .setAttribute(name: string, value: any, options?: { padding: boolean }).

One more change:
file.close() now throws when there are still open groups, as it would cause (weird) bugs further down the road not to.

Hi @romgrk , been away for a bit; let me look at why the travis failure and get this merged

Note;the code for node version v12 has been done in branch https://github.com/HDF-NI/hdf5.node/tree/v12

I'll write the essence of what you want in the two branches master and v12.
Still deciding if everybody is ready to go to v12 and make all prior versions pull from this repository and publish the v12 version to npm

on the file.close() how are you using the v8::Exception::Error's on the js side?  Hopefully not having to parse the error message.
My intention was to add a debug config that showed these; and users debug to make sure things are closing at the proper place & time.
Or would a list of h5 types and id's set for the v8::Exception::Error be useful?

in file.close(), I've made it an error to leave groups unclosed before closing a file. Otherwise the resulting file can be invalid.
You're free to cherry-pick the commits you wish to include in other branches. I'm using a local fork of this package for my use-case, I needed the ability to set the string-padding & other things when creating datatypes.

Hi @romgrk, I made file.close() an accumulated exception error when groups and other objects aren't closed.   The current exception error msg can be a bit messy and if you want to do more with it than just read it maybe a list could also be returned.
Will be looking again at a debug configuration with gyp to help users get objects closed
I'll be adding the fixed and padded strings; looking also at all the datatypes, attributes and dimensions in https://github.com/HDF-NI/hdf5.node/issues/105 to complete many more possibilities for users
The SyntaxError has been replaced with Error; will look thru the v8 categories to see what specific per error can be used

Ok! Well if it's all handled in other places you can close this.

Eventually, this is good set of things to work in.  Thank you @romgrk

HI @romgrk ,
when it comes to multi rank fixed string datasets what do you like for the javascript side?  My trend is to get away from nodejs Buffer which is a single Uint8Array and  implemented a Array of Array's interface.
I've been working on a pure JavaScript implementation that is compatible with nodejs but can be used with any javascript engine.  The Javascript side of hdf5.node will remain the same with additions and for a time I'll provide a nodejs Buffer but it will become deprecated.
Ill add to the gh pages more reference and tutorials

I assume this for strings, not numbers?

It sounds good to me for most cases. JS Strings have the inconvenience that .length returns the character length and not the byte length, but Buffer is less idiomatic.

@chadbr yes strings; numbers go to the JS typed arrays if possible or if you choose the Buffer; I'm not a big fan of the Buffer since it is Uint8Array which limits size by JS's largest int divided by bytes per entry
@romgrk  with the array of arrays the limiting size is on the column; not the whole dataset size.  You're right the JS representation/dimensioning is a factor.  I'm working to enable getting a subset of multi rank string datasets with the options {start: [1,2], stride: [1,1], count: [3,4]} as other datasets.  In case someone needs to go after an h5 with data larger than the upper limit of JS int(unless they've added it there is not even 64 bit but 53 bit)


fix #751

@josenavas @squirrelo @ElDeveloper could you review?

Coverage remained the same when pulling 49de8483a298e75c64329ec4dd65ea368ffcdd69 on antgonza:issue-751 into 0566851b2c4833a826c4ff0f9dc8685f690961ed on biocore:master.

:+1:

:+1:

@squirrelo I think you said that comment can be ignored. If so, can you please merge?


fix(manager): reveal_current_file attemp to call nil field

reveal_current_file still tries calling require("neo-tree").close_all_except(source_name) which is removed by 78d26168e0b4c7c06a0c59b26c2cd305e64f3d12 and causes this error.

Here's my usage:
mappings = {
  ['gc'] = function()
    local manager = require('neo-tree.sources.manager')
    manager.reveal_current_file('filesystem')
  end,
},

From my understanding, it needs to close all neo-tree window to bring focus to 'current file' and after that M.get_path_to_reveal() can find the path.
I tried using M.close_all_except(source_name) but since the neo-tree window is not closed M.get_path_to_reveal() can't find the path to reveal. So I think M.close_all() is the proper choice.

Honestly, the method you are trying to fix should have been deprecated and removed as well, I just missed it. It was definitely not meant to be used the way you are using it. i
It will not work as a mapping from a neo-tree window because if you are in the neo-tree window, then there is no "current file" to reveal. What you have done here with 'close_all' is create a hack that closes the window which will focus some other window and make the reveal logic work in that way. I suppose that is predictable if you only ever use a single window layout with no splits, but if that's the case you can also accomplish a similar thing without closing neo-tree with this:
vim.cmd("wincmd l")
vim.cmd("Neotree reveal")

The key here is that "revealing" the current file can only work if you are executing that action from the window that contains the file you want to reveal. That is why you use:
:wincmd l

to focus the window to the right of the current one, then:
:Neotree reveal

to reveal it in neo-tree.

Since this function does not even work right now I don't see much harm in just deleting it before someone else tries to use it. I will go ahead and do that.


Migrate our docs to a grav-based website

We are evaluating the feasibility of migrating our current RTD-based documentation website to a new one made with Grav. This issue was created to document, evaluate, decide and if so, plan that migration.

Dev website is up, finally.

Closing this Epic, as the GRAV version is up on dev. Fine tuning and new changes should be reported as new issues.