WIP: File/Folder Reorg Changes

Updated HIP Path additions
Package file/folder reorg applied for lib (ROCMPATH/lib) (removed component name prefix)
Package file/folder reorg applied for include (ROCMPATH/include/)
Package file/folder reorg applied for docs, samples, test binaries.

@asalmanp, @kiritigowda  Kindly share your input/comments if there are any

@arvindcheru this PR introduces lots of cmake warnings (i.e., ROCMChecks WARNING see this link for the log file from the CI). please take a look and fix these warnings (i.e., just run cmake ..)

At first look, This is warning generated by latest rocm-cmake on existing code since CMAKE_CXX_COMPILER is set in the code. Like set(CMAKE_CXX_COMPILER ${COMPILER_FOR_HIP}) in amd_openvx/openvx/hipvx/CMakeLists.txt line 36 for example.

@asalmanp I have built both the HIP and OCL backends with this PR and run some of the sanity tests. It built and ran the tests fine.

add_definitions(-w) — to suppress warnings @arvindcheru?

@asalmanp I tried to build the bubble pop app and got the following error: 14 | #warning "This file is deprecated. Use the header file from /opt/rocm/include/mivisionx/vx.h by using #include <mivisionx/vx.h>" | ^~~~~~~ /opt/rocm/mivisionx/include/VX/vx.h:16:10: fatal error: ../../../include/mivisionx/vx.h: No such file or directory 16 | #include "../../../include/mivisionx/vx.h" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Thanks for comments, at first looks the relative path generated in wrapper need to be analyzed. Will check this and update shortly

@arvindcheru update on warnings. The current TOT with rocm-cmake master does not throw warnings
cmake ../MIVisionX/
-- The C compiler identification is GNU 9.4.0
-- The CXX compiler identification is GNU 9.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
-- MIVisionX Version -- 2.1.0
-- MIVisionX Install Path -- /opt/rocm/mivisionx
-- MIVisionX Backend set to -- HIP
-- MIVisionX Build Type -- Release
-- MIVisionX Developer Options
--      -D NEURAL_NET=OFF [Turn OFF Neural Net Modules (default:ON)]
--      -D ROCAL=OFF [Turn OFF ROCAL Modules (default:ON)]
--      -D LOOM=OFF [Turn OFF LOOM Modules (default:ON)]
--      -D GPU_SUPPORT=OFF [Turn OFF GPU support (default:ON)]
--      -D BACKEND=OPENCL [select OPENCL for GPU backend [options:CPU/OPENCL/HIP](default:HIP)]
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- AMD OpenVX -- OpenVX built with HIP Support
-- AMD OpenVX -- OpenVX module added
-- OpenCL Found - /opt/rocm/lib/libOpenCL.so
-- ROCm OpenCL Found - Force OpenCL_LIBRARIES & OpenCL_INCLUDE_DIRS to use ROCm OpenCL
-- OpenCL - Setting CL_TARGET_OPENCL_VERSION=220
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.1")
-- Checking for module 'libavcodec'
--   Found libavcodec, version 58.18.100
-- Checking for module 'libavformat'
--   Found libavformat, version 58.12.100
-- Checking for module 'libavutil'
--   Found libavutil, version 56.14.100
-- Using FFMPEG -- Libraries:/usr/local/lib/libavcodec.so;/usr/local/lib/libavformat.so;/usr/local/lib/libavutil.so;/usr/local/lib/libswscale.so Includes:/usr/local/include
-- amd_media -- Building with HIP
-- AMD OpenVX Media Extension -- amd_media module added
-- WARNING: MIOPEN with HIP backend Not Found -- amd_nn module excluded
-- WARNING: RPP with the HIP backend Not Found (HINT: run setup script with the HIP backend first!) -- amd_rpp module excluded.
-- OpenCV Found -- Version-4.5.X Supported
-- AMD OpenVX OpenCV Extension -- amd_opencv module added
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- runvx built with HIP Support
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- Utilities -- runvx and mv_deploy modules added
-- Found HIP for rocAL: 5.0.13601-6b731c37/opt/rocm/hip
-- WARNING: RPP with the HIP backend Not Found (HINT: run setup script with the HIP backend first!) -- rocAL_rpp module excluded.
-- WARNING: rocAL module excluded
-- Configuring done
-- Generating done
-- Build files have been written to: /workspace/build

@asalmanp -- the warnings only come up with this PR, see comment above.
cmake ../MIVisionX/
-- The C compiler identification is GNU 9.4.0
-- The CXX compiler identification is GNU 9.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
-- MIVisionX Version -- 2.1.0
-- MIVisionX Install Path -- /opt/rocm
-- MIVisionX Backend set to -- HIP
-- MIVisionX Build Type -- Release
-- MIVisionX Developer Options
--      -D NEURAL_NET=OFF [Turn OFF Neural Net Modules (default:ON)]
--      -D ROCAL=OFF [Turn OFF ROCAL Modules (default:ON)]
--      -D LOOM=OFF [Turn OFF LOOM Modules (default:ON)]
--      -D GPU_SUPPORT=OFF [Turn OFF GPU support (default:ON)]
--      -D BACKEND=OPENCL [select OPENCL for GPU backend [options:CPU/OPENCL/HIP](default:HIP)]
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_COMPILER' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_COMPILER="/opt/rocm/bin/hipcc"'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_COMPILER' is set at
  /workspace/MIVisionX/amd_openvx/openvx/hipvx/CMakeLists.txt:<line#> shown
  below:
Call Stack (most recent call first):
  amd_openvx/openvx/hipvx/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  amd_openvx/openvx/hipvx/CMakeLists.txt:36 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=" -O3 "'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /workspace/MIVisionX/amd_openvx/openvx/hipvx/CMakeLists.txt:<line#> shown
  below:
Call Stack (most recent call first):
  amd_openvx/openvx/hipvx/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  amd_openvx/openvx/hipvx/CMakeLists.txt:37 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

-- AMD OpenVX -- OpenVX built with HIP Support

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=" -O3 -msse4.2 -std=gnu++14"'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /workspace/MIVisionX/amd_openvx/openvx/CMakeLists.txt:<line#> shown below:
Call Stack (most recent call first):
  amd_openvx/openvx/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  amd_openvx/openvx/CMakeLists.txt:182 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

-- AMD OpenVX -- OpenVX module added
-- OpenCL Found - /opt/rocm/lib/libOpenCL.so
-- ROCm OpenCL Found - Force OpenCL_LIBRARIES & OpenCL_INCLUDE_DIRS to use ROCm OpenCL
-- OpenCL - Setting CL_TARGET_OPENCL_VERSION=220
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.1")
-- Checking for module 'libavcodec'
--   Found libavcodec, version 58.18.100
-- Checking for module 'libavformat'
--   Found libavformat, version 58.12.100
-- Checking for module 'libavutil'
--   Found libavutil, version 56.14.100
-- Using FFMPEG -- Libraries:/usr/local/lib/libavcodec.so;/usr/local/lib/libavformat.so;/usr/local/lib/libavutil.so;/usr/local/lib/libswscale.so Includes:/usr/local/include
-- amd_media -- Building with HIP

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=" -O3 -Wno-deprecated-declarations -msse4.2 -std=c++11"'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /workspace/MIVisionX/amd_openvx_extensions/amd_media/CMakeLists.txt:<line#>
  shown below:
Call Stack (most recent call first):
  amd_openvx_extensions/amd_media/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  amd_openvx_extensions/amd_media/CMakeLists.txt:97 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

-- AMD OpenVX Media Extension -- amd_media module added
-- WARNING: MIOPEN with HIP backend Not Found -- amd_nn module excluded
-- WARNING: RPP with the HIP backend Not Found (HINT: run setup script with the HIP backend first!) -- amd_rpp module excluded.
-- OpenCV Found -- Version-4.5.X Supported

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=" -O3 -fdata-sections"'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /workspace/MIVisionX/amd_openvx_extensions/amd_opencv/CMakeLists.txt:<line#>
  shown below:
Call Stack (most recent call first):
  amd_openvx_extensions/amd_opencv/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  amd_openvx_extensions/amd_opencv/CMakeLists.txt:118 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

-- AMD OpenVX OpenCV Extension -- amd_opencv module added
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- runvx built with HIP Support

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=" -O3 -std=c++11"'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /workspace/MIVisionX/utilities/runvx/CMakeLists.txt:<line#> shown below:
Call Stack (most recent call first):
  utilities/runvx/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  utilities/runvx/CMakeLists.txt:134 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=" -O3 -std=c++11 -DHAVE_OpenSSL=1"'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /workspace/MIVisionX/utilities/runvx/CMakeLists.txt:<line#> shown below:
Call Stack (most recent call first):
  utilities/runvx/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  utilities/runvx/CMakeLists.txt:141 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=" -O3 -std=c++11 -msse4.1 -mf16c"'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm-5.0.0/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /workspace/MIVisionX/utilities/mv_deploy/CMakeLists.txt:<line#> shown
  below:
Call Stack (most recent call first):
  utilities/mv_deploy/CMakeLists.txt:9999 (rocm_check_toolchain_var)
  utilities/mv_deploy/CMakeLists.txt:35 (set)

*-----------------------------------------------------------------------------*
*******************************************************************************

-- Utilities -- runvx and mv_deploy modules added
-- Found HIP for rocAL: 5.0.13601-6b731c37/opt/rocm/hip
-- WARNING: RPP with the HIP backend Not Found (HINT: run setup script with the HIP backend first!) -- rocAL_rpp module excluded.
-- WARNING: rocAL module excluded
-- FILE_REORG - Install wrapper headers/Symlink files
-- Configuring done
-- Generating done
-- Build files have been written to: /workspace/build

