No os.family reported for Chrome on HP Chromebook

    {
        "description":"Chrome 40.0.2214.93",
        "layout":"Blink",
        "manufacturer":null,
        "name":"Chrome",
        "prerelease":null,
        "product":null,
        "ua":"Mozilla/5.0 (X11; CrOS armv7l 6457.83.0) AppleWebKit/537.36 (KHTML,like Gecko) Chrome/40.0.2214.93 Safari/537.36",
        "version":"40.0.2214.93",
        "os":{
            "architecture":null,
            "family":null,
            "version":null
        }
    }

Is this expected?
Thanks.

Currently, we haven’t added support for Chrome OS (totally forgot about it). We’ll add that in soon.

@d10 Thanks, good stuff.

Just here for a :+1: on this issue, I assumed the library wasn't working because I develop on a Chromebook
   "description":"Chrome 43.0.2357.46",
   "layout":"Blink",
   "manufacturer":null,
   "name":"Chrome",
   "prerelease":null,
   "product":null,
   "ua":"Mozilla/5.0 (X11; CrOS x86_64 6946.31.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.46 Safari/537.36",
   "version":"43.0.2357.46",
   "os":{
      "architecture":null,
      "family":null,
      "version":null
   }
}```

+1 :octocat:

+1 also :)


Consider displaying proto-repl with atom-ink

Having inline, Light Table evaluation and watches is really nice. Here's an Atom project that could allow you to do that: https://github.com/JunoLab/atom-ink
I think it would be cool to have the ability to toggle on and off these inline evaluations, along with a full-blown repl like you already have.

I added some very basic support for this in the latest released version. This is my attempt to try it out first to get an idea of how integration might work.
You can try it by doing the following:

Upgrade to the latest Proto REPL
Install Atom Ink
Reload atom (Restart it or go to View -> Reload)
Click "Show Inline Results" in the Proto REPL settings.

Now when you execute a block or selection the result will be displayed using Atom Ink as well as in the REPL. This is definitely not the final way this would be implemented. Some initial thoughts:

Need to pretty print results to display there. Or show non pretty printed results but when the tree is expanded show the pretty printed results.
Integrate Ivy for display documentation, lists of functions in a namespace
Take advantage of the Ivy's ability to show things loading

This is awesome. Those changes you mention above sound great.
Is it crazy to build it so that it automatically evaluates expressions? Now that you have the value positioned next to the functions, they could update as you update code, similar to how Light Table has watch expressions.
You could use a key binding to set a block as a watched expression. But I think it would be a cool experiment to just do it for all expression, even the one you are currently typing. It probably should have a delay, similar to how linting in JS constantly evaluates your syntax as you type. After using JS linting, I can't work without it. I could imagine this could be similar.
Then the repl is used for testing ideas out, but not the main receiver of values. If all functions are watched and evaluated as you type, you may eventually not even use the repl, because you could just test out code inline as you type.
Just an idea. Great work with what you've done - I love it.

Continuing in the saga of improving inline results I've added the ability to autoload the current file whenever there's a change. Results will display inline next to the expressions. You can even combine it with proto-repl-charts so a graph is displayed on every modification. To enable it for the current text editor bring up the command palette and select "Proto Repl: Autoload File". It can be turned off on a file with "Proto Repl: Stop Autoload File".

Jason, This is really great. Your repl plus parinfer just made Atom a serious Clojure/Clojurescript editor.
Are you still planning on adding inline docs, pretty print and loading indicator (mentioned in your comment above)? Once you're good with this you should consider reaching out to some clojure folks  to get their take on it. Maybe Hacker News as well.

For inspiration: https://www.youtube.com/watch?v=HnZipJOan54&feature=youtu.be

Thanks for the video. Shem and golem are very interesting. I like the way that golem shows bits of code next to the result where the code is editable and immediately re-evaluated. By splitting up the "REPL" into floating display areas you can mix visualizations and code with results. Mixing visualizations and code in the same display area is one of the next challenges for Proto REPL.

Agreed. It seems like you are building a good foundation on top of Atom for exploring those ideas.

This is really exciting! I already find it easier to use than light table, in terms of starting a repl and getting evaluation going.

Thanks, I'm really glad to hear it's working for you. If you (or anyone else) has ideas on improvements for the Atom Ink integration this thread is a good place to capture them and discuss. I can open new issues for specific improvements.

I had a few ideas:

When pretty printing a data structure for inline display convert it into a hierarchical tree so that each level can be expanded or collapsed. This would make exploring a large data structure easier.
Allow saving values inside an algorithm. Display the values inline next to where they were captured. I saw a tweet from Michal Srb on twitter showing Golem doing this. Here's a labeled screen cap showing Golem:

The way it would be displayed in Proto REPL would likely be different since it doesn't have the same display capabilities as Golem. But it could be displayed next to the specific log call where the values where saved. There should be a way to recall a specific saved value so that it could be used in further testing.

I can see a big benefit of displaying large datasets as hierarchical trees. I use that all of the time in the Chrome inspector when doing JS. I console.log or console.table out data structures or print them out at a debug point.
As for saving values inside an algorithm - it's interesting but at the moment it may be too far outside of my workflow to really understand it. I need to watch that video again and think about it.
This may be way outside the scope of proto-repl, but I keep thinking about error messages. From my experience, they are pretty obtuse. It doesn't sound like I'm alone: http://blog.cognitect.com/blog/2016/1/28/state-of-clojure-2015-survey-results
To this question:

What has been most frustrating or has prevented you from using Clojure more than you do now?

The number 1 response was error messages.
It's also an issue closely associated with beginner and intermediate Clojure developers. Experienced developers have already created a mental map and decision tree to understand and track down those errors. Experienced developers aren't likely on Atom and proto-repl either (they are typically on Emacs, with some converts over to Cursive). The perfect audience for Atom/proto-repl are people new to the language who don't want to learn Emacs on top of Clojure (And there are a lot of them, based on the survey). So it seems like if error messaging can be improved with proto-repl, that would be a huge draw.
For improved error messaging, it could give the original message along with hints as to what it could be:
#<CompilerException java.lang.NullPointerException (NO_SOURCE_FILE:0)>
Typically this is due to [something]. Try [suggestion]

Again, understandable if it's outside the scope of a repl. But maybe not. Language-level improvements in error messaging eventually needs to be done, but that is a very slow, precise process. A 3rd-party repl can be a lot quicker to implement and update the messages. It can also be given more leniency when incorrect, which is helpful for moving quick. I watched Colin Fleming give this talk in November: https://www.youtube.com/watch?v=kt4haSH2xcs. Clearly having access to the AST seems ideal in terms of what context is available for interpreting and tracking down errors. That is one approach, but there are tradeoffs on speed of development and platform requirements.  It reminds me of how early on Coffeescript made popular a lot of language constructs that gave the TC39 working group momentum and confidence to get language additions added to JS. Now we have Babel doing a similar thing and from what I've heard the working group appreciates and needs that kind of real-world usage.
You can still get a lot of information from the error message string and function the error threw in. I'm definitely not an expert at this. But I would explore two paths for this:

keyword search in a string with manual logic: does it contain java.lang.NullPointerException?
** Does it contain NO_SOURCE_FILE: do [a]
** Does it contain xyz: do [b]

Natural language processing + bayesian classification: I've used this library with success: https://github.com/NaturalNode/natural#classifiers

Whatever approach to classification you use, it may make sense to make it a separate repo from proto-repl. Then other people could contribute and use it in other projects. As another project I could see it really helpful as a website where people can paste error messages in and get suggestions. Maybe it's written in CLJS (if using something like NaturalNode) or CLJC. Just an idea. I love this project. Thanks.

That's a really good idea. I'm going to put some thought into that and start keeping track of typical Clojure errors I encounter. There may be reasonable error messages that can be displayed for many of them.
RE: Saving values in an algorithm, view results, and recall:
Inside of a function you might have a few different things going on. The function takes input and output and returns a value but you can't see inside it to know what's happening. That's why people typically reach for println and log statements. I want to improve on that approach with things like

The ability to save those values and recall them to use for manual testing in the REPL.
Showing values saved over time. (Atom ink inline display)
Visualizing values over time.

Golem was attempting to do what Brett Victor showed way back in his Inventing on Principle video. The relevant section starts at 17:30. Here's a screenshot from that video showing a very similar thing. It's showing the values captured during execution of his binary search function.

I want the same thing in Proto REPL. I want to show the values as they change within the code itself. Mr. Victor shows all of the values as soon as they are set. That's a bit harder to accomplish. An easier approach would be to trigger the save with a call to some kind of log function like in Golem. Saved values would automatically be displayed inline as soon as they were captured.

I implemented the inline display as an explorable tree. That's released with a bunch of other fixes in 0.14.0

This is really nice. Unfortunately I can only pull changes and play around right now because heads down in a non-Clojure project. In a couple weeks I'll be starting on a decent size CLJS project where I can really take advantage of this. I'll also keep track of errors that come up and their solutions. I am still thinking through the values saved during execution idea as well.
What's next for this project?

I'm working on the feature I described above to save local bindings and view them. I think I'll have a beta version of it available pretty soon. It's currently being developed in the save_and_recall branch.

I just released the save and view local bindings feature in 0.15.0. https://github.com/jasongilman/proto-repl#saving-and-viewing-local-binding-values It also improves the display of values from Atom Ink by changing to a mono-space font and increase the size of the inline display area.

So sweet. I totally get the saving and viewing local bindings now. It's kind of like the reductions function in Clojure (https://clojuredocs.org/clojure.core/reductions) but applied to any internal function.
Small typo:

They're not meant to be used in local development only.

You probably mean it's only meant to be used in local development.
As a side note, I talked with Julio Barros who is doing an online Clojure course and suggested he include Atom/proto-repl in his page of resources: http://e-string.com/clojure-resources/. It would be fun to get a bunch of people hammering on this.

Not directly related to atom-ink, but you might also look at using block decorations for larger results, and perhaps inline doc strings.
http://blog.atom.io/2016/02/03/introducing-block-decorations.html
https://github.com/atom/atom/pull/9930

I really like the idea of using block decorations. I don't have a great idea of what to put there yet. I like the idea of putting larger results there instead of inline on the right where there's not as much room. I've also thought of using them as a target for visualizations like those provided in proto repl charts instead of putting those in a new tab.

@jasongilman  So, I've been using LightTable for some time and have come to really love the inline results, which is why I was watching the block decorations closely, and like what your'e doing with proto-repl.
One area that I think could be improved on for inline results is when you have a larger result than can be displayed on the same line to the right of the end of the expression. With larger expressions they often run off the visible screen to the right. Formatting of the inline results would also be cool.
For instance a function that's being called and returns a number say 101 should just display on the same line. For a defn which returns a var name, perhaps that can be recognized as such and add a green checkmark on the same line to indicate it's been evaluated, and remove it on edit. For a function that's returning a large map, that should break to the next line. Taking it a bit further, format the result based on it's type, so if it's a map, rather than printing out just a raw string, actually format it and also allow it to be copy/pasted. If there's an exception, clean up the raw stack trace and display that inline as well.
Thanks for all your work on proto-repl, I believe having a good repl experience in Atom will really help clojure adoption.

Closing this issue now that Proto REPL has been using Atom ink for a while. Improvement ideas can be filed as new issues.


Integration of pyit600 to fhem is also available

Hi jvitkauskas,
there is also a integration of your package to fhem available by the module fhempy from dominikkarall: https://github.com/dominikkarall/fhempy.
Currently there is implementation for climate devices (tested with device VS20WRF) available.
https://github.com/dominikkarall/fhempy/blob/master/FHEM/bindings/python/fhempy/lib/pyit600/README.md
Maybe you want add this info also to your README.md.
BR
Volker

Ok, added this to readme.


Drop import_measures --definitions_only from pipeline

There's no need, as definitions get imported as part of import_measures.
This is being fixed now because it should depend on matrixstore_publish
(because it looks up all prescribed presentations from the MatrixStore)
and rather than fixing that, I'm just removing it.

I'm going to merge this myself now, because I'd like to check the e2e tests are passing before merging #2524.  A post-merge sense-checking review would be appreciated though!


action 'eosio::setcode' is on the action blacklist

Error 3130005: Action to execute is on the blacklist
Error Details:
action 'eosio::setcode' is on the action blacklist
pending console output:
in mainnet
Why is this?

The issue here. Any idea?

Same issue here.

I get the same error.

Same issue

Where is the news? I can't found this on eos.io @matthewdarwin

how to check the status of block list?

I have got this error since yesterday.
Some news?
are thre some workaround to deploy a contract?

According to discussion I see in telegram, the blacklist is proposed to be removed by end-of-day (US time) July 25.

A bunch of producers removed the blacklist now

Already working properly

@thbourlove An API has been created to retrieve the contents of the blacklist and whitelist.  It will be available in version 1.2. See #4853.
set code is generally available on mainnet again.


fix(do-dont-components): fixed padding and top line

Closes #903
Removed padding on the DoDontExample component and switched from rem to % padding for the text content so it doesn't flow outside of the box. Also changed the height of the top line from 2px to 3px.

This is happening with the top bar

I'm seeing the same issue as Tyler on typography page. I think it has to do with the default margin getting added to the bottom of images.

@alisonjoseph @designertyler Fixed the issue!


service set_room_mode not working for sensor.####betriebsart_heizkreis[abcd]

Version 0.9.0
Bug, missing feature or user error?
When trying to set heating mode for sensors sensor.[.*]betriebsart_heizkreis[abcd] the integration returns
"Entity sensor.warmepumpe_betriebsart_heizkreis_b does not support this service."
service: idm_heatpump.set_room_mode
data:
  target: sensor.warmepumpe_betriebsart_heizkreis_b
  value: eco
  acknowledge_risk: true

just seen the ciructe sensor use "CircuitMode" enum, not "RoomMode"

Yes, the values for these sensors (Modbus registers) are slightly different, so they need different enums. Setting CircuitMode is currently not implemented and haven't found a way to better filter the entities shown by Home Assistant for each service.
For now I added set_circuit_mode to #50, but I'm not sure, when I'll get round to implementing it. I'd gladly accept a PR.


NVA BGP Peering with Route Server across VNET peering

Looking to check if BGP Peering between NVA and RS across VNET peering is supported ? In my scenario bgp connection constantly dropped and re-established.
[ VNET1 ---NVA1] ---- [VNET2---RS--NVA0]
NVA0 can establish BGP with both ips of RS, but NVA1 , constantly dropped the connection.
NVA1
Neighbor        V           AS MsgRcvd MsgSent   TblVer  InQ OutQ Up/Down  State/PfxRcd
<IP_ADDRESS>        4        65515       3       4        1    0    0 00:00:16        1
<IP_ADDRESS>        4        65515       0       0        1    0    0 00:00:21 Idle
NVA0
Neighbor        V           AS MsgRcvd MsgSent   TblVer  InQ OutQ Up/Down  State/PfxRcd
<IP_ADDRESS>        4        65515      16      19        3    0    0 00:11:26        1
<IP_ADDRESS>        4        65515      17      19        3    0    0 00:11:26        1

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

ID: 55c39aea-70af-8b68-36b4-efb3bfc17d89
Version Independent ID: 32ca8dea-b00e-de3a-e2bf-dd8318f34550
Content: Frequently asked questions about Azure Route Server
Content Source: articles/route-server/route-server-faq.md
Service: route-server
GitHub Login: @duongau
Microsoft Alias: duau

@Andriy-dev, Thanks for reaching out. We will investigate and update this thread.

@Andriy-dev, as per your request, we are assigning this issue to the respective SME for providing further insights on your asks.

By nature of RouteServer it advertise back the Remote VNET space, that create recursive route from the NVA perspective (remote neighbor via itself ). Can be solved by selectively accepting prefixes or adding static routes to RS endpoints.
`
Neighbor        V           AS MsgRcvd MsgSent   TblVer  InQ OutQ Up/Down  State/PfxRcd
<IP_ADDRESS>      4        65515       3       6       60    0    0 00:00:18        2
<IP_ADDRESS>      4        65515       3       3       60    0    0 00:00:12        2
 Network          Next Hop            Metric LocPrf Weight Path

