fix: Restore "fix: nested lists roundtrip (#530)"

This restores commit e27ba08863e16ba8381e7ae06b5ea2a5ab18be01 which had been reverted in 88392b9dc174255dc5e7de442bfb19e4d0a08bf0.
The problem this caused last time with experience league got addressed, however we should do another round of testing.
Please ensure your pull request adheres to the following guidelines:

[ ] make sure to link the related issues in this description
[ ] when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues
Fixes #529
Thanks for contributing!

FWIW: We (DA) would love to have this merged sooner rather than later.

@buuhuu  did you check with experience-league if their code now works with collapsed lists?

I tested this with their dev env, but probably worth another iteration.

@tripodsan @buuhuu any update on this? This would be great to have


[move source language] Translated IR commands tests

Motivation

Translated the 'commands' directory
Updated test coverage file

Test Plan

ran em

@bors-libra r=tzakian

:pushpin: Commit 94b5e7f has been approved by tzakian

:hourglass: Testing commit 94b5e7f4c3126fc78663798c2179efdeceb7c3e9 with merge d59209f4fdb7ef6a973a66f4085deef1a549cf56...

:sunny: Test successful - checks-circle_commit_workflow
Approved by: tzakian
Pushing d59209f4fdb7ef6a973a66f4085deef1a549cf56 to master...


cannot find module npm/bin/npm/npm-cli.js

I cannot get npm install to run.

Windows 10 x64
Node v5.10.1
Atom 1.7.3

When I hit CTRLALTB, I get this error:
module.js:341
    throw err;
    ^

Error: Cannot find module 'C:\Users\Danny\Dev\handmade-quake-js\node_modules\npm\bin\npm-cli.js'
    at Function.Module._resolveFilename (module.js:339:15)
    at Function.Module._load (module.js:290:25)
    at Function.Module.runMain (module.js:447:10)
    at startup (node.js:146:18)

    at node.js:404:3

I made sure the only path I had in PATH related to node or npm was my node path.
Any advice on how to get atom-build-npm-apm to find my npm.cmd file?

Where do you get this error? In the build panel or as a notification (popup in Atom)?
How did you install npm?
Looking at the error message, it seems to be the other way around; npm.cmd is found, but it (which really is just a wrapper to run a node script) can't find its script (npm-cli.js).

Shows up in the build panel when i try to run any npm command such as npm install. I install node with choco, which should just be a standard install in Windows. npm works great through my command prompt. npm v3.7.0
You are guessing the wrong npm path is being created in the npm.cmd file? It becomes relative to where Atom is running instead of where node is which makes me suspicious it is apm or something. Boy really sure.

I'm confident that build correctly finds npm.cmd, however from there things seems flakey.
If you create an .atom-build.yml file containing only cmd: doesnotexist, then build will complain that doesnotexist doesn't exist :). It will print the PATH it's using. Make sure that there are no other npm.cmd on the path (Especially somewhere in C:\Users\Danny\Dev\...).

It is finding the correct npm.cmd. But apparently on Windows, it relies on %~dp0 to find node, but %~dp0 is being set to my project's cwd which ends up not finding node.
I'll poke around further.

Alright, figured it out. The package is trying to be a little too smart. At some point along the line, atom-build switched to cross-spawn-async which does not want commands to end in .cmd. Simply removing the Windows specific code around appending .cmd fixed it.


Generalizing reward models

Context
What is the purpose of this PR? Is it to

[x] add a new feature
[ ] fix a bug
[ ] update tests and/or documentation
[ ] other (please add here)

#1005
A hopefully quick add in parallel to my PPO PR to generalize reward models to also support e.g. llama2 reward models.
Model builders for the previous so-called classifier models are now correctly named as reward models, since they only classify a single label. I've also moved checkpoint conversion logic to modules/rlhf. This checkpointing logic can generally be useful for any classifier model, but I've tried to keep things simple and contained here without leaking the scope of this functionality to the rest of the API.
Previously, if someone wanted to use classifiers for their own recipes, they'd use the underlying component builder and set the model type to MISTRAL_REWARD. This hasn't really changed - they just now use REWARD, and the model builders are a bit clearer to indicate their default use is for reward modelling.
Test plan