@arvindcheru update on warnings. The current TOT with rocm-cmake master does not throw warnings
cmake ../MIVisionX/
-- The C compiler identification is GNU 9.4.0
-- The CXX compiler identification is GNU 9.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
-- MIVisionX Version -- 2.1.0
-- MIVisionX Install Path -- /opt/rocm/mivisionx
-- MIVisionX Backend set to -- HIP
-- MIVisionX Build Type -- Release
-- MIVisionX Developer Options
--      -D NEURAL_NET=OFF [Turn OFF Neural Net Modules (default:ON)]
--      -D ROCAL=OFF [Turn OFF ROCAL Modules (default:ON)]
--      -D LOOM=OFF [Turn OFF LOOM Modules (default:ON)]
--      -D GPU_SUPPORT=OFF [Turn OFF GPU support (default:ON)]
--      -D BACKEND=OPENCL [select OPENCL for GPU backend [options:CPU/OPENCL/HIP](default:HIP)]
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- AMD OpenVX -- OpenVX built with HIP Support
-- AMD OpenVX -- OpenVX module added
-- OpenCL Found - /opt/rocm/lib/libOpenCL.so
-- ROCm OpenCL Found - Force OpenCL_LIBRARIES & OpenCL_INCLUDE_DIRS to use ROCm OpenCL
-- OpenCL - Setting CL_TARGET_OPENCL_VERSION=220
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.1")
-- Checking for module 'libavcodec'
--   Found libavcodec, version 58.18.100
-- Checking for module 'libavformat'
--   Found libavformat, version 58.12.100
-- Checking for module 'libavutil'
--   Found libavutil, version 56.14.100
-- Using FFMPEG -- Libraries:/usr/local/lib/libavcodec.so;/usr/local/lib/libavformat.so;/usr/local/lib/libavutil.so;/usr/local/lib/libswscale.so Includes:/usr/local/include
-- amd_media -- Building with HIP
-- AMD OpenVX Media Extension -- amd_media module added
-- WARNING: MIOPEN with HIP backend Not Found -- amd_nn module excluded
-- WARNING: RPP with the HIP backend Not Found (HINT: run setup script with the HIP backend first!) -- amd_rpp module excluded.
-- OpenCV Found -- Version-4.5.X Supported
-- AMD OpenVX OpenCV Extension -- amd_opencv module added
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- runvx built with HIP Support
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- Utilities -- runvx and mv_deploy modules added
-- Found HIP for rocAL: 5.0.13601-6b731c37/opt/rocm/hip
-- WARNING: RPP with the HIP backend Not Found (HINT: run setup script with the HIP backend first!) -- rocAL_rpp module excluded.
-- WARNING: rocAL module excluded
-- Configuring done
-- Generating done
-- Build files have been written to: /workspace/build

Ok Thanks Kiriti, Did the MIVisionX latest version updated to latest rocm-cmake? So shall I remove all rocm-cmake download code from here?

As you can see the line number of these warnings - They are not from new PR changes.

@arvindcheru needs to sync with develop.

@arvindcheru needs to sync with develop.

I think we are comparing different rocm-cmake version
Kindly take a look on rocm-cmake version with "https://github.com/RadeonOpenCompute/rocm-cmake/blob/master/share/rocm/cmake/ROCMHeaderWrapper.cmake" file in it. this is in master rocm-cmake. Please check whether the latest version is with matching this version. I see it is not matching.

@arvindcheru now the develop branch uses master branch for rocm-cmake. Below is the log. Let me know if you can try this with the develop branch.
cmake ../MIVisionX/
-- The C compiler identification is GNU 9.4.0
-- The CXX compiler identification is GNU 9.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
-- MIVisionX Version -- 2.1.0
-- MIVisionX Install Path -- /opt/rocm/mivisionx
-- MIVisionX Backend set to -- HIP
-- MIVisionX Build Type -- Release
-- MIVisionX Developer Options
--      -D NEURAL_NET=OFF [Turn OFF Neural Net Modules (default:ON)]
--      -D ROCAL=OFF [Turn OFF ROCAL Modules (default:ON)]
--      -D LOOM=OFF [Turn OFF LOOM Modules (default:ON)]
--      -D GPU_SUPPORT=OFF [Turn OFF GPU support (default:ON)]
--      -D BACKEND=OPENCL [select OPENCL for GPU backend [options:CPU/OPENCL/HIP](default:HIP)]
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- AMD OpenVX -- OpenVX built with HIP Support
-- AMD OpenVX -- OpenVX module added
-- hip::amdhip64 is SHARED_LIBRARY
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.1")
-- Checking for module 'libavcodec'
--   Found libavcodec, version 58.18.100
-- Checking for module 'libavformat'
--   Found libavformat, version 58.12.100
-- Checking for module 'libavutil'
--   Found libavutil, version 56.14.100
-- Using FFMPEG -- Libraries:/usr/local/lib/libavcodec.so;/usr/local/lib/libavformat.so;/usr/local/lib/libavutil.so;/usr/local/lib/libswscale.so Includes:/usr/local/include
-- amd_media -- Building with HIP
-- AMD OpenVX Media Extension -- amd_media module added
-- Found Threads: TRUE
-- hip::amdhip64 is SHARED_LIBRARY
-- hip::amdhip64 is SHARED_LIBRARY
-- Found Protobuf: /usr/local/lib/libprotobuf.so;-lpthread (found version "3.12.0")
-- Inference Generator -- utilities/inference_generator module added
-- amd_nn -- Building with HIP
-- AMD OpenVX Neural Network Extension -- amd_nn module added with HIP backend
-- Found AMDRPP: /opt/rocm/rpp/lib/libamd_rpp.so
-- amd_rpp -- Building with HIP
-- AMD Radeon Performance Primitives -- amd_rpp module added with the HIP backend support
-- OpenCV Found -- Version-4.5.X Supported
-- AMD OpenVX OpenCV Extension -- amd_opencv module added
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- runvx built with HIP Support
-- Found HIP: /opt/rocm/hip version: 5.0.13601-6b731c37
-- Utilities -- runvx and mv_deploy modules added
-- Found HIP for rocAL: 5.0.13601-6b731c37/opt/rocm/hip
-- ROCm Augmentation Library -- rocAL module added
-- rocAL library is going to be built with HIP support
-- rocAL built with ENABLE_SIMD
-- rocAL - CMAKE_CXX_FLAGS: -O3 -O3 -fopenmp -msse4.2 -mavx2 -Wall  -fPIC -pg -pthread -std=gnu++14 -Wno-deprecated-declarations
-- rocAL library is going to be built with video decode functionality
-- Configuring done
-- Generating done
-- Build files have been written to: /workspace/build

@asalmanp I tried to build the bubble pop app and got the following error: 14 | #warning "This file is deprecated. Use the header file from /opt/rocm/include/mivisionx/vx.h by using #include <mivisionx/vx.h>" | ^~~~~~~ /opt/rocm/mivisionx/include/VX/vx.h:16:10: fatal error: ../../../include/mivisionx/vx.h: No such file or directory 16 | #include "../../../include/mivisionx/vx.h" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Just to avoid confusion, quoting here the latest update on this - should be addressed with this commit  https://github.com/GPUOpen-ProfessionalCompute-Libraries/MIVisionX/pull/786/commits/a8afcef76362f10fa58c5250a90dc8531097a105

@asalmanp @arvindcheru I can now build bubble pop. The error no longer appears. I can't test it however because I am building it on a remote system, which is all that I have available.

@arvindcheru updates to MIGraphX CMake needed - https://github.com/GPUOpen-ProfessionalCompute-Libraries/MIVisionX/blob/849679ad6ea7fb2cfbd2f4225063a7e36d2aef8f/amd_openvx_extensions/amd_migraphx/CMakeLists.txt#L21

@arvindcheru updates to MIGraphX CMake needed -
https://github.com/GPUOpen-ProfessionalCompute-Libraries/MIVisionX/blob/849679ad6ea7fb2cfbd2f4225063a7e36d2aef8f/amd_openvx_extensions/amd_migraphx/CMakeLists.txt#L21

@kiritigowda , yes these new files being updated now, will be merging shortly

@asalmanp  to verify AMD_MIGraphX

@asalmanp to verify AMD_MIGraphX

@kiritigowda both mnist and resnet50 migraphx node tests are building and running fine with this PR.


Handle Gaana songs with no artist information

Song URL: https://gaana.com/song/chalti-hai-kya-9-se-12
Expected behavior
Detect that the music is playing and notify that some information couldn't be identified.
Current behavior
No music is detected to be playing
Possible solution
I think we should assume that artist info might be missing while writing the code.
Steps to reproduce (for bugs)
Visit above URL; scrobbler can't detect music playing.
Context
Just trying to play a song.

Can't reproduce. Please make sure you're using v2.1.0 version.

Yeah, I am not able to reproduce it from the song page.
But @alexesprit try this: https://gaana.com/search/9 se 12
Playing the song from here, does not trigger last.fm scrobbling.

Works fine too.

@dufferzafar I also noticed player bar looks differently on our screenshots:

@dufferzafar I reproduced this issue on Firefox.
Unfortunately, I have no idea how to resolve it. Gaana connector is tricky and gets artist name via HTTP request using album URL. But by unknown reason this URL is missing in Firefox.


Save state reverts on Pokemon games

When I played Pokemon gold, and now currently Pokemon crystal, the save state reverts a little bit further back each day. It happens when I sleep and wake up to find the save state reverted. It probably has to do with the ingame clock on Pokemon Crystal & Gold.
A good function would be a manual save state on Google Drive. thats all thanks

@Kiwi-Koala Thanks for raising the issue. Do you think this could be a duplicate of #137?

Tagging as duplicate in advance of @Kiwi-Koala's answer.


Type link meta

Resolves #332

This change is

Review status: 0 of 37 files reviewed at latest revision, 3 unresolved discussions.

cs/cs.go, line 141 at r1 (raw file):

	Inputs       []string               `json:"inputs"`
	Tags         []string               `json:"tags"`
	Priority     float64                `json:"priority,omitempty"`

Priority is now mandatory. If not set its value is 0 instead of -inf.
I think it is not a problem because user always set priority or never. Is it ok for you?

cs/cs.go, line 185 at r1 (raw file):