*>   <IP_ADDRESS>/16      <IP_ADDRESS>                             0 65515 i

                <IP_ADDRESS>                             0 65515 i

<IP_ADDRESS>/16    <IP_ADDRESS>                             0 65515 i
*>                    <IP_ADDRESS>                             0 65515 i
`


Question about adding ppconsul library to my cmake project

Hi Andrey,
I ran into you git project (https://github.com/oliora/ppconsul) when I googled 'consul c++ client'.
I wanted to play around with the client that you wrote. However I'm having trouble creating a simple cmake project to link with your library.
Here is what is in main.cpp:
#include <iostream>
#include "ppconsul/agent.h"

int main(int argc, char const *argv[]) {
  using namespace ppconsul;

  // Create a consul client with using of a default address ("<IP_ADDRESS>:8500") and default DC
  Consul consul;
  // We need the 'agent' endpoint for a service registration
  agent::Agent agent(consul);

  return 0;
}

Very basic stuff. Here is what my simple CMakeList.txt looks like:
cmake_minimum_required(VERSION 2.8)

project(consuldemo)

#Enable C++14
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED on)

#On g++ this ensures -std=c++11 and not -std=gnu++11
set(CMAKE_CXX_EXTENSIONS OFF)

include_directories ("${PROJECT_SOURCE_DIR}/ppconsul/include")

FIND_PACKAGE(CURL)
IF(CURL_FOUND)
  MESSAGE(STATUS "Curl libraries found at: ${CURL_LIBRARIES}")
  MESSAGE(STATUS "Curl includes found at: ${CURL_INCLUDE_DIRS}")
else()
  MESSAGE(SEND_ERROR "Could not find cURL on your system")
ENDIF(CURL_FOUND)

add_executable(consuldemo main.cpp)

# set the path to the library folder. libppconsul.a is in /usr/local/lib
link_directories(/usr/local/lib)

target_link_libraries(consuldemo ${CURL_LIBRARIES} json11 ppconsul)

I'm having trouble in the link phase:
//usr/local/lib/libppconsul.a(http_client.cpp.o): In function `ppconsul::curl::(anonymous namespace)::CurlInitializer::CurlInitializer()':
http_client.cpp:(.text+0xb0): undefined reference to `curl_global_init'
....
....

Can you please suggest a best practice simple hello world type CMakeList.txt project which can link against libppconsul.a?
I am able to build the ppconsul cmake project fine so I should have curl installed properly. My cmake project is probably not correct.
Thanks in advance for any help you can provide.
Thanks,
Normin

Hi Normin,
Seems that CMake can't find libcurl library. Could you please post the full output of CMake and Make commands. Before running the CMake command make sure to clean the workspace directory thus CMake can't pickup cached variables and paths.
To make sure libcurl is found change the find command to FIND_PACKAGE(CURL REQUIRED).

This slightly changed CMakeLists.txt works fine for me:
cmake_minimum_required(VERSION 3.0)

project(consuldemo)

#Enable C++14
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED on)

#On g++ this ensures -std=c++11 and not -std=gnu++11
set(CMAKE_CXX_EXTENSIONS OFF)

FIND_PACKAGE(Boost 1.55 REQUIRED)
FIND_PACKAGE(CURL REQUIRED)

# PPConsul "finder"
set(PPCONSUL_INCLUDE_DIRS "$ENV{HOME}/projects/ppconsul/include")    # Replace with your path to ppconsul includes
set(PPCONSUL_LIBRARIES_DIR "$ENV{HOME}/projects/ppconsul/ws/output") # Replace with your path to ppconsul library
find_library(PPCONSUL_JSON11_LIB json11 HINTS ${PPCONSUL_LIBRARIES_DIR})
find_library(PPCONSUL_PPCONSUL_LIB ppconsul HINTS ${PPCONSUL_LIBRARIES_DIR})
set(PPCONSUL_LIBRARIES "${PPCONSUL_JSON11_LIB}" "${PPCONSUL_PPCONSUL_LIB}")

add_executable(consuldemo main.cpp)

target_include_directories(consuldemo PRIVATE
    "${Boost_INCLUDE_DIRS}"
    "${CURL_INCLUDE_DIRS}"
    "${PPCONSUL_INCLUDE_DIRS}"
)

target_link_libraries(consuldemo ${CURL_LIBRARIES} ${PPCONSUL_LIBRARIES})

This works fine on macOS.

I am trying to build on a Linux Ubuntu 16.04 platform.
I am still hitting the same issue after applying the CMakeLists.txt changes you suggested.
Here is the output:
n.emralino@ssna-eng-a1-stor7:/workspace/normin/consuldemo/build$ cmake -G "Unix Makefiles" ..
-- The C compiler identification is GNU 5.4.0
-- The CXX compiler identification is GNU 5.4.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Boost version: 1.63.0
-- Found CURL: /usr/lib/x86_64-linux-gnu/libcurl.so (found version "7.47.0")
-- Configuring done
-- Generating done
-- Build files have been written to: /workspace/normin/consuldemo/build
n.emralino@ssna-eng-a1-stor7:/workspace/normin/consuldemo/build$
n.emralino@ssna-eng-a1-stor7:/workspace/normin/consuldemo/build$
n.emralino@ssna-eng-a1-stor7:/workspace/normin/consuldemo/build$ make
Scanning dependencies of target consuldemo
[ 50%] Building CXX object CMakeFiles/consuldemo.dir/main.cpp.o
[100%] Linking CXX executable consuldemo
/workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function ppconsul::curl::(anonymous namespace)::CurlInitializer::CurlInitializer()': http_client.cpp:(.text+0xb0): undefined reference to curl_global_init'
/workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function ppconsul::curl::(anonymous namespace)::CurlInitializer::~CurlInitializer()': http_client.cpp:(.text+0xcf): undefined reference to curl_global_cleanup'
/workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function ppconsul::curl::HttpClient::HttpClient()': http_client.cpp:(.text+0x9db): undefined reference to curl_easy_init'
http_client.cpp:(.text+0xa41): undefined reference to curl_easy_setopt' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function ppconsul::curl::HttpClient::~HttpClient()':
http_client.cpp:(.text+0xb53): undefined reference to curl_easy_cleanup' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function ppconsul::curl::HttpClient::perform()':
http_client.cpp:(.text._ZN8ppconsul4curl10HttpClient7performEv[_ZN8ppconsul4curl10HttpClient7performEv]+0x18): undefined reference to curl_easy_perform' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function void ppconsul::curl::HttpClient::setopt<CURLoption, long>(CURLoption, long const&)':
http_client.cpp:(.text.ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionlEEvT_RKT0[ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionlEEvT_RKT0]+0x30): undefined reference to curl_easy_setopt' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function void ppconsul::curl::HttpClient::setopt<CURLoption, unsigned long ()(char, unsigned long, unsigned long, void*)>(CURLoption, unsigned long (* const&)(char*, unsigned long, unsigned long, void*))':
http_client.cpp:(.text.ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPFmPcmmPvEEEvT_RKT0[ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPFmPcmmPvEEEvT_RKT0]+0x30): undefined reference to curl_easy_setopt' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function void ppconsul::curl::HttpClient::setopt<CURLoption, decltype(nullptr)>(CURLoption, decltype(nullptr) const&)':
http_client.cpp:(.text.ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionDnEEvT_RKT0[ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionDnEEvT_RKT0]+0x2e): undefined reference to curl_easy_setopt' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function void ppconsul::curl::HttpClient::setopt<CURLoption, char const*>(CURLoption, char const* const&)':
http_client.cpp:(.text.ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPKcEEvT_RKT0[ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPKcEEvT_RKT0]+0x30): undefined reference to curl_easy_setopt' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o): In function void ppconsul::curl::HttpClient::setopt<CURLoption, std::__cxx11::basic_string<char, std::char_traits, std::allocator >>(CURLoption, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)':
http_client.cpp:(.text.ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEEvT_RKT0[ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEEvT_RKT0]+0x30): undefined reference to curl_easy_setopt' /workspace/normin/ppconsul/workspace/output/libppconsul.a(http_client.cpp.o):http_client.cpp:(.text._ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPSt5tupleIJNS_4http6StatusENS_15ResponseHeadersENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEEEEvT_RKT0_[_ZN8ppconsul4curl10HttpClient6setoptI10CURLoptionPSt5tupleIJNS_4http6StatusENS_15ResponseHeadersENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEEEEvT_RKT0_]+0x30): more undefined references to curl_easy_setopt' follow
collect2: error: ld returned 1 exit status
CMakeFiles/consuldemo.dir/build.make:97: recipe for target 'consuldemo' failed
make[2]: *** [consuldemo] Error 1
CMakeFiles/Makefile2:67: recipe for target 'CMakeFiles/consuldemo.dir/all' failed
make[1]: *** [CMakeFiles/consuldemo.dir/all] Error 2
Makefile:83: recipe for target 'all' failed
make: *** [all] Error 2
I have also attached my CMakeLists.txt and main.cpp
Desktop.zip
I will try on my Macbook tomorrow.

Hm, strange. I'll find my Ubuntu machine and try to reproduce the error.

I did not tried to build on Linux but I'm pretty sure it's about the order if linked libraries which matters on Linux and doesn't matter on macOS.
Could you please try to change the order of link liibraries to put Curl afer Ppconsul: target_link_libraries(consuldemo ${PPCONSUL_LIBRARIES} ${CURL_LIBRARIES}). Also, json11 should be linked after Ppconsul: set(PPCONSUL_LIBRARIES "${PPCONSUL_PPCONSUL_LIB}" "${PPCONSUL_JSON11_LIB}")

I'm thinking about changing Ppconsul into a dynamic library to hide all the dependencies and linking comexity inside the library. Does using of Ppconsul as a dynamic library sounds ok for you?

Changing the linking order worked :)
Here is the order I used:
target_link_libraries(consuldemo ${PPCONSUL_JSON11_LIB} ${PPCONSUL_PPCONSUL_LIB} ${Boost_LIBRARIES} ${CURL_LIBRARIES})

I also just noticed that the correct link order should be:
target_link_libraries(consuldemo ${PPCONSUL_PPCONSUL_LIB} ${PPCONSUL_JSON11_LIB} ${Boost_LIBRARIES} ${CURL_LIBRARIES})
As you suggested. This order also worked.
Thanks for the help Andrey!

You welcome!


AHashMap::default() gives a type inferrence error

(again, please ignore FxHashMap - it's really an AHashMap)
error[E0282]: type annotations needed for `FxHashMap<usize, snippet::Style, S>`
    --> compiler/rustc_errors/src/emitter.rs:1356:38
     |
1356 |                 let mut multilines = FxHashMap::default();
     |                     --------------   ^^^^^^^^^^^^^^^^^^ cannot infer type for type parameter `S` declared on the struct `AHashMap`
     |                     |
     |                     consider giving `multilines` the explicit type `FxHashMap<usize, snippet::Style, S>`, where the type parameter `S` is specified

The way FxHashMap deals with this is by making FxHashMap a straight type alias for using BuildHasherDefault: https://docs.rs/rustc-hash/1.1.0/rustc_hash/type.FxHashMap.html. Maybe AHashMap could do something similar?

new() has the same issue.
error[E0283]: type annotations needed for `FxHashMap<usize, snippet::Style, S>`
    --> compiler/rustc_errors/src/emitter.rs:1357:38
     |
1357 |                 let mut multilines = FxHashMap::new();
     |                     --------------   ^^^^^^^^^^^^^^ cannot infer type for type parameter `S`
     |                     |
     |                     consider giving `multilines` the explicit type `FxHashMap<usize, snippet::Style, S>`, where the type parameter `S` is specified
     |
     = note: cannot satisfy `_: BuildHasher`
     = note: required by `FxHashMap::<K, V, S>::new`

@jyn514
An alias does not work well for the reasons described here: https://github.com/tkaitchuck/aHash/pull/23
Does this do what you want? https://github.com/tkaitchuck/aHash/pull/54

@tkaitchuck indeed it does! Now it runs into other errors, but those I need to fix myself I think.
error[E0277]: the trait bound `FxHashMap<DepNode<K>, SerializedDepNodeIndex>: Encodable<_>` is not satisfied
  --> compiler/rustc_query_system/src/dep_graph/prev.rs:6:17
   |
6  |   #[derive(Debug, Encodable, Decodable)]
   |                   ^^^^^^^^^
   |                   |
   |                   the trait `Encodable<_>` is not implemented for `FxHashMap<DepNode<K>, SerializedDepNodeIndex>`
   |                   in this macro invocation
   |
  ::: /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/synstructure-0.12.4/src/macros.rs:94:9
   |
94 | /         pub fn $derives(
95 | |             i: $crate::macros::TokenStream
96 | |         ) -> $crate::macros::TokenStream {
   | |________________________________________- in this expansion of `#[derive(Encodable)]`
   |
   = note: required by `rustc_serialize::Encodable::encode`

