Integrate scalar subquery execution into ExecutionPlan building

So far we've had multiple steps to create a plan and execute them:

Logical planning -> returned a Plan
Execution planning -> (binding params) -> returned a ExecutionPlan
Task building -> returned a Task which was then executed

To support scalar subqueries we:

Built the ExecutionPlan upfront (with SelectSymbols in place)
During Task building we processed the dependencies
(sub-executionPlans) first, and replaced the SelectSymbols with
Literals

This worked so far, but it has the limitation that we cannot utilize
concrete values in all cases to build optimized execution plans.
One example is delete from t where partition_col > (select ...).  The
best possible execution-plan is a plan which deletes the partitions.
But without having the concrete values and evaluating the query for all
known partitions it is not possible to create such a plan - we had to
fallback to a slower delete-by-query plan.
This commit overhauls the plan creation and execution:

Logical planning -> returns a Plan
Plan is executed

How the plan is executed is encapsulated in the plan.  This allows us to
have different approaches. If there are subqueries we can execute them
first and built the executionPlan incrementally using the values
returned from the subqueries. In the case of DDL statements we could
avoid both the ExecutionPlan and the Task step.
This allows us to optimize the aforementioned case and should also help
further down the road when we begin to support correlated subqueries.
As a consequence of this we might not always be able to display the full
execution plan when using the EXPLAIN statement.

Couple of concerns / questions from myself:

Should we rename Plan/ExecutionPlan again? (I've no idea what names would fit better)
Can we come up with a better name for the TransportExecutor? It's now more of a god-object-dependency-carrier and not something that executes things. Generally I'm not happy that each plan now depends on that to be executed; but the only way to solve that would be to re-add a layer that does the dispatching.
Explain is a bit hacky for now; It would be cleaner to make explain explicit in the Plan interface (e.g. by extending the explainNode interface - but I'd prefer to do that as follow up given that this PR is already larger than what I would have liked)

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@9e03c6a). Click here to learn what that means.
The diff coverage is 91.05%.

@@            Coverage Diff            @@
##             master    #6593   +/-   ##
=========================================
  Coverage          ?   83.07%
  Complexity        ?    13254
=========================================
  Files             ?     1515
  Lines             ?    49310
  Branches          ?     5904
=========================================
  Hits              ?    40966
  Misses            ?     5562
  Partials          ?     2782

Impacted Files
Coverage Δ
Complexity Δ

.../crate/planner/projection/SysUpdateProjection.java
61.29% <ø> (ø)
8 <0> (?)

.../io/crate/planner/projection/FilterProjection.java
77.14% <ø> (ø)
11 <0> (?)

...ql/src/main/java/io/crate/planner/PlanPrinter.java
76.71% <ø> (ø)
3 <0> (?)

...lanner/projection/ColumnIndexWriterProjection.java
71.83% <ø> (ø)
19 <0> (?)

...crate/planner/projection/MergeCountProjection.java
81.81% <ø> (ø)
7 <0> (?)

.../io/crate/planner/projection/UpdateProjection.java
71.69% <ø> (ø)
14 <0> (?)

...o/crate/executor/transport/task/DropTableTask.java
51.85% <ø> (ø)
3 <0> (?)

...a/io/crate/planner/projection/GroupProjection.java
90.9% <ø> (ø)
14 <0> (?)

...rate/planner/projection/AggregationProjection.java
79.41% <ø> (ø)
11 <0> (?)

...n/java/io/crate/planner/projection/Projection.java
100% <ø> (ø)
7 <0> (?)

... and 78 more

Continue to review full report at Codecov.

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


go, buildGoModule, buildGoPackage: default to 1.22

Description of changes

Things done

Built on platform(s)

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

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

[ ] sandbox = relaxed
[ ] sandbox = true

[ ] Tested, as applicable:

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

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

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

[x] Fits CONTRIBUTING.md.

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

Go 1.22.1 is in PR:

https://github.com/NixOS/nixpkgs/pull/293580


Realtek ALC662

Layout11 - InsanelyDeepak  8/9 ports (Grey,Black,Orange, Pink, Green,
Blue)
Layout12 - InsanelyDeepak  5/6 ports (Orange, Pink, Green, Blue)
Layout14 - InsanelyDeepak  5/6 ports (Orange, Pink, Green, Blue)
Realtek ALC662 v2
Layout13 - InsanelyDeepak  5/6 ports (Orange, Pink, Green, Blue)

If there's issue still , then i think problem with PlistEditor i'm using

What is the difference between layouts 12, 13 and 14 in ALC662? 5/6 ports already exists in Mirone layout 7. Why your patches better? Why do we need 3 extra layouts with the same ports? Layout 11 is OK.
What is the difference between layouts 11 and 12 in ALC883? 5/6 ports already exists in Mirone layout 7. Why your patches better? Why do we need 2 extra layouts with the same ports? Layout 13 is OK.
As vit9696 told you yesterday - it is to be clear why a certain patch/layout is needed, and why do not the existing ones already cover the needs.
Thanks.

No i haven't said my patches are best than Mirone but fulfil requirement which they can't like ALC887 i have 4 layout one (11) which is only for AsusH61M-D and other three ports Mobo and other three won't works on my Mobo but for other they requested to make patches for them , Same story with ALC883 ,662 , 1150 and others .. i have alc668 12 version which for different laptop list is too big

Now i got your point just thinks to use Mirone and Toleda patches for all Desktop and Laptop ?? which is not  a good idea cuz they won't fulfill requirements for all but for some Example ALC269 series , there's flood around .. Thanks you i like your project cuz i was trying to make  i made which autdetect codec and install Patched AppleHDA , i was not a good coder .. i started support cuz i let people thinks their codec isn't supported

Suggestion
you can make list supported list of codec with How many ports of hardware is supported using particular layoutID . will be a great for new comers Thanks you
PS: plz remove P/R

@insanelydeepak I do not understand what you mean. Probably because you come to conclusions without any attentive reading.
In no way we are trying to use Mirone and Toleda patches for all the codecs. AppleALC incorporates lots of other configurations added by different people for their own hardware. What you do not understand is that these people gave reasons why certain configurations do not work for them, and why their configurations are necessary for their hardware.
We do gladly accept the configurations from different users, but we do ask them to provide as much detail as possible. This way we will know that they can replace the existing configurations or live together.
ALC269 is a very good example. There are numerous configurations specific to each laptop, which are completely different from the others. Clearly no generic variants will work for that.

will be no conflicting anymore whenever a P/R there will be more specific reasoning
Thank you

Alright :) We are counting on you. Thanks for your patience.


Update the search algorithm to filter by programming language and CDK type

We should support programming language and CDK type filters

Note: we currently do not have a way to determine cdk type on the front end


RAT-98: report skipped files

@Claudenw did you accidentally push an old branch as the build fails with spotbugs errors?

@ottlinger
Actually, it is a work in progress.  I had to rework the descending exclusion processing because it was returning incorrect results.  That triggered a number of changes in integration tests and the like because they were not set correctly to begin with.  Net result is a large change but accurate results and more use of the assertJ package.
My next step is to see how I can split this into smaller pieces.


Disable running test formatting on build

⚠ Is your feature request related to a problem? Please describe

Whenever I want to build my project but it is not formatted correctly, it will fail and not build.
💡 Describe the solution you'd like

I would like to have an option to disable this (if it is intended)
🤚 Do you want to develop this feature yourself?

[ ] Yes
[x] No

@TymanWasTaken Please let me know if this solves your issue so I can close it

Closing for inactivity. Feel free to reopen if the issue persists


Decomposition generalized and made ready for more complex projections

This draft PR serves as a collector for changes to the Decomposition class to make it ready for alternative orbit projection.

There seems to be a problem with histogram normalization in some bins in dyn.kinematics.Histogram.get_mean_sigma_gaussfit(). As this needs to be attended to I set this PR back to draft...

The problem was actually in fitting a Gaussian to some strangely-shaped losvd histograms. The fix in commit 6a16422 increases the stability of Histogram.get_mean_sigma_gaussfit() by

using a better estimate of the Gaussian amplitude
catching nan values in data
replacing the Levenberg-Marquardt curve fitting algorithm by the more robust Trust Region Reflective algorithm (see scipy.optimize.least_squares documentation)
catching (unexpected) scipy.optimize.curve_fit errors

Also, Decomposition now normalizes the losvd histogram data before determining mean and sigma values (last commit).
So far tested successfully with NGC6278 and FCC047 (2 kinematics sets) data.

Closes #292.


Fix blank input port

Pull Request Description
Fixes #6485
Conflicting requirements for the widget tree caused the issue:

The span tree node had a connection, and the text of the number1 label was changed to white (as per the Connected color state)
The node configuration did not consider it a valid port because the span tree kind was Operation, which is not a port usually. So the port shape was not displayed, making the label blend with the node background.

I fixed the issue by considering the existence of the current connection for Operation nodes. Remember that it does not turn the node into a port, so after removing the connection, it's not possible to connect it back. That makes sense, in my opinion, as the resulting AST is invalid anyway. But at least we can see the label on the invalid node.
https://github.com/enso-org/enso/assets/6566674/23934966-8f72-4675-abe3-78a3f0c0cda4
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:

[x] The documentation has been updated, if necessary.
[x] Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
[x] All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:

[x] Unit tests have been written where possible.
[x] If GUI codebase was changed, the GUI was tested when built using ./run ide build.

QA: Looks good to me! I tried a range of things and never experienced a blank input.
Unrelated to the issue of blank input ports, I noticed that creating a new node from an old one and typing "+1" gives a type error, which we might want to have a look at at some point.

Unrelated to the issue of blank input ports, I noticed that creating a new node from an old one and typing "+1" gives a type error, which we might want to have a look at at some point.

Tracked in https://github.com/enso-org/enso/issues/6484


Fix C# "Sample code that uses a named mutex"

The C# code in the Sample code that uses a named mutex paragraph was far from correct: it would not even compile.

Learn Build status updates of commit 6aafd8d:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

support/developer/visualstudio/csharp/language-compilers/single-instance-application-crashes.md
:white_check_mark:Succeeded

For more details, please refer to the build report.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
For any questions, please:Try searching the learn.microsoft.com contributor guidesPost your question in the Learn support channel


clini_table and slide_table template

I thought it would be useful to provide a clini_table and slide_table template (containing at least the key elements needed for model training) that would help me apply it to my own data set, otherwise I would have to drill down into the training code to de-reason the table structure.

We created a paper with templates and examples + troubleshooting for most of the issues that could occur during execution of the code: https://www.nature.com/articles/s41596-024-01047-2


Angular cli -ng build's bundles doesn't work in production