// It assumes the link is valid.
func (l *Link) GetMapID() string {
	return l.Meta.MapID

I think it is better to remove all getter which do nothing. What do you think?

rethinkstore/rethinkstore.go, line 178 at r1 (raw file):

// Rethink cannot retrieve nil slices, so we force putting empty slices in Link
func formatLink(link *cs.Link) *cs.Link {

Nil slices are stored as empty stores in rethink. So on retrieval, link has empty slices and linkHas is different than the original one

Comments from Reviewable

Reviewed 37 of 37 files at r1.
Review status: all files reviewed at latest revision, 9 unresolved discussions.

agent/client/utils_test.go, line 61 at r1 (raw file):

	var refs []cs.SegmentReference
	for refIdx, in := range inputSlice {
		// var ref client.SegmentRef

Remove?

cs/cs.go, line 139 at r1 (raw file):

	Action       string                 `json:"action"`
	Type         string                 `json:"type"`
	Inputs       []string               `json:"inputs"`

nit: should it really be []string and not []interface{}?

cs/cs.go, line 141 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
Priority is now mandatory. If not set its value is 0 instead of -inf.
I think it is not a problem because user always set priority or never. Is it ok for you?

It sounds good to me, but probably Adrien's input would be more important than mine on this.

cs/cs.go, line 185 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
I think it is better to remove all getter which do nothing. What do you think?

Definitely, it's good to clean it up and directly access the member instead. Except for getters that have a bit of logic like GetPrevLinkHash that converts to *types.Bytes32.

cs/cs.go, line 245 at r1 (raw file):

	}

	if v, ok := l.Meta.Data["priority"]; ok {

Shouldn't this be using l.Meta.Priority?

cs/util_test.go, line 45 at r1 (raw file):

func appendRefSegment(l, ref *cs.Link) {
	// var refs []interface{}

Remove?

cs/util_test.go, line 54 at r1 (raw file):

func appendRefLink(l *cs.Link, process, linkHash string) {
	// var refs []interface{}

Remove?

rethinkstore/rethinkstore.go, line 178 at r1 (raw file):

// Rethink cannot retrieve nil slices, so we force putting empty slices in Link
func formatLink(link *cs.Link) *cs.Link {

You probably don't need to return the link since you're modifying it in place.

Comments from Reviewable

so satisfying to see all those casts go away... amazing thank's !

Reviewed 37 of 37 files at r1.
Review status: all files reviewed at latest revision, 14 unresolved discussions.

agent/client/client.go, line 42 at r1 (raw file):

//SegmentRef defines a format for a valid reference.
type SegmentRef struct {

can this be imported from cs?

cs/cs.go, line 139 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
nit: should it really be []string and not []interface{}?

you may be right... (although it won't work when an input is an array) but I think it's a complicated question that we can answer later (UX-ly speaking, how would a user send custom objects ?)

cs/cs.go, line 141 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
It sounds good to me, but probably Adrien's input would be more important than mine on this.

It don't think it's a problem at all, -inf is not really different from 0 in this case since a user should never set a negative priority right ?

cs/cs.go, line 185 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
I think it is better to remove all getter which do nothing. What do you think?

agreed :+1:

cs/util_test.go, line 45 at r1 (raw file):

func appendRefSegment(l, ref *cs.Link) {
	// var refs []interface{}

commented code (same below)

rethinkstore/main_test.go, line 50 at r1 (raw file):

	containerName := "indigo_rethinkstore_test"
	p, _ := nat.NewPort("tcp", port)
	pa, _ := nat.NewPort("tcp", adminPort)

why is this needed ?

rethinkstore/rethinkstore.go, line 171 at r1 (raw file):

}

func avoidNilStringSlices(s *[]string) {

nit: not sure we need a dedicated function for that, but never mind

rethinkstore/rethinkstore.go, line 178 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
You probably don't need to return the link since you're modifying it in place.

the return value is used in BufferedBatch

rethinkstore/rethinkstore.go, line 194 at r1 (raw file):

	prevLinkHash := link.GetPrevLinkHash()

	formatLink(link)

passing pointers around and updating the values is not really idiomatic in go, I'd rather use the return value.

Comments from Reviewable

Review status: all files reviewed at latest revision, 14 unresolved discussions.

cs/cs.go, line 139 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
you may be right... (although it won't work when an input is an array) but I think it's a complicated question that we can answer later (UX-ly speaking, how would a user send custom objects ?)

Jeremie was talking about this, and ways to provide type annotations or a schema in the UI, and then the agent could cast a few types (for example string, int, float, bool to begin with, no need for complex types it would already be very helpful to have support for those simple types).
But I agree that it can be for later :).

Comments from Reviewable

Review status: all files reviewed at latest revision, 14 unresolved discussions.

agent/client/client.go, line 42 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
can this be imported from cs?

Why not, I'll try

agent/client/utils_test.go, line 61 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
Remove?

oops

cs/cs.go, line 139 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
Jeremie was talking about this, and ways to provide type annotations or a schema in the UI, and then the agent could cast a few types (for example string, int, float, bool to begin with, no need for complex types it would already be very helpful to have support for those simple types).
But I agree that it can be for later :).

I can put []interface{} to manage simple types and we can enhance it later

cs/cs.go, line 141 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
It don't think it's a problem at all, -inf is not really different from 0 in this case since a user should never set a negative priority right ?

negative values are allowed (cf. tests)

cs/cs.go, line 245 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
Shouldn't this be using l.Meta.Priority?

I didn't put Priority at the beginning to keep it optional. And I forgot removing the test.
Done.

cs/util_test.go, line 45 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
Remove?

Done.

cs/util_test.go, line 45 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
commented code (same below)

Done.

cs/util_test.go, line 54 at r1 (raw file):
Previously, t-bast (Bastien Teinturier) wrote…
Remove?

Done.

rethinkstore/main_test.go, line 50 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
why is this needed ?

To debug tests. 8080 is too standard, I can change it.
18080?

rethinkstore/rethinkstore.go, line 171 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
nit: not sure we need a dedicated function for that, but never mind

With generics, it could be great. But for 2 calls, I can repeat it.

rethinkstore/rethinkstore.go, line 178 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
the return value is used in BufferedBatch

Yes I return it to simplify rethinkBufferedBatch.CreateLink write

rethinkstore/rethinkstore.go, line 194 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
passing pointers around and updating the values is not really idiomatic in go, I'd rather use the return value.

To do not be confusing, I'll change the function to do not return value (as said @t-bast ).
I prefer changing the signature than be clean and make a deep copy of the link and affect it to link btw.
Are you ok, with that?

Comments from Reviewable

Review status: all files reviewed at latest revision, 12 unresolved discussions.

rethinkstore/main_test.go, line 50 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
To debug tests. 8080 is too standard, I can change it.
18080?

I didn't understand why 28015 wasn't enough, but 8080 is fine

rethinkstore/rethinkstore.go, line 194 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
To do not be confusing, I'll change the function to do not return value (as said @t-bast ).
I prefer changing the signature than be clean and make a deep copy of the link and affect it to link btw.
Are you ok, with that?

yes :)

Comments from Reviewable

Review status: 28 of 37 files reviewed at latest revision, 11 unresolved discussions.

rethinkstore/main_test.go, line 50 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
I didn't understand why 28015 wasn't enough, but 8080 is fine

I've put 18080 to avoid unit test failure if port 8080 is already used

rethinkstore/rethinkstore.go, line 178 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
Yes I return it to simplify rethinkBufferedBatch.CreateLink write

Changed

rethinkstore/rethinkstore.go, line 194 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
yes :)

Done.

Comments from Reviewable

Review status: 28 of 37 files reviewed at latest revision, 10 unresolved discussions.

Comments from Reviewable

Reviewed 9 of 9 files at r2.
Review status: all files reviewed at latest revision, 10 unresolved discussions.

Comments from Reviewable

Reviewed 9 of 9 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

Comments from Reviewable

HARD FORK!

Reviewed 28 of 37 files at r1, 9 of 9 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

cs/cs.go, line 185 at r1 (raw file):
Previously, simonvadee (Simon Vadée) wrote…
agreed :+1:

I would keep them for encapsulation but either is fine.

Comments from Reviewable

Review status: all files reviewed at latest revision, 5 unresolved discussions.

cs/cs.go, line 141 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
negative values are allowed (cf. tests)

@such, is it ok for you?

cs/cs.go, line 185 at r1 (raw file):
Previously, such (Adrien Montfort) wrote…
I would keep them for encapsulation but either is fine.

I prefer remove getter without logic to do not write all unuseful getters.
And I've put GetPrevLinkHash and GetTagsMap on LinkMeta

Comments from Reviewable

Reviewed 2 of 14 files at r3.
Review status: 31 of 43 files reviewed at latest revision, 5 unresolved discussions, some commit checks broke.

cs/cs.go, line 141 at r1 (raw file):
Previously, alexppxela (Alexandre Thibault) wrote…
@such, is it ok for you?

ok!

Comments from Reviewable

Reviewed 14 of 14 files at r3.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

Comments from Reviewable

Reviewed 13 of 14 files at r3, 4 of 4 files at r4.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable


Write tests as you develop your code!

This is an easy win because you are starting from scratch! It will save you time debugging, make the code more maintainable, and get you points!
300 pts

F17 ok


[v1.27.0] cli test packages command --all flag added. dev by default

Proposed changes
cli test packages command --all flag added. dev by default
Fixes
https://github.com/valory-xyz/open-autonomy/pull/1676#discussion_r1051874504
Types of changes

[ ] Bugfix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist
Put an x in the boxes that apply.

[ ] I have read the CONTRIBUTING doc
[ ] I am making a pull request against the develop branch (left side). Also you should start your branch off our develop.
[ ] Lint and unit tests pass locally with my changes and CI passes too
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have checked that code coverage does not decrease.
[ ] I have added necessary documentation (if appropriate)
[ ] Any dependent changes have been merged and published in downstream modules

Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...
DELETE INCLUSIVE THIS AND BELOW FOR STANDARD PR
Release summary
Version number: [e.g. 1.0.1]
Release details
Describe in short the main changes with the new release.
Checklist
Put an x in the boxes that apply.

[ ] I have read the CONTRIBUTING doc
[ ] I am making a pull request against the main branch (left side), from develop
[ ] Lint and unit tests pass locally and in CI
[ ] I have checked the fingerprint hashes are correct by running (aea packages lock --check)
[ ] I have regenerated the latest API docs
[ ] I built the documentation and updated it with the latest changes
[ ] I have added an item in HISTORY.md for this release
[ ] I bumped the version number in the aea/__version__.py file.
[ ] I bumped the version number in every Docker image of the repo and published it. Also, I built and published them with tag latest
(check the READMEs of aea-develop
and aea-user)
[ ] I have pushed the latest packages to the registry.
[ ] I have uploaded the latest aea to PyPI.
[ ] I have uploaded the latest plugins to PyPI.

Further comments
Write here any other comment about the release, if any.

Codecov Report
Base: 90.29% // Head: 91.85% // Increases project coverage by +1.55% :tada:

Coverage data is based on head (374274f) compared to base (8c684c4).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #504      +/-   ##
==========================================
+ Coverage   90.29%   91.85%   +1.55%
==========================================
  Files         352      352
  Lines       28700    28709       +9
==========================================
+ Hits        25916    26370     +454
+ Misses       2784     2339     -445

Flag
Coverage Δ

unittests
91.85% <100.00%> (+1.55%)
:arrow_up:

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

Impacted Files
Coverage Δ

aea/cli/test.py
83.87% <100.00%> (+7.73%)
:arrow_up:

aea/package_manager/v1.py
99.37% <100.00%> (+0.01%)
:arrow_up:

plugins/aea-cli-ipfs/aea_cli_ipfs/ipfs_utils.py
100.00% <0.00%> (+0.58%)
:arrow_up:

...ckages/valory/connections/p2p_libp2p/connection.py
33.65% <0.00%> (+18.64%)
:arrow_up:

...ges/valory/connections/ledger/ledger_dispatcher.py
100.00% <0.00%> (+28.15%)
:arrow_up:

...s/valory/connections/ledger/contract_dispatcher.py
90.00% <0.00%> (+36.36%)
:arrow_up:

packages/fetchai/connections/gym/connection.py
100.00% <0.00%> (+39.62%)
:arrow_up:

packages/valory/connections/ledger/base.py
100.00% <0.00%> (+46.96%)
:arrow_up:

...alory/connections/p2p_libp2p/check_dependencies.py
100.00% <0.00%> (+48.43%)
:arrow_up:

... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


fix test invocation with py-test

before:
E           TypeError: setup_method() takes exactly 1 argument (2 given)

after:
tests/store_test.py FE.EFEFE

http://doc.pytest.org/en/latest/xunit_setup.html#method-and-function-level-setup-teardown
./setup.py test still gives me
Ran 0 tests in 0.000s

Not sure how you're running the test suite; it would also be very nice if you could mock outputs of docker-credential-secretservice so we don't have to have it present on a system.

From your link:

As of pytest-3.0, the method parameter is optional.

I simply do py.test tests/ but I'll look into adding it to the setup.py.
And yeah I wanna look into mocking for when I have more time, it's a bit tricky because most of the code is just interacting with the binary, but I think I can figure something out.
$ py.test -v tests/
============================= test session starts ==============================
platform linux2 -- Python 2.7.12, pytest-3.0.2, py-1.4.31, pluggy-0.3.1 -- /home/joffrey/.envs/credstore/bin/python
cachedir: .cache
rootdir: /home/joffrey/work/docker-pycreds, inifile:
plugins: cov-2.3.1
collected 4 items

tests/store_test.py::TestStore::test_store_and_get PASSED
tests/store_test.py::TestStore::test_get_nonexistent PASSED
tests/store_test.py::TestStore::test_store_and_erase PASSED
tests/store_test.py::TestStore::test_unicode_strings PASSED

=========================== 4 passed in 0.36 seconds ===========================

So let's close then, since
pytest==3.0.2

is in requirements.txt.


Fix parsing bug when error body contains no spaces

Fix for issue https://github.com/twitter/twemproxy/issues/404
Parsing a redis error messages with no space in the body results in [part of] the following message being consumed and resulting errors.

Closing this as it's being fixed in a branch discussed on the issue. https://github.com/twitter/twemproxy/issues/404#issuecomment-134205334

@tom-dalton-fanduel why did you close this pr?
maybe @manjuraj will merge this later.
this is your patch :) please reopen and wait to merge :)

Sorry, I thought you were going to use your branch instead :-) Reopened.

@tom-dalton-fanduel I am just a contributor :)
commiters are @manjuraj and @idning :)

In the process of trying to get the tests to run so I can add a test case.

Not sure how to go about writing a deterministic test for this since it relies on forcing a specific pair of messages back into twemproxy - e.g. mocking out the real redis so that we can ensure twmeproxy has to handle a double-response such as "-keyalreadyexists\r\n$8\r\nbananas!\r\n"

Realised it would work through a pipeline, added a test which fails against master:
======================================================================
FAIL: Test the error described on issue 404 is fixed.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/twemproxy/tests/test_redis/test_lua_error_return.py", line 33, in test_lua_return_error
    self.assertEqual(get_result, "bananas!")
AssertionError: '-ERR Inv' != 'bananas!'
-------------------- >> begin captured logging << --------------------

but passes against this PR.

@manjuraj think this is ready for review, have tested it a fair bit internally and seems ok.

+1
@manjuraj Please review this ;)