[x] run pre-commit hooks and linters (make sure you've first installed via pre-commit install)
[ ] add unit tests for any new functionality
[x] update docstrings for any new or updated methods or classes
[x] run unit tests via pytest tests
[ ] run recipe tests via pytest tests -m integration_test
[ ] manually run any new or modified recipes with sufficient proof of correctness
[ ] include relevant commands and any other artifacts in this summary (pastes of loss curves, eval results, etc.)

Thanks so much for the review @felipemello1. Apologies for not providing a bit more context about the PR to help make things clearer. I discussed this briefly with @pbontrager on Discord in the "Discussing PPO" thread (which has been my defacto channel for pinging devs for contributing questions - maybe I should move to #dev..), it'd be great you have your input on some of the points we raised.
As you've correctly gathered, the main use for these classifier models is to support my work on RLHF to allow the use of pre-trained reward models during PPO. My primary aim for this PR was to reduce the review overhead on #1005, and I'd like my changes to be minimal and non-invasive (in that they can easily be refactored), and rely on future refactors (such as https://github.com/pytorch/torchtune/issues/1017#issuecomment-2155725874 ) to address some of the potential long-term effects on the codebase.

Are we comfortable with having 9 builders per model size? Maybe the answer is yes, and thats ok

There's been a few suggestions around this (see some comments in the original PR https://github.com/pytorch/torchtune/pull/837) - generally, there's some agreement that we can probably define these classifiers in a more minimal way like you suggested. However, we're touching on support for general classifiers vs support for classifiers as they're needed now in the codebase here. For now, I'd argue that we should revisit this further down the line when the need for generalisation is apparent and/or in a TransformerDecoder refactor.

Codecov Report
Attention: Patch coverage is 59.32203% with 24 lines in your changes missing coverage. Please review.

Project coverage is 68.96%. Comparing base (f4f83b2) to head (570dc8f).
Report is 4 commits behind head on main.

Files
Patch %
Lines

torchtune/models/llama2/_component_builders.py
18.51%
22 Missing :warning:

torchtune/models/llama2/_model_builders.py
71.42%
2 Missing :warning:

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1160      +/-   ##
==========================================
+ Coverage   68.63%   68.96%   +0.32%
==========================================
  Files         214      216       +2
  Lines        9687     9767      +80
==========================================
+ Hits         6649     6736      +87
+ Misses       3038     3031       -7

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

For now, I'd argue that we should revisit this further down the line

sounds good to me! Specially since you are already aligned with Philip and Rafi. Thanks for your reply!

Overall looks good to me. Do we have tests for the reward convert weights utilities?

There's a test here https://github.com/pytorch/torchtune/blob/570dc8f829dfaf2adf5a186ab1b93e11fb925bb4/tests/torchtune/utils/test_checkpointer.py#L443


Port to x64?

Hello,
I'm looking for a way to address some driver-related issues with some legacy code that uses DirectDraw from a 64 bit Windows environment. I'm wondering if it seems at all reasonable for me to consider porting your code to x64? I'm not asking you to do this, I'm simply wondering if you think this would be possible - are there any blocking issues that you can think of?
Many thanks for any thoughts you might wish to share on this.

It's probably not a old game I guess? cnc-ddraw does only work for fullscreen-exclusive applications, I assume that could probably already be a deal breaker for you.
I'm not sure if there's anything that couldn't be ported, would need to have a look through everything

Thanks for the response! No, not an old game. An old machine-vision library used in some decades-old factory automation code. Could you elaborate on "... does only work for fullscreen-exclusive applications"? The code I'm dealing with puts an image in an MFC "control" that is rarely full screen.

Yeah, it wouldn't work with it. cnc-ddraw needs the application to call SetCooperativeLevel(x, DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN) and SetDsplayMode() otherwise it wouldn't render anything. It doesn't have logic to handle windowed mode applications.
cnc-ddraw isn't a full replacement for the original ddraw.dll, it's intended to work with old games only.
Maybe have a look into wine's ddraw instead?

Ah, okay. Again, thanks for the response. I'm actually now diving deeper
into the legacy code to look for ways to avoid using the portion of the
DirectDraw interface that we're having trouble with. But I appreciate the
confirmation that cnc-ddraw is not the silver bullet that I might have
hoped for.
On Fri, Oct 27, 2023 at 12:38 PM FunkyFr3sh @.***>
wrote:

Yeah, it wouldn't work with it. cnc-ddraw needs the application to call SetCooperativeLevel(x,
DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN) and SetDsplayMode() otherwise it
wouldn't render anything. It doesn't have logic to handle windowed mode
applications.
cnc-ddraw isn't a full replacement for the original ddraw.dll, it's
intended to work with old games only.
Maybe have a look into wine's ddraw instead?
—
Reply to this email directly, view it on GitHub
https://github.com/FunkyFr3sh/cnc-ddraw/issues/256#issuecomment-1783204257,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/BDQZC3G3BWPJG7R45RC6UKDYBPPPPAVCNFSM6AAAAAA6PXEXZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBTGIYDIMRVG4
.
You are receiving this because you authored the thread.Message ID:
@.***>


Local shell & SSH: Storage access and possible implementations & alternatives.

Is your feature request related to a problem? Please describe.
Can't access local storage when using local shell, can't use PWD, LS or related commands
Describe the solution you'd like
An option to enable access of all directories to a shell, which would allow for use with Shizuku(Local ADB SHELL) or SFTP.
Describe alternatives you've considered
An option to add visible directories to a shell, so it only sees a folder you selected as its [/]
, also I think you could make ConnectBot create a org.connectbot directory at Android/Data/org.connectbot/ so the app can access it without additional permissions.

Yes, using a local ssh server on Magisk

Thanks!

When I use local connection I also cant do cd or ls ... And i have no idea how to ser a directory that i can access as my HOME ... (What should be HOME?)
I get error that HOME is not set
:/ $ ls
ls: .: Permission denied
1|:/ $ pwd
/
:/ $ cd
/system/bin/sh: cd: no home directory (HOME not set)
2|:/ $

When I use local connection I also cant do cd or ls ... And i have no idea how to ser a directory that i can access as my HOME ... (What should be HOME?)
I get error that HOME is not set
:/ $ ls
ls: .: Permission denied
1|:/ $ pwd
/
:/ $ cd
/system/bin/sh: cd: no home directory (HOME not set)
2|:/ $

Best solution would be using a local ssh server for now.


Added a fix for issue #2582

Thanks, @micahhainline, but this is not the correct resolution for this bug. It all has to do with the lines after the diff dealing with observed key paths. This will be resolved in the next release.


removed config-override for PS

For PS it is mandatory, have config-override in the project and .env

@VictorVelarde, I close this PR, the changes goes in CRA PR added


lib/os/heap: assorted improvements and optimizations

Looks like this is hanging on qemu_cortex_a53.

INFO    - -----------/workdir/zephyr/sanity-out/qemu_cortex_a53/tests/lib/heap/lib.heap/handler.log-----------
--
  |
  | INFO    - 4 Iteration:
  | INFO    - Adding tasks to the queue...
  | INFO    - 250/250 qemu_cortex_a53           tests/lib/heap/lib.heap                            FAILED Exited with 1 (qemu)
  | INFO    - -----------/workdir/zephyr/sanity-out/qemu_cortex_a53/tests/lib/heap/lib.heap/handler.log-----------
  | ERROR   - *** Booting Zephyr OS build zephyr-v2.3.0-633-g2a0d4de5d2f8  ***
  | Running test suite lib_heap_test
  | ===================================================================
  | START - test_small_heap
  | Testing small (2048 byte) heap
  | successful allocs: 1982/2161 (92%), frees: 1935,  avg usage: 894/2048 (44%)
  | PASS - test_small_heap
  | ===================================================================
  | START - test_fragmentation
  | Testing maximally fragmented (2048 byte) heap
  | successful allocs: 823/3376 (24%), frees: 720,  avg usage: 742/2048 (36%)
  | PASS - test_fragmentation
  | ===================================================================
  | START - test_big_heap
  | Testing big (262144 byte) heap
  |

On Wed, 1 Jul 2020, Andy Ross wrote:

Code looks good (though subtle).  I do see the heap test failing on a few platforms though

I'm trying to get at the ?&?"/"$%"/$% failure report and all I get is
this crap from the github CI interface (what am I missing?):
#       buildkite-agent pip…             $ Ran in 33s
# 1/20 ls -lsa .git …     $ Ran in 40m 55s

# 2/20 ls -lsa .git &&…     $ Ran in 1h 6m

# 3/20 ls -lsa .git …     $ Ran in 41m 30s

# 4/20 ls -lsa .git …     $ Ran in 32m 16s

# 5/20 ls -lsa .git …     $ Ran in 39m 10s

# 6/20 ls -lsa .git …     $ Ran in 35m 45s

# 7/20 ls -lsa .git …     $ Ran in 38m 41s

# 8/20 ls -lsa .git …     $ Ran in 26m 12s

# 9/20 ls -lsa .git &&…     $ Ran in 1h 8m
# 10/20 ls -lsa .git…     $ Ran in 27m 39s

" 11/20 ls -lsa .git…     $ Ran in 10m 35s

# 12/20 ls -lsa .git…     $ Ran in 29m 26s

# 13/20 ls -lsa .git …     $ Ran in 28m 0s

# 14/20 ls -lsa .git…     $ Ran in 46m 38s

# 15/20 ls -lsa .git…     $ Ran in 35m 16s

" 16/20 ls -lsa .git…     $ Ran in 51m 41s

# 17/20 ls -lsa .git …     $ Ran in 41m 6s

# 18/20 ls -lsa .git &…     $ Ran in 1h 2m
# 19/20 ls -lsa .git …          $ Ran in 51m 8s

# 20/20 ls -lsa .git…          $ Ran in 37m 57s

 Build will continue even if previous stage fails

On Wed, 1 Jul 2020, Andrew Boie wrote:

Looks like this is hanging on qemu_cortex_a53.

Works fine here, although it takes a significant amount of time:
INFO    - 1 of 1 tests passed (100.00%), 0 failed, 0 skipped with 0 warnings in 114.95 seconds
Backing out this PR and the test runs faster, but still takes a
significant amount of time:
INFO    - 1 of 1 tests passed (100.00%), 0 failed, 0 skipped with 0 warnings in 77.85 seconds
I did make sys_heap_validate() somewhat more expensive with extra
validation checks (about 47% from the looks of it) but I don't think it
is wrong per se.  Maybe the actual test is doing too many loops?

To CI, a test that runs past the timeout is indistinguishable from a hang.
I suggest making the test execute faster, or set a longer timeout in testcase.yaml

All tests pass locally, so I've increased the timeout to see what happens.


updating year to 2022 in copyright

Signed-off-by: Pravin Pushkar<EMAIL_ADDRESS>

@pravinpushkar time to update this PR? 😄


wallet connexion fails on website, works on local

strange thing, on the gem-bank,
wallet connexion works fine on localhost
but when app deploy on domain, it always fails to connect,
always getting the "oh no, failed to connect to wallet, try again" message in console
is it related to domain name ? to https/http ?
it's even stranger because it was working fine at first (for 10 days) now it's not working anymore
also on the VPS I changed from serving with Apache2 to Nginx
can that be related ?

I should also say that I tried on two different VPS, with different IP, and it's the same result

Update I also tested with a fresh version of Gem Bank out of github,
it works on Local, it doesn't works on network (after yarn serve command)
it's really weird, something must have changed recently

resolved: this is an https issue
wallet connexion won't work unless you're in HTTPS


clusteroperator: remove unused features

Signed-off-by: hejianpeng<EMAIL_ADDRESS>What type of PR is this?

What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:

/lgtm


Host not reachable in Home Assistant 2022.07.0

Describe the bug
After upgrading, the thermostat is unavailable and the following is logged:

Logger: custom_components.lennoxs30
Source: custom_components/lennoxs30/init.py:326
Integration: Lennox S30/E30 (documentation, issues)
First occurred: 6:42:08 PM (1 occurrences)
Last logged: 6:42:08 PM
async_setup unexpected error login Client Connector Error - failed due host not reachable url [https://thermostat.lan/Endpoints/homeassistant/Connect]

To Reproduce
Steps to reproduce the behavior:

Upgrade to Home Assistant 2022.07.1
See the above error in the logs

Expected behavior
The integration starts
Version

Home Assistant: 2022.07.01
This Integration: 0.2.7 and 0.2.9

**Integration Configuration **

Cloud or Local Connection: Local
Inverter or Sensor Diagnostics Enabled: No

Additional context
I'm running HAOS which influences DNS resolution. I was able to reach the thermostat both from the OS and inside the homeassistant container, so I don't think this is a more general issue.
I downgraded back to 2022.06 to fix this (thanks zfs!), but I can upgrade to test dependingon my family's schedule.

Looks like a problem.  I’ll upgrade my test system to 2022.07.0 and see if I can repro it.

It's indeed broken on 2022.07.0.  Looks like they upgraded to python 3.10.  Not sure of the solve yet.

Several custom interpretations are broken, including my own. Might want to check some other repositories and see what their doing about it.

Same issue here.

Several custom interpretations are broken, including my own. Might want to check some other repositories and see what their doing about it.

Thanks. I am on the hunt, let me know if you find anything.  Getting a 2022.7.0 dev environment setup.

Looks like the root cause is the default TLS version in python 3.10 is not compatible with the TLS version on the Lennox Thermostat.
2022-07-07 09:07:03 DEBUG (MainThread) [asyncio] <asyncio.sslproto.SSLProtocol object at 0x7f7d2750fdc0>: SSL handshake failed
Traceback (most recent call last):
  File "/home/pete/.pyenv/versions/3.10.0/lib/python3.10/asyncio/sslproto.py", line 631, in _on_handshake_complete
    raise handshake_exc
  File "/home/pete/.pyenv/versions/3.10.0/lib/python3.10/asyncio/sslproto.py", line 188, in feed_ssldata
    self._sslobj.do_handshake()
  File "/home/pete/.pyenv/versions/3.10.0/lib/python3.10/ssl.py", line 974, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLError: [SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:997)
2022-07-07 09:07:03 DEBUG (MainThread) [asyncio] <asyncio.sslproto.SSLProtocol object at 0x7f7d2750fdc0>: SSL error in data received
Traceback (most recent call last):
  File "/home/pete/.pyenv/versions/3.10.0/lib/python3.10/asyncio/sslproto.py", line 528, in data_received
    ssldata, appdata = self._sslpipe.feed_ssldata(data)
  File "/home/pete/.pyenv/versions/3.10.0/lib/python3.10/asyncio/sslproto.py", line 188, in feed_ssldata
    self._sslobj.do_handshake()
  File "/home/pete/.pyenv/versions/3.10.0/lib/python3.10/ssl.py", line 974, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLError: [SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:997)

Next step is to figure out how to change ths.

Presuming ssl is still used for the thermostats comms to the cloud, it seems like more reason to block internet access and switch to local only.
If the built in library doesn't have an override flag then might need to switch to a 3dd part ssl library. Unlike the other device I have I don't see a means to upgrade the thermostat or other wise enable tls.

I've tried a variety of combinations setting the SSL content. without much luck.  Line 235 of s30api_async.py
            context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
            context.options = ssl.OP_NO_SSLv3 | ssl.OP_NO_TICKET
            context.check_hostname = False
            context.verify_mode = ssl.CERT_NONE
            self.ssl = context
            self.initialize_urls_local()

This is the cipher setup we need
SSL connection using TLSv1.2 / AES256-GCM-SHA384

I couldn't figure out how to get to the API code in HA to test with the time I had. But I am hoping this is moving in the right direction.
cipher = 'DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384' context.set_ciphers(cipher)
If that doesn't work context.set_ciphers('ALL:@SECLEVEL=1') is more permissive but shouldn't be needed.

after a long and painful morning i have a fix!  It'll take a day or so to test.
            context = ssl.create_default_context()
            context.set_ciphers("DEFAULT")
            context.check_hostname = False
            context.verify_mode = ssl.CERT_NONE
            self.ssl = context
            self.initialize_urls_local()

I couldn't figure out how to get to the API code in HA to test with the time I had. But I am hoping this is moving in the right direction.
context.set_ciphers('DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384')
If that doesn't work context.set_ciphers('ALL:@SECLEVEL=1') is more permissive but shouldn't be needed.

Excellent and thank you!
I did create a patch with ALL, I will test with the more restrictive approach.

Link to api pull request for review.
https://github.com/PeteRager/lennoxs30api/pull/45

Running 4 RLC-520A with latest firmware.  None are working.

This issue should now be resolved in this release, that was just published now.
https://github.com/PeteRager/lennoxs30/releases/tag/0.2.10

What's the average delay before it appears in HACS?

What's the average delay before it appears in HACS?

Good question.  Should be available now.  Can you refresh HACS?


[STD]: dotenv package

NodeJS added this in latest LTS release so I though it would be funny to have this too right in the stdlib.
Could be separate package like os/dotenv or maybe just a component in os package

Should maybe have some support for .env.local and stuff like that
Not for MVP tho


improve error propagation

I noticed that I could set a TTL of 4, and it was happily accepted.  This made no sense, as there's only two bits to hold the ttl.
This set of changes adds error handling to ConnectionOptions, propagates errors up the stack a bit (from places where it wasn't being propagated, and most importantly actually passes the ttl options through in the default case.

Coverage decreased (-7.04%) to 58.653% when pulling 04b733921bfc0118452be5b3622e3fb75aae1702 on rspier:ttlerror into ca539aed4429aca3014966b4f7bc982f2d154a39 on abates:master.

Sorry for the coverage drop, it's mostly because of new if err == nil { } blocks.      Improving coverage is a project for another night.

Merged


python312Packages.cyclopts: 3.0.1 -> 3.1.0

Diff: https://github.com/BrianPugh/cyclopts/compare/refs/tags/v3.0.1...v3.1.0
Changelog: https://github.com/BrianPugh/cyclopts/releases/tag/v3.1.0

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/)
25.05 Release Notes (or backporting 24.11 and 25.05 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

[ ] Fits CONTRIBUTING.md.

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

nixpkgs-review result
Generated using nixpkgs-review.
Command: nixpkgs-review pr 358964

x86_64-linux

:white_check_mark: 4 packages built:

python311Packages.cyclopts
python311Packages.cyclopts.dist
python312Packages.cyclopts
python312Packages.cyclopts.dist


Allow none jti claim token type claim

Updated changes found here https://github.com/jazzband/djangorestframework-simplejwt/pull/489
to include tests and add a note.

@2ykwang @Andrew-Chen-Wang hey sorry to be so noisy, but I saw the build was failing so I updated the code with black formatting.

thanks!


Mutable exports lead to incorrect code removal

Given these two files (service repro):
// base.js
export function initialize(fn) {
    fn();
}

function reassignInitialize(wrapper) {
    initialize = function(fn) {
        fn();
    };
}

// main.js
import * as base from "./base.js";

console.log("Loaded main");

export function initialize() {
    console.log("Running initialize");
}

base.initialize(initialize);

The expected output is console.log("Loaded main");console.log("Running initialize"). However, only console.log("Loaded main"); appears.
If we drop the reassignInitialize function (service repro) then it works as expected.
I'm guessing that something in the mutable export implementation (679ea8fb4e8808ee725d22b8156ec09c96e437b0 - it uses getters) is not playing well with advanced optimizations, but I have not investigated further.

Created Google internal issue b/120230678.

I think I could explain what's happening if I saw the code as it looks just after module transpilation, but I can't tell from looking at the original code here.

@brad4d the code after module transpilation looks something like this:
// ==ClosureCompiler==
// @compilation_level ADVANCED_OPTIMIZATIONS
// @output_file_name default.js
// @language_out ES5
// ==/ClosureCompiler==

function initialize$$module$static$js$es6_modules$base(fn) {
  fn();
}
function reassignInitialize$$module$static$js$es6_modules$base(wrapper) {
  initialize$$module$static$js$es6_modules$base = function(fn) {
    fn();
  };
}
var module$static$js$es6_modules$base = {
  get initialize() {
    return initialize$$module$static$js$es6_modules$base;
  }
};
console.log("Loaded main");
function main$$module$static$js$es6_modules$main() {
  console.log("Running main");
}
function initialize$$module$static$js$es6_modules$main() {
  console.log("Running initialize");
}
module$static$js$es6_modules$base.initialize(
  initialize$$module$static$js$es6_modules$main
);

(compilation service version)

Funny thing is, that compiled version looks to me like it works.

module$static$js$es6_modules$base.initialize invokes the getter, which returns
initialize$$module$static$js$es6_modules$base
initialize$$module$static$js$es6_modules$base gets called with arguments (initialize$$module$static$js$es6_modules$main)
initialize$$module$static$js$es6_modules$base calls its argument without arguments.

I think @mihaip 's example is without optimizations, which are removing the getter and/or call somewhere.

Had some good discussion with @jplaisted about this yesterday.
Here's a simpler repro that doesn't involve modules.
function fn1() {
  alert('fn1');
}
// This gets removed entirely and fn1 above also
({ get fnProp1() { return fn1; } }).fnProp1();

function fn2() {
  alert('fn2');
}
// Normal property doesn't get removed
({ fnProp2: fn2 }).fnProp2();

https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540compilation_level%2520ADVANCED_OPTIMIZATIONS%250A%252F%252F%2520%2540language_out%2520ES6%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%2540formatting%2520pretty_print%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F%252F%2520ADD%2520YOUR%2520CODE%2520HERE%250Afunction%2520fn1()%2520%257B%250A%2520%2520alert('fn1')%253B%250A%257D%250A%250A%252F%252F%2520This%2520gets%2520removed%2520entirely%2520and%2520fn1%2520above%2520also%250A(%257B%2520get%2520fnProp1()%2520%257B%2520return%2520fn1%253B%2520%257D%2520%257D).fnProp1()%253B%250A%250A%250Afunction%2520fn2()%2520%257B%250A%2520%2520alert('fn2')%253B%250A%257D%250A%250A%252F%252F%2520Normal%2520property%2520doesn't%2520get%2520removed%250A(%257B%2520fnProp2%253A%2520fn2%2520%257D).fnProp2()%253B%250A%250A
Point of interest:
If you change both of the object literals to use the same property name, say fnProp, the removal doesn't happen. I can't explain that.


Panic mode arm rotation

You need to add a method to create an instance of the arm rotation subsystem to SubsystemFactory. That method should then be called in RobotContainer. If we ran this on the robot we would get a null pointer exception on the commands due to no instance of the subsystem existing.


I managed to get data!!!!!!!!!!!!!

There's two repo maybe you need to have a look.
bmi088-arduino and BMI08x-Sensor-API
The first repo looks like modified from the second repo, and they get data same way, by directly reading registers.
So you can have a look on bmi08x_defs.h, and change the configs such as ACCEL_CHIP_ID in the second repo's BMI088.h, then you can get data from the sensor.
But there's more configs need to be changed, such as accel's range(BMI088 and BMI085 have a different range) and I would try to deal with them next week.

That's good. You are more than welcome to fork this repo, add your SPI support, and submit a pull request. It's just not on my priority list right now. As you can see from the other repos in this organization, we mainly do hardware, and made this to support it. Just thought the community would benefit from a stepping stone for the 085.
-DS


Add validation

What should be done?
This is an epic to collect validation-related issues.
We need to add some basic validation methods for types.Document.
We can add two methods:

ValidateData to validate documents that represent data.
ValidateCmd to validate documents that represent commands.

Those methods should be called from handlers' implementations (for both Tigris and Postgres).
DoD:
Call data and command validation from the following methods:

[ ] insert
[ ] update
[ ] delete
[ ] findAndModify

List of tasks (validation rules) to be implemented:

Duplicate #871.


I want the AaC Visual Definition Editor to Implement Creating New Definitions

As an AaC User, I want the AaC Visual Definition Editor to implement creating new Definitions, so that I can create new definitions in the Visual Editor view.
AC:

[ ] There is a new task AaC Editor: Create new definition
[ ] The new task prompts the user to select one of the currently available root keys
[ ] The new task opens a blank editor with the JSON schema structure appropriate for the new definition's type
[ ] The editor has a "save" button that commits the new definition to the context

Maybe instead of a command we could add a '+' to the definition view and use that to kick-off command.


Need more deatail about "Persistent settings for use without a USB connection."

When I turn off my PC, 5V is still supplied to my USB ports.
This makes it that I always have my LEDS on.
I can disable this feature by enabling deep sleep but that would also disable my wake on lan setup I have and need.
I saw that in the features section of the README there is mention of "Persistent settings for use without a USB connection." which is what I assume needs to be changed in my case but I haven't found any further explanation in the documentation or the readme.

Persistent settings for use without a USB connection.

means that animations can be stored in the EEPROM of an arduino and are played back when powered without a connection to iCUE.
The solution to your question depends on how you power the LEDs, from what I understand from your description is, that you power them via the Arduino or an external power supply. If you power them via arduino I would suggest powering them from your PC power supply. Then the LEDs go off when the PC is off because the SATA and molex power should be off when the PC is off.
If you use an external power supply you should use it to turn off the LEDs. Alternatively you can set in iCUE the hardware lighting to a static black color.

Hardware lighting did the trick Thanks
/close


Code freezes when running OpenseaScraper.rankings()

If I try to run the script on Linux (computer in the same network) it stops right here without any errors like it's frozen. It works perfectly fine on Mac and it's the same code. It worked a week ago and nothing was changed.
✅ === OpenseaScraper.rankings() ===
=== OpenseaScraper.rankings() ===
...fetching 1 pages (= top 100 collections)
...opening url: https://opensea.io/rankings?sortBy=one_day_volume
...🚧 waiting for cloudflare to resolve
...exposing helper functions through script tag
...scrolling to bottom and fetching collections.
-> freezes here <-
There are no errors. It seems like its stuck in an infinite loop

Edit: After a lot of investigation i finally found what causes the freezing:
If I change the url in the rankings.js from "https://opensea.io/rankings?sortBy=total_volume" to "https://opensea.io/rankings?sortBy=one_day_volume" the script freezes, it works perfectly with the total_volume though. Seven_day_volume does also cause freezing. It is weird because it also worked with one_day_volume a few days ago

Thanks for the report, appreciate it!
You can always turn on debug mode and then you can actually watch the browser and see the scraping process. To turn on:
// options
const options = {
  debug: true,
  logs: true,
  sort: true,
  browserInstance: undefined,
}

const nbrOfPages = 2;
const ranking = await OpenseaScraper.rankings(nbrOfPages, options);

I run rankings() on a linux but cannot reproduce the bug. It might have something to do with screensize (smaller or larger screen might show another DOM due to responsive webdesign).
Can you rerun the function with debug turned on and report what you see actually happens?

appreciate

Thank you for answering I appreciate your help.
When debugging with total volume it opens the browser and waits for the collections to load. It then scrolls down to the bottom, closes the browser and returns the slugs.
When debugging with 24h volume on the same screen size it does the same but always stops scrolling at about position 50 - 54 (not consistent). After a few minutes it keeps scrolling a tiny but further to the next position and stops again. After another 3 - 5 Minutes it again scrolls 2 positions further. It maybe could go through after about 2 hours or so.
Whats also kinda funny, if I give it a small "nudge" and scroll down myself a little bit it finishes and returns all slugs correctly

this is really weird behaviour, will look into the code if I can find something that might cause this. Will let you know if I find something!

this is really weird behaviour, will look into the code if I can find something that might cause this. Will let you know if I find something!

Thank you, let me know if you need further information

moved to #36

Reopnening as I realized that this is not the same function, this issue is about the rankings() function, the other issue is about

Scrape top100 with this simple script
const nextDataStr = document.getElementById("__NEXT_DATA__").innerText;
const nextData = JSON.parse(nextDataStr);
const top100 = nextData.props.relayCache[0][1].json.data.rankings.edges.map(obj => obj.node);


On Home: Swan logo without hexagonal shape

Need to add hexagonal shape for swan logo as it is present for others on Home Screen.
The card colour is wrong, it should be the card colour specific in XD.

-Verified this issue on Dev app v1.7.0 (289)


Update Module Validity and error message

Error Message for valid modules (accept capital letters)
Modules found to be have 2 suffix: ASP1201CH

Based on this, should be allowed to fix in 1.4

👍  , Though ensure that your fix has minimal changes to the codebase


Documentation fails to provide all settings specifications

I have a solution built and tested with Outlook (Office 2016/365).
When I try to run it on a machine with Office 2013 installed, I get the error:
"You cannot debug or run this project because the required version of the Microsoft Office"
This document has no troubleshooting or examples or exact requirements posted beyond general information.  Could you please post more detailed settings instructions?  Thanks.

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

ID: 9c81ef68-355f-a059-91fa-904a7979ae48
Version Independent ID: 777a14e8-58e0-f08a-116f-315f8f1aa86f
Content: Run solutions in different versions of Microsoft Office - Visual Studio
Content Source: docs/vsto/running-solutions-in-different-versions-of-microsoft-office.md
Product: visual-studio-dev15
GitHub Login: @TerryGLee
Microsoft Alias: tglee

@DanRegalia Thanks for letting us know; this is in the backlog to fix. @John-Hart, FYI.

This document is a simple list of the various version of Office, VSTO runtime and .NET framework for a running VSTO app.  @DanRegalia, it sounds like the error you mentioned is actually coming from Visual Studio and it simple means either Office 2013 or Office 2016 was not installed on the same machine


Formatting of division

Missing / as the function name in the message.
(/)
Wrong number of arguments in ( ): the function  expects one or more arguments but was given no arguments.

This now corresponds to https://github.com/Clojure-Intro-Course/babel/issues/50 in the migration repository.


Navigating backwards to page with prompt locks up app

Issue:
While navigating GUS (gemini://gus.guru), Lagrange frequently locks up when using the "backwards" navigation action (both by clicking the toolbar button or using the Alt+LeftArrow binding).
It seems the app locks up when the navigation brings back a page where there should be a prompt (on GUS, it would be the search box), so it would probably not just impact browsing on GUS.
There is no relevant debug trace when this issue hits.
Expected behaviour:
I could see two scenarios to choose from here:

Either navigating back to a prompt page brings back the prompt, for the user to put a new entry
Navigating back brings the user to the page before the prompt, where there is effectively no prompt

Reproducing the issue:

Navigate to gemini://gus.guru/
Click "Search GUS" => this brings up a prompt
Type a query and press enter => the results appear
Navigate back (left arrow on the tool bar, or Alt+LeftWarrow on the keyboard)
Lagrange becomes unresponsive -- can't click anything, can't SIGTERM -- I have to SIGKILL

Version info:
Lagrange 0.8 ( edc5194)
Arch Linux x64 (Linux karth-dev-book 5.9.3-arch1-1 #1 SMP PREEMPT Sun, 01 Nov 2020 12:58:59 +0000 x86_64 GNU/Linux)

frequently locks up

"frequently" as in "not always"? This looks like a race condition related to thread exiting, I'll do some debugging in the_Foundation...

Sorry, i went back to poke it some more and it does seem to lock up always.

Please test with f2df52c755e4211fea50afbecf552bd99a8d5fe5. I updated the the_Foundation submodule with a potential fix.

Yes, on GUS, it brings me back and opens the search prompt. I couldn't get the UI to lock up anymore. It looks good from a user POV. Thanks!

Closing for now (0.8.1 is out), let me know if the issue still occurs in the future.

Thank you, I appreciate how reactive you are with this project. :bow:


ci(test): curl grcov

Используем скомпилируемый grcov из гитхаба, для ускорения

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 8.46%. Comparing base (3133cc2) to head (9639660).

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #255   +/-   ##
=====================================
  Coverage   8.46%   8.46%
=====================================
  Files          7       7
  Lines       3602    3602
  Branches     811     811
=====================================
  Hits         305     305
  Misses      3184    3184
  Partials     113     113

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


Remove other empty values

The empty maps can be detected and eliminated like the empty arrays.
Having the "0" for missing optional integer values is leading to some errors in some providers. As an example k8s is rejecting the most-be-positive keys that are "0"!
However there might be some string fields or reuired integer fields that will be broken by making this commit!
Do we need to check the field type and optionality before removing it?

I think something providers has resources with map is required.
Deleting attribute  because it's "0" it's very risky.
Maybe need to use with AllowEmptyValues and IgnoreKeys in resource level, because it's corner case.


feat: catch imports of private definitions in resolution

Description
Problem*
Resolves
Summary*
Currently Noir only defines import visibility for functions, while everything else is implicitly public. This PR is an experiment on adding proper import visibility to Noir so that we can.
I've modified ItemScope so that it can hold both public and private definitions and during as we traverse modules in resolution we check that dep:: paths don't enter any private modules. I've added pub mod syntax to allow this to be shown.
I've set the prelude to be pub mod so that we can compile most programs but all other modules within the stdlib are private to show how errors look like.
Additional Context
Documentation*
Check one:

[ ] No documentation needed.
[x] Documentation included in this PR.
[ ] [Exceptional Case] Documentation to be submitted in a separate PR.
I'll document this closer to the time of this being merged.

PR Checklist*

[ ] I have tested the changes locally.
[ ] I have formatted the changes with Prettier and/or cargo fmt on default settings.

Heads up that I'm going to be chopping this PR up into chunks to get some less potentially controversial sections through quicker and reduce the diff.

Is this an extension of the open and internal keywords to general Noir programs?

Nope this is an equivalent of https://doc.rust-lang.org/reference/visibility-and-privacy.html

I think that this is close enough to be ready for reviews now.

@TomAFrench would you like to supplement docs before merging the PR in?

Also will this be a breaking change in terms of libraries without pub will turn from public-by-default to private-by-default after the PR?

Closing as superceded by more recent work.


Add priorities and layer masks to avoid overlap

Adds priorities to fs25-texture-schema.json
Implements them into texture.py to sort the order layers are drawn in
Implement a cumulative mask in texture.py#draw to minimize overlap of roads and other features

Cyan indicates overlap

Without changes

With changes

With commit f88f for generating grass, the output became like the following

@ZenJakey, hey!
Thank you for the PR. It's really cool feature, we definitely need it.
Could you please check the comments and apply the changes?
Thank you
Sincerely


Remove RHEL7 - EOL June 2024

This pull request deprecates RHEL7 and updates README's

The grep over this repo like, 'grep -rn 'rhel7' *:
base/root/help.1:112:$ make build VERSIONS=base TARGET=rhel7
base/root/help.1:148:The Dockerfile for CentOS is called Dockerfile, the Dockerfile for RHEL7 is called Dockerfile.rhel7,
base/Dockerfile.rhel7:2:FROM rhscl/s2i-core-rhel7
base/Dockerfile.rhel7:17:      name="rhscl/s2i-base-rhel7" \
core/Dockerfile.rhel7:16:      name="rhscl/s2i-core-rhel7" \
core/Dockerfile.rhel7:27:    # TODO: There is a bug in rhel7.1 image where the PATH variable is not exported

The files are expected.

[test]

[test-openshift]


Overloaded methods cannot be called using function call syntax

Aim
Trying to call an overloaded method using function call syntax:
struct Foo<T> {}

impl Foo<u32> {
    fn foo(self) -> Field { 1 }
}

impl Foo<u64> {
    fn foo(self) -> Field { 2 }
}

fn main() {
    let f: Foo<u32> = Foo{};
    assert(Foo::foo(f) == 1);
}

Expected Behavior
The program to execute without failure
Bug
error: Could not resolve 'foo' in path
   ┌─ /.../short/src/main.nr:15:17
   │
15 │     assert(Foo::foo(f) == 1);
   │                 ---
   │

Aborting due to 1 previous error
exit 1

To Reproduce

Installation Method
None
Nargo Version
No response
Additional Context
No response
Would you like to submit a PR for this Issue?
No
Support Needs
No response

@jfecher I think this was fixed in #3087 but wasn't correctly linked, so we could close this. But let me know if this isn't correct! (the code doesn't seem to trigger the bug anymore)

@asterite this wasn't fixed by #3087 (rather that PR introduced this as a limitation) but it seems this was fixed sometime since then


[1.30.4] sap.ui.layout.DynamicSideContent wrong style class

Just played around with the new side content.
I think there is a small typo in line 17:
SPAN_SIZE_12_CLASS = "sapUiSCSpan12",
should be
SPAN_SIZE_12_CLASS = "sapUiDSCSpan12",
the missing 100% width killed my layout
I love this cool feature!
Fix: add custom style
.sapUiSCSpan12 {
width: 100%;
}

Thanks! Internally tracked as<PHONE_NUMBER>.

Hi,
The fix is submitted in the master branch and is down ported to rel-1.30.5
Thanks


NTR - metabolism: [GDP-mannose biosynthetic process from glucose]

Please provide as much information as you can:

Suggested term label: GDP-mannose biosynthetic process from glucose

ChEBI ID and label of the substance:
Please ensure that the ChEBI ID corresponds to the physiological form (pH7.3) of the compound. In the ChEBI entry, pH7.3 compounds have a 'UniProt synonym', as in the example for mycophenolate
has_primary_input	CHEBI:4167	D-glucopyranose
has_primary_output	CHEBI:21168	GDP-mannose

Definition (free text)
The chemical reactions and pathways resulting in the formation of GDP-mannose from glucose.

Reference, in format PMID:#######
PMID: 21273394
PMID: 9287314

Gene product name and ID to be annotated to this term
In fly:
Hex-A
Hex-C
Pgi
Mpi
Pmm2
Gmppa
Gmppb

Parent term(s)
GDP-mannose biosynthetic process GO:0009298

Synonym(s) (please specify, EXACT, BROAD, NARROW or RELATED)
Other than the standard synonyms below

Children terms (if applicable) Should any existing terms that should be moved underneath this new proposed term?

Cross-references
MetaCyc  PWY-5659 (start with glucose 6-phosphate)

Any other information
Cells can make GDP-mannose from either mannose or glucose. Depending on the cells, glucose is the predominant source.
Below is a schematic of the coverage of the new term compared to the already existing terms (the gene are from the fly).

Thank you
Rossana
@sjm41
@ukemi
@deustp01

FOR ONTOLOGY EDITOR

Logical definition

substance metabolic process:
'metabolic process'
and ('has primary input or output' some [substance]

substance biosynthetic process:
'biosynthetic process'
and ('has primary output' some [substance].

substance catabolic process:
'catabolic process'
and ('has primary output' some [substance]

Standard synonyms

substance metabolic process:

EXACT: substance metabolism

substance biosynthetic process:

EXACT: substance anabolism

EXACT: substance biosynthesis

EXACT: substance formation

EXACT: substance synthesis

substance catabolic process:

EXACT: substance breakdown

EXACT: substance catabolism

EXACT: substance degradation

Missed the following parent term(s):
mannose catabolic process GO:0019309

@rozaru
Just to make sure I understand:
Are you suggesting that the new term 'GDP-mannose biosynthetic process' is_a mannose catabolic process?
ie you'd consider mannose to mannose-6-phosphate a mannose catabolic process?

@pgaudet,
I should have mention that this comment is related to another ticket #27682 where I suggested to move "GDP-mannose biosynthetic process from mannose" under "mannose catabolic process".
If the new term "GDP-mannose biosynthetic process from glucose" is created it would go at the same level as "GDP-mannose biosynthetic process from mannose" is in the ontology.
This is the current ontology for mannose metabolic process:
mannose metabolic process
|__GDP-mannose biosynthetic process from mannose
|__mannose biosynthetic process
|__mannose catabolic process
|   |__glycolytic process from mannose through fructose-6-phosphate
|   |__mannose to fructose-6-phosphate catabolic process
This is my proposition:
mannose metabolic process
|__mannose biosynthetic process
|__mannose catabolic process
|   |__glycolytic process from mannose through fructose-6-phosphate
|   |__mannose to fructose-6-phosphate catabolic process
|   |__GDP-mannose biosynthetic process from mannose
|   |__GDP-mannose biosynthetic process from glucose (new term)
It is a bit difficult to decide what constitutes a catabolic or a biosynthetic process when the output and input belong to multiple pathways.
For "GDP-mannose biosynthetic process from mannose":
From the point of view of GDP-mannose this term cover a biosynthetic process
From the point of view of mannose this term cover its conversion into GDP-mannose which could been seen as a catabolic process.

@pgaudet
I realised that the commend above:
Missed the following parent term(s):
mannose catabolic process GO:0019309
is a mistake, as the potential new term is not part of the mannose metabolic process. Sorry about this.

+[Term]
+id: GO:0141199
+name: GDP-mannose biosynthetic process from glucose
+namespace: biological_process
+def: "The chemical reactions and pathways resulting in the formation of GDP-mannose from glucose." [PMID:21273394, PMID:9287314]
+xref: MetaCyc:PWY-5659
+intersection_of: GO:0009058 ! biosynthetic process
+intersection_of: has_primary_input CHEBI:4167 ! D-glucopyranose
+intersection_of: has_primary_output CHEBI:21168 ! GDP-mannose
+relationship: starts_with GO:0004347 ! glucose-6-phosphate isomerase activity
+property_value: term_tracker_item "https://github.com/geneontology/go-ontology/issues/27683" xsd:anyURI
+created_by: pg
+creation_date: 2024-04-26T08:43:27Z

@pgaudet
I just notice a small detail, but shouldn't
+intersection_of: has_primary_output CHEBI:21168 ! GDP-mannose
be
+intersection_of: has_primary_output CHEBI:57527 GDP-α-D-mannose(2−) ?

Thanks @rozaru !
I fixed this, and for the parent & siblings.

@rozaru @ukemi @deustp01 @pgaudet
Following the NTR in this ticket, we end up with this:
GDP-mannose biosynthetic process (GO:0009298) [Reactome:R-HSA-446205]
    |__GDP-mannose biosynthetic process from fructose-6-phosphate (GO:0061729)
    |__GDP-mannose biosynthetic process from glucose (GO:0141199)  [MetaCyc:PWY-5659]
    |__GDP-mannose biosynthetic process from mannose (GO:0061728)

Looking again now, I think the NTR actioned here (GO:0141199) should be merged with the pre-existing term 'GO:0061729 GDP-mannose biosynthetic process from fructose-6-phosphate.
Here's my reasoning:

If you look at Rossana's nice image in the original comment, the 'from fructose-6-phosphate' term is just describing the last 3 steps of the new 5-step 'from glucose' term.
Fig 1 and the accompanying text of PMID:9287314 Rossana mentions (shown below) says that "Man-6-P can be formed in two ways; either directly from mannose using hexokinase, or derived from the pathway of Glc → Glc-6-P ↔ Fru-6-P ↔ Man-6-P".  That is, there is one route from mannose, or another route from glucose via F6P.
Fig 1 of PMID:16339137 (used as the source for the 'from fructose-6-phosphate' (GO:0061729) term) also shows that the fructose-6-phosphate originates from Glucose, and again shows the same two routes - also shown below.
There are only 2 EXP annotations to the 'from fructose-6-phosphate' term (to human PMM2 and MPI), and they were both added recently by Rossana!

That said, there are still a few issues:

the new 'from glucose' term currently includes "relationship: starts_with GO:0004347 ! glucose-6-phosphate isomerase activity", which would indicate it actually starts with glucose 6-phosphate, so there's an inconsistency with the name/def/LD there.
That said, MetaCyc:PWY-5659 (xref on the new term GO:0141199) is stated to start from glucose-6-phosphate rather than glucose... (MetaCyc image shown below)
R-HSA-446205 (Synthesis of GDP-mannose) shows GDP-mannose being made from either mannose or fructose-6-phosphate (so it's appropriate they xref to the parent term "GDP-mannose biosynthetic process"), but it doesn't show where the fructose-6-phosphate has originated from - i.e. it doesn't show the Glc → Glc-6-P ↔ Fru-6-P steps mentioned in the refs above. (reactome image also shown below)

Maybe we can talk about this in the weeds call tomorrow afternoon?

From pathways2go discussion today:
Decision: Merge the new term (GDP-mannose biosynthetic process from glucose GO:0141199) into the pre-existing term (GDP-mannose biosynthetic process from fructose-6-phosphate GO:0061729), keeping the name and def of the pre-existing term. (That is, revert to what we had before, but add the MetaCyc:PWY-5659 xref to GO:0061729).
@pgaudet Can you take care of that please?
Justification: Although papers describe the second GDP-Man synthesis as starting from glucose, the first two steps (catalysed by HK and PGI) are best seen as being part of glycolysis. The resulting F(6)P is at a branch point - it can either continue to be metabolised in glycolysis (via PFK) or can be diverted into GFP-Man synthesis (using PMI, aka MPI). Though not perfect, this is the best (simplest) way of modularising and representing the information within GO-BP terms and GO-CAMs. (And it also mirrors the way Reactome models the situation.)
Other consequences: The way we are thinking about this specific example will hopefully help us with subsequent NTRs, and for reviewing other existing metabolic BP GO terms that appear to overlap or are nested in some way.... (e.g. based on the above arguments, there's a case for obsoleting "GO:0061619 glycolytic process from mannose through fructose-6-phosphate" and just keeping the "GO:0061611 mannose to fructose-6-phosphate catabolic process" term in order to connect mannose to core glycolysis.)
We will assess and review additional cases as we encounter them and aim to be consistent and establish guidelines!


Move to new carpet settings manager

TODO:

[x] Fix CarpetRulePrinter crashing cause translations aren't loaded early enough
[ ] Figure out if validators added by CarpetRulePrinter are still necesary

Other notable changes:

Update carpet version to 1.4.83
Update minecraft version and mappings to 1.19.1
Bumps carpet fixes version to 1.11.3

did some super quick sanity tests to make sure i didnt break anthing egregious and this looks ready to merge


[Feature Request] Remote Search Processor

Is your feature request related to a problem? Please describe
We are doing a lot of experimentations with search results and today, we are having to do most of this work client-side.  When we need to make code changes in a search response process to try something new, it is quite tedious and requires multiple steps.  We want more flexibility when it comes to changing the behavior of a processor.
Describe the solution you'd like
We basically want a RPC version of the processor.  Something that can be executed remotely, e.g. AWS Lambda or a REST endpoint.  It can be a Python extension?  I'm open to suggestions.
Related component
Search
Describe alternatives you've considered
No response
Additional context
No response

@dbwiddis What would it take for me to add remote processor functionality as one or more Python extensions.  I would like to expose this to OpenSearch as TransportActions.
1/ A SearchRequestProcessor TransportAction, e.g. SearchRequest processRequest(SearchRequest request)
2/ A SearchResponseProcessor TransportAction, e.g. SearchResponse processResponse(SearchRequest request, SearchResponse response)
Ideally, I would like to leverage the new Protobuf transport layer feature for this as well.
It looks like opensearch-sdk-py does not yet support registering transport actions?
Alternatively, we are looking at standing up a microservice outside OpenSearch (Flask or FastAPI).  We would really like Python to remote processors which is why we're considering the Python extension (opensearch-sdk-java seems to support what I need in terms of the transport action registration).

@austintlee  could you please share a usecase that would require this support?


test suite execution time is too long

Hi,
as discussed in #609 and other comments, I strongly believe the test execution time should be kept low.
I believe it has only recently increased to around 11 minutes, which is a 5x or 450% increase from the 2 minutes it took a few months ago.
I’m not opposed to that sort of thing, but I am pretty sure this can be cut back to 2 minutes quite easily by changing some parameters in the longest tests.
After inspecting things just on the surface, I’ve also noticed that the tutorials have become part of the tests. I think this should not be the case: sphinx-gallery is designed to handle failing examples (see here https://sphinx-gallery.github.io/stable/configuration.html#dealing-with-failing-gallery-example-scripts). A better way to test this would be to include documentation generation in the CI instead of copying and pasting examples in the tests!
I really think we can keep the same coverage but keep durations low.
Cheers,
Florian

Below a list of slowest durations on my machine.
======================================================================================================== slowest 10 durations =========================================================================================================
195.23s call     tofu/tests/tests04_spectro/test_01_fit12d.py::Test01_DataCollection::test03_fit1d
41.28s call     tofu/tests/tests05_nist/test_03_core.py::Test01_openadas::test01_search_online_by_wavelengthA
40.62s call     tofu/tests/tests03_openadas/test_03_core.py::Test01_openadas::test01_search_online
17.29s call     tofu/tests/tests01_geom/test_04_core_optics.py::Test01_Crystal::test14_plot_focal_error_summed
12.96s setup    tofu/tests/tests01_geom/test_03_core.py::Test03_Rays::test00_set_move
12.63s call     tofu/tests/tests02_data/test_03_core.py::Test01_DataCam12D::test19_plot_svd
11.33s call     tofu/tests/tests02_data/test_03_core.py::Test01_DataCam12D::test18_spectrogram
10.60s call     tofu/tests/tests01_geom/test_04_sampling.py::test05_sa_integ_map
9.16s call     tofu/tests/tests02_data/test_03_core.py::Test01_DataCam12D::test16_compare
8.66s call     tofu/tests/tests01_geom/test_03_core.py::Test01_Struct::test16_plot

Thanks @flothesof !

first line (195.23 s) : this can probably be reduced by tuning, this tests fits a large number of spectra and tests all combinations of options for the fitting parameters, but there is probably some redundancy, I'll look into it now

second and third line (41.28 s and 40.62 s) : this goes online, sends http requests to a website to check it successfully recovers a result, but again, I'm probably testing too many combinations

Others: would require more work, the others are testing core numerical routines, I'm not very tempted to risk reducing coverage

By looking closely into the first 3 lnes I should be able to save 1 or 2 minutes I guess

Great, thank you for your feedback!  That would be a nice start.
Another thing that surprised me while looking at the tests is: almost none of your test functions have a docstring. I’m not saying this to annoy you, but a single line docstring for tests does wonders sometimes (even though you tried to think hard about the name of the test). If you do a PR, it would be nice to include docstrings for the tests you change :D

commits #5833fe8 and #68e8f9a in PR #611 (the last 2 commits) partially address this issue.
But the PR cannot be merged as unit tests are not fully passing, opened issue #612 to solve this.


Find suspicious things from source code

Files are filled with admin passwords and passphrases to keys.

I think this was mostly fixed while working on issue #18, but might still deserve another look to be sure.

The only thing I could find was in lib/appleNotificationService.js, where there are some outdated credentials for apple notifications

So if you review this and agree, go ahead and close the issue.

I looked through the code and didn't find anything, closing the issue.

https://github.com/TheSoftwareFactory/lokki-server/blob/master/lib/appleNotificationService.js#L41


NPM install exits with code 1

When running npm install with the latest version of node (6.3.1) it crashes, with quite an extensive log (have a look in the attachments).
dump.txt
This crash seems to occur due to an old dependency..

Proposing fix in Pull Request #23

Accepted, thanks!


Add running status to MeiliSearchCheck

https://github.com/meilisearch/meilisearch/blob/b1844b0c270e6819e1e52a0eaa40cc1c3d9a45fb/meilisearch/src/routes/mod.rs#L282
It seems they changed the response message. This should add running as OK-status, but also keep the previous one, just to be backwards compatible.

Thanks!


CREATE TABLE LIKE for copying Basic Table definition using create table parser

This is for CASSANDRA-19664:
1、we can copy table under the same keyspace of under different keysapces.
2、only support basic table schema : primary key(partition key and clustering key), regular columns , static columns, column masking, table params(compaction, compression and so on).

Hello, can i take up this issue?


external GitRepository running into build error

Hello,
I was initially trying to test your CLI in our gitops repository but ran into errors:
flux-local error:  Error building Fluxtomization in '/Users/foo/gitops'  \
path 'manifests/monitoring/monitoring-config': Specified path is not a directory:  \
/Users/foo/gitops/manifests/monitoring/monitoring-config - \
Is a Kustomization pointing to a path that does not exist?

I believe it is because an external GitReoisitory source is unavailable locally.
We have quite a few of those external sources, it would be nice if you could support them at some point.
Here is a minimal example that runs into the error mentioned above.
---
apiVersion: source.toolkit.fluxcd.io/v1
kind: GitRepository
metadata:
  name: flux-source
  namespace: flux-system
spec:
  ref:
    tag: "v2.1.2"
  interval: 120m
  timeout: 60s
  url: https://github.com/fluxcd/flux2.git
---
apiVersion: kustomize.toolkit.fluxcd.io/v1
kind: Kustomization
metadata:
  name: flux-dashboards
  namespace: flux-system
spec:
  interval: 10m0s
  path: ./manifests/monitoring/monitoring-config
  prune: true
  sourceRef:
    kind: GitRepository
    name: flux-source
  timeout: 10m

Not referencing the files in the Kustomization files will result in successful commands but incomplete diffs.
Thank you

Thanks for the feature request. yes, right now there is a major simplifying assumption that all Kustomization references only the local repo. There is currently a way to manually map a GitRepository source to a filesystem path with sources which can map flux-source=./path/to/source but it won't automatically check out and clone the repo.
I think some of the challenges here would just be around checking out the repo in a local copy (determining where to store that) while still keeping everything fast.

Does it work for you to manually check out the repo as part of your CI then refer to it with --sources?

The above is just an example. We are working with 10+ external sources in our GitOps repo. A few of them are huge repositories.
I can understand that this might be out of scope, but I guess, I was hoping to see some kind of pull command, based on the GitRepository spec.ignore field.
For CI this might work, but I was mainly looking into local testing

Lets explore that in more detail, so the idea is:

Run a command to pull a GitRepository or OCIRepository (plus filtering from the ignore)
It's kept on disk somewhere?
Occasionally you run the pull command manually again
You can run it for all remote repos in a single command?

I think we'd additionally need these configuration options:

At least one GitRepository or OCIRepository points a the local repo so we'd need to explicitly figure out which one that is vs which ones are remote and should be pulled
Likely configuration on where to store the local repos since they persist across CLI runs
That same configuration could be used instead of or as part of --source so you don't have to specify more than once.

Let me know if you have better thoughts here on how this could work. I consider this still in a brainstorming mode to see if this is feasible/worthwhile where all ideas are welcome.
Aside: Last week i tried playing with kustomize localize (example) to pull in individual remote urls within a Kustomization and I was trying to determine if it made sense to include a command like flux-local localize kustomization to help with maintenance, but don't have a solid idea there yet.  This proposal could be something like flux-local locallize gitrepo though the functionality is very different, its kinda a similar idea.

I realize the other thing I didn't mention from https://github.com/allenporter/flux-local/issues/283  is that you can specify --sources=flux-system if you just want to limit flux-local to only a single GitRepository or OCIRepository with the specified name and completely ignore the other remote repositories. I realize I didn't say this explicitly before, so not sure if that is really what you're looking for as far as a workaround besides building out pulling of all remote repos.


Fixed and improved speed of permutation_entropy

Hi Nikolay (@nikdon) and Jakob (@jakobdreyer),
First of all, congratulations for your great package that has proven very useful to me!
I have noticed that the permutation entropy is (1) not super fast and (2) does not return the same values as in the Bandt and Pompe 2002 paper. This PR attempts to fix both issues. I have also added an argument to normalize the permutation entropy between 0 to 1, which might be useful in some cases (e.g. machine-learning).
1) Speed
Using %timeit on arrays of different lengths, I am getting consistent speed improvements by about ~40%. This is mainly done by avoiding unnecessary conversion of lists to numpy arrays and using np.nonzero() instead of np.argwhere().
2) Change of base
I replaced this line:
pe = -sum(p * np.log(p))
by
pe = -np.multiply(p, np.log2(p)).sum()
to be consistent with the Band and Pompe 2002 paper and return a value in base 2 (=bit).
3) Normalization
I've added a normalize argument to the function. If set to True, the function returns a normalized permutation entropy (i.e. value comprised between 0 to 1):
if normalize: pe /= np.log2(factorial(m))
4) Default values
I've added m=3 and delay=1 as default values as I feel that those are probably the most used parameters for this function, but feel free to change that! I also renamed the argument m to order, which seems easier to understand.
5) Testing
I have also updated the tests. I had to update the test values for multiscale_permutation_entropy too since it is based on the same function. However, I did only very little change to the raw code of this latter.
6) Code and docs
I took the opportunity to re-write the code and documentation using the PEP8 and NumpyDoc guidelines.
Hope this will be useful!

Hi @nikdon!
I have just pushed a much (much) faster version of the code (250X speed improvements). I removed the for loop as well as the util_hash_term function.
Raphael

Hey @raphaelvallat,
Sorry for the delayed response. Thank you for the PR - this will be definitely useful. Everything seems ok, so I'll merge it and prepare a new release.

@raphaelvallat thanks one more time, everithing is in pypi with version 0.5.0


Add ability to rename hostname manually

Currently, there's no way to edit them
Would be really helpful, especially when builtin hostname detection fails after taking too long (as reported in #5 )

I will work on that, thanks!


Unleash Hell's Fury

text: Is not a minion when it burns (eg. for Tension in the Ranks)
link: https://www.vekn.net/forum/rules-questions/76656-unleash-hell-s-fury-tension-in-the-ranks#107653

Duplicate #670


Errors when compiling all the contracts

Hi,
I get the follwing error when running:
docker run --rm -v "$(pwd)":/code \
  --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target \
  --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \
  cosmwasm/workspace-optimizer:0.12.4

Error:
Building "contracts/cw1-subkeys" ...
 Downloading crates ...
error: failed to download `pkcs8 v0.8.0`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/pkcs8-0.8.0/Cargo.toml`

Caused by:
  feature `edition2021` is required

  The package requires the Cargo feature called `edition2021`, but that feature is not stabilized in this version of Cargo (1.55.0 (32da73ab1 2021-08-23)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2021 for more information about the status of this feature.

Trying to follow the instructions on and running cargo fix --edition did not help neither.
Any idea on what can be happening?

Sorry for the wait! I think you need to bump the workspace-optimizer version:
docker run --rm -v "$(pwd)":/code \
  --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target \
  --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \
  cosmwasm/workspace-optimizer:0.12.6

I'll get the README updated.

Thanks, that works now.


Getting error *** Error while loading node-red-context-monitor:

"req_path" is not "found"
I am using user_dir and a non std installation.
Any ideas?

Could you please post the whole error message?! There should be a "scanned for" and "found" line below that announcement.


Feature Request: Azure Cloud Shell option to select PowerShell instead of bash

Description of the new feature/enhancement
Connecting to Azure Cloud Shell using Terminal 0.3 will automatically launch the bash cloud shell provider. Personally, I prefer PowerShell.
I am able to launch PowerShell from bash by using pwsh, but there is a bug: when running PowerShell Core (launched from bash) using the Azure Cloud Shell, the cursor position gets repositioned to 0,0 on every new command, and the output is overlaid on the previous output (gets very messy, hard to read).
While it is probably a good idea to address that bug, this request is just to add or modify a profile to support PowerShell Core. (I'm going to create a new issue for that bug and update this.)
UPDATED: Here is the bug report: https://github.com/microsoft/terminal/issues/2267
Proposed technical implementation details (optional)
I thought it could just be possible to have Terminal go directly to PowerShell (preferably PowerShell Core) as an option for the Azure Cloud Shell profile settings, or else as a whole new profile.
As one of the profiles listed for a new tab in Terminal, I'd be okay with just one Azure Cloud Shell profile, where I would need to modify the settings.json to configure that profile to select the PowerShell Core option instead of the default bash option, or else there could be two Azure Cloud Shell profiles, one for bash and one for PowerShell Core.

Currently blocked on #1883 as per comments here: https://github.com/microsoft/terminal/blob/886d018bb4032018ab69829705ea991a486c871e/src/cascadia/TerminalConnection/AzureConnection.cpp#L664-L667

I tried to run PowerShell from bash and it fails.

@Ayanmullick Lucky you! You hit a bug that I just filed upstream on PowerShell: https://github.com/PowerShell/PSReadLine/issues/1376


Added possibility to substitute network interfaces IP addresses

coreos-cloudinit will try to generate /etc/environment file populated by network interfaces information
As a result you will get something like that:
IFACE_DEFAULT_IPV4=<IP_ADDRESS>
IFACE_ETH0_IPV4=<IP_ADDRESS>
IFACE_ETH0_1_IPV4=<IP_ADDRESS>
IFACE_ETH0_IPV6=fe80::5054:ff:fed4:1856
IFACE_ETH0_1_IPV6=fe80::5054:ff:fed4:1857
IFACE_LO_IPV4=<IP_ADDRESS>
IFACE_LO_IPV6=::1

Also coreos-cloudinit will try to substitute $iface_eth0_ipv4 variables according to network interface information.
There is only one problem moment... Race condition with systemd-networkd.service. If we add:
Requires=coreos-setup-environment.service systemd-networkd.service
After=coreos-setup-environment.service systemd-networkd.service

to all our coreos-cloudinit unit files - that should work.
P.S. That's my first golang code =) I've used Kelsey's https://github.com/kelseyhightower/setup-network-environment as a base.
/cc @crawford
/cc @robszumski
#337

This change makes me really nervous. How do you know which IPv4 address to use given an interface name? How do you handle DHCP changing those addresses?
We considered doing this a while back and decided against it. I think I still agree with that original decision.

Yep, this will work only with DHCP server. So I guess we have to rename these variables to something like $dhcp_eth0_ipv4 or something else. If user has static IP addresses - he will have to enter them into cloud-config anyway.
Code generates ENV variables for all IPs on the network interface. So you can choose which one. First variable doesn't have index (IFACE_ETH0_IPV4=<IP_ADDRESS>), next one has index (IFACE_ETH0_1_IPV4=<IP_ADDRESS>). And so on.
I knew that you'll say that. But we have to fix that chicken-egg issue. It is hard to configure DHCP hosts. Especially when you have hundreds of them. Users have to use hacks, why don't we provide at least one solution?
BTW, another solution: Create something like early cloud-config code (like early docker). So we have to set something like flag inside cloud config options, or set specific section to configure network only. This part of instructions will be executed to configure network, then regular cloud-config should be executed and configure the rest. This solution will work on both static and DHCP configurations. What do you think?

My last note looks similar to https://github.com/coreos/ignition project. But for ignition you have to provide additional config file. I suppose people would not like to provide several config files.

Ignition replaces coreos-cloudinit so you're still only providing a single config. Once Ignition is available on all of our platforms, I am going to deprecate coreos-cloudinit and encourage people to migrate to Ignition. I don't want too much work going into this project knowing that it will be deprecated soon.

Random thought: this isn't too different than https://coreos.com/os/docs/latest/vmware-backdoor.html for the interface specification.

Yes it is. The VMware backdoor requires you to specify the address and role of the interface. This PR makes it implicit and dynamic.

just for history. commit is here: https://github.com/endocode/coreos-cloudinit/commit/17563db51de62ac97f32fe489bb10c4c1deb5122


Add to README : how to connect to the database.

I seem to understand that one should use knex to connect to the database, but then how do we use knex in relation to moron.js?
I must be a moron, but I'd like simple easy-to-follow instructions for setting up a connection to the database and using it with moron.js before showing how to use the models.

Did you look at the example project? It has a simple app.js that binds all the needed stuff together including knex initialization. Maybe I should add the instructions to README also though.

Yes I finally figured it out after going back and forth between the files in the example project.
but yes adding it to the README would be great! : explain that one needs knex to connect and then show an example of how to do it :+1:


Add highlight-deleted-and-added-files-in-diffs feature

LINKED ISSUES:
Closes #3438

TEST URLS:
https://github.com/sindresorhus/refined-github/pull/3062/files
https://github.com/yakov116/TestR/pull/28

SCREENSHOT:

I copied what you did for the avatars.
One issue I have is that I need to set repeat on ajax to false. But when you go from the conversation tab to the files tab, its an ajax request and it never fires.
Also sometimes its still creating the list (were just waiting for one mutation) and the icon is not available causing it to error

One issue I have is that I need to set repeat on ajax to false

Solution: https://github.com/sindresorhus/refined-github/pull/3412#discussion_r462318283

Yeah that makes sense. I am just not sure how to implement such a thing.

Yeah that makes sense. I am just not sure how to implement such a thing.

https://github.com/sindresorhus/refined-github/pull/3442

@fregante wow glad my "implementation" was good :haha:

Now I understand this is different from the diff blocks, I approve the current design. If this could benefit regular commits it would be perfect 👍

Test repo commit https://github.com/sindresorhus/refined-github/commit/ecad0ae6b0c8929ee264f5e810bd27511c6ece2b#diff-d6ab7dfee8e01c5e0c6d61abd3c7d41c

Works!

Uhm it's not working for me on this very PR

Let me check again

Ah yes it's working. Probably it was Webpack

The +/- icons are not vertically centered/aligned with the text.

Marked as draft. I don't know if I will have time before next week to finish.

Position updated

Position updated, this avoids causing jumps and makes it easier to visually align it

Makes sense, but I prefer the previous position. CSS for that:
    .file-info {
      display: flex;
    }

    .file-info a,
    .file-info clipboard-copy {
      order: 1;
    }

    .file-info > :not(button) {
      margin-left: 6px;
    }

    .file-info .tooltipped-s:before {
      bottom: 1px;
    }

    .file-info .tooltipped-s:after {
      margin-top: -2px;
    }


Add workflow specific engine logging to WES GetRunlog response

Description
Include any workflow specific engine log output in top level run_log element of the WES GetRunLog response
Use Case
If a workflow fails because a task fails, the corresponding logs are retrievable via the task_logs element of the WES GetRunLog response. However, if a workflow fails because of a workflow syntax or localization error, these are only available via the CloudWatch logs associated with an engine, retrieved by agc logs engine and mixed with engine logs from other unrelated workflow executions. This makes it difficult for tooling that only talks to the WES endpoint to retrieve engine related errors for a specific workflow execution.
Proposed Solution
Place either the output or a retrievable URI for a workflow specific engine log in the WES GetRunLog response. For example:
{
  "run_id": "string",
  "request": {...},
  "state": "UNKNOWN",
  "run_log": {
    "name": "string",
    "cmd": [
      "string"
    ],
    "start_time": "string",
    "end_time": "string",
    "stdout": "string", // <-- log output lines or s3 uri or cloudwatch logstream arn
    "stderr": "string",  // <-- log error lines or s3 uri or cloudwatch logstream arn
    "exit_code": 0
  },
  "task_logs": [...],
  "outputs": {...}
}

Other information

This probably relates a bit to #319, which teaches the AGC client to download and display those URL fields. Although that only implements fetching web URIs so far.
Toil as set up in #317 already sends nearly all the workflow-specific engine logs this way; some of the ZIP parsing logs still only show up in the main engine log stream.

@wleepang will this be scheduled soon? It would be great to have the information available in the WES api


[consensus] add ensure_round

Motivation
Unify the pre-processing of proposal/vote msg and update some comments.
Have you read the Contributing Guidelines on pull requests?
Y
Test Plan
Existing tests.
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/libra/website, and link to your PR here.)

@bors-libra r=davidiw

:pushpin: Commit aee5c84 has been approved by davidiw

:hourglass: Testing commit aee5c84a183dfb1b412b579ad24c044466bf9628 with merge fb5da600ad1eae3ba405114151894e0302d5d431...

:broken_heart: Test failed - checks-actions_land_blocking_test

@bors-libra retry

:hourglass: Testing commit aee5c84a183dfb1b412b579ad24c044466bf9628 with merge 827f8cf7485fd6716e319a4c61ccb0c90df4e569...

:sunny: Test successful - checks-actions_land_blocking_test, checks-circle_commit_workflow
Approved by: davidiw
Pushing 827f8cf7485fd6716e319a4c61ccb0c90df4e569 to master...


[sdk] devnet 0.15.1 has been activated but SDK with getEvents method not yet available

Devnet 0.15.1 apparently deprecated getEventsByStructName RPC method (and other similar methods for fetching events), but the SDK with the getEvents method  that's supposed to replace it has not yet been published.
I'm getting the following error
Error getting events by move event struct name: 0xffc427e5aa12f25423f579e9acb512c39d720988::amm::PoolCreationEvent, with error: Error: RPC Error: Method not found

but the SDK for 0.15.1 is not yet published on npm.

This works. Thanks!


Why do things work like this?

As I am working on pushing this forward, I am just curious as to a few engineering choices.

Why are all items required and then passed around as the deps variable? this makes it really hard to follow the code imho vs the normal process of just require what ya need.

Why is there a patata.yaml, and a config/config.yaml... both are required or are created yes none of the values are truly needed?

I get a default, but can we make the path to features a config option?

The log.exit method makes tracking down issues really hard due to lack of stack trace... can we improve on that?

The breakdown of features to step definitions is great on one hand, but I know for me and my team we would rather have JUST global, generic definitions, and only use specific ones if needed... Can we support that mindset here too?

init command is really cool, but if there is already a package.json can't we have it just add whats needed to the existing project?

Thank yo very much for indulging me here. Once I understand these items better I will be happy to help implement what needs done and PR it

Hi!
Here some answers.

I use deps as an easy option of dependency injection. All initialization are done on the index.js as bootstrap file. Doing that I avoid the overuse of require in each file. This may be my way to develop, maybe we can have further discussions on this if you guys are more involved in the future :-) Do not have any problem to improve the code :-)
patata.yml is the configuration for the engine. For instance, to set up HockeyApp token (https://patataio.github.io/#integrationshockeyapp) or other Patata settings (the CLI tool). config/config.yml are configurations for the test cases (https://patataio.github.io/#filestructureconfig)
Patata uses Cucumber and Appium. My idea with the project was to avoid have discussions about how to organize the file system and the structure of the project. Making an standard and something quick and easy to set up. The first versions on the projects were a bit more flexible with a lot of settings, but after some releases I figured out that the flexibility brought more troubles to the team for understanding and with conflicts. I decided to remove unnecessary settings and create a solid static structure based on feedback. Then I improved more the CLI tool with error messages and help. Anyway, I am open for changes, but I think we should keep an static way to do things and improve scaffolding/messages/errors.
Sure. Any improvement is welcome.
You can decide to have global definitions if you wish now. Patata reads all files previous to their run. In our company, we made global some GIVEN steps related with logins. Even it is possible, I promote to isolate features as much as possible, but sometimes is needed. Do you have any suggestion about how we could support both mindset without messing up? All ideas are welcome :-)
Sure thing! Excellent idea.

I am happy to hear you wanna be involved! And it could be great to receive any PR from you, there are a lot of improvements I would like to add too that we could discuss if there are beneficial for your team.

Ok so some follow ups (thinking I should have had multiple tickets now to make this easier lol)

OK, I get where your coming from, I think there is room for improvement but for now just getting this somewhere where it works for my team is more important to me then cleaning that up so we can leave it be.

I am embarrassed now because a saw this earlier and forgot... my bad

I think my concern here is so many extra directories off of root make it hard to read... can we just point it to ./integration-tests/*?

I was thinking of just adding an extra default search paths of:

      .concat(glob.sync(`features/definitions.${platform}.js`))
      .concat(glob.sync(`features/definitions.common.js`))

Though ideally, imho definitions belong in their own folder... at the same level of features so more ideal to be would be:
      .concat(glob.sync(`definitions/default.common.js`))
      .concat(glob.sync(`definitions/default.${platform}.js`))
      .concat(glob.sync(`definitions/${platform}.js`))
      .concat(glob.sync(`definitions/common.${platform}.js`))

As for Pos, you bet ya! We are betting the future of our company on improving quality, and integration tests we feel are key to that... so we are very vested in this idea

Actually, for 2... but why are they required? in particular the patata.yml... sure use it... but we should be able to run without it right?

Hi @mattapperson,
For 2 and 3, have you considered the possibility to use the command patata --cwd [PATH] [COMMANDS/OPTIONS]. So you can set the tests any place you wish?
You could update your package.json project with a script similar to this:
"scripts": {
  "patata": "patata --cwd ./integration-tests"
}

And run it like
npm run patata [COMMAND/OPTIONS]

(I did not test it using it on the scripts section of package.json, but I believe it should work)
For 2, the only reason to have patata.yml as mandatory file is for the CLI tool to realize that you are inside a patata project.  This can be solved with patata --cwd [PATH] [COMMANDS/OPTIONS] too. If we are able to make the CLI understand where the patata project is, we may not need this file in the future.
If you think my comments does not work for you, let's keep the discussion open :-)
For 4, let me see if I understand you correctly. Here an example of the file system I understood from you:
+ root
  + components
  + config
  + features
     + a-feature
        - a-feature.android.js
        - a-feature.ios.js
        - a-feature.feature
     + definitions
        - default.common.js
        - default.android.js
        - default.ios.js

If I understood this correctly, inside the features folder, you can be flexible right now. The CLI has a scaffolding tool to create new features, but you can actually organize them in the way you wish :-) It is only required to use *.android.js, .ios.jsand.common.js` as suffixes.
In my team we created independent JavaScript files without be attached to features to be reused on other features :-)
If I did not understood you well, could you give me an example of folder structure? :-)
Thanks for your comments, @mattapperson

@eridem ok, so for 2 and 3... I did not understand that this was how the cwd flag worked, but here is how I expressed things to work fwiw... not saying it's right:

patata would consider the base directory to be wherever the patata command was run.
cwd flag could be used to alter the assumed base directory, overwriting the assumption that it should be the dir that the command was run in
patata.yml is not required, but if it existed, patata would just look for it in whatever directory it was run.
there would be a flag/config (including being able to set it in patata.yml)  for where to look for the components, config, features etc... This way patata.yml can exist in root but not everything has to

Your solution gets the job done, it is just not what I expected...
as for 4... here is what I am shooting for:
+ root
  + integration-tests
    + components
    + config
    + features
        - a-feature.feature
        - b-feature.feature
        - c-feature.feature
     + definitions
        - default.common.js
        - default.android.js
        - default.ios.js
        - b-feature.common.js

The reason is we have non-programer project managers writing our acceptance tests here... we want to give them a folder so they can create these without causing any confusion. We then have default generic definitions that we use for all projects, and our engineers add support for special cases when needed

also having gotten this "working", it does not seem to actually see my tests.
I have an app, in root I run patata --cwd tests/intigration/ run ./testProj.app/
in tests/intigration/ I ran patata feature "A wonderful feature" and then in the .feature file I added my test. in the a-wonderful-feature.common.js file I have some definitions...
But when I run patata --cwd tests/intigration/ run ./testProj.app/ from root I get:
[Patata] Version 6.0.1. More info: http://patata.io
[Patata] Executing tests...
[Patata] Verifying platform conditions...
[Patata] Running Patata...
0 scenarios
0 steps
0m00.000s
[Patata] Everything alright! Happy testing!

Any ideas?

Note:
if I console.log includedFiles in run-generic.js I get:
[ 'features/a-wonderful-feature/a-wonderful-feature.common.js',
  'hooks/hooks.common.js',
  'hooks/hooks.ios.js' ]

OK... so it seems that if you don't include a tag it runs nothing, and I would assume it would run all tags then...

hmmm ok now it "runs" with I added a tag, but fails because the app is not running... it never starts like a simulator or anything...

Sorry for so many posts here 😞 just reporting as I see things...
so run-generic.js has the platform var set to lets say 'ios', that var is used to select the included files... but it also seems to wind up (no idea the code path TBH) in capabilities.js... am I missing a flag?
I set the udid flag and it crashes because there is no capability for 'ios'... does that make sense?

Hi @mattapperson,
Sorry for late answers. I think we may be in different time schedules.
-I agree that sometimes you would like that POs/Managers write the acceptance criteria there (I promote this in my company too). You can decide how to distribute your files right now inside the features folder. As your example, you can separata the implementation from the specifications. You example is valid and it should work on Patata, with the exception of using *.android.feature, *.ios.feature and *.common.feature. If you want to have specifications for both platform, the POs/Managers can use my-feature.common.feature so it works in both platforms.
+ root
  + integration-tests
    + components
    + config
    + features
        - a-feature.common.feature
        - b-feature.common.feature
        - c-feature.common.feature
        + definitions
          - default.common.js
          - default.android.js
          - default.ios.js
          - b-feature.common.js

So, my answer is that you can distribute your files as you wish :-) It is only important to keep the suffixes in the files. In my team, we have the same app for both iOS and Android, but some features were a bit different between platforms so that's why we separated them, keeping common in case the feature works in the same way in both platforms.

Yes, patata works with tags better. But if you do not use tags, it should run all of them.

We did few tests with iOS, I guess we need to improve better this platform. Even the intention was working on both platform, we were using it only in Android. We should revise the iOS platform from scratch.

@mattapperson , would you mind if we close this issue and we create multiple ones for each question? I feel we talk about different things in one thread all time XD

Heh agreed


Could You Make a Splash Skin Visualiser/Preview on the menu options like its already with the Note Skin Visualiser/Preview?

What feature do you want to get added on the base engine?
At least for me, searching between all the splashes only guiding by the name is very annoying and tiring, so some kind of splash visualiser/preview like theres already one for the noteskins aka the notes above the noteSkin selector that shows the current selected noteSkin Strums, same but with the (up note) splash at the side, something like

To test your sight, and reliability, please select the option of what should NOT be requested.
Proper credit that was forgotten

Done in the latest commit!


allow read from replicas in pipeline

Can you merge this and make a 2.1.1 version first?
Using mget is not acceptable as it's adding too much cost to us(3x as the expected cluster size).

@duke-cliff Yes i can make a 2.1.1 releaes after i merge your two PR:s

Also even if all the current tests actually works, do you know know that the current pipeline algorithm actually works out in basically all pipeline cases even with read-only mode? I have no tests for this and this PR do not include any extension to the tests or at least any manual steps or a validation script to validate that it actually works out as we expect and that it handles a lot of the error scenarios and failover scenarios that actually can happen that the algorithm have to take care of.

@Grokzen Yup, it works. The redirect logic is working the same as before, the only difference is when it could get a connection from the slaves for a slot. The rest logic is the same to how you deal with exceptions/redirects(moved/ask) on masters:

You can see our CPU was dropped from 16% to 5-6% because of this change.

I will have more optimization for this PR coming. testing locally first.

Hi, I noticed that https://github.com/Grokzen/redis-py-cluster/pull/402 is already open for this, is there any way we could help make this happen? We'd be very interested in the ability to be able to send read commands to replicas within pipelines.

When I merged master into this branch, the diff went away. I think this PR might be safe to close.


Championify no longer starts (error 0xc0000005)

According to the log, the last time Championify successfully ran for me was on 3rd of February 2018. After this date, although I had not changed anything (not even updated it), the application would no longer start and instead immediately throws the above error (Translation: "The application could not be started correctly (0xc0000005). Press "OK" to close the application").
I suspect that some Windows update changes something about this. I am on Windows 10 N, 64 Bit. I have confirmed that version 2.1.3 through 2.1.5 are affected and probably lower versions as well. Even the installer throws an error when I try to install Championify: it's the exact same error 0xc0000005 for installer version 2.1.3, and the same error message but with 0xc000007b instead for installer version 2.1.5.
No other applications on my computer are affected, thus I believe the error lies in the way Championify works with a recent Windows update.

Not experiencing any issues myself. This looks to be something directly related to electron. Feel free to try uninstall and reinstall and see if it helps.

Again, the same thing happens when I attempt to uninstall or reinstall Championify. So I really don't know what to do.


bump master to v.0.33.2

Coverage increased (+0.0007%) to 26.267% when pulling ce0294e6cbcf95a117bac525ce1454299ba8ae5f on patthiel:master into 37e0e76c236cf4a66780cd9f2126d10b0aca25e5 on linode:develop.


Split into packages

Group by social media, and split into multiple packages (oh, and move all credentials to external files).

Fixed


Added back-to-top links in ReadMe.md

Related Issue

Solves #29

Description of Changes

Added section-wise 'back-to-top' links in ReadMe.md

Checklist:

[x] My code adheres to the established style guidelines of this project.
[x] I have conducted a self-review of my code.
[x] I have included comments in areas that may be difficult to understand.
[x] I have made corresponding updates to the project documentation.
[x] My changes have not introduced any new warnings.

Great work 🚀Thank you for your contribution !


[config] Move hard coded test fixture to files

Tracker #2
The config package seems to be finished, but it took me a while to remember what is it doing

gofmt use tab while yaml does not allow it, and when using long string `, it's pretty likely to use tab instead of space
a lot of tests share fixtures, and there should be test helpers for reading fixture files

fail directly if file not found/can't read (pass testing)

(optional) provide example that can run in godoc/playground

Fixed in 4d42e64e0a868308aee463434ce22da63fd6d5fe


Fix bug where config interface wouldn't allow port > 32767

Change port to be parsed as uint16 instead of int16 to allow for full range of valid ports.

Good catch! I will merge it tomorrow and make it available in the upcoming release.
Claudio