That looks like it is related to this https://docs.rs/rustc-serialize/0.3.24/rustc_serialize/trait.Encodable.html
You may need to add a .into() to get the hashmap in/out.

This error I don't understand ... it looks like insert() takes a different mutability than before? But FxHashMap also takes &mut self for insert(), so I don't know what could have changed.
error[E0502]: cannot borrow `*self` as immutable because it is also borrowed as mutable
  --> compiler/rustc_resolve/src/def_collector.rs:53:92
   |
53 |             let old_index = self.resolver.placeholder_field_indices.insert(field.id, index(self));
   |                             --------------------------------------- ------                 ^^^^ immutable borrow occurs here
   |                             |                                       |
   |                             |                                       mutable borrow later used by call
   |                             mutable borrow occurs here

error: aborting due to previous error

I'm also seeing errors like this, which might be related to the K: Borrow<Q> bound on get()? Not sure why that would be an error, though.
error[E0308]: mismatched types
   --> compiler/rustc_mir/src/interpret/memory.rs:627:30
    |
627 |                         .get(&id)
    |                              ^^^
    |                              |
    |                              expected struct `rustc_middle::mir::interpret::AllocId`, found `&rustc_middle::mir::interpret::AllocId`
    |                              help: consider removing the borrow: `id`

I think you might want Q: Borrow<K>, not the other way around.

This error I don't understand ... it looks like insert() takes a different mutability than before? But FxHashMap also takes &mut self for insert(), so I don't know what could have changed.

Is there an extra deref that wasn't there before?

I think you might want Q: Borrow<K>, not the other way around.

Where are you referring to? The only location I see doing Borrow<Q> is the impl for the Index trait, and it's identical to HashMap's impl.

Oh I misread - the Q is from HashMap::get from the Deref impl, not from AHashMap. https://docs.rs/ahash/0.5.3/ahash/struct.AHashMap.html#method.get
Hmm, something weird is going on with type inference ... I'll look into it.

This also happens with the other codebase I tried to switch to ahash! (well, from hashbrown::HashMap to ahash::AHashMap.) https://github.com/launchbadge/sqlx/pull/737
error[E0502]: cannot borrow `n` as immutable because it is also borrowed as mutable
   --> sqlx-core/src/sqlite/connection/explain.rs:139:34
    |
139 |                     n.insert(p3, n.get(&p2).copied().unwrap_or(true));
    |                     - ------     ^ immutable borrow occurs here
    |                     | |
    |                     | mutable borrow later used by call
    |                     mutable borrow occurs here

error[E0502]: cannot borrow n as immutable because it is also borrowed as mutable
--> sqlx-core/src/sqlite/connection/explain.rs:139:34

I wonder if it's because of the deref trait, and if it would work better if the class directly implemented insert and get. I'll try it out.

@jyn514
Done. Pushed to the same PR. Please confirm that this works for you.

Works great :) There was another error I don't think you can solve without getting rid of deref altogether:
error: function cannot return without recursing
   --> compiler/rustc_mir/src/const_eval/machine.rs:120:5
    |
120 | /     fn contains_key<Q: ?Sized + Hash + Eq>(&mut self, k: &Q) -> bool
121 | |     where
122 | |         K: Borrow<Q>,
123 | |     {
124 | |         FxHashMap::contains_key(self, k)
    | |         -------------------------------- recursive call site
125 | |     }
    | |_____^ cannot return without recursing