@manjuraj ping! Please review this :)

build fails with this commit: https://github.com/twitter/twemproxy/compare/0e8708be4d36...34f369ff9107

could you submit a fix that doesn't break the build? thanks!

@manjuraj I don't know why it fails :) this commit is not related with test_redis.test_mget_large_binary.test_mget_binary_value
and I pass all redis test with @tom-dalton-fanduel 's branch :)

@manjuraj Looking at the build history: https://travis-ci.org/twitter/twemproxy/builds It looks like that test also failed before my branch was merged - when the "Add Uber to the list of Adopters" commit was added. Maybe a problem with the Travis build/environment rather than the code/tests?

@manjuraj Could you take another look at this PR? I believe the build failure was unrelated to my change.


UI Issues on Newsletter Subscription and Contributor Page Alignment

Description
There are two distinct issues present in the interface:

The confirmation message after signing up for the newsletter is unclear and not prominent.
The contributor page does not have a proper center alignment, making the layout look
unorganized.

Steps to Reproduce

Sign up for the newsletter on the homepage.
Observe the confirmation message that appears after subscribing.
Navigate to the contributor page.
Check the alignment of contributor cards.

Expected Behavior

The confirmation message should be bold and clearly visible.
The contributor page should have all elements aligned to the center for a clean appearance.

Actual Behavior

The confirmation message appears faded and lacks prominence.
The contributor cards are unevenly aligned, resulting in a disorganized layout.

Screenshots

Additional Information
The issue may affect UX and could potentially discourage users from engaging with the newsletter and contributor sections.
What browser are you seeing the problem on?

[X] Firefox
[X] Chrome
[X] Safari
[X] Microsoft Edge

Checklist

[X] I have checked the existing issues
[X] I have read the Contributing Guidelines
[X] I want to work on this issue. (optional)

@Lokesh-Bijarniya assigning this issue to you.
Make sure to complete this issue within 2 days.

@Lokesh-Bijarniya before creating make sure to sync the repository, as i have updated the files and clustered the files in one way.


python 2/3 compatibility

Trivial change to allow install in Python 2 and Python 3.

Actually, I think the change should be more like this one:
https://github.com/astropy/astroquery/pull/715/files
Could you update the PR to be more like that?

Six is what I use for this

The tests seem broken for other regions.  I'll merge now.  Thanks!


Custom feature doesn't work

What happened + What you expected to happen
As your doc mentions it should be possible to add custom feature  (I copy paste your function from README)
=> but nothing happens after a few longs minutes
Could you please check ?
Versions / Dependencies
0.4.2 (the last one)
Reproduction script
import pandas as pd
import numpy as np
from tsfeatures import tsfeatures
periods = 24
ind = pd.date_range(start='2021-01-01', periods=periods, freq='MS')
vals = np.random.rand(periods)
df = pd.DataFrame({'ds':ind, 'y':vals, 'unique_id':1})
def number_zeros(x, freq):
number = (x == 0).sum()
return {'number_zeros': number}
features_df = tsfeatures(df,freq=12, features=[number_zeros])
features_df
Issue Severity
None

I'm having a similar issue. If I understand  correctly, the number_zeros function will count the number of zeros for each unique_id.
``python
def number_zeros(x, freq):
number = (x == 0).sum()
return {'number_zeros': number}

features = tsf.tsfeatures(data, features=[tsf.stl_features, number_zeros], dict_freqs={'MS': 12,})

Result is wrong because `number_zeros` is not supposed to be all zeros like this. In the data there are some unique ids that contain zeros.

|    |   unique_id |   number_zeros |
|---:|------------:|------------------:|
|  0 |      282998 |                 0 |
|  1 |      347809 |                 0 |
|  2 |      489552 |                 0 |
|  3 |      594474 |                 0 |
|  4 |      594861 |                 0 |
|  5 |      595209 |                 0 |
|  6 |      595956 |                 0 |
|  7 |      600426 |                 0 |
|  8 |      600429 |                 0 |
|  9 |      600458 |                 0 |
| 10 |      600460 |                 0 |
| 11 |      618974 |                 0 |
| 12 |      626202 |                 0 |
| 13 |      651946 |                 0 |
| 14 |      652565 |                 0 |
| 15 |      653545 |                 0 |
| 16 |      653546 |                 0 |
| 17 |      659776 |                 0 |
| 18 |      695699 |                 0 |
| 19 |      709426 |                 0 |
| 20 |      756965 |                 0 |
| 21 |      757751 |                 0 |

I think that the issue is that the scale argument in ts_features is set to True by default. You should try to change that to False and then rerun.


Do not receive POST request in user domain sent from webhook post method

Hi all,
I've setup 2 node servers application. The 1st is mailin server, receive email and send to 2nd server (http://localhost:3000/mailin/incomming).
The code in 1st server:
'use strict';
var mailin = require('mailin');
exports.start = function() {
mailin.start({
    port: 25,
    disableWebhook: false, // Disable the webhook posting.
    webhook: 'http://localhost:3000/mailin/incoming'
});

mailin.on('startMessage', function (connection) {
    console.log(connection);
});

mailin.on('message', function (connection, data, content) {
    console.log(data);
    require('../test/MailinService').closeConnection().then(function successFunc(){
        logger.info("Close connection");
        process.exit();
    });
 });

};
************* I write a test to send email: ******************
client = new SMTPConnection({
port: 25,
ignoreTLS: true,
host: "localhost"
});
client.connect(function () {
logger.info("connect success, sent now");
client.send({
from: {
name: '',
address<EMAIL_ADDRESS>},
to: [{
name: '',
address<EMAIL_ADDRESS>}]
}, fs.createReadStream(path.join(__dirname, 'test.eml')),function (err) {
if (err) {
logger.error(err);
}
});
});