Please provide us with the following information:

OS?

Windows 7, 8 or 10. Linux (which distribution). Mac OSX (Yosemite? El Capitan?)

Windows Server 2012
Versions.

Please run ng --version. If there's nothing outputted, please run in a Terminal: node --version and paste the result here:
28.3

Repro steps.

Was this an app that wasn't created using the CLI? What change did you do on your code? etc.
Yes using CLI. No other changes.

The log given by the failure.

Normally this include a stack trace and some more information.

Mention any other details that might be useful.
I have generated a new app using angular cli. I used ng build to bundle the application and hosted it on my local machine. The app works fine. When I use this bundles to host it on my dev server, the app doesn't work. It shows loading. Do I need to install anything over there?

Thanks! We'll be in touch soon.

Check out your console output, it there any errors?

This is the same looking error that happens when you try and run angular on a too old version of IE (<10). Make sure you are not accidentally running compatibility mode in the emulation tab in the dev tools for something IE9 and before.

It looks like you're missing polyfills for the browser you're using in your server. Check out the src/polyfills.ts files.


.gitignore eclipse stuff

The eclipse-specific project settings conflict with the Java VSCode extension, causing the files to get changed. To me this is the fault of vscode, but it would be nice if we didn't commit the eclipse stuff in.
Does removing it cause any problems for eclipse users?

Do not ignore .settings files, they're required for formatting and checks. We discussed it before, only .settings and .classpath need to be ignored.

OK, I took a look at the .settings files and determined that only one of them is related to formatting. I've gone ahead and .gitignored the .classpath and .project files, as well as all .settings that are unrelated to formatting


ZAM/ZAT/ZAS restructure

:koala:
/cc @zendesk/quokka
References

Jira link:

Risks

I will break some apps, but only in Development mode, since I moved app id from 0 to -1, -2, -3. That means that is someone did:
this.id() > 0 it will still work
!this.id() it will stop working, since negative numbers are truthy.

Are we moving some of the deleted specs/tests from ZAM here?

This is awesome, I've been trying to get this de-duplicated for ages :clap:

:+1: when :green_heart: / conflicts resolved

:+1:

looking good

:+1:


ARM template aks-custom-headers

I'd like to try out containerd support for our product but I have to provision the cluster from an ARM template instead of CLI. I can't find a way to use aks-custom-headers in an ARM template. I couldn't find any documentation around that.

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

ID: e1e42fdb-aa59-786b-ba0d-659f67db3cf3
Version Independent ID: cc199e1d-6737-5f52-f2a2-b6c0b603dbdc
Content: Cluster configuration in Azure Kubernetes Services (AKS) - Azure Kubernetes Service
Content Source: articles/aks/cluster-configuration.md
Service: container-service
GitHub Login: @palma21
Microsoft Alias: jpalma

Are you trying to use Ubuntu 18.04 as the default OS? The containerd runtime is only supported on nodes and node pools using the AKS Ubuntu 18.04 image. Let me know.

Yes, I'd like to try 18.04 also before becomes default.

I've already opened this for the AKS team:
https://github.com/Azure/AKS/issues/1638

Thanks for sharing the issue number here.
CC: palma21 for awareness.
@ohorvath We will track this issue: Azure/AKS#1638 and share updates to this thread as well.
Thanks for bringing this to our attention. We will now close this issue. If there are further questions regarding this matter, please tag me in a comment. I will reopen it and we will gladly continue the discussion.


[FEATURE] Add KopEventManager to repair metadata consistency of cluster and consistency of consumer group status

Fixes #617
Motivation
When investigating #617, I found the root cause of TimeOut exception and incorrect consumer group status.
The first question is a new problem I found while investigating the cause
1. For example, when the cluster contains two kop nodes, broker1 is the owner broker of all topics in the current cluster, and broker2 is not the leader of any partition.

1.1. When the client describesGroup, it needs to connect to the configured brokerList first.
1.2. Suppose it connects to broker2 to obtain metadata. For the current server-side metadata processing logic, if the leader nodes of all partitions are not broker2, they are all broker1. Since metadata acquisition proceeds normally, the metadata returned by the brokers node only contains broker1.
1.3. So next, the client will send FindCoordinatorRequest to broker1. Broker1 calculates based on the consumer group hash and finds that the coordinator is on broker2.
1.4. Because the cluster attribute in the client metadata only has the broker1 node, the client can no longer send DescribeGroupRequest to the broker2, and keep sending the update metadata request to broker1 until it times out.

The second question is described in #617
2. The current GroupCoordinator will not remove the deleted partition from the GroupMetadata after the topic is deleted
Modifications
Add a single-threaded event processing queue on the kop server. It will obtain the current cluster node metadata once at startup and place it locally, and will also register to monitor the /loadbalance/broker subnode and /kop/delete_topics subnode, /loadbalance /broker is used to update the metadata of the kop node list, and /kop/delete_topics is used to update the GroupMetadata which had these deleted topics, and try to change the state of the consumer group.

When a new node started, the kop brokers node metadata update is triggered.

Remove the deleted partition from the GroupMetadata and try to update the consumer group status.

@BewareMyPower There are still some bugs in the code that need to be fixed and related tests added. Before that, can you first take a look at the solution described by this pr? thanks.

@BewareMyPower My PR and I are ready. Zk client has been replaced with MetadataStore.  PTAL :)

I've left some comments, PTAL. The main problem is that we should avoid waiting a future in any callback, which was very dangerous and hard to investigate the reason if deadlock happened. There was a similar problem before, see https://github.com/streamnative/kop/pull/556.

I've left some comments, PTAL. The main problem is that we should avoid waiting a future in any callback, which was very dangerous and hard to investigate the reason if deadlock happened. There was a similar problem before, see #556.

Thank you for your meaningful comments. Based on your comment, I fixed the relevant code, and for the test, I added timeout=6000 ms. In my local area, the actual test can be completed in just a few hundred milliseconds. It took 1 minute before because I got the judgment condition wrong. I mistakenly wrote describeGroup.get(groupId).state().name().equals("DEAD") as describeGroup.get(groupId).state( ).name().equals("Dead")). PTAL :)

A single test only took around 190 milliseconds.

The failed test does not seem to be related to the PR.

Overall LGTM, just left a few comments, PTAL.

@BewareMyPower I have addressed your comments. PTAL.


Fix: Update readme contact information

After April 15th my contact information needs to be updated to my new email address.

Closed in #408


Don't use prototype extensions.

So this is just a basic refactor - correct? I see all the tests are passing --- anything else we need to add?
The only update I'd ask for -would you mind renaming the commit to have the word [REFACTOR]: as a prefix? I'm trying to do a better job than I've done in the past (labeling what I did /etc)

@toranb correct, is a simple refactor. re: tagging, done.

Last question - what version of ember and ember-cli do you need to get this new computed right now?

@toranb this doesn't depend on any version of ember-cli. About Ember's I'm almost  sure I've used this as far as back as 1.7, .property and .on are just sugar  ... @stefanpenner can you confirm about the versions please?

This syntax has existed since the beginning of time. In-fact all of ember is written this way.

Excellent - thanks for the clarification @stefanpenner

@abuiles just did an npm publish of 0.0.11 that includes this change


Wrong like link in this page

Makes impossible to link the map
https://augustmiller.cartodb.com/viz/746f6e62-4aec-11e5-85f3-0e49835281d6/public_map

I cannot find a ticket about this exactly new thing but in google trends maps.

After cleanning my cache it worked... :S


Use Array.prototype.every in CompositeTag example

The current example uses a short-circuiting return within an each which would do the right thing in Ruby but not, I think, in JavaScript.

Nice! I caught the bug the other day and was planning to replace it with a loop. But this is even better!


Remove the first empty line for uv tree --package foo

Summary
When using uv tree --package foo, an extra empty line appears at the beginning, which seems unnecessary since uv tree without the package option doesn’t have this. It’s possible that the intention was to add separation between packages, i.e. the correct implementation shoule be:
if !std::mem::take(&mut first) {
    lines.push(String::new());
}

Even if corrected, this extra spacing might be redundant as uv tree doesn’t include these empty lines between packages by default.
$ uv init project
$ cd project
$ uv init foo
$ uv tree
Using CPython 3.12.5
Resolved 2 packages in 1ms
foo v0.1.0
project v0.1.0

$ uv tree --package project
Using CPython 3.12.5
Resolved 2 packages in 1ms

project v0.1.0

Good catch. Seems fine to just remove it.


Recent attempt to refactor leapp's tmt-workflow to enable both

7to8 and 8to9 upstream regression testing has shown that we
need some tooling to get most recent leapp-repository's master
copr build id.
So shamelessly copying the refactored utils/get_latest_copr_build
script from leapp-repository to be used in tmt-tests workflow.

Can one of the admins verify this patch?

@fernflower fix pls commit msg to keep the expected format
title

description

(hint: the missing new line between title & description)


Auth.liping

Please review the following for this change:

VMODL definition (the java file docker-volume-auth-vmodl.java, anything is missing or need to change?)
Python code that implement the API of auth VMODL (code in auth_data.py)
Python code that provide API for vmdk_ops to check the authorization check (code in auth.py)
Python code change in vmdk_ops.py made to support authorization check (vmdk_ops.py, vmdk_utils.py)
Python code to test the API of auth VMODL (auth_data_test.py)
Changes in vmdk_ops_test.py (fix for “test esx” failure)

The purpose of this PR is not to check-in the code, I just need someone to do a initial review to make sure the code change is on the right track.

The direction generally looks good Liping. I only had a few comments. Keep up the good work!

Address Andy's comments and repush the change.

New code added to handle the following:

auth check in vmdk_ops
add code to remove volumes created by a tenant when removing a tenant

The code is not ready for formal review, and it includes many debug logs for debugging purpose.
The doc string for modules/classes/functions for newly added code is not completed yet.
But please to take a look for an informal review for the functionality itself. Thanks!

I think Mark had a lot of great comments. Will review after those are addressed.

Fix CI test failure issue and address all comments.
Repush the change, please review it.
I want to merge the change to master if everything looks OK to you @msterin @andrewjstone

@msterin  I think the test for actual admission/access control should not be put in auth_db_test.py.
I plan to put those test in auth_test.py.
The db file used in auth_db_test.py is different from the one used in production (/etc/vmware/vmdk/auth-db).
In order to do the actual admission/access control test:

we need insert/delete in the production DB, which I think it is dangerous
need change the code in vmdk_ops auth check code to make sure it can query different DB.

I think we should go for option 2.