What happened is this is in a trait implementation; previously this would call the inherent method, but now it calls the method from the trait. Fortunately rustc has good warnings and this is the fix:
diff --git a/compiler/rustc_mir/src/const_eval/machine.rs b/compiler/rustc_mir/src/const_eval/machine.rs
index 73ca7e0d471..b7a142ce330 100644
--- a/compiler/rustc_mir/src/const_eval/machine.rs
+++ b/compiler/rustc_mir/src/const_eval/machine.rs
@@ -121,7 +121,7 @@ impl<K: Hash + Eq, V> interpret::AllocMap<K, V> for FxHashMap<K, V> {
     where
         K: Borrow<Q>,
     {
-        FxHashMap::contains_key(self, k)
+        (**self).contains_key(k)
     }

     #[inline(always)]

This error looks a little odd, though:
error[E0308]: mismatched types
   --> src/tools/clippy/clippy_lints/src/copies.rs:393:58
    |
393 |         FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());
    |                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `ahash::random_state::RandomState`, found struct `BuildHasherDefault`
    |
    = note: expected struct `ahash::random_state::RandomState`
               found struct `BuildHasherDefault<_>`

I think you might have changed too many things to using RandomState? I wouldn't expect with_hasher to require a specific type.

@jyn514
Does https://github.com/tkaitchuck/aHash/pull/54 do everything you want now? Should I merge it?

@tkaitchuck it has the same error I mentioned above:
error[E0308]: mismatched types
   --> src/tools/clippy/clippy_lints/src/copies.rs:393:58
    |
393 |         FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());
    |                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `ahash::random_state::RandomState`, found struct `BuildHasherDefault`
    |
    = note: expected struct `ahash::random_state::RandomState`
               found struct `BuildHasherDefault<_>

Why is that happening? https://github.com/tkaitchuck/aHash/blob/master/src/hash_map.rs#L44
It is generic with respect to builder.

The full code was
    let mut map: FxHashMap<_, Vec<&_>> =
        FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());

The next thing I tried was to use let mut map: FxHashMap<_, Vec<&_>, _>:
error[E0282]: type annotations needed for `FxHashMap<u64, Vec<&T>, BuildHasherDefault<H>>`
   --> src/tools/clippy/clippy_lints/src/copies.rs:393:58
    |
392 |     let mut map: FxHashMap<_, Vec<&_>, _> =
    |         ------- consider giving `map` the explicit type `FxHashMap<u64, Vec<&T>, BuildHasherDefault<H>>`, where the type parameter `H` is specified
393 |         FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());
    |                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type for type parameter `H` declared on the struct `BuildHasherDefault`

The fix was to explicitly say I wanted an AHasher:
let mut map: FxHashMap<_, Vec<&_>, BuildHasherDefault<FxHasher>> =


Import caffe / torch model?

Is that a way to import caffe / torch pretrained model? Can you show me how to do that?

We have a caffe2neon utility (https://github.com/NervanaSystems/caffe2neon) for converting caffe models using the protobuf formatted Caffe model definition file and a binary protobuf model parameter file. For more information, see the instructions in that repository.

I'll see how it works. Thanks.


PS 6/7 are 4-5x slower than PS 5 due to not using web connection pool

I'm calling an API repeatedly (500,000 times) and I noticed it runs 4-5x faster on PowerShell 5. The code below is not the actual API, but a minimal example that demos the issue.
Steps to reproduce
I'm running this on 2 different "machines":

PowerShell 5 and 6 on Windows 10 (physical)
PowerShell 5 and 7 on Windows Server (VM)

The results are pretty consistent. I noticed PowerShell 6 or 7 for Mac was really slow -- that's why I started benchmarking.
You can adjust $max on your machine to get a more reasonable run duration for comparison.

PowerShell Version
Duration (sec.)

5
1

6
4

7
5

<#
Ver.   Sec
----   ---
   5     1
   6     4
   7     5
#>

$max = 10
$start = Get-Date -Format "HH:mm:ss"
foreach ($i in 1..$max) {
    Invoke-RestMethod "https://postman-echo.com/get?i=$i"
}
$end = Get-Date -Format "HH:mm:ss"
write-host "started at" $start
write-host "  ended at" $end

Unless somebody is aware of why this might be, I'm willing to take a look.

PowerShell 6/7 seems to use the HttpClient class which takes about 5-7 seconds of my system under PowerShell 7.0.1 or Windows PowerShell 5:
Measure-Command -Expression `
{
   Add-Type -Assembly System.Net.Http
   $Max = 100
   $Size = 0
   ForEach ($I in 1..$Max)
   {
      $Url = "http://postman-echo.com/get?i=$i"
      $Client = [System.Net.Http.Httpclient]::new()
      $Request = [System.Net.Http.HttpRequestMessage]::new('Get',$Url)
      $CancelToken = [System.Threading.CancellationToken]::new($false)
      $Option = [System.Net.Http.HttpCompletionOption]::ResponseContentRead
      $Response = $Client.SendAsync($Request, $Option, $CancelToken).GetAwaiter().GetResult();
      $Size += $Response.Content.ReadAsByteArrayAsync().Result.Length
   }
   Write-Host 'Total Size' $Size
}

Windows PowerShell uses System.Net.WebRequest; this request completes in about 2-3 seconds on my system under Windows PowerShell:
Measure-Command -Expression `
{
    $Max = 100
    $Size = 0
    ForEach ($I in 1..$Max) {
        $Url = "http://postman-echo.com/get?i=$i"
        $Request = [System.Net.WebRequest]::Create($Url)
        $Response = $request.BeginGetResponse($null, $null)
        $Response.AsyncWaitHandle.WaitOne()
        $Result= $Request.EndGetResponse($Response)
        $Reader = [System.IO.BinaryReader]::new($Result.GetResponseStream());
        $Size += $Reader.ReadBytes($Result.ContentLength).Length
    }
   Write-Host 'Total Size' $Size
}

Interestingly, this latter code also runs much 2x-3x slower under PowerShell 6/7 compared to Windows PowerShell.  This kind of surprised me.
So the question may actually be two-fold: 1) Why the switch to HttpClient? This is not an obvious problem at first glance... just a curiosity.  2) Why the slowdown in System.Net.WebRequest from Windows PowerShell to PowerShell Core (may be an underlying DotNet Core question).

WebClient was simply nonexistent in .NET Core during pretty much all of the development of 6.x, and we needed web cmdlets to work with. Even now, it's only really been brought back in for compatibility and isn't recommended for new code as I understand it.
Nor sure about much beyond that, though. 🙂

PowerShell 5 is using a single persistent TCP connection, PowerShell 7 is opening a new connection for each HTTP request.
If the -DisableKeepAlive parameter is used with Invoke-RestMethod there is no difference in performance.

@NoMoreFood If you set $Request.KeepAlive = $false in your example the performance is also the same.

PowerShell 7 actually sends the Connection: Keep-Alive header, but than opens a new connection for every request.

According to https://github.com/dotnet/runtime/issues/31267, .NET Core does not currently support TCP KeepAlive in System.Net.HttpClient.
There is an open issue to fix this: https://github.com/dotnet/runtime/issues/28721, which is tracking the 5.0 milestone. Since PowerShell 7.1 will be targeting .NET 5.0, I expect this issue will be be resolved by the end of 2020.

That's really good info! Thank you.
I did a little more digging and here's what I found. Node.js (using node-fetch) and Python (2/3, using requests) are also very slow, but JS in Chrome is as fast as PowerShell 5. I also used Wireshark, and noticed that the slow ones seem to negotiate HTTPS each time, but the fast ones do it only once. If you try it using HTTP instead of HTTPS, it's much faster. hth.
// JavaScript

// To run this in a browser, first go to https://postman-echo.com/get to avoid CORS issues.

const fetch = require("node-fetch"); // Only for node.js. Remove for Chrome.

(async function () {
    const MAX = 10;
    const start = new Date();
    for (var i = 0; i < MAX; i++) {
        const r = await fetch(`https://postman-echo.com/get?i=${i}`);
        const d = await r.json();
    }
    const end = new Date();
    console.log(start);
    console.log(end);
})();

# Python 3

import requests
from datetime import datetime

MAX = 10
start = datetime.now()
for i in range(MAX):
    requests.get(f'https://postman-echo.com/get?i={i}').json()
end = datetime.now()
print("started at", start)
print("  ended at", end)

It appears that TCP KeepAlive is supported in certain circumstances. If a singular HttpClient is declared and reused for each request, performance is much better, especially when using HTTPS:
$Client = [System.Net.Http.HttpClient]::new()
$Client.BaseAddress = "https://postman-echo.com"
(Measure-Command {
    (1..100).ForEach{
        $Request = [System.Net.Http.HttpRequestMessage]::new('Get', "/get?i=$_")
        $Response = $Client.SendAsync($Request).GetAwaiter().GetResult();
        Write-Output $Response.Content.ReadAsStringAsync().Result
    }
}).TotalSeconds

The problem here is that the HttpClient needs to be reused between calls of Invoke-RestMethod. This enables connection pooling.
Note that this is not TCP Keepalive (a technology to detect broken idle connections), and the two issues linked above will not help to resolve this.

So the the difference in perfomance is due the fact Windows PowerShell 5 pools connections between seperate instances of [System.Net.WebRequest], and PowerShell 7 does not.
As far as I can see, the only way to ensure use of a persistent connection (in PowerShell 7) is to use a raw HttpClient instead of the cmdlets Invoke-RestMethod/Invoke-WebRequest. Perhaps we could open a new issue to discuss whether to implement this functionality.

I guess I was only partially correct above. Python requests has a Session() object which allows for connection pooling. This only takes about 1 second as opposed to about 4 seconds for the example in my earlier comment (which doesn't use connection pooling).
import requests
from datetime import datetime

session = requests.Session()

MAX = 10
start = datetime.now()
for i in range(MAX):
    session.get(f'https://postman-echo.com/get?i={i}').json()
end = datetime.now()
print('started at', start)
print('  ended at', end)

HttpClient creates SocketsHttpHandler where connection pool is implemented.
If we will share the HttpClient or SocketsHttpHandler per process how do we resolve config conflicts between runspaces?
It seems we can not utilize a global pool.
Perhaps we could add new parameter in web cmdlets to get shared SocketsHttpHandler.

We could utilize a thread-static member to work with that. That'll resolve a majority of cases, and we can have backup code to generate a new handler/client when needed on a secondary thread.

I have a concern about any implicit cache because it can have side effects. We have pool for runspaces. Also we should take into account ForEach -Parallel.

for fun, i tried the -SessionVariable / -WebSession options to see if it would like (kinda like Python above). it didn't

for fun, i tried the -SessionVariable / -WebSession options to see if it would help (kinda like Python above). it didn't.

These parameters did not work for me either to enable connection pooling.

We could enhance WebRequestSession with SocketsHttpHandler.

We could utilize a thread-static member to work with that. That'll resolve a majority of cases, and we can have backup code to generate a new handler/client when needed on a secondary thread.

Sorry I am not super familiar with PowerShell, so I am probably misunderstanding. HttpClient is thread-safe and does not need a per-thread handler/client -- generally one instance per process works just fine. The only reason to have more than one is if you're setting options on the SocketsHttpHandler itself.

Hello,
any news about this issue ? I have same problem

Hello,
any news about this issue ? I have same problem

Waiting for someone to want to do this job :-)