And the log console when run this:
2016-04-09T16:57:41+07:00 - info PID-19309 Configuration loaded, starting mailin server.
warn: Either spamassassin or spamc are not available. Spam score computation is disabled.
2016-04-09T16:57:41+07:00 - info PID-19309 Open connection success
2016-04-09T16:57:41+07:00 - info PID-19309 Connect and send mail
info: Mailin Smtp server listening on port 25
2016-04-09T16:57:42+07:00 - info PID-19309 connect success, sent now
info: 8a662cf1-0eb4-414d-bbe7-8b4b5d07c14a Receiving message from<EMAIL_ADDRESS>{ id: '8a662cf1-0eb4-414d-bbe7-8b4b5d07c14a',
remoteAddress: '<IP_ADDRESS>',
clientHostname: '[<IP_ADDRESS>]',
hostNameAppearsAs: 'lebaphis-macbook-pro.local',
xClient: {},
xForward: {},
envelope:
{ mailFrom: { address<EMAIL_ADDRESS>args: false },
rcptTo: [ [Object] ] },
transaction: 1,
mailPath: '.tmp/8a662cf1-0eb4-414d-bbe7-8b4b5d07c14a' }
info: 8a662cf1-0eb4-414d-bbe7-8b4b5d07c14a Processing message from<EMAIL_ADDRESS>2016-04-09T16:57:42+07:00 - info PID-19309 Send mail success
{ html: 'Hello world!',
text: 'Hello world!',
headers:
{ 'x-mailer': 'Nodemailer 1.0',
from: '"Me"<EMAIL_ADDRESS>to: '"First Receiver"<EMAIL_ADDRESS><EMAIL_ADDRESS>'content-type': 'multipart/mixed; boundary="----mailcomposer-?=_1-1402581589619"',
'mime-version': '1.0' },
priority: 'normal',
from: [ { address<EMAIL_ADDRESS>name: 'Me' } ],
to:
[ { address<EMAIL_ADDRESS>name: 'First Receiver' },
{ address<EMAIL_ADDRESS>name: '' } ],
attachments:
[ { contentType: 'text/plain',
fileName: 'dummyFile.txt',
contentDisposition: 'attachment',
transferEncoding: 'base64',
generatedFileName: 'dummyFile.txt',
contentId: '6e4a9c577e603de61e554abab84f6297@mailparser',
checksum: 'e9fa6319356c536b962650eda9399a44',
length: 28,
content: <Buffer 6d 79 20 64 75 6d 6d 79 20 61 74 74 61 63 68 6d 65 6e 74 20 63 6f 6e 74 65 6e 74 73> } ],
dkim: 'failed',
spf: 'failed',
spamScore: 0,
language: 'pidgin',
cc: [],
connection:
{ id: '8a662cf1-0eb4-414d-bbe7-8b4b5d07c14a',
remoteAddress: '<IP_ADDRESS>',
clientHostname: '[<IP_ADDRESS>]',
hostNameAppearsAs: 'lebaphis-macbook-pro.local',
xClient: {},
xForward: {},
envelope: { mailFrom: [Object], rcptTo: [Object] },
transaction: 1,
mailPath: '.tmp/8a662cf1-0eb4-414d-bbe7-8b4b5d07c14a' },
envelopeFrom: { address<EMAIL_ADDRESS>args: false },
envelopeTo: [ { address<EMAIL_ADDRESS>args: false } ] }
info: 8a662cf1-0eb4-414d-bbe7-8b4b5d07c14a Sending request to webhook http://localhost:3000/mailin/incoming
info: Convert attachments to strings
{ durationMs: 0 }
2016-04-09T16:57:42+07:00 - info PID-19309 Close connection

In 2nd server, code to receive request in 1st is:

.......
appRouter.post('/mailin/incoming', cache, controllers.IntegrationController.receiveMailin);
appRouter.head('/mailin/incoming', function (req, res) {
logger.log('Received head request from webhook.');
res.send(200);
});
.......
But it just hit the method HEAD and DOES NOT hit the method POST...Does anyone got problem like me or plz give me some ideal?
Thanks

Try to create a request bin http://requestb.in/ and to use this url as you webhook url, just to see if the mailin webhook hits your request bin.


Added functionality to give custom inset for bottom and trailing cons…

…traints

@rajtharan-g Thanks for the PR!
Could you also update the documentation here and here?
Meanwhile I'll try to fix checks on circle ci 😬

Thank You! 👋


Simplify byte_pair_merge

Based on suggestion in https://github.com/openai/tiktoken/pull/239 (specifically 8f5dd7d)
Like that commit, this:

Does the init in a single loop and saves a loop if there are no merges
Simplifies get_rank and no longer uses it in init (so you don't need multiple skip values)

Unlike that commit:

We drop optimisations enabled by ignoring single tokens. These didn't show any benefit on benchmarks for me (this makes sense given typical piece sizes, but let me know if that's unexpected!). Given this, I opted for the simpler version.
I preserve some of the comments from the original that I think are still useful

Let me know what you think! Once we figure this one out, we'll look at the linearithmic fix (I have some thoughts there, still doing some benchmarking).
Co-authored-by: @paplorinc

We drop optimisations enabled by ignoring single tokens.

the parts.len() > 3 means that once we're down to 2 tokens, we don't need more iterations, we don't have to try to merge it into a single token since we've already filtered those out.
This would need a corpus with many double token words to be measurable in a benchmark.
Thanks for adding the comments back, please see my inline comments.

Thank you for the original change and follow-up review! I've marked you as co-author on the commit :-)


python3.7 posix broken

Getting this error
File "/usr/local/lib/python3.7/site-packages/Crypto/Random/OSRNG/posix.py", line 66
except IOError, e:
                  ^
SyntaxError: invalid syntax

Comparing posix.py between working and broken
 ❯ diff posix.py /Users/rwong/tmp/2022-04-22/bad/site-packages/Crypto/Random/OSRNG/posix.py                                 [11:24:09]
32c32
< from .rng_base import BaseRNG
---
> from rng_base import BaseRNG
66c66
<             except IOError as e:
---
>             except IOError, e:

python:3.7 updated 7 hours ago. Looks to have broken posix.
https://hub.docker.com/layers/python/library/python/3.7/images/sha256-0827a545174692[…]de6f275c61cffffdec172af2220bb05cb3e0eb8e6efce?context=explore

This was caused by pipenv 2022.4.21; Reverted to pipenv 2022.4.8 to resolve
https://pypi.org/project/pipenv/#history

Opened https://github.com/pypa/pipenv/issues/5074

I have the error too and I don't use pipenv, is there any other workaround?


Add .merlin

I use vim with merlin and I have to add a .merlin file to make it work in all the exercises.
Is it ok to make a PR to add this file on all the exercises?

Yeah, I think that's fine. I'll make sure that dotfiles get detected and served properly.

Update: I've added support for dotfiles in the API.

Merlin seems to be widely accepted in OCaml, I'm in favour of adding it. (Plus I use it myself in Emacs)

I will try to arrange some time this weekend to do this

bump @ismaelga

@tmcgilchrist I will send a PR today. Thanks for the bump :smiley_cat:

@tmcgilchrist What do you think about adding PKG findlib on .merlin? Is it wide used?

Done.


Rusty Anyhow

Google API Anyhow for Firefox Nightly.  Issued project.  Rusty start.

https://github.com/dtolnay/anyhow/actions/runs/9966926221/job/27539823551

https://github.com/dtolnay/anyhow/actions/runs/9966926221/job/27539823551

Thank you


Unable to launch or attach to firefox

Hi and thanks for this plugin. The last few releases were really 🔥 !
While the chrome adapter works without any issues I can't seem to get firefox to work in either launch or attach mode.
I'm getting the following error in the Sublime console:
Debugger: Nothing to read from process, closing
Debugger: error: event ignored not implemented unknownSource

This is my config for launching firefox:
{
  "type": "firefox",
  "request": "launch",
  "reAttach": true,
  "name": "Launch firefox 🦊",
  "url": "http://localhost:3000/index.html",
  "webRoot": "${folder}",
  "port": 3000,
  "firefoxExecutable": "/Applications/Firefox Developer Edition.app/Contents/MacOS/firefox"
},

Its probably because its like 2 years old on openvsx https://open-vsx.org/extension/firefox-devtools/vscode-firefox-debug

Well that's depressing.. any way to install it manually?

You could build it and replace what was placed in Packages/Debugger/data/adapters/firefox

Thanks! I'll give it a shot

The error I saw before is gone, but firefox is still not launching 🤔
This is what I'm seeing in the Debugger Protocol tab:
⟸ process/stopped ::
⟸ process/starting :: ['/usr/local/bin/node', '/Users/lev/Library/Application Support/Sublime Text/Packages/Debugger/data/adapters/firefox/extension/dist/adapter.bundle.js']
⟸ process/started ::
⟸ request/initialize(1) :: {'clientID': 'sublime', 'clientName': 'Sublime Text', 'adapterID': 'firefox', 'pathFormat': 'path', 'linesStartAt1': True, 'columnsStartAt1': True, 'supportsVariableType': True, 'supportsVariablePaging': False, 'supportsRunInTerminalRequest': True, 'supportsMemoryReferences': True, 'locale': 'en-us'}
⟹ response/initialize(1) :: {'supportsConfigurationDoneRequest': False, 'supportsEvaluateForHovers': False, 'supportsFunctionBreakpoints': False, 'supportsConditionalBreakpoints': True, 'supportsSetVariable': True, 'supportsCompletionsRequest': True, 'supportsDelayedStackTraceLoading': True, 'supportsHitConditionalBreakpoints': True, 'supportsLogPoints': True, 'supportsDataBreakpoints': True, 'supportsBreakpointLocationsRequest': True, 'exceptionBreakpointFilters': [{'filter': 'all', 'label': 'All Exceptions', 'default': False}, {'filter': 'uncaught', 'label': 'Uncaught Exceptions', 'default': True}]}
⟸ request/launch(2) :: {'name': 'Launch firefox 🦊', 'port': 3000, 'reAttach': True, 'request': 'launch', 'type': 'firefox', 'url': 'http://localhost:3000/index.html', 'webRoot': '/Users/lev/Projects/zencity/community/client'}
⟹ event/initialized :: None
⟸ request/setExceptionBreakpoints(3) :: {'filters': ['uncaught'], 'filterOptions': [ExceptionFilterOptions(filterId='uncaught', condition=None)]}
⟸ request/setDataBreakpoints(4) :: {'breakpoints': []}
⟹ response/setExceptionBreakpoints(3) :: None
⟹ response/setDataBreakpoints(4) :: {'breakpoints': []}