@lipingxue - there is nothing dangerous is testing :-) We need to test the actual code, with the DB in the actually correct place. So while the test db (/tmp/xxx) is used for testing auth manipulation with the DB - and this is the correct approach -  we also need to test the admission control , so we need to have tests which create the actual DB in the correct place, put some access control and tenant info, maybe create a fake VM and then run vmdk_ops operation  on behalf of this VM, and make sure the operations fails as expected, with correct exception info.
So we need to take option 1

@msterin OK. make sense, I will add this test case in vmdk_ops_test.py.

Fixes #559 #557 #502

Address comments from Mark and Andy, repush the change.


PMM-9919 Adding tests for smart metrics, merging changes from Main for RC

https://pmm.cd.percona.com/job/pmm2-ui-tests-matrix/720/

Can you add test run result pls?

added, https://pmm.cd.percona.com/job/pmm2-ui-tests-matrix/720/


Upgrade to modern version of Jekyll and use kramdown

#920 reminded us that our older versions of Jekyll, and using redcarpet instead of kramdown for rendering Markdown, are causing various problems.

Hi, I went through #920 to understand the motivation behind this issue. I would like to work on this.
To start with, I ran the project locally with the following changes in jekyll.yml:
+markdown: kramdown
-markdown: redcarpet
- redcarpet:
-   extensions:
-     - fenced_code_blocks
-     - no_intra_emphasis
-     - autolink
-     - with_toc_data
-     - strikethrough
-     - lax_html_blocks
-     - superscript
-     - tables

Looking at the redcarpet extensions I expected some issues in the final built docs, especially with code snippets and tables, but surprisingly the kramdown build looks to be the same as redcarpet.
Is this expected?
Can you guide me as to how I can check feature parity with the current docs site?
Also, can you point me to few articles in the docs where I can confirm if the duplicate IDs issue has been fixed by switching to kramdown?

Hello @vividh - thanks for your interest!

Hi, I went through #920 to understand the motivation behind this issue. I would like to work on this.
Awesome! I'm here to help :)

To start with, I ran the project locally with the following changes in jekyll.yml:
+markdown: kramdown
-markdown: redcarpet
- redcarpet:
-   extensions:
-     - fenced_code_blocks
-     - no_intra_emphasis
-     - autolink
-     - with_toc_data
-     - strikethrough
-     - lax_html_blocks
-     - superscript
-     - tables

Looking at the redcarpet extensions I expected some issues in the final built docs, especially with code snippets and tables, but surprisingly the kramdown build looks to be the same as redcarpet.
Is this expected?

I don't know - I'm afraid I don't know enough about how this all works to be able to predict the effect of this change.
Hopefully @allejo could come comment here, as his terrific work on https://github.com/Kong/docs.konghq.com/pull/920 makes him better qualified than I to do so.

Can you guide me as to how I can check feature parity with the current docs site?

Off the top of my head:

Run the site locally, with your change
Visit a bunch of pages, and compare them (visually) to the pages on the current production site at https://docs.konghq.com/
Try out the Table of Contents on a few pages in the https://docs.konghq.com/latest/ and https://docs.konghq.com/enterprise/ and https://docs.konghq.com/hub/ parts of the site to ensure they work as expected
Try out the sidebar links on those pages

Also, can you point me to few articles in the docs where I can confirm if the duplicate IDs issue has been fixed by switching to kramdown?

Great question! This page https://docs.konghq.com/enterprise/changelog/ used to show the duplicate ID problem, but then we "fixed" it by changing the otherwise duplicate headers to plain old bold text here https://github.com/Kong/docs.konghq.com/pull/920/files#diff-1f5b1371b59158788d4a59f3089d44d0
A similar fix occurred here https://github.com/Kong/docs.konghq.com/pull/932/files
I guess you could try reverting one of those changes, or manually editing a page to have what would otherwise be duplicate IDs?

Ah, I found a live page with the duplicate IDs issue: https://docs.konghq.com/0.14.x/admin-api/

I don't know - I'm afraid I don't know enough about how this all works to be able to predict the effect of this change.
Hopefully @allejo could come comment here, as his terrific work on #920 makes him better qualified than I to do so.

Based on the redcarpet README, here's what I've concluded based on the redcarpet usage Kong is using.

redcarpet
kramdown

fenced_code_blocks
:white_check_mark:

no_intra_emphasis
:white_check_mark:

autolink
:x:

with_toc_data
:white_check_mark:

strikethrough
:white_check_mark:

lax_html_blocks
:question:

superscript
:x:

tables
:white_check_mark:

I can't seem to find what exactly lax_html_blocks does but by the sounds of it, I would think that it means parse markdown inside of HTML elements.
So in order to make sure kramdown works,

the docs would need to look for any superscript uses and replace it with manual  calls
Autolinks should be replaced with proper links
if there is any markdown inside of HTML blocks, they should be replaced with markdown="1"

Everything else, should be compatible with kramdown but a double check through the important and current pages would be ideal.

Thanks @allejo! Will do the parity checks locally for all these features and give an update.


Fix preprocessdir and findfile to be platform agnostic

fixes issue #36 by fixing some tests and using filepath and os packages to handle paths instead of handrolled string concatenations

Coverage decreased (-0.2%) to 80.163% when pulling 7ee3b2f99f75ba5de6ce5651b85748f1093081b3 on serbrech:fix-preprocessdir-platform-agnostic into af517d5b70db9c4b0505e0144fcc62b054057d2a on containous:master.


[Task]: Update candy machine preview NFTs to new design

Task description.

Requirements.

Empty
Cache loaded with NFTs grid and cache filename
Loading status if necessary

For simplicity and stability, and to save work for now, show only 10 nfts. We will add pagination later on.


Validate Reference_Allele string when annotating (if present)