There's a lot of caveats here, it looks like. If we could get an agreed on set of requirements for this improvement before work starts, that would be helpful to whoever wants to give it a stab.

I also ran into a problem due to the change in behavior. I have a script that talks to the web server on a Siemens LOGO!8 PLC, it has a very peculiar authentication protocol and I recently discovered the HTTP requests during the handshake need to be sent over the same TCP connection or it doesn't work (obviously this isn't PowerShell's problem but just adding for context).
Overall execution of the script in PS 7 was so much snappier than PS 5 that it didn't even occur to me there was a new TCP connection opened each time, although no SSL in my use case.
That said, there is at least a documentation issue here: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-restmethod?view=powershell-7.1

-DisableKeepAlive
Indicates that the cmdlet sets the KeepAlive value in the HTTP header to False. By default, KeepAlive is True. KeepAlive establishes a persistent connection to the server to facilitate subsequent requests.

Wireshark shows no Connection: Keep-Alive header being sent by default (whereas PowerShell 5.1 does). And Connection cannot be specified via the -Headers parameter as a workaround either.
As previously suggested by @iSazonov I think caching SocketsHttpHandler per PS session / runspace would make sense, probably need a separate instance for each set of redirect settings as those get baked as part of the internal message handler chain. However it looks like SocketsHttpHandler does the heavy lifting internally as far as adapting to different proxy and retry handling settings. I don't know whether PS presents any special considerations as far as parallelism but believe the best practice with respect to System.Net.Http.HttpClient has always been to reuse the instances, or rather reuse the handler, even in highly parallel applications like ASP.NET.
Although I'm not sure about requiring the use of WebRequestSession to take advantage of connection pooling is a good idea though, the two concerns seem orthogonal. For example you may want to take advantage of connection pooling without sharing cookies (or visa versa).

That said, there is at least a documentation issue here: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-restmethod?view=powershell-7.1

-DisableKeepAlive
Indicates that the cmdlet sets the KeepAlive value in the HTTP header to False. By default, KeepAlive is True. KeepAlive establishes a persistent connection to the server to facilitate subsequent requests.

Wireshark shows no Connection: Keep-Alive header being sent by default (whereas PowerShell 5.1 does). And Connection cannot be specified via the -Headers parameter as a workaround either.

https://en.wikipedia.org/wiki/HTTP_persistent_connection#HTTP_1.1
In HTTP 1.1, all connections are considered persistent unless declared otherwise
Нou can open a new issue if you do not agree.

I also ran into a problem due to the change in behavior.

Please use latest 7.2 Preview for testing and share results.

Reproduced in 7.2.0-preview.4. Can this be fixed?

Reproduced in 7.2.0-preview.4. Can this be fixed?

Same here with same version.
From what I've read above, it would appear that the KeepAlive feature is broken by default with P6/7's Invoke-RestMethod and similar cmdlets, where it's wasting time standing up new connections every request instead of reusing connections in a connection pool.
Interestingly though, it's slow even on the first request in a session, so there might also be other issues that are making it slow in general, but nothing definitive on what. I am wondering if it really is the different between using one class over the other :/
Hopefully, as some have said, the requirements can be reached soon so we can work towards submitting a PR

Even just a basic Measure-Command { Invoke-WebRequest "https://postman-echo.com/get?i=test" } is twice as slow as PowerShell 5, which has nothing to do with persistent connections (at least to my knowledge). Now I feel a bit dumb for replacing all the calls to curl.exe in my code with Invoke-WebRequest.

any progress here?

The HttpClient instantiated for every single  request in the processRecord method.
I have made Cmdlets that has a HttpClient as a field that is instantiated in the beginProcess method.  This would help if passing multiple body objects to a uniform url but not in a for-each loop.
I don't know how splats are handled. I imagine they are interpreted in a loop so caching HttpClient in class no help, but also not likely to insulate against Configuration pollution.

@CraigChamberlain No need to "cache" HttpClient. Dotnet implementation contains all "caching" logic in SocketsHttpHandler. We could use the same SocketsHttpHandler in (1) WebSession, (2) pipeline. First one is simple, universal and reliable. Second one is under question for me but we could investigate how this works in different scenarios and implement as an experimental feature.

@iSazonov Thanks for your consideration.  I agree electing to reuse a WebSession would likely be the ideal.  Most control to the end user wanting to optomise and covers for-each loops not just pipeline input.

Initially we could internally store SocketsHttpHandler in WebSession.
Any public API changes require more complex process discussion and approval.

So extend this class with an internal property?

Yes, I think it is good start point to investigate the issue. At least we could measure the issue scenario and see whether this resolve the issue.

Yes, I think it is good start point to investigate the issue. At least we could measure the issue scenario and see whether this resolve the issue.

Do you have some news ? (Make measure issue scenario ?)

Reproduced in 7.3.1 (no connection sharing by Invoke-WebRequest)

Meanwhile, I suggest to fix the misleading description of PowerShell:
https://github.com/PowerShell/PowerShell/blob/master/README.md
PowerShell is a cross-platform (Windows, Linux, and macOS) automation and configuration tool/framework that works well with your existing tools and is optimized for dealing with structured data (e.g. JSON, CSV, XML, etc.), REST APIs, and object models.

@iSazonov I'll try to investigate after I finish working on a couple other issues

Tested:
Measure-Command -Expression `
{
    $Max = 100
    $Size = 0
    ForEach ($I in 1..$Max) {
        $Url = "http://postman-echo.com/get?i=$i"
        $Request = [System.Net.WebRequest]::Create($Url)
        $Response = $request.BeginGetResponse($null, $null)
        $Response.AsyncWaitHandle.WaitOne()
        $Result= $Request.EndGetResponse($Response)
        $Reader = [System.IO.BinaryReader]::new($Result.GetResponseStream());
        $Size += $Reader.ReadBytes($Result.ContentLength).Length
    }
   Write-Host 'Total Size' $Size
}

System.Net.WebRequest: 11 seconds
Measure-Command -Expression `
{
   $Max = 100
   $Size = 0
   ForEach ($I in 1..$Max)
   {
      $Url = "http://postman-echo.com/get?i=$i"
      $Client = [System.Net.Http.Httpclient]::new()
      $Request = [System.Net.Http.HttpRequestMessage]::new('Get',$Url)
      $CancelToken = [System.Threading.CancellationToken]::new($false)
      $Option = [System.Net.Http.HttpCompletionOption]::ResponseContentRead
      $Response = $Client.SendAsync($Request, $Option, $CancelToken).GetAwaiter().GetResult();
      $Size += $Response.Content.ReadAsByteArrayAsync().Result.Length
   }
   Write-Host 'Total Size' $Size
}

System.Net.Http.HttpClient : 24 seconds
Measure-Command -Expression `
{
   $Max = 100
   $Size = 0
   $handler = [System.Net.Http.HttpClientHandler]::new()
   ForEach ($I in 1..$Max)
   {
      $Url = "http://postman-echo.com/get?i=$i"
      $Client = [System.Net.Http.Httpclient]::new($handler)
      $Request = [System.Net.Http.HttpRequestMessage]::new('Get',$Url)
      $CancelToken = [System.Threading.CancellationToken]::new($false)
      $Option = [System.Net.Http.HttpCompletionOption]::ResponseContentRead
      $Response = $Client.SendAsync($Request, $Option, $CancelToken).GetAwaiter().GetResult();
      $Size += $Response.Content.ReadAsByteArrayAsync().Result.Length
   }
   Write-Host 'Total Size' $Size
}

System.Net.Http.HttpClientHandler: 12 seconds
Measure-Command -Expression `
{
   $Max = 100
   $Size = 0
   $handler = [System.Net.Http.SocketsHttpHandler]::new()
   ForEach ($I in 1..$Max)
   {
      $Url = "http://postman-echo.com/get?i=$i"
      $Client = [System.Net.Http.Httpclient]::new($handler)
      $Request = [System.Net.Http.HttpRequestMessage]::new('Get',$Url)
      $CancelToken = [System.Threading.CancellationToken]::new($false)
      $Option = [System.Net.Http.HttpCompletionOption]::ResponseContentRead
      $Response = $Client.SendAsync($Request, $Option, $CancelToken).GetAwaiter().GetResult();
      $Size += $Response.Content.ReadAsByteArrayAsync().Result.Length
   }
   Write-Host 'Total Size' $Size
}

System.Net.Http.SocketsHttpHandler: 11 seconds
Using a handler should make System.Net.Http.HttpClient as fast as System.Net.WebRequest
From this test it doesn't seem like we would benefit from replacing System.Net.Http.HttpClientHandler with System.Net.Http.SocketsHttpHandler (but I can do it anyway). There might be a problem with the current implementation of the handler, I'll investigate more

@CarloToso HttpClientHandler is a wrapper for SocketsHttpHandler.

@CarloToso HttpClientHandler is a wrapper for SocketsHttpHandler.
Today we always create HttpClientHandler in GetHttpClient(). Optimization could be - we check WebSession for HttpClientHandler and use it if presents, otherwise create new one and save in WebSession.

Only in latest (.NET5+ I think) versions, and all the prescriptive guidance around "only use one httpclient per process" problems have mostly been solved by this change in architecture. However, it would make sense to reuse the httpclient from the websession if present. Even better, give me a parameter to supply my own httpclient so I can inject my own middlewares or default settings (like use HTTP/3 only), but that might be too advanced.

@CarloToso Have you interest to work on the issue? I think WebRequestSession is designed for this. So we could add new internal field to save SocketsHttpHandler.

I have made an attempt at implementing this; however it's not as straight-forward as just storing a SocketsHttpHandler in the WebRequestSession because

Current implementation uses HttpClientHandler, and the interfaces are different so a lot of parameters that directly set values on the HttpClientHandler would need to be updated to translate to similar behaviour on SocketsHttpHandler. I have chosen to keep the change minimal and do what you suggested except keeping HttpClientHandler instead of changing to SocketsHttpHandler.
The HttpClient is created in a number of places, and uses using to ensure it's disposed as soon as it's no longer required. This also causes the associated HttpClientHandler to be disposed, so subsequent requests using the same handler will fail because it is already disposed.
The HttpClientHandler properties are set from the command parameters. Once a handler has been used, these values cannot be changed because they will throw. The only way around this is to do one of the following:

compare the new parameters with the old ones and create a new handler if any have changed;
ignore any parameters related to setting options on the handler if a websession is passed to the cmdlet; or
throw an error if any such parameters are provided and the websession is provided.