I think you are asking firefox to use the same port for remote debugging that you are using for serving your content which is probably causing issues

thanks! now firefox launches but the debugger fails to attach.
this my current config:
{
	"type": "firefox",
	"request": "launch",
	"reAttach": true,
	"name": "Launch firefox 🦊",
	"url": "http://localhost:3000/index.html",
	"webRoot": "${folder}",
},

and this is the error i'm seeing in the debugger console:
connect ECONNREFUSED ::1:6000
Debugging ended

if i understood this correctly, port 6000 is supposed to be used in attach mode only.
any idea what's going on?

As far as I can tell the launch request is just opening firefox and configuring it for remote debugging. Port 6000 is the default port they use.
You can look to see what port is being used by firefox with about:debugging

Tried the port that's configured there (6080) but still getting the same error. btw, I tried the same config in VSCode and it works there so I'm kinda stuck.


Added Expat XML parser.

http://expat.sourceforge.net/
Usage:
hunter_add_package(EXPAT)

find_package(Expat REQUIRED)

add_executable(foo foo.c)
target_compile_definitions(foo PRIVATE XML_STATIC)
target_link_libraries(foo PRIVATE ${EXPAT_LIBRARIES})
target_include_directories(foo PRIVATE ${EXPAT_INCLUDE_DIRS})

Thanks.  Fixed.

@alamaison Hi, Alex! I've fixed some issues with applying toolchain flags to C project (see details). Now some CI testing start failing at analyze toolchain (issue was that wrapper for clang analyzer was not used for C code but only for C++ code).
See Expat errors: https://travis-ci.org/ruslo/hunter/jobs/93227575#L606
I will disable it for now. Let me know if you want to fix it - I will enable it back after update.
Thanks, Ruslo

Let me know if you want to fix it - I will enable it back after update.

Sorry @ruslo, I've not had much free time lately.  Will look at this if I get a chance, but it might not be soon :(  Looks like issues in the upstream projects.


Disable via config

Is there currently or can a way be made to disable loading the assets through a config setting?

Closing for now, let me know if you have some more information on why this feature would be needed :)


ci: fix check for forks in mobile_docs job

The previous logic meant that we were never installing the deploy key.
E.g.: https://github.com/envoyproxy/envoy/actions/runs/3673844011/jobs/6211361666#step:7:7
Commit Message:
Additional Description:
Risk Level: No Envoy code changes, moderate risk of keeping this CI job broken in some cases.
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Seems to be working now.
A PR run from envoyproxy/envoy: https://github.com/envoyproxy/envoy/actions/runs/3676707021/jobs/6217702104

And from a fork (jpsim/envoy): https://github.com/envoyproxy/envoy/actions/runs/3676731585/jobs/6217754980

/retest


New option to allow overwriting the CSS

Great theme! but I't would be even better if it would be possible to customize some parts.
Taken from the forum, this allow to drop a css file and overwrite the main.css.

Amazing. I wasn't aware this was possible. This will be very helpful for all of the theme users, thank you very much!


[Request] Linux Repo or Inbuilt Automatic Updates For Stable and Nightly

To my knowledge, right now you have to manually update to each new version.
Instead can you implement automatic updates for both stable and nightly, either by a repo or inbuilt automatic updates.
Thanks

The resolvers lists are automatically updated.
But for the software itself: it would be impossible for a single person to maintain packages for every single linux distribution (and why only Linux?), especially since I don't use Linux.
Packages are available for most operating systems/distros already (including Windows and all BSDs), so you should probably use them. dnscrypt-proxy will be updated like everything else on your system.


⚠ GTru has degraded performance

In 22ed32a, GTru ($GTRU) experienced degraded performance:

HTTP code: 200
Response time: 12311 ms

Resolved: GTru performance has improved in d7004fa after 6 minutes.


[building] potential browserslist conflict

Because we are calling browserslist ourselves here: https://github.com/open-wc/open-wc/blob/master/packages/building-utils/find-supported-browsers.js it's possible to end up with a different version than the one used by @babel/preset-env in one of the build configs.
This creates situations where one version of browserslist passes versions to another version, creating errors as it doesn't know about certain new browsers.

I had this issue when updating an existing project.

@petrubocsanean if you regenerate your lockfile, does the error go away?

Yes, removing the lockfile and a clean install solves the error.

This was solved in v1 of building-rollup.


RFC: porting libpod to containers/image/pkg/sysregistriesv2

Why moving to sysregistriesv2?
The new package [1] allows to configure more aspects related to distribution than the old v1 API, including mirrors, specifying a URL with an http{,s} scheme, setting them to be insecure and also to allow searching in case an unqualified image is specified (e.g., "alpine:latest" instead of "docker.io/library/alpine:latest"). Moreover, we can use the Prefix semantics to translate namespaces and repositories or to just match certain namespaces on a given repository.
What's the problem moving to sysregistriesv2?
So far, the biggest obstacle is that we cannot separate the registry URI (http/https) and if it's a secure one or not, which has been a core requirement when discussing the design [2]. One way to approach this is extending the types.ImageReference interface with something like RegistryURL() but there are more ways to approach that. We also need to extend the docker_client code, which seems to only use http if DockerInsecureSkipTLSVerify is set and use https by default, and make the code use the URI scheme specified in the registries URL.
Hence the questions:

How shall we proceed? What's a potential way of addressing that?

Shall we introduce the concept of URI schemes or stick with how it's done at the moment? (This weould mean that we remove the URI handling from sysregistriesv2, and that insecure implies using http and skipping verification.)

Additional issues I may have missed?

[1] https://github.com/containers/image/pull/429
[2] https://github.com/containers/image/issues/397#issuecomment-359125521

I like what you are doing @vrothberg I think this will make configuring registries a lot more straight forward.

Regarding registry protocol specified in the URI: I'm of slightly mixed feelings here... We currently maintain a list of registries we allow insecure operations against, so I don't think we should respect the registry's requested scheme if it wants HTTP and it's not listed as an insecure registry (we should produce a good error describing why we won't pull from it, though).

@mtrmac PTAL, this looks like it will require some changes in the containers/image interface

So far, the biggest obstacle is that we cannot separate the registry URI (http/https) and if it's a secure one or not, which has been a core requirement when discussing the design [2].

Am I overlooking something? The sysregistriesv2 package, as merged in https://github.com/containers/image/pull/429, does have an independent “insecure” flag both for registries and mirrors. So we have already separated the two.

One way to approach this is extending the types.ImageReference interface with something like RegistryURL() but there are more ways to approach that. We also need to extend the docker_client code, which seems to only use http if DockerInsecureSkipTLSVerify is set and use https by default, and make the code use the URI scheme specified in the registries URL.

As the libpod code stands right now, it is not necessary to cram the security flag into the “image name” as parsed by alltransports.ParseImageName / ImageTransport.ParseReference; it is passed separately in types.SystemContext.DockerInsecureSkipTLSVerify, and AFAICT that approach works just as well with sysregistriesv2; https://github.com/projectatomic/libpod/blob/b44d4fb1ec05aec958a63bdfa3d344cdf4b4da8f/libpod/image/pull.go#L182 could be directly adapted to sysregistriesv2 with no changes to c/image.
AFAICS, in principle, nothing prevents c/image users from interpreting the Prefix→URL mapping, or using the list of mirrors, either (although the code would be a bit non-trivial and should not need to be repeated all over the place, see below).
Or maybe I am missing the problem that needs to be solved.

While integration of sysregistriesv2 as a direct replacement of sysregistries in all users seems to be fairly straightforward, I think we should seriously consider consolidating most of that code in to c/image itself, so that it acts on the config file without every caller having to do it itself.  The caller should ideally be just able to do ~ alltransports.ParseImageName(userSpecifiedString).NewImageSource() and get a client that Just Works.
(Well, maybe not the search, because I hate it, notably it pretty much breaks reasoning about user intent when verifying signatures, and the way it changes handling of existing docker://busybox references would be an invisible API break.)
The Insecure flag, the Prefix→URL mapping look like things that can and should be handled transparently in c/image without the callers having to care. The Mirrors list seems likely to be better handled in c/image as well, although there is a stronger case that the caller may want to have control over that process, maybe distributing the use of mirrors when pulling many images at once, so that they all don’t hammer the first one in the list, or something like that. But the default is likely to work for most callers, so we can start with c/image handling mirrors, and we can always add an opt-out via types.SystemContext.

As the libpod code stands right now, it is not necessary to cram the security flag into the “image name” as parsed by alltransports.ParseImageName / ImageTransport.ParseReference

(BTW as much as having a string format is useful and necessary for a CLI, it’s also a pain to design and maintain because it is either unusably complex for humans to use (e.g. strictly semantically correct JSON), or it uses meta-characters which complicate or prevent handling of unusual strings, or it heuristically tries to guess what the user meant depending on context, and is unpredicatable/unreliable.  E.g. we’ve ended up with several transports unable to handle paths which contain colons because they use a colon as a separator.
A better UX is for the “image name” not to embed various options, and for those options to be instead available in separate, individually documented and validated, CLI options.  The downside to that is that every single caller needs to implement, document and validate the options, so there certainly is a balance to be struck. At least for the “insecure” flag, we mostly already have the options in place.)

So far, the biggest obstacle is that we cannot separate the registry URI (http/https) and if it's a secure one or not, which has been a core requirement when discussing the design [2].

Am I overlooking something? The sysregistriesv2 package, as merged in containers/image#429, does have an independent “insecure” flag both for registries and mirrors. So we have already separated the two.

… Ah, right. The difficulty is not with the “insecure” flag, it’s that the c/image behavior, as described above, makes the protocol specified in the config file ignored and pointless.
As far as the linked comment goes, I was only worried about a config that forces the user to say http to mean means “HTTPS but without certificate verification”, and we no longer have that problem.  As discussed above, I would be fine with not having the protocol in the config at all, I guess (with absolutely no supporting data, to be fair) that the distinction is rarely likely to matter in practice.
If you think this does matter:

One way would be for c/image/docker to parse and interpret the config file itself. Then the question of how to express HTTP/HTTPS in the API mostly goes away.

It would be consistent with the other cases, although not all that pretty, to express this as an option in types.SystemContext, maybe an yes/no/autodetect enum DockerUseTLS.

I’d prefer either dropping the schema from the config, or option 1.

While integration of sysregistriesv2 as a direct replacement of sysregistries in all users seems to be fairly straightforward, I think we should seriously consider consolidating most of that code in to c/image itself, so that it acts on the config file without every caller having to do it itself.

I agree. I really want to get rid of libpod's registries package as having a wrapper for an API usually means that the API isn't complete. Things such as registries.GetInsecureRegistries() could safely be moved into c/image.

The Insecure flag, the Prefix→URL mapping look like things that can and should be handled transparently in c/image without the callers having to care.

I wouldn't put that into c/image, as I want to have full control over the c/image library. In case we move all the sysregv2 semantics into c/image pulling, for instance, having full control over what's going to happen would mean that we had to extend the API with yet more boolean switches (e.g., disable_mirrors, disable_prefixing, etc.). If we keep the logic at the user site, c/image can just pull an image without further reasoning and the user can decide if the mirrors or prefix semantics shall be used or not.

… Ah, right. The difficulty is not with the “insecure” flag, it’s that the c/image behavior, as described above, makes the protocol specified in the config file ignored and pointless.

Luckily there is no user of sysregv2 yet :-) I am quite happy to know we don't have to go down the rabbit hole. I must have misread your comment when working on the sysregv2 API as I thought you wanted to have https with TLS verification skip as well.

I’d prefer either dropping the schema from the config, or option 1.

I completely agree. As mentioned above, we could add an extra API to get the insecure registries as well.
@mtrmac I propose to follow your suggestion and remove the URI schemes from sysregsv2 (and unconditionally throw a warning if one is specified) and to also extend the API to get insecure registries. We can then remove the redundant libpod registries package. After that, supporting sysregv2 is pretty straight forward. Did I miss anything?
Once I get an ACK from your side, I will start preparing a PR to polish sysregv2.

@mtrmac I was playing a bit with the idea of forbidding any URI schemes in sysregsv2, and it turns out that the URL validation gets pretty screwed. Without an URI scheme, net/url's parsing isn't able to distinguish between user:password and the URI scheme yielding very imprecise error messages.
This made me reflect more on the issue, so I want to propose another alternative to the two you proposed above:

Use http and https URI scheme to distinguish insecure and secure registries. I know there are (valid) concerns, but Docker is already using this semantics. The issue discussed above can then be solved by setting the skip verify flag when http is used.

A more or less related topic, but something that makes me prefer option 3: For the SUSE CaaS Platform and openSUSE's Kubic project, we are already using registry mirrors (configurable via the Velum dashboard using a backport of the current upstream PR). Once things are merged into Docker and CRI-O, it would be nice to be as close as possible (semantically) without having to special-case too many details.
Sorry for changing my mind, but I believe it's more usable that way.

The Insecure flag, the Prefix→URL mapping look like things that can and should be handled transparently in c/image without the callers having to care.

I wouldn't put that into c/image, as I want to have full control over the c/image library. In case we move all the sysregv2 semantics into c/image pulling, for instance, having full control over what's going to happen would mean that we had to extend the API with yet more boolean switches (e.g., disable_mirrors, disable_prefixing, etc.). If we keep the logic at the user site, c/image can just pull an image without further reasoning and the user can decide if the mirrors or prefix semantics shall be used or not.

The point of a system-wide config file is that the behavior is system-wide :) Individual applications should typically not be deciding to interpret only parts of it, at least not in the common case and without a good reason.
Applications which do have a good reason can already exercise full control, by pointing types.SystemContext.SystemRegistriesConfPath at a custom file (including /dev/null). (This allows the application to do everything, but small localized edits like “respect the users’ config, but use insecure mode for a single host name” would be very difficult. Does that matter? What do you need?)