Problem
When annotation api requests specify deleted nucleotides, genomenexus currently does not check whether those nucleotides match the content of the reference genome assembly which is in use by VEP to make variant effect predictions. This is because of simplification of the query format before passing the request to VEP.
When a genome nexus server uses the public Ensembl vep annotation endpoints for predictions, queries are submitted in the (standard simplified) HGVS format. This format no longer includes the nucleotides which are deleted, for example with this deletion-insertion variants: /annotation/genomic/5,138163255,138163256,CC,TT
the query which is made Ensembl does not mention the replaced "CC" nucleotides but is encoded like this:
5:g.138163255_138163256delinsTT
So a predicted effect is generated using the genome assembly available to VEP (which has sequence "TC" rather than "CC" at these positions.
When a genome nexus server uses a local deployment of the VEP command line tool the query for the same case is formatted in a REST-style region format which also does not mention the deleted nucleotides:
5:138163255-138163256 :-/TT
There are subtle differences in output response between these two configured VEP service providers when invariant nucleotides are part of the "replacement". In other words, the results can differ if the query "replaces" a T with a T in the first position of a 2 NT delins where the second position is a genuine replacement (C>T).
To avoid these inconsistencies, we have decided to require that any query which specifies a reference allele must be validated to make sure the provided reference allele matches the genome assembly in use by VEP. During annotation of our own studies and variants we will provide the reference allele, increasing reliability and standardization in results.
Strategy
Two services are available for providing annotation. One using HGVS format making requests to the Ensembl /annotation endpoint, One using REST-style region format to make requests to a locally deployed VEP web service. Calls to these services will be redirected to a higher level service which both makes the appropriate service call for annotation, and then verifies that the annotation returned from the service contains a reference genome sequence which matches the input Reference_Allele sequence. For some types of variants (SNV) this information will always be available and validation can be done with one request. For other types of variants (delins) the annotation returned from the service may not contain the full reference genome sequence if the extent of the variation has been reduced (as discussed above when part of the "replacement" is actually invariant). In those cases a subsequent request to the annotation service provider will be made for a full deletion of the coordinates covered by the Reference_Allele. Then the full reference genome sequence will be compared to the Reference_Allele. When it is not a match, the service will report a failure to annotate.

integration tests are currently being debugged. Note that resetting to a PR with only minimal changes from the current head of master led to all continuous integration tests passing


[NFC][test] XFAIL NewArray test cases for iOS and tvOS devices.

rdar://problem/33926468

@swift-ci Please smoke test and merge


frdm_k64f: Run some testcases timeout failed by using twister

Describe the bug:
I find some testcases timeout failed after twister changed. More detail as below:
frdm_k64f platform:
Just select a testcase to demo this failure.
command: twister -p frdm_k64f --device-testing --device-serial /dev/ttyACM0 -T tests/kernel/stack/ -j 1 -vv
and it showed me timeout failed, and device.log shows:
WARNING: runners.pyocd: hex file (None) does not exist; falling back on .bin (/home/zn/zephyrproject/zephyr/twister-out/frdm_k64f/tests/kernel/stack/stack/kernel.stack.usage/zephyr/zephyr.bin). Consider enabling CONFIG_BUILD_OUTPUT_HEX.
0005001:INFO:loader:Erased 77824 bytes (19 sectors), programmed 77824 bytes (19 pages), skipped 4096 bytes (1 page) at 19.53 kB/s

And I reset to commit c5caa9b916d1 ,  It is good to work.
sam_e70b_xplained platform:
command: twister -p sam_e70b_xplained --device-testing --device-serial /dev/ttyACM1 -T tests/kernel/stack/stack/  -j 1 -vv
and it showed me timeout failed, device.log shows:
Open On-Chip Debugger 0.10.0+dev-01508-gf79c90268-dirty (2021-01-24-19:03)
Licensed under GNU GPL v2
For bug reports, read
	http://openocd.org/doc/doxygen/bugs.html
Info : flash bank command
Info : CMSIS-DAP: SWD  Supported
Info : CMSIS-DAP: FW Version = 03.25.01B6
Info : CMSIS-DAP: Serial# = ATML2637010000000000
Info : CMSIS-DAP: Interface Initialised (SWD)
Info : SWCLK/TCK = 1 SWDIO/TMS = 1 TDI = 1 TDO = 1 nTRST = 0 nRESET = 1
Info : CMSIS-DAP: Interface ready
Info : clock speed 1800 kHz
Info : SWD DPIDR 0x0bd11477
Info : atsame70q21.cpu: hardware has 8 breakpoints, 4 watchpoints
Info : starting gdb server for atsame70q21.cpu on 3333
Info : Listening on port 3333 for gdb connections
    TargetName         Type       Endian TapName            State
--  ------------------ ---------- ------ ------------------ ------------
 0* atsame70q21.cpu    cortex_m   little atsame70q21.cpu    running

target halted due to debug-request, current mode: Thread
xPSR: 0x01000000 pc: 0x00403638 msp: 0x20402c00
Info : device id = 0xa1020e01
Info : erasing lock regions 0-4...
Info : erasing lock region 0
Info : erasing lock region 1
Info : erasing lock region 2
Info : erasing lock region 3
Info : erasing lock region 4
auto erase enabled
wrote 81920 bytes from file /home/xuhao/work/zephyrproject/zephyr/twister-out/sam_e70b_xplained/tests/kernel/stack/stack/kernel.stack.usage/zephyr/zephyr.hex in 1.864619s (42.904 KiB/s)

target halted due to debug-request, current mode: Thread
xPSR: 0x01000000 pc: 0x00402ec0 msp: 0x20402400
verified 70972 bytes in 1.719761s (40.301 KiB/s)

shutdown command invoked

And I reset to commit c5caa9b916d1 ,  It is good to work.
mimxrt1050_evk platform:
command: twister -p mimxrt1050_evk --device-testing --device-serial /dev/ttyACM0 -T tests/kernel/stack/stack/  -j 1 -vv
and it showed me build failed, and there is nothing wrong in build.log file.
And I reset to commit c5caa9b916d1 ,  It is good to work.
And reel_board platform is also not work, more detail refer above.
Environment
OS: Fedora33
Toolchain:zephyr-sdk-0.12.2
Commit ID: 4118ed1d4d6fe50f4

just bisected to
commit f1a27a8189e338f2c2d59c146002952bd9537feb
Author: Ioannis Glaropoulos<EMAIL_ADDRESS>Date:   Mon Mar 1 11:25:49 2021 +0100

    arm: cortex_m: assert if DebugMonitor exc is enabled in debug mode

    Assert if the null pointer de-referencing detection (via DWT) is
    enabled when the processor is in debug mode, because the debug
    monitor exception can not be triggered in debug mode (i.e. the
    behavior is unpredictable). Add a note in the Kconfig definition
    of the null-pointer detection implementation via DWT, stressing
    that the solution requires the core be in normal mode.

    Signed-off-by: Ioannis Glaropoulos<EMAIL_ADDRESS>
 arch/arm/core/aarch32/cortex_m/Kconfig  | 1 +
 arch/arm/include/aarch32/cortex_m/dwt.h | 8 ++++++++
 2 files changed, 9 insertions(+)

It looks that the issue is fixed partially. We still see it on reel_board.

It looks that the issue is fixed partially. We still see it on reel_board.

Confirmed. I'll share a fix to this for us to test out.

@LeiW000 I submitted a PR that addresses the issue with the reel_board (essentially, a pyocd issue). Could you please check if you can run CI on reel_board using #33386 ?


Added tmux links

@smacz42 can you add the below overview before the links
"tmux is a "terminal multiplexer", it enables a number of terminals (or windows)
to be accessed and controlled from a single terminal. It can be installed via sudo apt-get install tmux "
This is not installed by default and users may not know about Tmux, This overview will help them know how useful it is and how quickly it can be installed.


Question about and2_output_verilog.v in full_testbench

Hello, I'm new to this software and I'm going through the initial tutorials.
I have a question regarding the file _output_verilog.v that is generated in the full testbench tutorial.

Shouldn't this be the netlist? Why is it implemented using shifts rather than with a LUT?

Thanks @yunuseryilmaz18 for the prompt answer. The verilog is an output from yosys, which is a rewrite for your original RTL. We use this file as a reference in openfpga_flow to validate the correctness of FPGA fabric.


SceneObjectBase: Force render when there are pending state chages before any state subscription

This tries to address a weird timing issue that can happen when an async action in the model that updates model state happens before state subscription is created in renderer.
For exampe in VizPanel, plugin can load quicker than the VizPanelRenderer renders and sets subscription to the state. So technically there was a state change, that will be undetected by VizPanel.
In this PR I'm trying to solve this with a simple counter in SceneObjectBase that counts state updates (pending state updates) until a state subscription is created. When such subscription is added, and we have indicated some pending changes, the component is force updated and pending changes counter cancelled.

closing in favor of https://github.com/grafana/scenes/pull/161


Fix bug in SFind and RFind commands

Fix bug where find using multiple keywords does not return expected results

Codecov Report

Merging #228 (f68abd0) into master (4d75a91) will increase coverage by 0.20%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #228      +/-   ##
============================================
+ Coverage     65.07%   65.27%   +0.20%
- Complexity      583      589       +6
============================================
  Files           101      101
  Lines          2067     2082      +15
  Branches        234      239       +5
============================================
+ Hits           1345     1359      +14
- Misses          666      667       +1
  Partials         56       56

Impacted Files
Coverage Δ

...c/main/java/foodwhere/logic/parser/ParserUtil.java
75.80% <100.00%> (+2.59%)
:arrow_up:

...ava/foodwhere/logic/parser/RFindCommandParser.java
93.33% <100.00%> (+2.42%)
:arrow_up:

...ava/foodwhere/logic/parser/SFindCommandParser.java
93.33% <100.00%> (+2.42%)
:arrow_up:

src/main/java/foodwhere/ui/ReviewCard.java
0.00% <0.00%> (ø)

src/main/java/foodwhere/model/stall/Stall.java
97.91% <0.00%> (ø)

.../main/java/foodwhere/model/stall/StallBuilder.java
100.00% <0.00%> (ø)

src/main/java/foodwhere/model/stall/Address.java

src/main/java/foodwhere/model/commons/Address.java
90.00% <0.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


will not load QR code/digit code, or status

I am running homebridge via systemd on boot and homebridge is in fact running and iOS is able to use it. However, I am unable to get homebridge-config-ui-x to see the status of homebridge.
I used this guide for auto-running homebridge: https://timleland.com/setup-homebridge-to-start-on-bootup/
How can I correct this? I used "sudo npm install -g --unsafe-perm homebridge-config-ui-x" to install.

It looks like the websocket is not connecting. Are you behind a reverse proxy?

It’s running on a RPI that is used as a pi-hole. Is there something I need to whitelist?

On Apr 24, 2019, at 1:55 AM, oznu<EMAIL_ADDRESS>wrote:
It looks like the websocket is not connecting. Are you behind a reverse proxy?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/oznu/homebridge-config-ui-x/issues/267#issuecomment-486078774, or mute the thread https://github.com/notifications/unsubscribe-auth/ACBMXL6V4IKTIJFFHT2BMTLPR7Y5XANCNFSM4HIASI3Q.

What does the javascript console say (Open developer tools).

That all looks fine, what about the systemd homebridge logs?

Apr 24 06:04:01 pihole systemd[1]: Started Node.js HomeKit Server.
Apr 24 06:04:08 pihole homebridge[352]: [4/24/2019, 6:04:08 AM] Loaded config.json with 1 accessories and 1 platforms.

Apr 24 06:04:08 pihole homebridge[352]: [4/24/2019, 6:04:08 AM] [Config] Initializing config platform...
Apr 24 06:04:08 pihole homebridge[352]: [4/24/2019, 6:04:08 AM] [Config] Spawning homebridge-config-ui-x with PID 578
Apr 24 06:04:08 pihole homebridge[352]: [4/24/2019, 6:04:08 AM] Loading 1 accessories...
Apr 24 06:04:08 pihole homebridge[352]: [4/24/2019, 6:04:08 AM] [Xbox] Initializing Xbox accessory...
Apr 24 06:04:08 pihole homebridge[352]: Setup Payload:
Apr 24 06:04:08 pihole homebridge[352]: X-HM://0023ISYWYDMM3
Apr 24 06:04:08 pihole homebridge[352]: Scan this code with your HomeKit app on your iOS device to pair with Homebridge:
Apr 24 06:04:08 pihole homebridge[352]:
Apr 24 06:07:30 pihole homebridge[6229]: Or enter this code with your HomeKit app on your iOS device to pair with Homebridge:
Apr 24 06:07:30 pihole homebridge[6229]:
Apr 24 06:07:30 pihole homebridge[6229]:     ┌────────────┐
Apr 24 06:07:30 pihole homebridge[6229]:     │ 031-45-154 │
Apr 24 06:07:30 pihole homebridge[6229]:     └────────────┘
Apr 24 06:07:30 pihole homebridge[6229]:
Apr 24 06:07:30 pihole homebridge[6229]: [4/24/2019, 6:07:30 AM] Homebridge is running on port 51826.
Apr 24 06:07:36 pihole homebridge[6229]: [4/24/2019, 6:07:36 AM] [homebridge-config-ui-x] Console v4.0.6 is listening on :: port 8080

That looks fine to.. hmm what about your config.json?

I installed the xbox accessory to test rpm in this, ignore it.
{
                "bridge": {
                "name": "Homebridge Pi",
                "username": "CC:22:3D:E3:CE:55",
                "port": 51826,
                "pin": "031-45-155"
        },
        "description": "This is an example configuration",
        "platforms": [{
                "platform": "config",
                "name": "Config",
                "port": 8080,
                "sudo": false
        }],
        "accessories": [{
                "accessory": "Xbox",
                "name": "Xbox",
                "ipAddress": "<Xbox IP address>",
                "liveId": "<Xbox Live ID>"
        }]
}

Can you try adding "websocketCompatibilityMode": true to the UI config?

So, not sure what happened. I added that line, restarted homebridge and it didn’t work. I deleted the line from config.json, restarted homebridge and logged in to the panel and it’s working.

Great, I couldn't see anything wrong in the logs or in the console, so I'll close this for now.


Fixes gh-994

https://github.com/spring-cloud/spring-cloud-commons/pull/996

Merged 3.0.x PR into main


Display multiple authors for workflows

For https://github.com/dockstore/dockstore/issues/4295
zeplin:
https://app.zeplin.io/project/5fc690a2717ee9144814f190/screen/60219b1cf92a1d19c95a33a2
https://app.zeplin.io/project/5fc690a2717ee9144814f190/screen/602c16fafaa1a71a3aac49e7
Added a table that displays multiple authors for workflows. Note that Orcid Authors only have the orcid id because the work is not complete yet (there's a ticket to grab orcid author info via the orcid public api https://ucsc-cgl.atlassian.net/browse/SEAB-3220).
I made the styling for the table global because I browsed through zeplin and it looks like all the tables have the same style as the Authors table. I went through all the other tables and made sure they looked acceptable (mainly just removed the elevation style).
Removed some td and th styles that were being overriden by mat-table so they weren't being used anywhere.
Screenshots of Authors Table
My Workflows with authors

Published workflow with authors

The published workflow page has a fixed width which makes the Authors table quite squished if all the fields are filled out. Not sure what to do about this. Are there plans to make the published workflow page fit the width of the screen, like the my workflows page?

My workflows with no authors

Published workflows with authors

Misc. screenshots of tables across the site, since the styling changed a bit

For this specific case I wonder if it is worth moving the ORCID icon to the header since the number of ORCID characters should be fixed? (i.e. it seems like it will always wrap if the icon is inline)

I'm not sure if it's worth it because if we do it for the ORCID icon, the wrapping can still happen with the email


Is there a way to construct a dataclass directly from the command line inputs?

Hi, is there a way to directly construct a dataclass from the command line inputs with typer? Like an argparser would do.
For a large amount of arguments this would improve readability of the code, and would permit inheritances of dataclasses like in the example below:
import typer
from dataclasses import dataclass

@dataclass
class BaseFlags:
    arg1: int = typer.Argument(help='something', default=1)
    arg2: int = typer.Argument(help='something', default=2)

@dataclass
class Flags(BaseFlags):
    arg3: int = typer.Argument(help='something', default=3)
    arg4: int = typer.Argument(help='something', default=4)

def main(args:Flags=Flags()):
    print(args)

if __name__ == '__main__':
    typer.run(main)

Thanks a lot for the great package and your help!

This would be really helpful; in particular it would be easier to re-use a set of command line flags across different commands.

Maybe it's better to allow direct use of result of Flags() from snippet above.
I found that it can be achieved this way (via late binding though):
import typer
from dataclasses import dataclass

@dataclass
class BaseFlags:
    arg1: int = typer.Argument(help='something', default=1)
    arg2: int = typer.Argument(help='something', default=2)

@dataclass
class Flags(BaseFlags):
    arg3: int = typer.Argument(help='something', default=3)
    arg4: int = typer.Argument(help='something', default=4)

    def __post_init__(self):
        main(self)

def main(args: Flags):
    print(args)

if __name__ == '__main__':
    typer.run(Flags)

Or with helper:
from dataclasses import dataclass
from typing import Any, Callable, Type

import typer

T = TypeVar('T')

def run_dataclass(tp: Type[T], callback: Callable[[T], Any]) -> ...:
    assert is_dataclass(tp)

    @dataclass
    class BindType(tp):
        def __post_init__(self):
            super().__post_init__()
            callback(self)

    return typer.run(BindType)

First snippet can be reduced to:
from dataclasses import dataclass

@dataclass
class BaseFlags:
    arg1: int = typer.Argument(help='something', default=1)
    arg2: int = typer.Argument(help='something', default=2)

@dataclass
class Flags(BaseFlags):
    arg3: int = typer.Argument(help='something', default=3)
    arg4: int = typer.Argument(help='something', default=4)

def main(args: Flags):
    print(args)

if __name__ == '__main__':
    run_dataclass(Flags, main)

Hi @arquolo, thanks for your answer!  Is it supposed to work as is?

Here's another helper which is based upon decorators. It might still be a bit rough around the edges (and I didn't test this with Typer because I need to get back to work) but at least the general idea should be clear:
from dataclasses import dataclass, is_dataclass
from typing import Callable, Type, TypeVar

@dataclass
class MyArguments:
    name: str
    formal: bool = False

T = TypeVar('T')
R = TypeVar('R')

# decorator generator
def inject_dataclass_args(cls: Type[T]):
    assert is_dataclass(cls)

    def decorator(func: Callable[[T], R]) -> R:
        def wrapped(*args, **kwargs):
            args_as_data_obj = cls(*args, **kwargs)
            return func(args_as_data_obj)

        wrapped.__annotations__ = cls.__init__.__annotations__
        return wrapped

    return decorator

# Usage:

@inject_dataclass_args(MyArguments)
def goodbye(data_obj: MyArguments):
    if data_obj.formal:
        print(f"Goodbye Ms. {data_obj.name}. Have a good day.")
    else:
        print(f"Bye {data_obj.name}!")

if __name__ == "__main__":
    typer.run(goodbye)

One issue that I'm already foreseeing is that Typer won't notice that formal is an optional parameter because it's not part of the annotations. I suppose Typer does some inspect-module-related magic here to check for optional parameters, so one would have to do some similar reverse magic to ensure that inspect yields the right results and not *args, **kwargs.
A quick test shows that @arquolo's solution works better here because @dataclass already sets up the right signature for MyArguments.__init__. So to fix my solution one would either have to look into how @dataclass achieves this or, inspired by @arquolo's suggestion, one could replace the above decorator with this monster:
def inject_dataclass_args(cls: Type[T]):
    assert is_dataclass(cls)

    def decorator(func: Callable[[T], R]) -> R:
        @dataclass
        class wrapped(cls):
            def __post_init__(self):
                super().__post_init__()
                func(self)

        return wrapped

    return decorator

Either way, the nice thing about a decorator here is that it keeps the signature rewriting close to the function whose signature it's changing.

Side note: On a meta level this issue is somewhat related to determining (and "reproducing") the full type of a function which has already been discussed here.

I tweaked @codethief 's code and came up with this: https://gist.github.com/tbenthompson/9db0452445451767b59f5cb0611ab483

I have an already productionized version of this idea, and it's here:  https://github.com/rec/dtyper

I have an already productionized version of this idea, and it's here: https://github.com/rec/dtyper

@rec From a skim of the docs, I think dtyper is doing the reverse of what's asked for here. It's making a dataclass from a CLI function. I already have a dataclass and I want to create a CLI function from that existing dataclass. Am I misunderstanding?

Oops, no, the misreading is mine. You are exactly right that is the reverse
thing.
However, I don't quite see how you can really get this to work. How can you
set the help for each argument? Or the names of command line flags?
On Fri, May 12, 2023 at 11:18 PM Ben Thompson @.***>
wrote:

I have an already productionized version of this idea, and it's here:
https://github.com/rec/dtyper
@rec https://github.com/rec From a skim of the docs, I think dtyper is
doing the reverse of what's asked for here. It's making a dataclass from a
CLI function. I already have a dataclass and I want to create a CLI
function from that existing dataclass. Am I misunderstanding?
—
Reply to this email directly, view it on GitHub
https://github.com/tiangolo/typer/issues/197#issuecomment-1546312127,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAB53MQ7FAFIDNAEJXB3DF3XF2SKTANCNFSM4UBMIWRA
.
You are receiving this because you were mentioned.Message ID:
@.***>

--
/t
PGP Key: @.***
https://tom.ritchford.com https://tom.ritchford.com
https://tom.swirly.com https://tom.swirly.com

Oops, no, the misreading is mine. You are exactly right that is the reverse thing. However, I don't quite see how you can really get this to work. How can you set the help for each argument? Or the names of command line flags?
…

The solution above manages this by copying the type signature of the dataclass' __init__ function. This gets you flag names and defaults:
@dataclass
class Test:
    config: str = ""
    hi: int = 1
    bye: str = "bye"

@dataclass_cli
def main(c: Test):
    """docstring test"""
    pass

you get a CLI like:
> python config.py --help

 Usage: config.py [OPTIONS]

 docstring test

╭─ Options
│ --config        TEXT
│ --hi            INTEGER  [default: 1]
│ --bye           TEXT     [default: bye]
│ --help                   Show this message and exit.
╰─

This is enough for making a quick and easy CLI for personal use or a rough project.


the '[' character doesn't work in editor but does in settings view

I only have this error in windows. In linux it works fine.
When I type the character in settings view it does work. In the editor if I do "alt gr + [" it doesn't print the character. Instead it prints nothing at all.

This seems like a duplicate of atom/atom-keymap#35-can you check that out please and see if that's what your issue is talking about?

Yes indeed installing keyboard-localization solved the problem. My bad, but please fix it out of the box on final release. It's confusing for new users.
Thanks for the help!

@Vincentdc94 it'll hopefully get fixed as part of Google's Summer of Code :smile:.  Glad to hear that you fixed your issue, though!


Add pipeline trait

Add Redis Pipelining fonctionnality: http://redis.io/topics/pipelining
It is the same thing of "Transaction" but commands MULTI and EXEC are not sent and it can be run on master or on slave.

@etaty could you show an example of how is it possible to use pipelining without transactions? Not very clear from docs 🙂


Provide consistent examples of bindings for all SDKs

Thank you for helping make the Dapr documentation better!
Please follow this checklist before submitting:

[x] Read the contribution guide
[x] Commands include options for Linux, MacOS, and Windows within codetabs
[x] New file and folder names are globally unique
[x] Page references use shortcodes instead of markdown or URL links
[x] Images use HTML style and have alternative text
[x] Places where multiple code/command options are given have codetabs

In addition, please fill out the following to help reviewers understand this pull request:
Description
Modified bindings documentation page (under building blocks) for dapr.io
Issue reference
This partially fulfills: #1969

Looks like this comment from me is not yet addressed. Please either accept and commit the suggestion, or else resolve the comment if you are taking this in another direction.
https://github.com/dapr/docs/pull/2021/files#r762207024


fix fishing xp and rarities, remove unnecessary rare tag

fixes fishing xp and rarities based on data from the official wiki and in-game bestiary
removed rare tag from taurus and deep sea protector since i dont think most people would consider them a "rare" mob
(sharks were excluded from getting their rarity updated since they're apparently all special and that is weird)

should've seen sooner but the same changes in #73 are also here

Should Zombie Miner be a rare sea creature? (not the Hypixel rarity, the rare tag for SkyHanni features)

i'd say yes, if you're fishing while having the requirements to catch it, you're most likely trying to catch it specifically

you can add the rare tag in this pr as well if you want


Feature request: Allow don't care _ as default argument for out arguments

Personnel

[x] Owner: @apinski-cavium
[x] Supporters: @mbudiu-vmw

Design

[x] Document: See below

Implementation

[ ] p4-spec:
[ ] p4c:

Process

[ ] LDWG discussed:
[ ] LDWG approved:
[ ] Merged into p4-spec:
[ ] Merged into p4c:

=======================================
It would be a good idea to allow don't care _ as default arguments for out arguments so you don't need to specify them when doing the call.
I can see cases where it would be useful in the general sense.
An example of simple usage would be like:
void f(out bit a = _){ a = 1; }
void g(out bit b)
{
  f();
  f(b);
}

You could do this right now with a wrapper function:
void f(out bit a){ a = 1; }
void f(){ f(_); }
void g(out bit b)
{
  f();
  f(b);
}

So it just becomes a nice feature of the language.

Solved by https://github.com/p4lang/p4-spec/pull/1074

Solved by #1074

Huh? No that is a different feature request unrelated to this one.
That only just solves f(a = _); for the call. I am asking for calls like f(); where the default argument was don't care.

In the interest of tidying up the set of active issues on the P4 specification repository, I'm marking this as "stalled" and closing it. Of course, we can always re-open it in the future if there is interest in resurrecting it.


Refactor Token Management: Introducing getTokenSilently for Multi-Tab Consistency and Enhanced Authentication Reliability

Why is this pull request needed?
Limitations of the Current Approach
The existing token management strategy, which relies on periodic checks via setInterval and application context state, introduces critical issues, especially in multi-tab environments. Without a unified storage mechanism, each tab may store and update different token values, leading to conflicting states and unreliable authentication.
Additionally, this approach does not effectively support refresh token rotation. With rotation enabled, each token refresh generates a unique new token, which can lead to errors when multiple tabs hold outdated or mismatched tokens. Although using static refresh tokens reduces the chance of errors by sharing the same token across tabs, it is less secure, as a single compromised token could grant unauthorized access.
Solution: getTokenSilently
To solve these issues, a new approach using getTokenSilently was introduced. This method ensures only one token refresh occurs at a time across all tabs, preventing conflicts by using a mutex to control access and check token expiration before refreshing. By synchronizing token updates, getTokenSilently effectively supports refresh token rotation, keeping authentication states consistent and reducing errors across tabs.
What does this pull request change?

Prevent state conflicts with multiple tabs: The removal of setInterval and introduction of a mutex-based strategy ensures that token refreshing does not conflict between tabs.
Improved token retrieval strategy: The new getTokenSilently function ensures consistent access to the token and handles expiration more reliably, reducing errors.
Unified state management: Combining logoutInProgress and loginInProgress into isLoading simplifies the logic and usage within the application.
Better authentication state tracking: Introducing isAuthenticated provides a clear and direct way to check if the user is authenticated.
Consistent state storage: Refactoring useBrowserStorage to use only browser storage eliminates state synchronization issues during token refreshes.
Manual event dispatch: Ensures that storage events are handled correctly within the current tab, maintaining consistency.
Flexible storage change handling: Adding an onChange callback offers more flexibility in responding to storage changes.
Simplified error handling: Removing handleExpiredRefreshToken shifts error handling to the application, providing more control to developers.
Post-logout actions: Adding postLogout callback enhances the customization of the logout process.
Streamlined context: Removing the token and loginInProgress from the context reduces redundancy and encourages the use of the new mechanisms.
Preventing batch requests: Using async-mutex ensures that only one refresh token request is processed at a time, avoiding potential rate limiting issues.

Moving Forward
I understand that not all features in this pull request may be merged as-is, but moving in this direction could significantly improve the system’s reliability and address the issues pointed out, especially around multi-tab environments and token rotation.
Please note that this is a breaking change, which may require adjustments in other parts of the application. I’m open to discussing any aspect of this proposal to ensure we find the best solution for stable, secure, and consistent token management.

Hey @kleysonfiretail ! Thanks for contributing to the library!
I'm still reviewing the contents of the PR, so I have some preliminary comments/questions. When it comes to whether this is a direction we want to go with the library, I'll have to discuss with @soofstad.

I see that you list 11 separate changes you've made with this PR, and I'm wondering if it would be too much to ask to make 11 separate PR's from main?
The problem with a PR like this is the sheer volume of change, and even if we wish to take in some of the changes, we might not want to take in all of them, and in such a case, accepting the PR may be troublesome.

Another comment, that I'm sure @soofstad will make as well, is that this PR actually adds a dependency to "async-mutex": ">=0.5.0", while the library strives to be dependency-free.

I see in types.ts that in addition to adding some keys to IAuthContext (which is fine), the PR also removes loginInProgress. This is a breaking change, and we don't want any of those if we can avoid them. Any way to keep the loginInProgress property? I know that we use that in our application to support some messages in the user's UI.
If we introduce this breaking change, we'll have to bump the major version and library users will have to change their code to continue using the latest version of the library. Not something we take lightly.

Totally support @sebastianvitterso.
Thanks for the effort and interest @kleysonfiretail.
Lets discuss these changes in separate PR's/Issues :)

Hi @kleysonfiretail
Hope you haven't lost interest in helping to improve the library.
Could you please elaborate on:

each tab may store and update different token values, leading to conflicting states and unreliable authentication.

Have you been testing with the latest version, and if so, how can we replicate any issues you have been having?


chore(BDD): add integration test to provision & de-provision pvc by disabling volume-events-exporter

This PR adds integration test verify PVC provisioning and de-provisioning
by disabling volume-event-exporter sidecar. Once PVC is de-provisioned then
enabling volume-event-exporter should export both sent and delete events to
metrics server.
Signed-off-by: mittachaitu<EMAIL_ADDRESS>

Codecov Report

Merging #8 (29cb98d) into develop (e50bac3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop       #8   +/-   ##
========================================
  Coverage    26.64%   26.64%
========================================
  Files            5        5
  Lines          334      334
========================================
  Hits            89       89
  Misses         235      235
  Partials        10       10

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 e50bac3...29cb98d. Read the comment docs.


Default http server connection buffer size

I am on branch 0.1.13-release.
I see that the default BOOST_NETWORK_HTTP_SERVER_CONNECTION_BUFFER_SIZE is 1024 bytes.
I did some tests (by uploading 250 MB of file) and I see that setting it to 8192 bytes increases the transfer rate by 7 folds. With 4096, the transfer rate is increased by close to 5 folds. The test code is in here
Can we increase the default value of BOOST_NETWORK_HTTP_SERVER_CONNECTION_BUFFER_SIZE to 8192 bytes or may be 4096 bytes (for memory constrained systems)? (Few KB's shouldn't matter, I guess).

@deanberris Can you please suggest?

I thought I responded to this but I can see that I hadn't. ☹
I'm fine with increasing the default. I'd also consider making this a value that's configurable through the HTTP server options, and use the macro to define the default.
Happy to review a pull request that changes it to 4K to begin with.
Thanks @carun for the testing and suggestion!

PR on the way. hg user here.  :(


[Run-Plugin]  [Unit Converter] Add LocProject.json to UnitConverter plugin

Provide a description of the new feature / enhancement

Add LocProject.json to UnitConverter plugin after localization team added translations
Update the installer

What's still needed here?
cc @jaimecbernardo

Hi @dedavis6797 ,
I see no loc folder in https://github.com/microsoft/PowerToys/tree/master/src/modules/launcher/Plugins/Community.PowerToys.Run.Plugin.UnitConverter
I think the localization team still has to pick this up.

Hi @dedavis6797 ,
I see no loc folder in https://github.com/microsoft/PowerToys/tree/master/src/modules/launcher/Plugins/Community.PowerToys.Run.Plugin.UnitConverter
I think the localization team still has to pick this up.

These folder is missing because there is no LocProject.json. And btw. we can only translate nane, description and context menu name.

This is superseded by #15242.
Closing it out -- let me know if that's not right :)

What @DHowett said

(might want to set status in 0.53 project?)

(might want to set status in 0.53 project?)

@crutkas


Ensure test groups include those with variety of accessibility issues

After discussion on how we show results as viewed, wider discussion on our testing process and more detail relating to 'tiers' of accessibility. We need to ensure we have people from a variety of those tiers testing and feeding back so we can make the site as accessible as possible.

@neslihanu I've assigned this one over to you. It would be brilliant if you get a chance to review where @avielbassalian-freeukgen got to with this..

I talked to Aviel regarding this task. Here is the update;
In summary, he didn't do anything for that because A) He didn't have the means to find these people to test with and B) testing with these people isn't neccessary to meet WCAG accessibility standards. He recommended using  online testing tools to meet WCAG AA.
If you  think we need these people to test with, you can send out a questionnaire to the mailing list that Denise and Aviel put together. Ask for anyone with disabilities to categorise themselves so that you can contact them directly.

Thanks @neslihanu (and indeed @avielbassalian-freeukgen). That sounds fine for now - I'll create a separate story in regards to testing using WCAG tools, keep this in mind when we roll out to a closed and indeed open beta group. We can close this one.


deal with deprecated vcat warnings

This is to deal with warnings such as:
WARNING: [a] concatenation is deprecated; use collect(a) instead
 in depwarn at deprecated.jl:73
 in oldstyle_vcat_warning at ./abstractarray.jl:29
 in lifter at /home/james/.julia/v0.4/MFCC/src/rasta.jl:242

and
WARNING: [a,b] concatenation is deprecated; use [a;b] instead
 in depwarn at deprecated.jl:73
 in oldstyle_vcat_warning at ./abstractarray.jl:29
 in vect at abstractarray.jl:38
 in lifter at /home/james/.julia/v0.4/MFCC/src/rasta.jl:242

Thanks for the PR.  I happened to have been busy with MFCC just this week after a long period of standstill, and found the same deprecation warning as you, and fixed these.  I hadn't pushed them yet.
Could you see if the latest checkout (with will not merge with your PR, you'll have to step back) does it for you?

Sure. Those changes worked. Thanks.


Block container's TCP traffic to the special IP address <IP_ADDRESS>.

<IP_ADDRESS> is a special Azure IP that hosts various info, including priviledged settings, thus we need to block container/pod's access to it.

just curious, have you tested it with kubenet and azure cni?

just curious, have you tested it with kubenet and azure cni?

Yes, it worked for both the default kubenet network setting and the "azure" network plugin.
"Worked" here means the containers running in pods cannot "curl http://<IP_ADDRESS>..."


Add volumeMount for /etc/istio/config in istio-auth.yaml

Without this PR if I do:
kubectl apply -f ./kubernetes/istio-auth.yaml

I get ingress/egress pods in error state and in pod's log I see:
Error: failed to read mesh configuration. cannot read mesh config file open /etc/istio/config/mesh: no such file or directory

Hi @mwieczorek. Thanks for your PR.
I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

I signed it

/ok-to-test

@sebastienvas @andraxylia are we not testing auth YAMLs file? How did this slip through?

@kuat @yutongz is currently working on this.

Current e2e smoke test doesn't including auth. I am working to deploy istio-auth in e2e test.

/test all

/lgtm

/lgtm

/approve no-issue

/lgtm

Do we need a similar change in pilot?

@yutongz For whatever reason the submit queue seems to be on Vacation.

@andraxylia which pilot you meant?

@sebastienvas  I didn't notice IBM test is not required any more. Merge-bot still supposes that's required and waits it green. Updating it.

Any way that the bot uses the same list as GitHub?


auto-start break time should be optional

Is your feature request related to a problem? Please describe.
Not always I can simply stop working and have a break.
It's like reading a book, before sleep. If you feel asleep while reading, you can't stop in the middle of a sentence and sleep. It's best to finish the whole page or a sentence, at least.
Describe the solution you'd like
When work time finishes, play button could be replaced by another button representing break time. When user clicks on it, then breaks starts. Otherwise, break time may start in a not so desired time. Hence, user may loose control of the app.
Additional context
I'm taking all of these ideas from TickTick. It may help downloading their mobile app to improve yours.

You have a point with this. I will try to make it soon as I can. 👍

Hello @hunkjazz . I tried to implement the feature you've requested.
I found it conflict with Full-screen Break feature.
Let say, the user enabled Full-screen Break and disabled Auto-start Break Time.
Since there's no play, paused, skip or reset button while user's still on full-screen mode. It is impossible for the user to exit the app. The only way to quit the app is to restart the operating system.
What I am proposing instead, I will add feature that enable the user from extending the work time if needed.
Like for example, it takes 5 - 10 minutes to finish the task and the user wants to finish it first before he/she takes the break.
The user can extend the timer duration up to let say 10 minutes. It makes the app flexible enough from what the users need.
This feature might helpful too if I started to develop Productivity Report.
What do you think?

best of both worlds!

I will just put Time Extender feature to Roadmap as of now.
I tried to develop it and used the notification property of node-notifier for that but unfortunately Linux doesn't support it.
I will try to do some alternatives as soon as I can.

Hi Roldan and thanks for your great app.
The idea with extending the time work time sounds good on paper, but I’m a bit worried about it in real life situations. When am I supposed to extend the time? Before the break begins? In real life, I’m working, I’m focused for the whole time (and I want to keep that this way) and don’t know there is a break coming. When the break hits, should I just press “add 5 minutes” button, the break ends, and the work time continues again? I don’t know, it seems strange.
There is another solution, to notify user when the break is coming, but it’s honestly contra productive solution, while the app is supposed to let me be focused. So many notifications are disturbing. And see a notification saying “you have 30 second to delay the break or the brake force you too…take a break” :D is just ridiculous and totally kills my ongoing focus.
In other apps, the straightforward solution is to just stop the timer after 25 mins and wait for the user to start the break timer. I’m quite happy with this solution even though it’s not perfect. I understand, you mentioned the problem with full screen mode. But doesn’t that mean there is a need to fix/enhance the full screen mode to have some button for this purpose, rather than twist and complicate other parts of the app? Or maybe just simply disable the fullscreen feature when using this discussed feature?
Other way it can work and it would fit to your Productivity Report: the work timer doesn’t stop after 25 minutes – it gives the user notification, that the work time is over and is running over (the timer can get red).
This solution is great for those reasons.

The user knows now, he should take a break but isn’t force to do something in 30 seconds
His focus isn’t broken
No other “prenotification” bothers me
I can decide if I take a break now or just finish the feature, which takes me for example 7 minutes and 36 second – which is something I don’t know in the beginning off course, so it makes sense to just start the break when I finish, not to guess “hmm, give me another 5 or 10” and then add another time if I don’t make it or finish early and have some spare time – it’s silly. I’ll get just the right time I need and then stop.
The over time will be logged for the purpose of report.

What do you think about it?


Variable Font decoding

Description
In Rive, we're using SF Pro which is a variable font. Rive just added support for variable fonts, as far as we understand.
We set this font as referenced as to not include it in the main file (to avoid a larger file size).
Since the font is referenced, we need to set it within code or no text is displayed.
In code, we want to be able to set the font to the native font on iOS (also SF Pro) and specify the size, weight, and width. However, if we set it using fontAsset.font(riveFont), that means all text using that font use the same size/weight/width setting whereas in the actual Rive file we use bold for some of the text and regular for other parts despite using the same font.
What's the appropriate method for being able to specify how the variable font in Rive should map in code?

On follow-up, it looks like Rive (correctly) overrides the font size, weight, and width.
I'll keep this issue open though if there's more to it.

Perfect, thanks @dskuza. May be a nice addition to the documentation for the future.
I'll close this issue.


Rudimentary filtering added

Closes issue #20

The filter works on only one field per table / page (at this time).

Trips: filters on the "Cave" field
Cavers: filters on the "Name" field
Caves: filters on the "Name" field


Allow the WireMock standalone to specify notifier.

Allow the WireMock standalone to specify notifier.
Currently WireMock standalone server always uses 'ConsoleNotifier' as its notifier. A nice improvement would be to make this configurable. This would allow request mismatch logging to slf4j.
Our specific use-case is, we run WireMock standalone in Mesos and we don't have easy access to those logs but we can forward slf4j to an ELK stack making the logs available for all our devs.

Hello!
I faced the same problem trying to configure WireMock to log in a specific format. For example: if I want to use SL4J to deliver logs to Splunk, or GrayLog. Currently WireMock logs requests journal as a multiline pretty json. These logs cannot be parsed easily by Splunk. It would be nice to have the ability to set custom notifier to transform logs.

Ready for review

PR needed merging again after 2 months of nothing. Not interested in only resolving merge conflicts without result.


Creating new group didn

Students being added to the group didn't appear to be added until the page was refreshed.
This occurred on Chrome.
This was attempted after the marks were released.
Students had previously been in a group but were manually removed.
(Not sure which of these details are relevant)
This happened to Tom in CSC108

I tried to reproduce the issue but wasn't successful.
I tried many combinations of actions.
Removing 2 student from the same group, creating a new group, adding the 2 students to it
Removing 2 student from 2 different groups, creating a new group, adding the 2 students to it
Removing 2 student from the same group, adding the 2 students in an existing empty group
Removing 2 student from 2 different groups, adding the 2 students in an existing empty group
Tried these 4 combinations with marks released & not released, to no avail.

Couldn't reproduce.


delete message

how to delete message in use this module?
if (responses.status === 200) {
messages.forEach((obj) => {
console.log(obj.type)
SmsAndroid.delete(

        obj._id,
        (fail) => {
          console.log('Failed with this error: ' + fail);
        },
        (success) => {
          console.log('SMS deleted successfully');
        },
        );
     })
   }
  }, 1000);

//error
LOG  1 // obj._id
LOG  Failed with this error: SMS not found //this error
help me\

Hi Updour. Apologies for the delay, was off for a while. Going to look into this and get back to you.


WIP: Add Freesurfer 7

I added Freesurfer 7 and some additional jobs on circleci to test and deploy it. Circlci config will likely need to be fixed before this is complete.

@PeerHerholz and @andysworth this resolves #57 with the potential exception of the replacement recon-all script. I'd be happy to get your feedback on it.

Ahoi hoi @Shotgunosine,
thx for the fantastic work and effort. The same to you @andysworth.
I checked the code and tests, LGTM.  The plan re the recon-all script would be include it once we have it, eh?

Yeah, That's what I'm thinking.

Awesome! @andysworth, do you want to have a look as well? After that or otherwise, we could go ahead and merge!?

Sorry for the slow response.  This looks good to me.  I just noticed that I got a response on the Freesurfer mailing list from Doug Greve:

Hi Andy, this is a bug introduced in 7.1.1, some kind of race condition.
You can fix it by commenting out this line in recon-all
if($OMP_NUM_THREADS > 1 && ! $LHonly && ! $RHonly) set cmd = ($cmd --parallel) # just hemis
doug

Thanks for your strong work @Shotgunosine!

Cool, thanks @AndysWorth! @Shotgunosine should we go ahead and merge? WDYT re the recon-all script adaptation above?

We should add a sed command to the docker command, probably the best place
would be in neurodocker. @AndysWorth, do you think you could open an issue
there and submit a PR?
On Fri, Nov 13, 2020 at 8:42 AM Peer Herholz<EMAIL_ADDRESS>wrote:

Cool, thanks @AndysWorth https://github.com/AndysWorth! @Shotgunosine
https://github.com/Shotgunosine should we go ahead and merge? WDYT re
the recon-all script adaptation above?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/BIDS-Apps/freesurfer/pull/67#issuecomment-726771234,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABILLGBJ27Y7SRH3TGMV4MTSPUZU7ANCNFSM4TI2FGUA
.

Will do!

Hi folks,
just checking: we can merge this, right? @Shotgunosine @AndysWorth

Sorry, I was looking forward to making that contribution but it seems that I'm slowing this down.  It looks like I won't have time until I'm off work for the holidays.  @Shotgunosine if you want to go ahead with your suggestion in the meanwhile, that would be great.  I know it's a small change but I don't feel confident to just do it without taking the time to make sure I fully understand what's going on.

I might not be able to get to it until around the same time period. Maybe @llevitis would be able to take a look

I've installed the fix here at Flywheel -- once I have had a chance to run it a few times I'll submit it to neurodocker.

I've installed the fix here at Flywheel -- once I have had a chance to run it a few times I'll submit it to neurodocker.

@AndysWorth have you had a chance to send the fix over to neurodocker?

@Shotgunosine Sorry, no 😞 .  There are two places that need patching in recon-all and the fixes have worked well at Flywheel so it's about time!  I'll work on making the time.  Thanks for the reminder.

Hey @Shotgunosine , I (finally) managed to patch Freesurfer 7.7.1 in neurodocker.  See neurodocker PR#384.  I did this exact same change to the official Freesurfer docker image version of 7.1.1 and it has been run successfully many times on various Flywheel instances.  I have not tested this new neurodocker version, which is for a slightly different version of centos (6 vs. 7).  Is this PR sufficient for you to give it a try?  All it should do is prevent crashing.

@Shotgunosine I've created a repository recon-all-v7.1.1-parallel-patch.  Right now it just has the patch file that can be installed with
patch /usr/local/freesurfer/bin/recon-all parallel.patch
Is this enough?  If not, what else should I do?

@AndysWorth, can you update the neurodocker PR to pull that repo and apply the patch?

@Shotgunosine I'm not familiar enough with neurodocker.  Where would I do that?

@Shotgunosine It sounds like they don't want the patch in neurodocker so the following should go somewhere around here?
curl -LJO https://github.com/AndysWorth/recon-all-v7.1.1-parallel-patch/tarball/master
tar zxf AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe.tar.gz
cd AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe
patch $FREESURFER_HOME/bin/recon-all parallel.patch
patch $FREESURFER_HOME/bin/quantifyThalamicNuclei.sh quantifyThalamicNuclei.patch
cd ..
rm -rf AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe

I’ll see if I can use the script you wrote to get a command into
neurodocker that’ll make them happy.
On Thu, Apr 1, 2021 at 4:57 PM Andrew J. Worth @.***>
wrote:

@Shotgunosine https://github.com/Shotgunosine It sounds like they don't
want the patch in neurodocker so the following should go somewhere around
here
https://github.com/Shotgunosine/freesurfer/blob/fs_7/generate_freesurfer_images.sh#L96
?
curl -LJO https://github.com/AndysWorth/recon-all-v7.1.1-parallel-patch/tarball/master
tar zxf AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe.tar.gz
cd AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe
patch $FREESURFER_HOME/bin/recon-all parallel.patch
patch $FREESURFER_HOME/bin/quantifyThalamicNuclei.sh quantifyThalamicNuclei.patch
cd ..
rm -rf AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/BIDS-Apps/freesurfer/pull/67#issuecomment-812168436,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABILLGC46EKVCXGLUICFAPTTGTM4FANCNFSM4TI2FGUA
.

With the new clarifications can you take care of it @Andysworth?
On Thu, Apr 1, 2021 at 5:15 PM Dylan Nielson @.***> wrote:

I’ll see if I can use the script you wrote to get a command into
neurodocker that’ll make them happy.
On Thu, Apr 1, 2021 at 4:57 PM Andrew J. Worth @.***>
wrote:

@Shotgunosine https://github.com/Shotgunosine It sounds like they
don't want the patch in neurodocker so the following should go somewhere
around here
https://github.com/Shotgunosine/freesurfer/blob/fs_7/generate_freesurfer_images.sh#L96
?
curl -LJO https://github.com/AndysWorth/recon-all-v7.1.1-parallel-patch/tarball/master
tar zxf AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe.tar.gz
cd AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe
patch $FREESURFER_HOME/bin/recon-all parallel.patch
patch $FREESURFER_HOME/bin/quantifyThalamicNuclei.sh quantifyThalamicNuclei.patch
cd ..
rm -rf AndysWorth-recon-all-v7.1.1-parallel-patch-e5ff2fe
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/BIDS-Apps/freesurfer/pull/67#issuecomment-812168436,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABILLGC46EKVCXGLUICFAPTTGTM4FANCNFSM4TI2FGUA
.

Hi, are there any blockers to merging that very old PR?
It would be great to have latest versions integrated into repronim/containers.
Anything I can do to help?
Should we restart a PR from https://github.com/e-dads/freesurfer ?
Thanks!

I haven't looked at this in a while, but I think this convo with Satra is where things were left off:
https://github.com/ReproNim/neurodocker/pull/384#issuecomment-812187770
@bpinsard, are you comfortable with the structure of everything to just make that change? It'd take me a while to reestablish context here.

Actually, looks like we could just jump to Freesurfer 7.4.1. Would that be alright?

I am not sure I have played enough with neurodocker to fully grasp the issue to be fixed.
If I understand, that would mean:

inserting the patch in your branch as @satra suggested into that file: https://github.com/Shotgunosine/freesurfer/blob/fs_7/generate_freesurfer_images.sh
regenerating Dockerfile and Apptainer recipes.
The latest version would be great! I don't think there is really an interest in redeploying each older version.

I can have a look to try to fix some of the merge conflicts on the circle-CI config. I have tried to "standardize" across bids apps to make the maintenance easier.

I'm working on 7.4.1 now, to do that I've got to update the neurodocker version, which breaks some of the syntax. I'm working on it though.

I had the generate_freesurfer_images.sh running with the following changes:
https://github.com/courtois-neuromod/freesurfer/tree/pr/67
If I understand correctly the patch are not needed with that version, right?

@Remi-Gau I took a first pass at resolving merge conflicts on the circleci config, but I've been out of the circleci game for a minute. Do you think you can fix the config now, or have I just made a mess of things?

had a quick look
it seems to make sense but may need to go for a deeper dive: those diffs are hard to read

also seems that some file changes are showing in the PR when they are already in the default branch, no?

I tried to rebase the PR thinking that that would be more straightfoward, but may have just made a mess. Happy to do a force rollback if you think that'd be easier.

yeah if you can try to get a cleaner diff it'd be great
let me know if I can help

Alright, should be cleaner now.

yup indeed

also just thinking: if we agree that we want both freesurfer 6 and 7 to be available it should probably be mentioned in the readme how to pull each from docker hub.
Does not have to be part of this PR: can be done once a release has been done and a tagged image has been pushed

I think we will want both freesurfer 6 and 7 available. HCP still uses freesurfer 6.

having a look locally at the whole config.
it looks very much like this is doing exactly the same thing for both images, no?
like literally running the same commands with both containers
sounds like this should probably be refactored, no?

Definitely, do you think you could refactor it? It should be straightforward, I just don't know circle's syntax very well.

Definitely, do you think you could refactor it? It should be straightforward, I just don't know circle's syntax very well.

yup I will give it a go

ok refactored things now let's see how this baby flies on the circle-ci side...

Note that given that one of the preivous build did not complete because we hit the circle CI "wall time" I think we may have to move to github actions that will give us 6 hours before we get cancelled. I am not as good with caching things in GHA action though...
https://app.circleci.com/pipelines/github/bids-apps/freesurfer/82/workflows/bc88a5ed-4c03-4c1e-ba7a-dd0b33577e60/jobs/340
But let's wait and see how things are going on this build... Not holding my breath.

note that the image is using an antiquated bids-validator version (0.19.8 when the current one is 1.12.0): may be worth bumping this up.

ok the image build is failing because of some conda hanging: https://app.circleci.com/pipelines/github/bids-apps/freesurfer/87/workflows/d6fe415d-5abd-427d-95c9-792ab1bf0073/jobs/355
trying locally

The only that changes in the bash script used to generate the different recipe is the freesurfer version, no?
I think this should be refactored too

@Shotgunosine can you check if you can build the image using the docker recipe: it seems to fail in CI and for me locally.
May need to adapt the neurodocker command

If hangs forever solving the environment for me locally as well. I'm trying to see if there's a way to install pandas that doesn't hang, but I haven't found anything yet. Might need to add mamba to neurodocker.

If hangs forever solving the environment for me locally as well. I'm trying to see if there's a way to install pandas that doesn't hang, but I haven't found anything yet. Might need to add mamba to neurodocker.

maybe specifying the exact version of each package may help?

I was able to get it to build with a local version of neurodocker that I added a mamba option to, and I've submitted a pull request for that: https://github.com/ReproNim/neurodocker/pull/562
Note that for generate_freesurfer_images.sh I actually used my local neurodocker image, not that sha that's in there now.

@Remi-Gau I've got no idea why circle is trying to do anything with /home/circleci/license/license.txt (which is the source of the failure here: /home/circleci/license/license.txt). As far as I can tell I've changed all references to that directory and the build ran past it when I changed the entry point to python. Do you have any ideas? I'm stumped.

Edit: Nevermind , figured it out.

🚀

will make a dummy change to retrigger circle ci

getting somse validator error: will bump its version

getting somse validator error: will bump its version

totally missed that you had done that and completely refactored the bash script: you've been busy

Bah, looks like circleci webhooks are broken at the moment. Is there anyone who can trigger a manual build?

Is there anyone who can trigger a manual build?

just did: let's see how this one goes

see if we can create specific dependencies between the steps of the circle ci so that test for fs6 starts when the build for fs6 is done.
Currently the tests can only start when both builds are done.

see if we can create specific dependencies between the steps of the circle ci so that test for fs6 starts when the build for fs6 is done.
Currently the tests can only start when both builds are done.

Yep, found the docs on matrix and they explain how to set it up. The circleci graph makes it looks like the dependencies are correct now.

Yep, found the docs on matrix and they explain how to set it up. The circleci graph makes it looks like the dependencies are correct now.

nice!!!
thanks for looking into this

@Remi-Gau, the tests are failing because they can't find anything at /home/circleci/data/ds114_test1_freesurfer_precomp_v6.0.0/00_group*. That part of the circleconfig predates this PR. Do you know what's going on with that?

get the same bug locally:
# get download script
wget https://raw.githubusercontent.com/bids-apps/maintenance-tools/main/utils/get_data_from_osf.sh

# get gata into home dir from https://osf.io/9q7dv/
bash get_data_from_osf.sh ds114_test1_freesurfer_precomp_v6.0.0

# create target dir
mkdir -p ${HOME}/outputs1 ${HOME}/outputs2

# try to copy
sudo mv ${HOME}/data/ds114_test1_freesurfer_precomp_v6.0.0/00_group* ${HOME}/outputs1/

no group folder
$ ls ${HOME}/data/ds114_test1_freesurfer_precomp_v6.0.0/

fsaverage  lh.EC_average  rh.EC_average  sub-01  sub-02

let me dig into the history of the repo to see what was happening in the good old days

here is the circle ci "step" that was used before I started working in the circle-ci:
https://github.com/bids-apps/freesurfer/blob/8c364a62ffbdef3b3e4f81a1c6bf4de5e9c2b5cd/.circleci/config.yml#L66C67-L66C67
more readable
docker run -ti --rm --read-only \
  -v ~/license/license.txt:/license.txt \
  --tmpfs /tmp --tmpfs /var/tmp \
  -v ${HOME}/data/ds114_test1:/bids_dataset \
  -v ${HOME}/data/ds114_test1_freesurfer_precomp_v6.0.0:/outputs \
    bids/${CIRCLE_PROJECT_REPONAME} /bids_dataset /outputs group2 \
      --license_file=/license.txt && \
mkdir -p ${HOME}/outputs1/ && \
sudo mv ${HOME}/data/ds114_test1_freesurfer_precomp_v6.0.0/00_group* ${HOME}/outputs1/ && \
cat ${HOME}/outputs1/00_group2_stats_tables/lh.aparc.thickness.tsv && \
cat ${HOME}/outputs1/00_group2_stats_tables/euler.tsv

I think I it is "just" a path issue: trying to fix in https://github.com/bids-apps/freesurfer/pull/67/commits/765a747844da7745a49b19c1087a8098ce213589

@Remi-Gau, it passed! Is there anything else we should fix for this PR?

woohoo!!!
I will merge and make a new release: this should trigger a push to docherhub.

Wow, glad my ping triggered a new release! Thanks for all the hard work! Waiting on the dockerhub push and will make a PR to repronim, can't wait to try it out!

Wow, glad my ping triggered a new release! Thanks for all the hard work! Waiting on the dockerhub push and will make a PR to repronim, can't wait to try it out!

FYI it is already on dockerhub but it is a bit of a mess at the moment:
see here
https://github.com/bids-apps/freesurfer/issues/75#issuecomment-1699911795
any thoughts on what would make most sense for users?


Does electron-push-receiver work in China?

my electron app not working in china region,  something weird happens there. checked in greatfirewallofchina.org "google.com" is blocked, but "firebase.google.com" succeeds yet app not working and not generates an fcm token.

push-receiver needs to access android.clients.google.com and mtalk.google.com. According to the site you've shared, they work in China. Do you see any error ?

Chrome Web Push / Firebase Cloud Messaging are blocked in China, therefore no, unfortunately electron-push-receiver cannot work in China.
Pushy, a commercial product, can deliver notifications in China and supports Electron.
Full disclosure - I work at Pushy.