For problem 2 I have checked for existence of WebSession or SessionVariable parameters and constructed the HttpClient with the option to not dispose the handler if either are present. This is not perfect because there is no way to Dispose the handler on sessions that are no longer required without adding additional APIs or switches (e.g. -DisposeSession)
For problem 3 my current implementation uses the second option, simply because it works and was relatively easy to implement.
I compared the performance with/without the change by running on 7.3.1 and the latest main with my changes, routed via Fiddler proxy to validate that no SSL tunnels were being created with the patch (and were without). Note that when using HTTP requests the difference with the patch was only minor.
Measure-Command {
    Write-Host (Invoke-WebRequest https://postman-echo.com/get?i=1 -SessionVariable Session).StatusCode
    1..10 | ForEach-Object {
        Invoke-WebRequest https://postman-echo.com/get?i=$_ -WebSession $Session
    }
}

7.3.1:
TotalMilliseconds : 12113.3189
Persistence patch:
TotalMilliseconds : 5279.7051
I have pushed my changes into a branch on my fork at the URL below. I'm happy to submit as a PR but I am expecting to need to make changes first.
https://github.com/stevenebutler/PowerShell/tree/12764-support-persistent-http-connections-in-cmdlets

2. The HttpClient is created in a number of places, and uses using to ensure it's disposed as soon as it's no longer required. This also causes the associated HttpClientHandler to be disposed, so subsequent requests using the same handler will fail because it is already disposed.

See https://source.dot.net/#System.Net.Http/System/Net/Http/HttpClient.cs,146

3. The HttpClientHandler properties are set from the command parameters. Once a handler has been used, these values cannot be changed because they will throw.

I would rather expect that this is the right behavior. Since we are essentially caching and reusing an existing connection, not all parameters can be changed.

I would rather expect that this is the right behavior. Since we are essentially caching and reusing an existing connection, not all parameters can be changed.

While that is true, it is still a potentially breaking change if there are scripts written that use -Session and expect the old behaviour. Because the handler isn't part of the session right now, it is possible to tweak settings between requests in a session.

See https://source.dot.net/#System.Net.Http/System/Net/Http/HttpClient.cs,146

Yes, that is the constructor I used in the patch. However there are some corner cases around using it that I outlined at the end of my post (repeated below).

For problem 2 I have checked for existence of WebSession or SessionVariable parameters and constructed the HttpClient with > the option to not dispose the handler if either are present. This is not perfect because there is no way to Dispose the handler > on sessions that are no longer required without adding additional APIs or switches (e.g. -DisposeSession)

That might be a way to support the feature without any potential breaking change. Before making such a change we'd have to decide if we want the new behaviour to be opt-in, or supported without effort for existing scripts, and if the small chance of scripts being broken is important in this case.
There are other options for handling the changes to my simplistic implementation. For example, the WebRequestSession could track the relevant parameters used to construct the HttpClientHandler and simply dispose the existing handler if the user passes in a different set of parameters on a subsequent run.
Most of the parameters are set from the websession anyway so aren't going to cause a compatibility issue, so it would only affect compatibility for the following parameters if I am reading the source correctly: -NoProxy -SslProtocols .
The interesting one is that GetHttpClient method is also called with a bool handleRedirect parameter which is controlled by these parameters:
               bool keepAuthorizationOnRedirect = PreserveAuthorizationOnRedirect.IsPresent
                                                   && WebSession.Headers.ContainsKey(HttpKnownHeaderNames.Authorization);

                bool handleRedirect = keepAuthorizationOnRedirect || AllowInsecureRedirect;

It is conceivable the same websession might be used with and without AllowInsecureRedirect or PreserveAuthorizationOnRedirect - I'd probably only add those switches to a single request where it was needed and leave them off elsewhere. When true, it looks like the cmdlet handles the redirection itself and redirection is disabled in HttpClientHandler.

I would rather expect that this is the right behavior. Since we are essentially caching and reusing an existing connection, not all parameters can be changed.

While that is true, it is still a potentially breaking change if there are scripts written that use -Session and expect the old behaviour. Because the handler isn't part of the session right now, it is possible to tweak settings between requests in a session.

Current WebSession design looks wrong. We should rather use splatting to store parameters, while a session is something else entirely. This is a question for the workgroup. If they don't want to restrict the WebSession by making it readonly and almost readonly, we'll have to make a new class to store the session. I would prefer the breaking change, as it's a better design and this cmdlet already has too many parameters.
But first it would be useful to analyze every parameter of this handler - are there really scenarios where some of the parameters need to be changed in the same session. I am sure the handler works correctly disabling changes after startup and we should follow this too.

I hadn't understood this well enough earlier. All the properties of the WebSession can get updated everytime PrepareSession() method is called so it's definitely possible to have different certificates, proxies, credentials etc on every request. Whether anyone would actually want to do this is anybody's guess, but my limited testing shows that it is possible to do so (e.g. when I supply Proxy to IWR with a session it tries to use the new proxy I supply on each line.
The headers are cumulatively added to the WebSession too, so each time you provide headers argument they override or add to the headers from the last request on the same session (and user agent can be changed just fine on the same session too).
If the workgroup decides that using sessions means you don't get to mess with all those settings between requests then would also need to decide if providing them is considered an error (like providing SessionVar and WebSession together is an error now) and should throw, or should just silently ignore them. At least if it breaks loudly, anyone doing this would know they need to update their scripts rather than it silently ignoring their parameters and not doing what they expected.
The parameters that impact on the use of a persistent handler are anything that gets set into the WebSession that is later set onto the handler in GetHttpClient:

Users could potentially change the CookieContainer in the WebRequestSession public API
UseDefaultCredentials or Credentials changes in the WebRequestSession public API, or by providing different parameters for the same items in the IWR command line
Proxy is public set in WebRequestSession and also via -Proxy parameter or -NoProxy switch
Certificates is public set in WebRequestSession but should be okay when set via IWR parameters since it just adds them to the existing container, but only if the container already existed on the first request that initiates the session since the container is created lazily.
-SkipCertificateCheck switch alters the handler's callback if provided on any request - if this were done on the first request it would therefore disable all certificate handling for the session and conversely you wouldn't be able to disable certificate checking for just one request in a sequence of requests in a session.
ContentHeaders and UserAgent in WebRequestSession should be fine since they are only set on individual requests and not on the handler itself.
-MaximumRedirection, PreserveAuthorizationOnRedirect and  -AllowInsecureRedirect all have an influence on handler settings for each request.

It looks like IWR uses redirection handling in the HttpClientHandler except in some specific circumstances where the user specifies authorization and allows insecure redirects because the .net handler explicitly won't do an insecure redirect while authenticating. You can't change this property on the handler after it is constructed so to keep existing behaviour IWR might need to always turn off  HttpClientHandler redirects and always handle the redirect logic itself.
An alternative might be to require users to opt-in to more strict session handling through a switch (not sure what would be a good name for it that makes it clear why it's there and what impact it has on the session e.g. -PersistentSessionVariable MySession.  The WebRequestSession could throw if you try to alter any of the public properties when created using this option.
One concern would be how to use these sessions and get the benefits of a persistent connection while still allowing the user to use a different session but share cookies between sessions in the case they need to alter handling for a single request. The most likely reason I'd want to do this is to disable certificate checks or use test certificates in a test environment for a specific site that may not be setup correctly but keep certificate handling normally for other requests. A workaround would be to allow a new -PersistentSessionVariable session to be initialized with an existing cookie container (e.g. from another session ) either using a -CookieContainer parameter or by copying from an existing session if both are provided -PersistentSessionVariable NewSession -WebSession $ExistingSession. The problem with the latter option is it currently throws when -SessionVariable NewSession -WebSession $ExistingSession is passed.

@stevenebutler I don't think we need additional parameters. You could avoid them and break changes with a simple trick - recreate the handler if it already exists and the parameters have changed. This doesn't even need to be implemented as an experimental feature. If you output a warning when this happens, users will start using this cmdlet correctly.

Yes, I was forming the same opinion, but it would need to also want to keep
the handler in the absence of those parameters, meaning keep the previous
values.
The only disadvantage is the complexity it adds to an already quite complex
cmdlet.
On Sun, 5 Feb 2023 at 6:08 pm, Ilya @.***> wrote:

@stevenebutler https://github.com/stevenebutler I don't think we need
additional parameters. You could avoid them and break changes with a simple
trick - recreate the handler if it already exists and the parameters have
changed. This doesn't even need to be implemented as an experimental
feature. If you output a warning when this happens, users will start using
this cmdlet correctly.
—
Reply to this email directly, view it on GitHub
https://github.com/PowerShell/PowerShell/issues/12764#issuecomment-1417068208,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AEADFJNUONKHRNTS4XL34HDWV5NWDANCNFSM4NIB3TCA
.
You are receiving this because you were mentioned.Message ID:
@.***>

--
Regards,
Steven Butler

The only disadvantage is the complexity it adds to an already quite complex cmdlet.

Perhaps there are unexpected obstacles there, but I don't see them now. Most likely the changes will be minimal.

The only one I see is that users could still change items on the web
session through direct assignment in their scripts. However this would be a
fairly strange thing to do I would hope. The implementation might need to
be pushed into the web session class to detect when this happens.
On Sun, 5 Feb 2023 at 6:30 pm, Ilya @.***> wrote:

The only disadvantage is the complexity it adds to an already quite
complex cmdlet.
Perhaps there are unexpected obstacles there, but I don't see them now.
Most likely the changes will be minimal.
—
Reply to this email directly, view it on GitHub
https://github.com/PowerShell/PowerShell/issues/12764#issuecomment-1417099383,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AEADFJO5EERQMOHDE3XVADDWV5QJTANCNFSM4NIB3TCA
.
You are receiving this because you were mentioned.Message ID:
@.***>

--
Regards,
Steven Butler

I have made changes to detect when the WebSession has changed between invocations on my branch here. Please have a look and let me know if this approach is likely to work.
https://github.com/PowerShell/PowerShell/compare/master...stevenebutler:PowerShell:12764-support-persistent-http-connections-in-cmdlets

Looking at this code I think it is too confusing. We need to do a refactor first.  My main idea is based on the fact that both handler and HttpClient can't be changed after the start. That means there is no point in calling the GetHttpClient() method on the fly. In other words, all initializations should be in BeginProcessing() and the cmdlets could be IDisposable. I hope it possible.
Perhaps @CarloToso knows the code better and could refactor it.

You can still call multiple invocations of iwr in sequence using the web
session parameter to tie them together and most of the time I would expect
gethttpclient to be called only once per invocation, so I’m not sure moving
the call to beginprocessing will make much difference for most use cases.
A refactoring is definitely not something I’d be willing to take on so I
guess I’ll leave this issue the way it is for now.
On Mon, 6 Feb 2023 at 3:59 pm, Ilya @.***> wrote:

Looking at this code I think it is too confusing. We need to do a refactor
first. My main idea is based on the fact that both handler and HttpClient
can't be changed after the start. That means there is no point in calling
the GetHttpClient() method on the fly. In other words, all initializations
should be in BeginProcessing() and the cmdlets could be IDisposable. I hope
it possible.
Perhaps @CarloToso https://github.com/CarloToso knows the code better
and could refactor it.
—
Reply to this email directly, view it on GitHub
https://github.com/PowerShell/PowerShell/issues/12764#issuecomment-1418557750,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AEADFJO4FWXUTP5I3RI5H6DWWCHLHANCNFSM4NIB3TCA
.
You are receiving this because you were mentioned.Message ID:
@.***>

--
Regards,
Steven Butler

@stevenebutler I tested your code and I the speedup was impressive, please keep working on it and open a PR.
Test:
Measure-Command {
    Write-Host (Invoke-WebRequest https://postman-echo.com/get?i=1 -SessionVariable Session).StatusCode
    1..10 | ForEach-Object {
        Invoke-WebRequest https://postman-echo.com/get?i=$_ -WebSession $Session
    }
}

Ps 7.3.2 --> 5538 milliseconds
your code --> 468 milliseconds

I tested your code and the speedup was impressive, please keep working on it and open a PR.

To keep working on it I'd need to have some idea of what needs to be done to make it acceptable to merge.
I can start that process through a PR if that is the recommended course of action or if there's some improvements that should be made first, then I can try that first. I would assume at minimum it would need to merge with changes in #19109

@stevenebutler You should merge with changes in #19109 and open a [W.I.P] PR, then continue working on it there.

@ImportTaste responding to your query on my repo that got lost when I force pushed some changes...
I wouldn't be able to make this an exteranl cmdlet without copying most of the existing code base into the third party module and I'm not really willing to do that.
The reason you'd have to do that is it depends on a lot of internal interfaces that an external module can't access. It also adds internal properties to the WebRequestSession you'd have to do externally as well.
If you're desperate for an interim solution you can write your own simplified PowerShell script that uses httpclienthandler/httpclient directly that supports only what you need. That is what I have been doing but it was relatively simple since I don't need 95% of the features on IWR.


Flaky unit test (drcluster_controller_test.go:485)

I see many random failure in the unit tests. I this is is the test failing most of the time
but I'm not sure. Adding an issue for every random failure will help to find the flaky test.
------------------------------
• [FAILED] [0.007 seconds]
DRClusterController DRCluster resource CIDR validation when provided CIDR value is changed to be correct [It] reports validated
/home/runner/work/ramen/ramen/controllers/drcluster_controller_test.go:485

  Timeline >>
  2023-05-29T11:53:54.387Z	INFO	DRPCPredicate.DRCluster	Update event
  2023-05-29T11:53:54.387Z	INFO	controllers.DRPolicy	reconcile enter	{"DRPolicy": "sync-drpolicy-drcluster-tests", "rid": "8ecb717f-b7ca-4aeb-85fb-da4cdd351dab"}
  2023-05-29T11:53:54.387Z	INFO	controllers.DRCluster	Updated drCluster Status {Phase: Conditions:[{Type:Validated Status:False ObservedGeneration:1 LastTransitionTime:2023-05-29 11:53:54 +0000 UTC Reason:DrClustersDeployFailed Message:Operation cannot be fulfilled on manifestworks.work.open-cluster-management.io "ramen-dr-cluster": the object has been modified; please apply your changes to the latest version and try again}] MaintenanceModes:[]}	{"name": "drc-cluster0", "rid": "8de9185a-033e-4783-bed7-ec209df13467"}
  2023-05-29T11:53:54.387Z	INFO	controllers.DRCluster	reconcile exit	{"name": "drc-cluster0", "rid": "8de9185a-033e-4783-bed7-ec209df13467"}
  2023-05-29T11:53:54.387Z	ERROR	Reconciler error	{"controller": "drcluster", "controllerGroup": "ramendr.openshift.io", "controllerKind": "DRCluster", "DRCluster": {"name":"drc-cluster0"}, "namespace": "", "name": "drc-cluster0", "reconcileID": "ffaecf0b-c8f2-4b4c-8af5-bde00d45f1e6", "error": "drclusters deploy: Operation cannot be fulfilled on manifestworks.work.open-cluster-management.io \"ramen-dr-cluster\": the object has been modified; please apply your changes to the latest version and try again"}
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
  	/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:329
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
  	/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:274
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
  	/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.5/pkg/internal/controller/controller.go:235
  2023-05-29T11:53:54.387Z	INFO	controllers.DRCluster	reconcile enter	{"name": "drc-cluster0", "rid": "b73cadc2-3900-4f1c-a1c3-4[88](https://github.com/RamenDR/ramen/actions/runs/5111545867/jobs/9188622618#step:4:89)2dc8bee39"}
  2023-05-29T11:53:54.387Z	INFO	controllers.DRCluster	create/update	{"name": "drc-cluster0", "rid": "b73cadc2-3900-4f1c-a1c3-4882dc8bee39"}
  2023-05-29T11:53:54.3[89](https://github.com/RamenDR/ramen/actions/runs/5111545867/jobs/9188622618#step:4:90)Z	INFO	controllers.DRPolicy	create/update	{"DRPolicy": "sync-drpolicy-drcluster-tests", "rid": "8ecb717f-b7ca-4aeb-85fb-da4cdd351dab"}
  [FAILED] in [It] - /home/runner/work/ramen/ramen/controllers/drcluster_controller_test.go:487 @ 05/29/23 11:53:54.39
  << Timeline

  [FAILED] Expected success, but got an error:
      <*errors.StatusError | 0xc001648320>:
      Operation cannot be fulfilled on drclusters.ramendr.openshift.io "drc-cluster0": the object has been modified; please apply your changes to the latest version and try again
      {
          ErrStatus: {
              TypeMeta: {Kind: "", APIVersion: ""},
              ListMeta: {
                  SelfLink: "",
                  ResourceVersion: "",
                  Continue: "",
                  RemainingItemCount: nil,
              },
              Status: "Failure",
              Message: "Operation cannot be fulfilled on drclusters.ramendr.openshift.io \"drc-cluster0\": the object has been modified; please apply your changes to the latest version and try again",
              Reason: "Conflict",
              Details: {
                  Name: "drc-cluster0",
                  Group: "ramendr.openshift.io",
                  Kind: "drclusters",
                  UID: "",
                  Causes: nil,
                  RetryAfterSeconds: 0,
              },
              Code: 409,
          },
      }
  In [It] at: /home/runner/work/ramen/ramen/controllers/drcluster_controller_test.go:487 @ 05/29/23 11:53:54.39
------------------------------

The failing run:
https://github.com/RamenDR/ramen/actions/runs/5111545867/attempts/1
Next attempt was successful:
https://github.com/RamenDR/ramen/actions/runs/5111545867

these types of issues are mostly seen when there is more than one request to update the resource while the previous request is still updating. this error occurs intermittently. One way to solve this is to get latest-drcluster from API server before updating the drcluster.

@rakeshgm Why does it happen in the unit tests? how can we avoid it?


Get UX an explanation for the Pipeline/Workspace column in artifacts

What is a workspace here? How are pipelines being grouped, what is the difference between default/grouped column?

Looks like workspaces are a multi-tenanct concept: https://www.kubeflow.org/docs/components/multi-tenancy/design/
I don't think it should be applicable to our solution as we rely on Data Science Projects.

Migrated to Jira: https://issues.redhat.com/browse/RHOAIENG-1702


Cannot run/compile

I followed all the instructions you gave to install, and after doing this, it throws error no cannot import flax error. When I install flax, it throws errors like mismatch of the settings. when I finally figure them out. It throws this error:
from jax.nn.initializers import constant
ImportError: cannot import name 'constant'
Can you please re-create the environment and check if it is working flawlessly on your side? Because I think you created an environment long ago and now when we try to recreate it, it throws tons of errors.

I appreciate your interest in our paper and am sorry for the late reply! :(
On my side, I can run the code without the error that you mentioned.
I checked the installed library versions, and it's all the same as those in the requirements.txt.
I've also seen errors like the one you bumped into, and it was usually the mismatch of the versions between flax, jax, or jaxlib.
I suggest you check and adjust the related libraries' versions; please let me know if it still does not work. :)


Debugging in VSCode

Hi,
I'm trying to figure out how to actually debug the fib-debug example in VSCode using code-lldb. I tried creating a .vscode/launch.json which looks like this:
{
    "version": "0.2.0",
    "configurations": [
        {
            "name": "Launch",
            "type": "lldb",
            "request": "launch",
            "program": "target/debug/fib-debug",
            "args": [],
        },
    ]
}

But it doesn't seem to work; breakpoints are skipped, and when I add a panic to the wasm code it's just logged but it doesn't stop the execution on it.
What else needs to happen for me to be able to debug the wasm module in VSCode?

Lol wrong repo..


Fix test names in skipped tests list

Changing these to reflect their new names.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

/approve


Iterate on test case editor

Yet another draft PR for the editor (not ready for review yet) ; this aims at providing editors for supported data types (most are not yet implemented)

I think we could merge this iteration rather than drag it for a while, @vincent-botbol would you agree? (we could just squash commits)


Support AnyOf for required keys instead of flattening

While this is valid for JSON schema, it is not valid for Open API


DO NOT MERGE - Placeholder to remember past idea

Things needed to test / need to check

test nginx conf on a development build
Actually test app on port 443 (I only got to opening the site, I haven't played with it yet)
-  Do this on ALS-test directory of the instance
remove debug messages from /nginx/Dockerfile

Update:

dev builds do not use nginx, just node to run commands. So nginx changes to prod builds do not affect dev builds.
App seems to be working on port 443. No issues with api calls.
debug messages have been removed.

I will push the bug fixes I did do and then merge into docker-compose-fixes to merge to staging


custom throw in model constructor

I am not able to see my error from constructor of the model.
I see just your error:
Model.relationMappings.role: join.from must have format TableName.columnName. For example "SomeTable.id" or in case of composite key ["SomeTable.a", "SomeTable.b"].
It is because you are catching everything in the objection/lib/relations/Relation.js  function createRelationProperty

How could that be improved while keeping the current functionality? Why are you throwing from a constructor?

The code path that executes createRelationProperty is only ran once per relation the first time you use the model. The problem is that objection cannot initialise the relation because your invalid constructor throws every time when used inside objection.
The error message you see is correct for the errors thrown from createRelationProperty 99% of the time. It's not more specific because I had a lot of people filing bug reports caused their typos in relation mappings before I wrote that error message there.
We could add the actual error as a cause for the error you are seeing, but ecmascript and node.js have a support for that (that I know of.. Some libraries support a cause property on error, but I'm pretty sure node itself doesn't).
Your constructor is invoked in createRelationProperty because this function is used:
function columnNameToPropertyName(ModelClass, columnName) {
  // This is your model class's constructor.
  const model = new ModelClass();
  const addedProps = Object.keys(model.$parseDatabaseJson({}));

  const row = {};
  row[columnName] = null;

  const props = Object.keys(model.$parseDatabaseJson(row));
  const propertyName = difference(props, addedProps)[0];

  return propertyName || null;
}

Whatever you do in your constructor, you need to prepare for no input arguments, because that's how it is sometimes used internally. You probably shouldn't do anything in your Model's constructor though.
What are you trying to do in the constructor? There probably is a better way to achieve that in objection.

My models need to have static property modelName and jsonSchema as well.
I added this check in the constructor of the "Base" class because I am doing a big refactor from bookshelf to objection. (You are doing same check for relations there as well) I was confused because I saw bad relation but it was caused by my throw.
  constructor(...args) {
    super(...args);

    if (!this.constructor.modelName) {
      throw new Error('Model must have a static property modelName');
    } else if (!this.constructor.jsonSchema) {
      throw new Error('Model must have a static property jsonSchema');
    } else if (!this.constructor.relationMappings) {
      throw new Error('Model must have a static property relationMappings');
    }
  }

Maybe you override static fromJson and move that check there?


ARTEMIS-2258 The FileLockNodeManager directory should be configurable

I'm doing some local tests to see if it works as expected, but the configuration bits are already covered by tests.

Whislt i see tests for the config, is there any way to make an automated test to check what this is trying to fix?

Nice one, I can modify the test used for shared store HA.

@michaelandrepearce I've added support for the NettyFailoverTest that's using the node managers for every mechanics with the added benefit that FileLockNodeManager has a much improved coverage now, given that the test wasn't originally used to test it.


python312Packages.zha-quirks: 0.0.125 -> 0.0.128

Automatic update generated by nixpkgs-update tools. This update was made based on information from passthru.updateScript.
meta.description for python312Packages.zha-quirks is: ZHA Device Handlers are custom quirks implementations for Zigpy
meta.homepage for python312Packages.zha-quirks is: https://github.com/dmulcahey/zha-device-handlers
meta.changelog for python312Packages.zha-quirks is: https://github.com/zigpy/zha-device-handlers/releases/tag/0.0.125
Updates performed

Ran passthru.UpdateScript

To inspect upstream changes
Impact
Checks done

built on NixOS
The tests defined in passthru.tests, if any, passed
found 0.0.125 with grep in /nix/store/w74hlzbnrlpln9vx4iqhqffk88s9wdxs-python3.12-zha-quirks-0.0.125
found 0.0.125 in filename of file in /nix/store/w74hlzbnrlpln9vx4iqhqffk88s9wdxs-python3.12-zha-quirks-0.0.125

Rebuild report (if merged into master) (click to expand)

10 total rebuild path(s)

10 package rebuild(s)

First fifty rebuilds by attrpath

home-assistant-component-tests.hassio
home-assistant-component-tests.homeassistant_green
home-assistant-component-tests.homeassistant_hardware
home-assistant-component-tests.homeassistant_sky_connect
home-assistant-component-tests.homeassistant_yellow
home-assistant-component-tests.otbr
home-assistant-component-tests.zha
python312Packages.zha
python312Packages.zha-quirks

Instructions to test this update (click to expand)

Either download from Cachix:
nix-store -r /nix/store/w74hlzbnrlpln9vx4iqhqffk88s9wdxs-python3.12-zha-quirks-0.0.125 \
  --option binary-caches 'https://cache.nixos.org/ https://nix-community.cachix.org/' \
  --option trusted-public-keys '
  nix-community.cachix.org-1:mB9FSh9qf2dCimDSUo8Zy7bkq5CX+/rkCWyvRCYg3Fs=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(The Cachix cache is only trusted for this store-path realization.)
For the Cachix download to work, your user must be in the trusted-users list or you can use sudo since root is effectively trusted.
Or, build yourself:
nix-build -A python312Packages.zha-quirks https://github.com/r-ryantm/nixpkgs/archive/802572c7eca663778d3255bbfff60389a009754d.tar.gz

Or:
nix build github:r-ryantm/nixpkgs/802572c7eca663778d3255bbfff60389a009754d#python312Packages.zha-quirks

After you've downloaded or built it, look at the files and if there are any, run the binaries:
ls -la /nix/store/w74hlzbnrlpln9vx4iqhqffk88s9wdxs-python3.12-zha-quirks-0.0.125
ls -la /nix/store/w74hlzbnrlpln9vx4iqhqffk88s9wdxs-python3.12-zha-quirks-0.0.125/bin

Pre-merge build results
We have automatically built all packages that will get rebuilt due to
this change.
This gives evidence on whether the upgrade will break dependent packages.
Note sometimes packages show up as failed to build independent of the
change, simply because they are already broken on the target branch.
nixpkgs-review result
Generated using nixpkgs-review.
Command: nixpkgs-review

x86_64-linux

:x: 9 packages failed to build:

home-assistant-component-tests.hassio
home-assistant-component-tests.homeassistant_green
home-assistant-component-tests.homeassistant_hardware
home-assistant-component-tests.homeassistant_sky_connect
home-assistant-component-tests.homeassistant_yellow
home-assistant-component-tests.otbr
home-assistant-component-tests.zha
python312Packages.zha
python312Packages.zha.dist

:white_check_mark: 2 packages built:

python312Packages.zha-quirks
python312Packages.zha-quirks.dist

Maintainer pings
cc @fabaff for testing.

[!TIP]
As a maintainer, if your package is located under pkgs/by-name/*, you can comment @NixOS/nixpkgs-merge-bot merge to automatically merge this update using the nixpkgs-merge-bot.

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

Automatic update generated by nixpkgs-update tools. This update was made based on information from passthru.updateScript.
meta.description for python312Packages.zha-quirks is: ZHA Device Handlers are custom quirks implementations for Zigpy
meta.homepage for python312Packages.zha-quirks is: https://github.com/dmulcahey/zha-device-handlers
meta.changelog for python312Packages.zha-quirks is: https://github.com/zigpy/zha-device-handlers/releases/tag/0.0.128
Updates performed

Ran passthru.UpdateScript

To inspect upstream changes
Impact
Checks done

built on NixOS
The tests defined in passthru.tests, if any, passed
found 0.0.128 with grep in /nix/store/zf5w70k814cxwljk85d6prfgl459mzlz-python3.12-zha-quirks-0.0.128
found 0.0.128 in filename of file in /nix/store/zf5w70k814cxwljk85d6prfgl459mzlz-python3.12-zha-quirks-0.0.128

Rebuild report (if merged into master) (click to expand)

12 total rebuild path(s)

12 package rebuild(s)

First fifty rebuilds by attrpath

home-assistant-component-tests.hassio
home-assistant-component-tests.homeassistant_green
home-assistant-component-tests.homeassistant_hardware
home-assistant-component-tests.homeassistant_sky_connect
home-assistant-component-tests.homeassistant_yellow
home-assistant-component-tests.otbr
home-assistant-component-tests.zha
python312Packages.zha
python312Packages.zha-quirks
python313Packages.zha
python313Packages.zha-quirks

Instructions to test this update (click to expand)

Either download from the cache:
nix-store -r /nix/store/zf5w70k814cxwljk85d6prfgl459mzlz-python3.12-zha-quirks-0.0.128 \
  --option binary-caches 'https://cache.nixos.org/ https://nixpkgs-update-cache.nix-community.org/' \
  --option trusted-public-keys '
  nixpkgs-update-cache.nix-community.org-1:U8d6wiQecHUPJFSqHN9GSSmNkmdiFW7GW7WNAnHW0SM=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(The nixpkgs-update cache is only trusted for this store-path realization.)
For the cached download to work, your user must be in the trusted-users list or you can use sudo since root is effectively trusted.
Or, build yourself:
nix-build -A python312Packages.zha-quirks https://github.com/r-ryantm/nixpkgs/archive/8b6f41682aceb66117c6f06bb8fbcdab39fc6044.tar.gz

Or:
nix build github:r-ryantm/nixpkgs/8b6f41682aceb66117c6f06bb8fbcdab39fc6044#python312Packages.zha-quirks

After you've downloaded or built it, look at the files and if there are any, run the binaries:
ls -la /nix/store/zf5w70k814cxwljk85d6prfgl459mzlz-python3.12-zha-quirks-0.0.128
ls -la /nix/store/zf5w70k814cxwljk85d6prfgl459mzlz-python3.12-zha-quirks-0.0.128/bin

Pre-merge build results
We have automatically built all packages that will get rebuilt due to
this change.
This gives evidence on whether the upgrade will break dependent packages.
Note sometimes packages show up as failed to build independent of the
change, simply because they are already broken on the target branch.
nixpkgs-review result
Generated using nixpkgs-review.
Command: nixpkgs-review

x86_64-linux

:x: 11 packages failed to build:

home-assistant-component-tests.hassio
home-assistant-component-tests.homeassistant_green
home-assistant-component-tests.homeassistant_hardware
home-assistant-component-tests.homeassistant_sky_connect
home-assistant-component-tests.homeassistant_yellow
home-assistant-component-tests.otbr
home-assistant-component-tests.zha
python312Packages.zha
python312Packages.zha.dist
python313Packages.zha
python313Packages.zha.dist

:white_check_mark: 4 packages built:

python312Packages.zha-quirks
python312Packages.zha-quirks.dist
python313Packages.zha-quirks
python313Packages.zha-quirks.dist

Maintainer pings
cc @fabaff for testing.

[!TIP]
As a maintainer, if your package is located under pkgs/by-name/*, you can comment @NixOS/nixpkgs-merge-bot merge to automatically merge this update using the nixpkgs-merge-bot.

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


unable to get nodeid

I am unable to get nodeID from atlas node explorer. We should be able to click on the nodeID section to copy it to the clipboard. This will make the values usable. Right now, we are unable to use nodes on the node explorer to get the data.

Great idea!
Here is what we can do. Add a v-on:dblclick to that column, where the event handler simply just copies it to the user's clipboard. Maybe we also display a modal for a few seconds saying NodeID Copied!.


Need to edit code, scipt makefiles and readmes

Waiting for all the scripts to come in and we will need to update the makefiles and readmes accordingly
@pigriver123 please comment when all sciprts are done

Check #144

@BenjaminHsieh I think I have added all the scripts

For the readmes whats the threshold for warning the user about run time. some of the scripts take ~15 minutes

Not sure but you can just add the estimated run times in the readme, I just labeld the things that take more than 45 mins

closing


code cleaning

The CI tests are very long these days, it seems they never finish.
I will nonetheless accept this PR, it looks good on review.

Today Travis has problems with MacOS VM. It doesn't start or our tests stop in the middle.