I was playing a bit with the idea of forbidding any URI schemes in sysregsv2, and it turns out that the URL validation gets pretty screwed. Without an URI scheme, net/url's parsing isn't able to distinguish between user:password and the URI scheme yielding very imprecise error messages.

Ouch.

Use http and https URI scheme to distinguish insecure and secure registries. I know there are (valid) concerns, but Docker is already using this semantics.

Pre-existing practice would certainly be a strong argument. Where is this being used?

Another possible alternative: Instead of parsing the values as url.URL, use the regexes in c/image/docker/reference.  That’s what the data actually is; docker.io/library/busybox is actually the URL https://registry-1.docker.io/v2/library/busybox/manifests/latest + all the referenced blob files.  The namespace/repo parts of the docker/distribution protocol do not accept arbitrary characters allowed in URLs; ?query=string#anchor is not allowed either.
The flip side is that, assuming we want the URL and Prefix fields to always be hostname[/namespaces…], c/image/docker/reference does not provide an API to do that right now; there’s ParseNormalizedNamed which can accept such input but interprets differently for hostname-only values (example.com == docker.io/library/example.com.  It’s seems plausible enough to make DomainRegexp public and make the parsing possible to do externally, or to add another parser to the package.  (Using regexps like that my not give very friendly error messages either, though.)
This seems to be conceptually correct but a not-quite-trivial amount of code.

Or we could go with a two-line hack, uri, err := url.Parse("https://" + input); …; uri.Scheme = "".  Not correct by any means but the if url.Parse does report an error, it will be appropriately matched to the relevant component of the input, and the simplicity is a bit appealing.

Use http and https URI scheme to distinguish insecure and secure registries. I know there are (valid) concerns, but Docker is already using this semantics.

Pre-existing practice would certainly be a strong argument. Where is this being used?

It's part of the design of https://github.com/moby/moby/pull/34319#issuecomment-381766390 for a long while (see quote below). It's not yet merged, but those semantics are carved into stone.

"Registry URLs must have a scheme (http:// or https://); this largely makes --insecure-registry obsolete, but if desirable, we can add an TLSnoVerify option in future, to account for (e.g.) MITM proxies etc."

You might notice that we're about to celebrate the Moby PR's birthday soon, so we'll certainly be quicker with c/images and libpod. While working on sysgregsv2 I tried to be close to the Moby PR, but I wasn't aware that the insecure switch controls that for image pulling.
In case we decide to remove URI schemes from sysregsv2, I would prefer to use the https fallback to allow a sane parsing, just as you suggested:

Or we could go with a two-line hack, uri, err := url.Parse("https://" + input); …; uri.Scheme = "". Not correct by any means but the if url.Parse does report an error, it will be appropriately matched to the relevant component of the input, and the simplicity is a bit appealing.

However, then I would change {Registry,Mirror}.URL to be a string and do the parsing only to validate and to sanitize the input.

@mtrmac Sorry to be pushy, but I am eager to get mirror support into Podman. What's your take on my last comment?

It's part of the design of moby/moby#34319 (comment) for a long while (see quote below). It's not yet merged, but those semantics are carved into stone.
"Registry URLs must have a scheme (http:// or https://); this largely makes --insecure-registry obsolete, but if desirable, we can add an TLSnoVerify option in future, to account for (e.g.) MITM proxies etc."

It’s a bit hard to take an unmerged PR as semantics “carved into stone” :)
And anyway, this design has AFAICS “http: means no TLS”, not “http: allows both no TLS and unauthenticated TLS”.  It means dropping support for the TLSnoVerify case entirely (until some “future”).

More or less every TLS client ends up having an option to ignore certificate failures.  I don’t have a really good argument that such an option should exist; probably in the majority of cases where I’ve seen the option used it shouldn’t have been. Still, I do see skopeo --tls-verify=false often enough in various shell transcripts, for whatever reasons.
The existence of such an option may be just a cargo-culted practice of making such a concession to convenience, something that may not actually be necessary.  I’d be happy to hear that none of these are actually needed, I don’t know how to tell whether that’s the case. (And there’s also the question whether c/image wants to be a trailblazer for the hard-line “TLS means TLS” approach.) @rhatdan , others, WDYT?

WRT specific use cases: MITM proxies should be handled by actually installing their certificate as a CA root, to be at all practical. I’m more worried about disaster recovery, “special situations”, and the like:

Legitimate disagreements between the maintainer of the server and of the client about the needed security for the connection.

A notable special case for docker/distribution registries: with digest references, and possibly signatures, the integrity of the TLS connection to the registry may not matter for clients who rely on the digests—but the registry still needs to support TLS for integrity of images delivered to clients who don’t know the digests.
Also, forensic investigations, where a former production server is examined on a separate unquestioned network, and the goal is only to see the contents of the server and not to protect against the network attacking, ideally without changing the server configuration in any way.

Temporary workarounds (“I have a new laptop and I urgently need to check an image without spending hours to install a full corporate build”. This one is typically iffy.)
Recovering from CA certificate mismanagement (rolled over the root of trust without deploying it to consumers, which would need to be updated from the rolled over registry), both for manual changes and possibly when the CA is automatically managed by a product like OpenShift (I do hope we would not fail this badly, but Kubernetes does seem to be moving towards shipping core parts of the infrastructure as containers).
Crazy corporate box-ticking/department politics (“we must use TLS but we don’t have to care”, “the other department set up the box and we can’t wait for them to fix their certificates”).  (Years ago, in a completely unrelated project, I have encountered a customer who insisted on using TLS but not doing any verification. The customer ultimately got the option they asked for added.)

OTOH, there is a genuine big risk that temporary workarounds stay enabled forever; and even if they are very temporary, an attacker may need to only successfully compromise an image transport once.

Overall, with the little information I have, I don’t currently really feel comfortable with not having the ability to use TLS in an insecure manner at all. It’s a risk to use such an option, but it seems to be a risk users want to take fairly frequently.
And with the strings not being URLs semantically at all, using schemes is not really adding clarity.
So, I’d prefer a format with docker-like references (whether parsed through c/image/docker/reference or url.Parse("http://"+…) and letting some invalid strings through), and a separate “insecure” boolean.
BUT I do know I have a pretty bad track record on UX design (volume_key!?); I’d love other opinions.

However, then I would change {Registry,Mirror}.URL to be a string and do the parsing only to validate and to sanitize the input.

Yes, that makes perfect sense. (The fields may not even need to be called URL, pedantically, but that doesn’t matter that much.)

I agree, we need to have a get out of jail free card.  Where user just tells the tool to make it happen.  --tls-verify=false.  These things get broken too often and easily.

@mtrmac @rhatdan Thanks a lot for checking. It looks like we are going for removing the URI schemes and keeping the insecure switch. I will prepare an update to c/image then. I am pretty busy right now, but I will try to work on that asap.

I see this issue as resolved. We can discuss the details in the upcoming PR in c/image.


Add missing properties to stdlib

Fixes #38676
Fixes #38677

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.:x: devsnek sign nowYou have signed the CLA already but the status is still pending? Let us recheck it.

@rbuckton thanks for merging. y'all might want to spend some time thinking about ways to improve contribution flow so that pinging ts devs on twitter isn't needed.


record type for window options

For more than a few possible options, the list approach probably is better, but for BrowserWindow options, I think a plain record is ok for the start.

Thanks, Patrick!
In the short run I agree with you but considering the set of options we want to cover and that new releases can still extend it, it seems to me the list approach is better in the long run and I'd be tempted to keep it. What do you think?

Agreed. Given the plethora of (partially nested) options, records are not a good idea. Pull request revoked, sorry for the noise. :)

Nothing to be sorry for :)


Add public function to delete objects from ADF instance

It looks like when an ADF object is excluded from deployment (via the Excludes property of the options) it is still considered to be "in source" and DeleteNotInSource does not apply to it.
For instance: if a given ADF object is already in the target ADF, and if a new deployment excludes that object from deployment and DeleteNotInSource flag is set to true, the excluded object will not be deleted from the target.
I would expect it to be deleted after the deployment.

Yes, this is exactly how the feature should work like.
'Excludes' property means you want to skip deployment of particular (set of) object(s), but definitely won't delete any of excluded objects. As latter property says "Delete Not in Source" - but the object is still in the source (repo) and should NOT be removed from ADF service.
I consider exposing the method to delete some objects explicitly if needed. Let me know if that would help in your scenario.

Hey Kamil,
Thanks for your note.

I see what you mean, Excluded does not mean that is no longer "in
source" (they are considered to be in source), as a result "Delete not in
source" (true) will skip the deletion of them if they were present at the
target. This is not a big deal.

Yes, a way of explicit delete of objects in a given folder would
definitely be helpful. Again, just nice to have. This could be extended by
the object type (pipeline, dataset, etc.), recursive flag to delete
subfolders, perhaps even a pattern to match folders to delete.

We normally use ARM template deployment, and it works pretty well,
except when we need to automatically rebuild templates. There is no API for
that, and Microsoft is very slow in adding it. I know they are planning to
revamp ADF, but not sure when. We keep enquiring them for improvements
related to remote ADF calls, better workflow capabilities, ability to fail
a pipeline etc, all slated to come but no one knows when.
Anyway, nice package you have created. Well done. Probably the most
comprehensive that I have seen.
Thanks
Maciej
On Sun, Sep 6, 2020 at 4:13 AM Kamil Nowinski<EMAIL_ADDRESS>wrote:

Yes, this is exactly how the feature should work like.
'Excludes' property means you want to skip deployment of particular
(set of) object(s), but definitely won't delete any of excluded objects. As
latter property says "Delete Not in Source" - but the object is still in
the source (repo) and should NOT be removed from ADF service.
I consider exposing the method to delete some objects explicitly if
needed. Let me know if that would help in your scenario.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/SQLPlayer/azure.datafactory.tools/issues/30#issuecomment-687747274,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AMI26ROGXX652GNRAKROGHLSENOE5ANCNFSM4Q4DGOVQ
.

Thanks! Sounds good. I will do it as soon as I can.

@NowinskiK @gidziu indeed, this feature will be really cool.
At the moment I'm migrating a project to a different Team Data Factory, and I thought about something related to this.
It would be great to have a "delete deployment" feature (all included adf objects in the deployment config files). Kind of a "reverse deployment" thing to be able clean ADF.


What port does forever (start.sh) run the server on?

I can't seem to access my app in a web browser when running the app in production mode.
Thanks!

Are you using Docker version or straight-up?

Straight up :-)

Can you check the logs, please? What's in the logs?

Thanks! I'm getting:
TypeError: Cannot read property 'domain' of undefined
    at Domain.add (domain.js:171:27)
    at errorHandler (/home/ec2-user/deploy/node_modules/error-handler/errorHandler.js:32:7)
    at /home/ec2-user/deploy/server.js:28:42
    at Object.exports.setup (/home/ec2-user/deploy/node_modules/nodebootstrap-server/app.js:41:3)
    at Object.<anonymous> (/home/ec2-user/deploy/server.js:17:8)
    at Module._compile (module.js:399:26)
    at Object.Module._extensions..js (module.js:406:10)
    at Module.load (module.js:345:32)
    at Function.Module._load (module.js:302:12)
    at Function.Module.runMain (module.js:431:10)
    at startup (node.js:141:18)
    at node.js:977:3
domain.js:171
  if (this._disposed || ee.domain === this)
                          ^

@nikkwong,
were you able to find out what the issue was with this one?

I don't remember unfortunately. Hopefully won't run into it again :-)


moved kibana url types to exclude from custom exported metadata

What this PR does / why we need it:
Moves kibana url type definitions from broad Metadata to specific TestrunSummary and StepSummary to avoid "polluting" all custom exported metadata with this "huge" URL.
Adds specific kibana urls to StepSummary so that we can present a direct link to the logs of a specific TestStep.
Release note:

Adds kibana URLs to elasticsearch metadata to directly link to the logs of a respective TestRun or TestStep.

Please rebase to not include output-dir-path changes

This PR is currently ahead of master by one commit, nothing to rebase.

Changed and commented, PTAL

This PR is currently ahead of master by one commit, nothing to rebase.
You probably refer to an output-dir-path change I added to the collector command (it was missing there)

Thanks for the fix;)


Repeatable migrations

Being able to specify a script (via filename or special directory) that gets re-applied whenever the hash changes. It would be dependent on #120 getting done (adding file hashes), but IMO would bridge the feature parity gap between Flyway and Dbops for 99% of use cases.
Thanks again for a great tool! I'd be happy to help do some work towards this if needed.

Now that I have a working example of an enhanced table, I would like to get more input on how you envision this working.
Currently, there is already an option to run a modified script if you're using DBOps packages, which store hashes internally and are able to distinct if a script on a filesystem is different from how it was imported into a build previously.
I imagine that you're talking about achieving similar functionality in Install-DBOScript without having to build packages - by comparing every script's hash with a recorded hash from when that particular script was previously executed. Is that right?

That's correct. In our setup we aren't quite able to utilize the full fledged CI/CD workflow currently, so we're just relying on the Install-DboScript command to do migrations.
Comparing the hashes would make it a lot easier to have a View definition live in a single script, versus having to create a new migration script for it per change when using Install-DboScript. I'm not sure if it makes more sense to just apply this logic to all scripts, or only apply it to a folder like PreScripts.
This may be a bit of an edge case, so thanks for looking into it!

The more I look at it, the more I think that this functionality would benefit the module greatly, perhaps offering a third potential pipeline scenario that supports modified files.
It seems, however, that it's not going to be a quick win. The only validation DbUp UpgradeEngine class does is script name validation, and the overrides might not be so easy to implement. But I'll give it an honest attempt at the very least.


Upload new values to Multicore, Memory and Performance params to workload doc

This actually needs to be discussed whether we can completely overwrite the original values or whether we should somehow provide an API containing the exact parameters provided during request creation.

Can this have more meat so that one understand what is the impact on operation of workflows ? Is this a monitoring feature or an actual feature that impacts wf behavior ?

This is mostly to investigate issues and check what were the original parameters and etc. The document I'm talking about is:
https://cmsweb-testbed.cern.ch/couchdb/_utils/document.html?reqmgr_workload_cache/amaltaro_TaskChain_RelVal_Multicore_HG1608_Validation_160809_232142_450
where the Memory/Multicore parameters could get overwritten and then by looking at this document you don't know anymore what was the original value.

This is actually overwritten for ReqMgr2 requests. So my question remains, is it Ok if we overwrite the original values with those values provided by Unified during assignment? My main concern is with future clones of the original request, since it would clone the values provided during the request assignment instead of the original one submitted by McM.
Please assume the structure of this document cannot change (so, no special key for assign_param and whatnot), so no third option in your answer :)
PS.: The same applies to AcqEra/ProcStr/ProcVer that I asked you in another issue...

Fixed in the last month by a couple of PRs


Ensure that the tekton dashboard validates the secret sent by git

When the tekton dashboard registers a webhook with git, it should send a shared secret that git will include in the future requests to the dashboard.  Ensure that the dashboard is sending this secret to git during the webhook registration process and validates it properly when git makes the call to drive a pipeline.

@jtmulvey just FYI.  Opening this here so I can keep track of all the webhook related issues under one epic. Think you mentioned you were going to be discussing this with Mark this week.

This is currently handled by virtue of our using the Knative Webhooks sample, https://knative.dev/docs/serving/samples/gitwebhook-go/

Closing based on Mark's comment that this is handled.


Delay when loading "Docker Containers using [Docker Extension]"

Does this issue occur when all extensions are disabled?: Yes/No
Using Docker Extension

Steps to Reproduce:

Use Docker Extension to Load Containers

am still debugging this one, this doesn't seem to happen when you are over dockerized TCP, however, it seems to happen locally

/causedByExtension
This is caused by Docker or the Docker extension. Please file a new issue for it: https://github.com/microsoft/vscode-docker/issues


How do I use RunCommand?

Hello,
I couldn't find anything on the API documentation about RunCommand. How do I use it inside a custom plugin? I tried using server.RunCommand('notify',plr.Name,'You gave $' .. args[2] .. ' to ' .. player.Name) but it returns an error saying "attempt to call a nil value".

Nevermind, found the solution.
service.TrackTask("Thread: Notifying", server.Admin.RunCommand, server.Settings.Prefix.."notify "..plr.Name.. " You removed $" .. args[2] .. " from " .. player.Name)