GH-3838 Add Support for S3DownloadStrategy to Bottles

Add Support for S3BottleDownloadStrategy to Bottles

[x] Have you followed the guidelines in our Contributing document?
[x] Have you checked to ensure there aren't other open Pull Requests for the same change?
[x] Have you added an explanation of what your changes do and why you'd like us to include them?
[ ] Have you written new tests for your changes? Here's an example.
[x] Have you successfully run brew style with your changes locally?

→ brew style
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
Inspecting 665 files
.........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

665 files inspected, no offenses detected

[x] Have you successfully run brew tests with your changes locally?

2246 examples, 0 failures, 10 pendings

Took 104 seconds (1:44)

Preface
No tests written yet. I am happy to write them if we determine this approach is acceptable.
I am taking your contribution notes at heart, and wish to deliver an improvement that fits within the guidelines of the quality bar Homebrew has set. Thus, I expect to iterate on this, as needed, until hopefully we can determine what all changes are needed.
Overview
This is an attempt at implementing #3838 by allowing for a :using => statement to be optionally added to the root_url of a bottle configuration.
I can specify a new download strategy called S3BottleDownloadStrategy which extends the behavior of the CurlBottleDownloadStrategy by allowing it to use S3 URLs, but still retaining the bottle "Pouring" text.
Example
class MyApp < Formula
  desc "MyOrg's MyApp"
  homepage "https://github.com/myorg/myapp"
  url "https://myorg-private-source-dist.s3.amazonaws.com/myapp/1.0/my-app-1.0.0.tar.gz", :using => S3DownloadStrategy
  version "1.0.0"
  sha256 "big-long-sha"

  bottle do
    root_url "https://myorg-private-bin-repo.s3.amazonaws.com/myapp/1.0", :using => S3BottleDownloadStrategy
    cellar :any_skip_relocation
    sha256 "big-long-sha" => :high_sierra
  end

  def install
    system "./configure", "--disable-silent-rules",
                          "--prefix=#{prefix}"
    system "make", "install"
  end

  test do
    true
  end
end

Code Changes
Library/Homebrew/download_strategy.rb
CurlBottleDownloadStrategy is the default bottle download strategy. To support S3, I needed a new class which inherits from this one, and adds the S3 Behavior.

Added S3BottleDownloadStrategy which inherits from CurlBottleDownloadStrategy
Moved the _fetch override from S3DownloadStrategy to a new mixin inside CurlDownloadStrategy called S3DownloadExtension
added include S3DownloadExtension into S3DownloadStrategy and S3BottleDownloadStrategy

Library/Homebrew/software_spec.rb
BottleSpecification has a root_url attribute, I needed to be able to pass a :using => setting into it, so I used a pattern similar to resource.url to support optionally passing in a hash of 'specs'.
I then store those specs in a new read-only attribute called :root_url_specs
-  def root_url(var = nil)
+  def root_url(var = nil, specs = {})
     if var.nil?
       @root_url ||= "#{DEFAULT_DOMAIN}/#{Utils::Bottles::Bintray.repository(tap)}"
     else
       @root_url = var
+      @root_url_specs.merge!(specs)
     end
   end

Bottle.initialize normally defaults to CurlBottleDownloadStrategy and ignores whatever resource.url had set it to. I needed to be able to remove the default override and instead use a default suggestion to resource.url which would allow the new S3BottleDownloadStrategy to be optionally specified.
-    @resource.url(build_url(spec.root_url, filename))
-    @resource.download_strategy = CurlBottleDownloadStrategy

+    @resource.url(build_url(spec.root_url, filename),
+                  select_download_strategy(spec.root_url_specs))

I added a helper method select_download_strategy which takes the root_url_specs hash from the BottleSpecification
It's here that I default to CurlBottleDownloadStrategy, except now I'm doing it by specifying it as a :using flag, and this allows the existing Formula.url behavior to set @resource.download_strategy appropriately.
+  def select_download_strategy(specs)
+    specs[:using] = CurlBottleDownloadStrategy unless specs.include?(:using)
+    specs
+  end

Hey @MikeMcQuaid,
Thanks again for your comments.
I've attempted to address them. Let me describe the changes for your review:
Code Changes
Library/Homebrew/download_strategy.rb

class AbstractDownloadStrategy

I noticed that the resource.specs were getting saved to the download strategy as meta, so I'm leveraging that to store a new :bottle flag.
During initialize, AbstractDownloadStrategy will extend BottleExtension if meta[:bottle] == true (This adds the stage behavior that was previously in CurlBottleDownloadStrategy and LocalBottleDownloadStrategy)
Thus, in other contexts when the download strategy is being initialized, the caller can specify resource.specs[:bottle] = true before passing the resource to that strategy's constructor.

class CurlDownloadStrategy

Removed my S3DownloadExtension module, as it was moved back into the original S3DownloadStrategy class.

class CurlBottleDownloadStrategy

Removed this class entirely since it's now covered by CurlDownloadStrategy with meta[:bottle] = true

class LocalBottleDownloadStrategy

This class is still used to allow the cached path to be specified during creation, so I left this as-is other than removing the stage behavior which was duplicated across both this and CurlBottleDownloadStrategy (it's now covered in AbstractDownloadStrategy with extend BottleExtension)

class S3DownloadStrategy

Moved the _fetch behavior back into this class, since this strategy is now valid for both regular formula urls as well as a bottle root_url
Also as per your comments, created a helper function load_aws_sdk which leverages Homebrew.install_gem_setup_path! to automatically install the aws-sdk-v1 gem.
Library/Homebrew/constants.rb
To match the other uses of Homebrew.install_gem_setup_path!, I've added a new constant to specify the gem version for the aws-sdk-v1 gem (used pessimistic matching to match any version within 1.x)
Library/Homebrew/software_spec.rb

class Bottle

I now specify @resource.specs[:bottle] = true when creating the resource, so that the DownloadStrategy will end up extending with BottleExtension
Updated the private helper select_download_strategy so that it will default to CurlDownloadStrategy, since this strategy now works for bottles and non-bottles (depending on the meta bottle flag).
Library/Homebrew/formulary.rb

class BottleLoader

This is the most confusing part, because this code wasn't getting exercised when I was making the change, but I found the reference to CurlBottleDownloadStrategy here, so I updated it to instead use CurlDownloadStrategy with a resource that has specs[:bottle] = true.
What I'm not sure of is the context in which this gets called??
New Example
Updated example usage, notice that I can now use S3DownloadStrategy for both the url and for bottle root_url:
class MyApp < Formula
  desc "MyOrg's MyApp"
  homepage "https://github.com/myorg/myapp"
  url "https://myorg-private-source-dist.s3.amazonaws.com/myapp/1.0/my-app-1.0.0.tar.gz", :using => S3DownloadStrategy
  version "1.0.0"
  sha256 "big-long-sha"

  bottle do
    root_url "https://myorg-private-bin-repo.s3.amazonaws.com/myapp/1.0", :using => S3DownloadStrategy
    cellar :any_skip_relocation
    sha256 "big-long-sha" => :high_sierra
  end

  def install
    system "./configure", "--disable-silent-rules",
                          "--prefix=#{prefix}"
    system "make", "install"
  end

  test do
    true
  end
end

Great work @eedwards-sk, I really like your approach and glad I encouraged this as it's a massive improvement. Would love to see a few tests to cover this behaviour.

Thanks for the feedback. I'm currently working on adding tests. Unfortunately, there weren't any tests for S3DownloadStrategy before, at all, so as I'm attempting to add coverage I'm starting to question the sanity of the current strategy. It feels a bit difficult to test...
We're basically putting a contract on S3DownloadStrategy#fetch that if the aws sdk gem is not installed, it will get installed.
However, testing this is extremely difficult. I don't want to mock a bunch of the internals with fragile tests, I just want to validate the contract.
Ideally I'd be able to set up a clean, repeatable state e.g. given an environment without the aws sdk gem, confirm that after calling subject._fetch, it gets installed. (except it would get installed once and the state would be dirty...)
Since so much of the 'gem environment' is dynamic and set up by invoking through brew tests, I can't really 'mock' the environment.
To make things even more confusing, the dynamic install and immediate require works when running directly through a brew install, but when running through rspec (well, brew tests) it fails to require the gem after installation:
code
def _fetch
...
    require "rubygems"
    aws_sdk_s3 = "aws-sdk-s3"
    Homebrew.install_gem_setup_path! aws_sdk_s3, HOMEBREW_AWS_SDK_S3_VERSION, nil
    require aws_sdk_s3

rspec
     Failure/Error: subject._fetch

     LoadError:
       cannot load such file -- aws-sdk-s3

Finally, to properly create aws sdk stubbed responses, I'll need the aws sdk gem (now the aws-sdk-s3 gem) in the rspec environment, which means adding it to the test/Gemfile... doing this also causes the gem to already be installed into the vendored ruby's gems, again ruining the ability to test if the dynamic install will work.
I guess it would also help if there was already coverage for Homebrew.install_gem_setup_path!, as I really don't want to test whether or not it actually works, I just want to ensure that our _fetch method will call it.
Basically, trying to figure out if I need to extract something here into a separate util method or not. The struggle with that is doing it and making it testable while maintaining the 'dynamic gem installation' behavior.
I'll keep hacking away at it, but if you have any thoughts please feel free to share.

The gem environments are definitely changing in a way I'm not clear on.
If I add a line with
raise RuntimeError, `gem list`

Immediately after the call to Homebrew.install_gem_setup_path!
and then I run a brew install...
==> Installing or updating 'aws-sdk-s3' gem
Fetching: jmespath-1.3.1.gem (100%)
Successfully installed jmespath-1.3.1
Fetching: aws-partitions-1.68.0.gem (100%)
Successfully installed aws-partitions-1.68.0
Fetching: aws-sigv4-1.0.2.gem (100%)
Successfully installed aws-sigv4-1.0.2
Fetching: aws-sdk-core-3.17.0.gem (100%)
Successfully installed aws-sdk-core-3.17.0
Fetching: aws-sdk-kms-1.5.0.gem (100%)
Successfully installed aws-sdk-kms-1.5.0
Fetching: aws-sdk-s3-1.8.2.gem (100%)
Successfully installed aws-sdk-s3-1.8.2
6 gems installed
Error: aws-partitions (1.68.0)
aws-sdk-core (3.17.0)
aws-sdk-kms (1.5.0)
aws-sdk-s3 (1.8.2)
aws-sigv4 (1.0.2)
bigdecimal (1.2.8)
bundler (1.16.1)
byebug (10.0.0)
CFPropertyList (2.2.8)
coderay (1.1.2)
did_you_mean (1.0.0)
io-console (0.4.5)
jmespath (1.3.1)
json (1.8.3)
libxml-ruby (2.9.0)
method_source (0.9.0)
minitest (5.8.5)
net-telnet (0.1.1)
nokogiri (1.5.6)
power_assert (0.2.6)
pry (0.11.3)
pry-byebug (3.6.0)
psych (2.1.0)
rake (10.4.2)
rdoc (4.2.1)
sqlite3 (1.3.11)
test-unit (3.1.5)

but shortly thereafter, as it thinks that Bottle installation failed: building from source., it ends up using S3DownloadStrategy for the source, as well, which when it dumps gem list results in:
Error: aws-partitions (1.68.0)
aws-sdk-core (3.17.0)
aws-sdk-kms (1.5.0)
aws-sdk-s3 (1.8.2)
aws-sigv4 (1.0.2)
bigdecimal (1.2.8)
io-console (0.4.5)
jmespath (1.3.1)
json (1.8.3)
psych (2.1.0)
rdoc (4.2.1)

Much different already, even in the same run!
Then if I run it from brew tests:
     Failure/Error: subject._fetch

     RuntimeError:
       ast (2.3.0)
       bundler (1.16.1)
       codecov (0.1.10)
       diff-lcs (1.3)
       docile (1.1.5)
       json (2.1.0)
       parallel (1.12.0)
       parallel_tests (2.17.0)
       parser (<IP_ADDRESS>)
       powerpack (0.1.1)
       rainbow (3.0.0)
       rspec (3.6.0)
       rspec-core (3.6.0)
       rspec-expectations (3.6.0)
       rspec-its (1.2.0)
       rspec-mocks (3.6.0)
       rspec-retry (0.5.6)
       rspec-support (3.6.0)
       rspec-wait (0.0.9)
       rubocop (0.52.1)
       ruby-progressbar (1.9.0)
       simplecov (0.15.1)
       simplecov-html (0.10.2)
       unicode-display_width (1.3.0)
       url (0.3.2)

Which is totally different. So I thought, hmm... maybe a difference in environment? Well, let's switch the bail to gem environment...
RubyGems Environment:
  - RUBYGEMS VERSION: 2.5.2
  - RUBY VERSION: 2.3.3 (2016-11-21 patchlevel 222) [universal.x86_64-darwin17]
  - INSTALLATION DIRECTORY: /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0
  - USER INSTALLATION DIRECTORY: /Users/eedwards/.gem/ruby/2.3.0
  - RUBY EXECUTABLE: /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin/ruby
  - EXECUTABLE DIRECTORY: /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/bin
  - SPEC CACHE DIRECTORY: /Users/eedwards/.gem/specs
  - SYSTEM CONFIGURATION DIRECTORY: /Library/Ruby/Site
  - RUBYGEMS PLATFORMS:
    - ruby
    - universal-darwin-17
  - GEM PATHS:
     - /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0
  - GEM CONFIGURATION:
     - :update_sources => true
     - :verbose => true
     - :backtrace => false
     - :bulk_threshold => 1000
  - REMOTE SOURCES:
     - https://rubygems.org/
  - SHELL PATH:
     - /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/bin
     - /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin
     - /usr/bin
     - /bin
     - /usr/sbin
     - /sbin
     - /usr/local/Homebrew/Library/Homebrew/shims/scm

It's identical in all 3 cases, except for      - /usr/local/Homebrew/Library/Homebrew/shims/... which is scm for the first output during brew tests, which then changes to super for the second output. Through rspec, it's scm again. That's the only difference, is that related or a red herring?

I've tracked down part of the issue, when running brew tests, you're inside a 'bundle', and the newly installed gems inside the 'homebrew setup path' aren't seen, as they are rejected for not being a part of the 'bundle'.
The environments, otherwise, appear about the same.
I'm going to investigate if there's a way that I can get the bundled environment that rspec runs within to recognize the new gem after it's installed.
Once I can validate that, then I can probably start to extract away some of the 'install gem setup path' behavior into separate methods.
Then I can implement testing around 'dynamically install a gem into ANY gem repo' (thus setup and validate against an isolated test repo), and just make sure that 'install gem setup path' calls those functions with the correct args.

I've now confirmed that when building from source, due to the way that the environment is sanitized, that we cannot easily automatically install a gem (it fails).
I will do some minor digging into seeing if there's a simple way to make it work, otherwise I will likely back out from attempting to use install_gem_setup_path! which is seeming to be not the ideal solution for this use case (and I will go back to the prior behavior that was implemented, which requires the gem to already be installed).
To reiterate, the following issues crop up due to attempting to dynamically install a gem:

The bottle fetching environment is different from the build environment

installing the gem inside the bottling phase means it will work in the build phase, BUT...
installing the gem in the build phase does not work

rspec runs within a bundle, meaning the gem list is limited to those in the Gemfile it's using

running with a bundle.clean_env doesn't fix it, since bundle already overwrote a bunch of Gem commands and expects a Gemfile
running a bundle install wouldn't work, since it needs to occur after the gem is installed, but before it's required... it also wouldn't propagate to the build run

@MikeMcQuaid It unfortunately does appear that Homebrew.install_gem_setup_path! cannot be a solution for this use case, as it, during the build phase, runs into Mac System Integrity Protection (SIP) issues, which appears to be similar to #3849
==> Installing or updating 'aws-sdk-s3' gem
Error: Operation not permitted @ dir_s_mkdir - /Users/eedwards/.gem
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/fileutils.rb:253:in `mkdir'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/fileutils.rb:253:in `fu_mkdir'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/fileutils.rb:227:in `block (2 levels) in mkdir_p'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/fileutils.rb:225:in `reverse_each'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/fileutils.rb:225:in `block in mkdir_p'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/fileutils.rb:211:in `each'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/fileutils.rb:211:in `mkdir_p'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/source.rb:157:in `fetch_spec'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/resolver/api_specification.rb:77:in `spec'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/resolver/installer_set.rb:78:in `add_always_install'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/dependency_installer.rb:474:in `resolve_dependencies'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/commands/install_command.rb:239:in `install_gem'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/commands/install_command.rb:300:in `block in install_gems'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/commands/install_command.rb:296:in `each'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/commands/install_command.rb:296:in `install_gems'
/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/commands/install_command.rb:203:in `execute'
/usr/local/Homebrew/Library/Homebrew/utils.rb:227:in `install_gem_setup_path!'
/usr/local/Homebrew/Library/Homebrew/download_strategy.rb:507:in `install_aws_s3_sdk'
/usr/local/Homebrew/Library/Homebrew/download_strategy.rb:476:in `initialize'
/usr/local/Homebrew/Library/Homebrew/resource.rb:59:in `new'
/usr/local/Homebrew/Library/Homebrew/resource.rb:59:in `downloader'
/usr/local/Homebrew/Library/Homebrew/resource.rb:137:in `fetch'
/usr/local/Homebrew/Library/Homebrew/resource.rb:90:in `stage'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1872:in `stage'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1090:in `brew'
/usr/local/Homebrew/Library/Homebrew/build.rb:111:in `block in install'
/usr/local/Homebrew/Library/Homebrew/utils.rb:552:in `with_env'
/usr/local/Homebrew/Library/Homebrew/build.rb:108:in `install'
/usr/local/Homebrew/Library/Homebrew/build.rb:189:in `<main>'

from ps during a run:
73889 ttys003    0:00.04 /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/build.rb /usr/local/Homebrew/Library/Taps/myorg/homebrew-private/Formula/my-app-1.0.rb --debug

So the issues I'm having with using install_gem_setup_path! in the build phase don't seem to be SIP related after all, but are likely due to me now realizing that it's using /usr/bin/sandbox-exec when it runs, which likely is stopping it from being able to write to the user directory.

@MikeMcQuaid
Please see latest changes :)
I have refactored S3DownloadStrategy to use the latest AWS SDK Gem (v3), and more specifically, to use their new 'service level' gem, which allows you to require only the gem for the specific service you're using. Thus in this case, it now requires aws-sdk-s3.
In addition, this gem has been added to the test Gemfile, to satisfy the rspec runs, but it is still not a requirement for regular brew use. (Added the v1 gem to the Gemfile would have lead to a conflict on the current json version and a bunch more gem version changes, so using the newer gem actually has a much smaller impact!)
As there was no test coverage for this strategy before, I've now added unit test coverage. To do so I had to split apart the behavior a bit. It probably could use another refactor pass later at some point, or even moving some of the AWS / S3 related behavior to dedicated classes with their own testing. However, this should at least ensure the initial contract cases are covered.
TODO: Add integration testing for the Bottle / not bottle behavior in AbstractDownloadStrategy

Added unit tests for the bottle / pourable behavior in AbstractDownloadStrategy.
Please review for a possible merge.
Let me know if you have any questions, comments, concerns, etc. :)
→ brew style
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
Inspecting 668 files
<snip>

668 files inspected, no offenses detected

→ brew tests
Randomized with seed 33573
8 processes for 223 specs, ~ 27 specs per process
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

Finished in 17.49 seconds (files took 2.04 seconds to load)
452 examples, 0 failures, 3 pending

Finished in 31.49 seconds (files took 1.89 seconds to load)
232 examples, 0 failures, 1 pending

Finished in 31.9 seconds (files took 1.92 seconds to load)
278 examples, 0 failures, 1 pending

Finished in 59.18 seconds (files took 1.9 seconds to load)
234 examples, 0 failures

Finished in 1 minute 0.33 seconds (files took 1.89 seconds to load)
259 examples, 0 failures

Finished in 1 minute 0.8 seconds (files took 1.8 seconds to load)
289 examples, 0 failures

Finished in 1 minute 14.52 seconds (files took 2 seconds to load)
301 examples, 0 failures, 2 pending

Finished in 1 minute 46.43 seconds (files took 1.88 seconds to load)
229 examples, 0 failures, 3 pending

2274 examples, 0 failures, 10 pendings

Took 108 seconds (1:48)

I noticed that build was failing, discovered a bug in the original implementation of S3DownloadStrategy that was not surfaced until I added tests.
The behavior that was attempting to use object_summary.public_url would never have been exercised in the real world.
Credentials are actually required to create an s3 object summary, thus you wouldn't be able to instantiate an object summary and call public_url on it.
Thus I've defaulted the behavior in that case to instead just use the url as-is, as it's the same url you would get back from the 'authenticated' public_url method, anyway.
Theoretically the only use case for this behavior would be specifying a download with :using => S3DownloadStrategy, but the user doesn't have any aws credentials configured, and the resulting S3 url ends up being public anyway so it didn't need them.
That is a rare use case though, since if it's a public S3 url you want to just use the normal CURL downloader, instead (and thus not use :using => S3DownloadStrategy).

So the issues I'm having with using install_gem_setup_path! in the build phase don't seem to be SIP related after all, but are likely due to me now realizing that it's using /usr/bin/sandbox-exec when it runs, which likely is stopping it from being able to write to the user home directory.

How about trying to install it in advance e.g. in formula_installer.rb?

@eedwards-sk Any news on this?

@MikeMcQuaid thanks for following up, I've been busy at work and let this slip :)
I'll jump on it tonight. I think I'm going to skip trying to install the gem automagically and leave that for a follow-up PR (e.g. maybe @amancevice wants to take a stab at it?)

No worries and thanks both @eedwards-sk and @amancevice!

Myanmar

@MikeMcQuaid
Added latest changes with your suggestions. Note that I'm getting a failing test to something unrelated when I run brew tests --online, so I guess we'll see if it fails in travis.
As an aside, if I can get on my soapbox for a moment: this entire approach for testing and utilizing the S3 library feels like a total smell.
The tests I added, while adding some coverage, are basically just testing the implementation, and with the amount of mocking needed to not actually exercise the s3 library, I'm basically mocking the implementation, too.
I think a more holistic downstream improvement would include:

moving the s3 utilization into a 'service' class
use dependency injection to provide the service
mock the service during tests
write tests for the service, as well

The last line (tests for the service) would actually require a dependency on the s3 gem. I honestly don't see how you can expect to utilize a 3rd party library and then not include it when testing it. Without that, you're not actually exercising their contract AT ALL, so the tests are going to hide any changes in their API, which feels like a smell.
I'd like to see an 'officially blessed' approach to using 3rd party libraries or gems, maybe to make this an officially supported concept where we can treat bringing in gems to the 'brew repo' as a first-class concept. As of now it feels more like a hack if we proceed with trying to auto-install them.
Anyway, that's enough of my soap box. I hope this should at least take what already existed, improve it, and leave it easier to improve next time.

The check failure appears to be completely unrelated to this PR. Is master currently broken?

Adding to @eedwards-sk’s soapbox: it may have made more sense to avoid installing the aws-sdk-v1 gem because it pulled in some heavy requirements (hello, nokogiri), but the newer aws-sdk-s3 gem is much slimmer. It only requires a handful of lightweight gems, mostly other aws stuff.

I think a more holistic downstream improvement would include

These seem overkill to me, I'm afraid. Additional indirect and abstraction makes the code harder to read and in my general experience integration tests catch far more than unit tests do.

The last line (tests for the service) would actually require a dependency on the s3 gem. I honestly don't see how you can expect to utilize a 3rd party library and then not include it when testing it. Without that, you're not actually exercising their contract AT ALL, so the tests are going to hide any changes in their API, which feels like a smell.

I agree. That said I'm not sure how you can test a dependency without actually accessing S3. I'd be 👍 on something that made brew tests --online actually download something small from a public S3 bucket (e.g. one of Amazon's).

I'd like to see an 'officially blessed' approach to using 3rd party libraries or gems, maybe to make bringing in gems to the 'brew repo' a first-class concept. As of now it feels more like a hack if we proceed with trying to auto-install them.

This won't happen I'm afraid. The only reason we're allowing it on this download strategy is because it's a pretty niche use-case. It's a feature rather than a bug that Homebrew works without requiring any non-vendored gems.

but the newer aws-sdk-s3 gem is much slimmer. It only requires a handful of lightweight gems, mostly other aws stuff.

If it's really slim we can consider vendoring it in Library/Homebrew/vendor/.

Aside from the above this PR and the others are all looking good and thanks for all the work, folks. There's merge conflicts here that need resolved but they should be the last ones.

Bah! There were no merge conflicts you just waited too long to merge :P
I'll fix this today.

Sorry, @eedwards-sk this is because of my contribs.
FYI, You shouldn't need to require the aws-sdk-s3 gem in the S3DownloadStrategy anymore.
Also, feel free to overwrite my changes to continue using the Aws::S3::ObjectSummary method if you have reasons, but I think the Aws::S3::Presigner method is simpler.
Lastly, you may want to move the "Bad S3 URL" error up in the stack so it doesn't need to be run twice (once for the bucket parse & once for the key parse). This would also make it easier to add s3://-style URLs to the mix.

@amancevice Would have been much easier if you had waited for this improvement before getting yours merged.
I can't commit to making those changes soon, I may need to revisit this later. Maybe you should take over the PR.

@MikeMcQuaid This was ready to be merged with no conflicts prior to you merging their PR which then blocked it...
As I said above, I expected the gem installation improvement to be a follow-up PR:

and leave that for a follow-up PR (e.g. maybe @amancevice wants to take a stab at it?)

Thus my point. Appreciate the improvements either way, but I believe my point is valid. That's all I'll say on the topic. Thanks.

This was ready to be merged with no conflicts prior to you merging their PR which then blocked it...

I did not consider it ready to be merged yet or I would have merged it, sorry.

@eedwards-sk the following should resolve the conflicts:
Rebase from master:
git rebase master -s recursive -X theirs

Download GH-3838.txt to ~/GH-3838.txt
Apply the diff:
git apply ~/GH-3838.txt

Review & commit the diff, then force-push onto your feature branch. That should resolve the conflicts. It's up to you if you want to make the changes I recommended above.

@amancevice The scope of this PR was originally quite simple but was ballooned excessively over time due to asks for 'nice to haves', so at this point I am not interested in iterating on it any further until we get the core improvement (bottle support) merged.
If this work is not mergeable as-is then I wholly support anyone iterating on it further as necessary, but I won't have time to address it again for at least 2 weeks.

The scope of this PR was originally quite simple but was ballooned excessively over time due to asks for 'nice to haves', so at this point I am not interested in iterating on it any further until we get the core improvement (bottle support) merged.

@eedwards-sk I've tried to show some restrain so far as you're a first-time contributor and we appreciate your contribution but to be clear: this PR was not merged because it was not ready and all the things you consider "nice to haves" I consider requirements to merge this functionality (which is why it was not merged). You appear to be relatively new to GitHub open source contribution and I hope you continue to do more but arguing with maintainers about how they run their project and criticising other contributors on your first PR is not a recipe for a productive interaction.

If this work is not mergeable as-is

It's not. If you look at the diff it's pretty obviously now including changes that are currently in the master branch.

@MikeMcQuaid

this PR was not merged because it was not ready and all the things you consider "nice to haves" I consider requirements to merge this functionality (which is why it was not merged)

The feature I've touched here was not my creation -- this is already part of homebrew, and is missing tests and is basically half baked (working only on source URLs and not bottles).
If having it 'auto install the gem' and adding more coverage were required functionality then the initial PR(s) that added S3DownloadStrategy should have been addressed as such.
It's also no longer clear what the actual requirements are to get this merged, you've simply said twice you would not merge it without explaining specifically.
Please keep that context in mind when considering why I might be frustrated.

basically half baked (working only on source URLs and not bottles).

That's not half-baked, that's something that applies to literally every download strategy.

If having it 'auto install the gem' and adding more coverage were required functionality then the initial PR(s) that added S3DownloadStrategy should have been addressed as such.

This is not how open source works. Your changes revealed a deficiency (in code which you did have to change) which needed addressed before this was ready to merge.

It's also no longer clear what the actual requirements are to get this merged, you've simply said twice you would not merge it without explaining specifically.

That's because you've said how busy you are and are unwilling to make further changes for several weeks for this PR so I didn't see the point in doing so until the existing issues (merge conflicts) had been addressed.
Again, advice on contributing to open source: the maintainers are need to review and merge (and create) more PRs than you are contributing so please try not to waste their time or your time arguing with them and telling them they are wrong.


Typescript Code Lens is shown above all type declarations

VS 1.55.0, TS stable & nightly

export const foo = 1;

const bar = 2;

export type foo = 1;

type bar = 2;

Expected no Code Lens above type bar
Expected Code Lens behavior to be the same for all declarations

This is by-design. We do the same with classes and interfaces, even if they are only used in the current file
We don't do this on locals (non-exported) variables because it would be very noisy


Regarding the topic of switching frameworks

In the section *2. Change to a different frontend Web framework the first section talks about Node.js. This is the first time (I think) that we mention Node. Do we need to? Is it clear why we're doing this? Will the educators be clear?
Immediately after this we show how to switch from React to another framework by removing files. Will they know what files to remove?  It might take some further discussion to figure out the best way to do this ... or perhaps for now, we don't show this?

I appreciate you mentioning this @johnpapa! It is a great point, and while going through the material again to ensure an educator focus, it occurred to me that I missed noting the included linter. I have now replaced the step to change the framework to how to adjust the linter rules. This felt to be a better fit for educators as it would allow them to set code rules and quality for students' projects. As always, feedback and thoughts on this change are appreciated!

@johnpapa closing this issue out since I have this now replaced with section with a linter step, and in prep to get this repo transferred. Look forward to continuing to iterate on this as we get feedback. 😊


Receive's memory usage continues to grow in v0.31.0-rc.0

Thanos, Prometheus and Golang version used:
Thanos: v0.31.0-rc.0
Object Storage Provider:
S3
What happened:
I went from 0.30.2 to the new version around 8 o'clock and noticed that the memory kept growing until I rolled back.

Args:
receive
--log.level=info
--log.format=logfmt
--grpc-address=<IP_ADDRESS>:10901
--http-address=<IP_ADDRESS>:10902
--remote-write.address=<IP_ADDRESS>:19291
--objstore.config=$(OBJSTORE_CONFIG)
--tsdb.path=/var/thanos/receive
--label=thanosreplica="$(NAME)"
--label=receive="true"
--tsdb.retention=1d
--receive.local-endpoint=$(NAME).$(NAMESPACE).svc.cluster.local:10901
--receive.grpc-compression=snappy
--tsdb.out-of-order.time-window=1h
--store.limits.request-samples=1000
--store.limits.request-series=10000

Would you mind posting a graph of head series and samples ingested for the same time period?
The metrics are prometheus_tsdb_head_series and prometheus_tsdb_head_samples_appended_total.

I'm afraid we didn't collect these metrics, the figure below is the series_count_by_metricname data in the /api/v1/status/tsdb, which is actually not a big increase compared to 0.30.2.

@PhilipGough @saswatamcode would it be possible to test the RC with your load testing framework to see if there's a memory regression with Thanos itself?

@yutian1224 do you know if there might have been queries being executed in your cluster that could be touching the "hot data" in Receives?
You said you rolled back, but the right edge of the chart still shows a trend upwards. How's the memory usage since you rolled back?

@douglascamata
Our receive is mainly used for grafana chart query and alarm settings. I don't know if the "hot data" here refers to the alarm part.
The alarm query usually has a fixed interval and is continuous.
As shown in the figure below, the memory usage before updating 0.31.0 and after rolling back is relatively stable.

@fpetkovski I wont get a chance to do so this week due to other commitments but I can check next week.
What I can say now is that we are already running the RC in one of our production environments and we are not seeing the issues reported here

@PhilipGough I tested the instance without the --store.limit parameter and the results were stable.
So I suspect it is the problem caused by --store.limit

@yutian1224 interesting, thanks for confirming. We were indeed running the RC without those limit flags.
cc @fpetkovski

I can test later this week, thanks for looking into it.

Interesting, @fpetkovski I assume it then could be https://github.com/thanos-io/thanos/pull/6074, I just realized we only added those flags this release.
Would be interesting to see a profile, where all the memory is being hogged.

I enabled these flags in our staging environment but could not reproduce the described memory issue. @yutian1224 are you able to reproduce this problem consistently?

@fpetkovski Yes, except for the first time, I also made a comparison before and after removing the limit flag, and this problem was also reproduced.
So will it might be a problem of large amounts of data? At present, each receive instance in our environment receives about 3 million series.

3M series should not be that much data. Would you mind providing a heap profile when you reproduce the issue? You can get it by hitting the /debug/pprof/heap endpoint on the receiver port 10902.

@fpetkovski Sure, I'll test it on the weekend

I enabled these two flags in staging and ran Receivers for about a day. I cannot reproduce the memory leak so I think we can release 0.31.0 as it is now. Once we have the heap profile we can check whether the limits are the culprit of this and cut 0.31.1 if necessary.

Sounds good to me, thanks for checking @yutian1224 and @fpetkovski 👍

@fpetkovski I deployed 0.31.0 and enabled limit flags yesterday, the memory problem reappeared.
The zip file is the pprof heap file captured when the memory usage was about 48%.

receive.prof.zip

FYI I took the profile and uploaded to this web visualization tool: https://flamegraph.com/share/7c78f5a0-cfa9-11ed-9b0d-d641223b6af4.
I'm not sure what's the problem, but github.com/thanos-io/thanos/pkg/receive.newReplicationErrors caught my attention: 7.6 GB of heap there. 🤔

I wonder is there some contention caused by these low read limits on receivers that is effecting the ingestion path.
@yutian1224  Can you confirm if the limits were being hit or not? Can you increase your previous limits * 100 and see if the problem remains?

@PhilipGough I am pretty sure the limits were being hit.
The right side is the network traffic at that time. After adding the limit, you can see that the out traffic has decreased significantly, but the in traffic has not changed significantly.

This is really interesting. Is the yellow line outgoing traffic and why is it negative?

@fpetkovski
For the convenience of display, we display the incoming and outgoing traffic in one panel, which is represented by positive and negative.😄


Request new module: cp-truststore

Related user story
Provide the issue number for the user story related to this request - https://github.ibm.com/gsi-labs/industry-cloud-planning/issues/550
Type of module
gitops
If requesting a terraform module, what is the target cloud platform?
N/A
What is the purpose of the module?
Please describe what this automation module will do
What is the category of the module?
infrastructure

/displayName CP TrustStore


mfa for multi speaker.

In the code, group MFA inputs for better parallelism. For multi speaker,  it maybe go wrong.
For input g_uang3 zh_ou1 n_v3 d_a4 x_ve2 sh_eng1 d_eng1 sh_an1 sh_i1 l_ian2 s_i4 t_ian1 j_ing3 f_ang1 zh_ao3 d_ao4 i2 s_i4 n_v3 sh_i1.
The TexGrid is
	item [1]:
		class = "IntervalTier"
		name = "words"
		xmin = 0.0
		xmax = 9.4444
		intervals: size = 56
			intervals [1]:
				xmin = 0
				xmax = 0.5700000000000001
				text = ""
			intervals [2]:
				xmin = 0.5700000000000001
				xmax = 0.61
				text = "eng"
			intervals [3]:
				xmin = 0.61
				xmax = 0.79
				text = "s_an1"
			intervals [4]:
				xmin = 0.79
				xmax = 0.89
				text = "eng"
			intervals [5]:
				xmin = 0.89
				xmax = 1.06
				text = "i1"
			intervals [6]:
				xmin = 1.06
				xmax = 1.24
				text = "eng"
			intervals [7]:
				xmin = 1.24
				xmax = 1.3
				text = ""
			intervals [8]:
				xmin = 1.3
				xmax = 1.36
				text = "s_an1"
			intervals [9]:
				xmin = 1.36
				xmax = 1.42
				text = ""
			intervals [10]:
				xmin = 1.42
				xmax = 1.49
				text = "eng"
			intervals [11]:
				xmin = 1.49
				xmax = 1.67
				text = "s_i4"
			intervals [12]:
				xmin = 1.67
				xmax = 1.78
				text = "eng"
			intervals [13]:
				xmin = 1.78
				xmax = 1.91
				text = ""
			intervals [14]:
				xmin = 1.91
				xmax = 1.96
				text = "er4"
			intervals [15]:
				xmin = 1.96
				xmax = 2.06
				text = "eng"
			intervals [16]:
				xmin = 2.06
				xmax = 2.19
				text = ""
			intervals [17]:
				xmin = 2.19
				xmax = 2.35
				text = "i1"
			intervals [18]:
				xmin = 2.35
				xmax = 2.53
				text = "eng"
			intervals [19]:
				xmin = 2.53
				xmax = 3.03
				text = "i1"
			intervals [20]:
				xmin = 3.03
				xmax = 3.42
				text = "eng"
			intervals [21]:
				xmin = 3.42
				xmax = 3.48
				text = "i1"
			intervals [22]:
				xmin = 3.48
				xmax = 3.6
				text = ""
			intervals [23]:
				xmin = 3.6
				xmax = 3.64
				text = "eng"
			intervals [24]:
				xmin = 3.64
				xmax = 3.86
				text = "i1"
			intervals [25]:
				xmin = 3.86
				xmax = 3.99
				text = "eng"
			intervals [26]:
				xmin = 3.99
				xmax = 4.59
				text = ""
			intervals [27]:
				xmin = 4.59
				xmax = 4.869999999999999
				text = "er4"
			intervals [28]:
				xmin = 4.869999999999999
				xmax = 4.9799999999999995
				text = "eng"
			intervals [29]:
				xmin = 4.9799999999999995
				xmax = 5.1899999999999995
				text = "s_i4"
			intervals [30]:
				xmin = 5.1899999999999995
				xmax = 5.34
				text = ""
			intervals [31]:
				xmin = 5.34
				xmax = 5.43
				text = "eng"
			intervals [32]:
				xmin = 5.43
				xmax = 5.6
				text = ""
			intervals [33]:
				xmin = 5.6
				xmax = 5.76
				text = "i1"
			intervals [34]:
				xmin = 5.76
				xmax = 6.279999999999999
				text = "eng"
			intervals [35]:
				xmin = 6.279999999999999
				xmax = 6.359999999999999
				text = "s_an1"
			intervals [36]:
				xmin = 6.359999999999999
				xmax = 6.47
				text = ""
			intervals [37]:
				xmin = 6.47
				xmax = 6.6
				text = "eng"
			intervals [38]:
				xmin = 6.6
				xmax = 6.9399999999999995
				text = "i1"
			intervals [39]:
				xmin = 6.9399999999999995
				xmax = 7.039999999999999
				text = "eng"
			intervals [40]:
				xmin = 7.039999999999999
				xmax = 7.289999999999999
				text = "s_an1"
			intervals [41]:
				xmin = 7.289999999999999
				xmax = 7.369999999999999
				text = "eng"
			intervals [42]:
				xmin = 7.369999999999999
				xmax = 7.6
				text = "s_i4"
			intervals [43]:
				xmin = 7.6
				xmax = 7.699999999999999
				text = "eng"
			intervals [44]:
				xmin = 7.699999999999999
				xmax = 7.869999999999999
				text = ""
			intervals [45]:
				xmin = 7.869999999999999
				xmax = 8.049999999999999
				text = "er4"
			intervals [46]:
				xmin = 8.049999999999999
				xmax = 8.26
				text = ""
			intervals [47]:
				xmin = 8.26
				xmax = 8.299999999999999
				text = "eng"
			intervals [48]:
				xmin = 8.299999999999999
				xmax = 8.36
				text = "s_i4"
			intervals [49]:
				xmin = 8.36
				xmax = 8.389999999999999
				text = ""
			intervals [50]:
				xmin = 8.389999999999999
				xmax = 8.42
				text = "eng"
			intervals [51]:
				xmin = 8.42
				xmax = 8.45
				text = ""
			intervals [52]:
				xmin = 8.45
				xmax = 8.59
				text = "s_an1"
			intervals [53]:
				xmin = 8.59
				xmax = 8.83
				text = ""
			intervals [54]:
				xmin = 8.83
				xmax = 9.1
				text = "eng"
			intervals [55]:
				xmin = 9.1
				xmax = 9.44
				text = "i1"
			intervals [56]:
				xmin = 9.44
				xmax = 9.4444
				text = ""

The mfa code is borrowed from the official repo of NATSpeech. Unfortunately I have not studied the mfa module in depth. We encourage you to push your commits on multi-speaker chinese corpus. Thanks a lot!

hello, I push a commit. But I don't know it suits for your planning. The parameter of mfa_group seems unnecessary. It doesn't prompt the train speed compared with wetts(it uses version 2.1.0). Also need add the parameter of spk_name.


Implement interchange timeout

I have recently encountered error states where the interchange gets stuck, leaving the main parsl process hanging. It's not completely obvious where the hangup is. It would be nice if the parsl process would raise an exception if it hasn't heard from the interchange within a configured timeout.

I don't think we have seen any hangs in the Interchange over the past couple months of testing. While it is possible that the Interchange could hang, the risk of that is pretty low. I vote to close this one unless anyone has seen any cases of hangs.


Console: upgrade JLine dependency

Upgrade JLine from 3.5.1 to e.g. 3.8.2 3.16,
And check if it's worth the effort to replace the bundled jar by a series of more fine grained jars (jline-terminal / jline-terminal-jansi / jline-reader / jline-style / jline-builtins / jline-console). This will require a few CQs

[x] JLine3 parent CQ 22622

Actually there already is an approved CQ for using 3.6.2 (CQ 16792), already an improvement

Replacing the bundled jar with smaller jars doesn't make that much (size) difference, but it would require more CQ's and entries in the pom / more jars, so probably not worth the effort...

Seems that ConvertTest bumps into issue https://github.com/jline/jline3/issues/477

See #1394


Error running with eBPF on Kernel >= 5.8.5

Describe the bug
Running Falco with BPF on kernels 8.5 and 9 result in runtime error

How to reproduce it
Compile Falco 0.26.1
git clone git@github.com:falcosecurity/falco.git
cd falco
git checkout tags/0.26.1 -b tag-0.26.1
mkdir build
cd build
cmake ../ \
      -DBUILD_BPF="ON" \
      -DBUILD_WARNINGS_AS_ERRORS="OFF" \
      -DCMAKE_BUILD_TYPE="Release" \
      -DCMAKE_INSTALL_PREFIX="/usr" \
      -DFALCO_ETC_DIR="/etc/falco" \
      -DUSE_BUNDLED_DEPS=ON
make bpf
make falco
sudo cp driver/bpf/probe.o /root/.falco/falco-bpf.o
sudo cp userspace/falco/falco /usr/bin/falco

Run Falco with eBPF
sudo FALCO_BPF_PROBE="" falco

Runtime Error
1178: (25) if r1 > 0x80 goto pc+679
 R0=inv(id=0) R1_w=inv(id=0,umax_value=128,var_off=(0x0; 0xff)) R2_w=inv(id=0) R3_w=map_value(id=0,off=0,ks=4,vs=1612,imm=0) R5_w=inv(id=0) R6_w=ctx(id=0,off=0,imm=0) R7_w=map_value(id=0,off=0,ks=4,vs=77,imm=0) R8=inv(id=0) R9=inv4294967294 R10=fp0 fp-72=mmmmmmmm fp-80=map_value fp-88=map_value fp-96=map_value fp-104=map_value fp-112=map_value fp-120=ctx fp-128=map_value fp-136=mmmmmmmm fp-144=mmmmmmmm fp-152=map_value fp-160=inv16 fp-168=mmmmmmmm fp-176=map_value fp-184=mmmmmmmm fp-192=mmmmmmmm
1179: (79) r1 = *(u64 *)(r10 -96)
1180: (bf) r3 = r8
1181: (85) call bpf_probe_read#4
R2 min value is negative, either use unsigned or 'var &= const'
processed 2228 insns (limit 1000000) max_states_per_insn 1 total_states 93 peak_states 93 mark_read 39
Wed Oct 28 10:40:39 2020: Runtime error: bpf_load_program() err=13 event=filler/sys_read_x message=0: (bf) r6 = r1
1: (b7) r1 = 0
2: (63) *(u32 *)(r10 -72) = r1
last_idx 2 first_idx 0
regs=2 stack=0 before 1: (b7) r1 = 0
3: (bf) r2 = r10
4: (07) r2 += -72
5: (18) r1 = 0xffff8e787d0b4400
7: (85) . Exiting.

Expected behaviour

Screenshots

Environment

Falco version: 0.26.1

System info:

{
  "machine": "x86_64",
  "nodename": "emily",
  "release": "5.9.1-arch1-1",
  "sysname": "Linux",
  "version": "#1 SMP PREEMPT Sat, 17 Oct 2020 13:30:37 +0000"
}

Cloud provider or hardware configuration:
OS: Arch

Kernel: Linux emily 5.9.1-arch1-1

Installation method: Compile from source

Additional context

I’ve been looking into this for a long time.
It is a bit more complex than just the kerne being >= 5.8.5 and also happens in older kernel with different kernel/clang combinations.
Let’s keep the discussion on the original one where the works is happening
https://github.com/falcosecurity/falco/issues/1251

I was able to fix this by using clang version 9.0.1
see  https://github.com/falcosecurity/falco/issues/1251#issuecomment-718324102 for instructions on how I was able to fix this


Karpenter nodes are not coming up

Karpenter nodes are not coming up, instead of having unscheduled pods.
karpenter version : v0.33
pod:
dcpui               serviceaccount-59cb5b648d-8vpw2                 0/1     Pending   0          23s
Events:
Type     Reason            Age   From               Message

Warning  FailedScheduling  54s   default-scheduler  0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod..
Normal   Nominated         53s   karpenter          Pod should schedule on: nodeclaim/default-zjrnm
sh-4.2$ kubectl get nodes
NAME                                         STATUS   ROLES    AGE   VERSION
ip-172-25-18-28.us-west-2.compute.internal   Ready       55m   v1.26.6-eks-a5565ad

Are the nodes ever joining the cluster? Have you looked at the instances after they are launched by the NodeClaim to see if the instance is getting terminated as soon as it's launched?

@jonathan-innis I am having same issue here: https://github.com/aws/karpenter-provider-aws/issues/6323. Can you kindly add your advice here


ValueError: Error when checking : expected input_1 to have shape (None, 128, 128, 1) but got array with shape (10, 128, 128, 3) using explanation = explainer.explain_instance(test_explaination, model.predict)

Hi,
I am trying to explain the predictions of my VGG19 model and get the error below:
KERNEL_SIZE = 3
NUM_CLASSES = 2
LEARN_RATE = 10e-4
EPOCHS = 15
DECAY = LEARN_RATE/EPOCHS
IMG_SIZE = 128
input_shape = [
128,
128,
1,
]
#model.fit --> VGG19
.
.
pred = Dense(NUM_CLASSES, activation = "softmax", name = 'predictions')(fc)
model = Model(inputs=inputs, outputs=pred, name='vgg19')
model.compile(loss = binary_crossentropy,optimizer=SGD(lr=LEARN_RATE,decay=DECAY,momentum=0.1,),
metrics = ['accuracy']
)
I am using one-hot
test_explain.shape = (128, 128) #gray scale 2D
explanation = explainer.explain_instance(test_explain, model.predict)
Traceback (most recent call last):
File "lime_test.py", line 211, in
explanation = explainer.explain_instance(test_explain, model.predict)
File ".../tensorflow/lib/python2.7/site-packages/lime/lime_image.py", line 187, in explain_instance
batch_size=batch_size)
File ".../tensorflow/lib/python2.7/site-packages/lime/lime_image.py", line 248, in data_labels
preds = classifier_fn(np.array(imgs))
File "/Library/Python/2.7/site-packages/Keras-2.0.5-py2.7.egg/keras/engine/training.py", line 1576, in predict
check_batch_axis=False)
File "/Library/Python/2.7/site-packages/Keras-2.0.5-py2.7.egg/keras/engine/training.py", line 139, in _standardize_input_data
str(array.shape))
ValueError: Error when checking : expected input_1 to have shape (None, 128, 128, 1) but got array with shape (10, 128, 128, 3)
I have no idea where this '10' comes from either the '3' , but got array with shape (10, 128, 128, 3)!

I currently have the same issue, I guess this is related. The comment mentions that this works now for shape=2 but is not necessary for shape=1 ... I dont really understand why, since my gray scale images usually have shape [x, y, 1]...

LIME generate instances in the neighborhood of the image you want to explain. The default batch size for those images is 10, which is where you get the 10 in (10, 128, 128, 3) from.
As per this line, we assume that the image is either 3d or 2d (in which case we convert it to 3d). This implies that the predict function must take a 3d RGB image with 3 channels in the last dimension (x, y, 3), while yours takes in (x, y, 1). A quick fix is to fold a conversion function into the predict function, something like:
def new_predict_fn(images):
    images = convert_to_1channel(images)
    return model.predict(images)

(and use this when explaining)

Can you write an example of a conversion function?

Hi! I can't do it either. I want to use explain_instance() on a grayscale image.
I've tried two ways :

Giving the image with shape (1, 70, 70, 1) and I have :
ValueError: the input array must be have a shape == (.., ..,[ ..,] 3)), got (1, 70, 70, 1)
Giving the image with shape (70, 70) and I have :
ValueError: Error when checking input: expected conv2d_4_input to have shape (70, 70, 1) but got array with shape (70, 70, 3)

Because my CNN is trained on grayscale images. How can I do the image conversion?

Could the image conversion affect the results of the explainer in some way?


Extending tests to ensure — and introducing — Windows and Mac OS compatibility..

Supersedes: https://github.com/bids-standard/bids-specification/pull/1295#event-7426140754

@effigies could you also add a patch increment release tag for BST after this is merged? Or can I also do that?

@effigies it was originally introduced for testing, but it finds some niche usage in DANDI as well for parsing entities in paths returned from objects without needing to download the files or even the symlinks (many datasets have tons of files, so even just creating placeholder files would take a while). I'm thinking it could have broader use as well in that paradigm.


Support for trimmable (ILLink) assemblies

Proposal: Support for trimmable (ILLink) assemblies
Summary
Trimming is a great feature introduced with .NET 5, but CsWinRT does not support it. This should be fixed, so assembly sizes can be reduced.
Rationale
Trimming with ILLink provides great results when publishing self contained applications. I have reduced the size of the Microsoft.Windows.SDK.NET assembly by a factor of 45, taking it from 25mb to 0.55mb! I see the same great result in other projected libraries as well.
Since WinUI is using CoreCLR, trimming will be an important tool to keep assemblies small. Same goes for Xamarin apps using the desktop SDK.
Important Notes
Issues I have encountered so far:

Internal constructors, used by internal factories, are incorrectly trimmed.
Missing interfaces and types in the ABI namespace. Perhaps an explicit interface implementation could fix this.

As you can expect, the reflection-based factories in the ABI namespace quickly break, however, these types can manually be preserved by whitelisting them using a TrimmerRootDescriptor.
ILLink can be enabled by specifying the following properties:
<PublishTrimmed>true</PublishTrimmed>
<TrimMode>link</TrimMode>

To whitelist the Windows SDK for trimming:
<ItemGroup>
    <ManagedAssemblyToLink Condition="'%(Filename)' == 'Microsoft.Windows.SDK.NET'">
        <IsTrimmable>true</IsTrimmable>
    </ManagedAssemblyToLink>
</ItemGroup>

Further reading: https://github.com/mono/linker/blob/main/docs/data-formats.md

Thanks for opening this @trympet.  We are aware of some of the issues that you mention and are tracking this with #373 which also has some changes we are considering to address them.

@manodasanW, awesome! I'll close this issue, since it's already being tracked.


Update README.md and CONTRIBUTING.md

updated both contributing and readme as per request

Thanks! :smiley:

Thanks. I've cherry-picked your commit from this PR to #77.


Fix Bootstrap on Mingw64 via MSYS2

Hey guys,
.
I'm trying to get ninja to build under mingw64 with a mingw64 compiled python from within msys2.
The issue is that if MSYSTEM is defined python even though it is a native windows exe will use '/' for paths instead of '' because it knows that it is running from within a msys shell.
The result is
[25/26] AR build/libninja.a
FAILED: cmd /c ar cqs build/libninja.a.tmp build/build.o build/build_log.o build/clean.o build/debug_flags.o build/depfile_parser.o build/deps_log.o build/disk_interface.o build/edit_distance.o build/eval_env.o build/graph.o build/graphviz.o build/lexer.o build/line_printer.o build/manifest_parser.o build/metrics.o build/state.o build/util.o build/version.o build/subprocess-win32.o build/includes_normalize-win32.o build/msvc_helper-win32.o build/msvc_helper_main-win32.o build/getopt.o && move /Y build/libninja.a.tmp build/libninja.a
The system cannot find the path specified.
[25/26] CXX build/ninja.o
ninja: build stopped: subcommand failed.
Mingw-python is a native windows exe just like cpython but it just has extra checks to support the posix layer.
Would it be possible to get ninja to check windows for an environment variable MSYSTEM at runtime and if it is set then use mv instead of move.
The result would be a windows native windows exe that works just as normal but can detect if it is in a posix layer and change to suit.
please look here for more info
https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-ninja/PKGBUILD
basically I want to remove CPython as a dependancy and use mingw-python instead from within the package manager.
Can someone tell me if this small change is possible?

This is not an issue anymore :)


C#: Callable.From() on async lambda throws a harmless exception

Tested versions

Reproducible in v4.3.stable.mono.official [77dcf97d8]

System information
Godot v4.3.stable.mono - macOS 15.0.1 - Vulkan (Forward+) - integrated Apple M3 Max - Apple M3 Max (14 Threads)
Issue description
Using an async lambda as a Callable throws a System.InvalidOperationException because it is attempting to convert the System.Threading.Tasks.Task to a Variant.
The code still runs fine, and no side-effects are present from that interaction, so I think the engine should treat a non-generic Task as a void value and not throw that exception.
I also checked that Godot gracefully handles this case internally, which is the case, as this Variant conversion is the very last thing that is done, and falls back to a default Variant value on failure.
https://github.com/godotengine/godot/blob/fd4c29a189e53a1e085df5b9b9a05cac9351b3ef/modules/mono/glue/GodotSharp/GodotSharp/Core/DelegateUtils.cs#L84-L94
https://github.com/godotengine/godot/blob/fd4c29a189e53a1e085df5b9b9a05cac9351b3ef/modules/mono/glue/GodotSharp/GodotSharp/Core/Callable.generics.cs#L279-L288
Steps to reproduce
Calling this code from a _Ready function is enough to exhibit this issue.
Callable.From(async () =>
{
    GD.Print("Waiting for delay");
    await ToSignal(GetTree().CreateTimer(1), "timeout");
    GD.Print("Delay complete");
}).Call();

Minimal reproduction project (MRP)
MRP-CallableFromException.zip

Task is not a Variant-compatible type. Removing this check would mean you'd run into issues when trying to use the type as a Variant (i.e.: trying to use Task with some Godot API).
The exception only seems harmless to you because you are not using the Variant returned from the Callable.Call invocation. The value returned from the Callable's delegate must be converted to Variant so it can be returned, but if the value can't be converted it throws. If the conversion didn't throw for incompatible values, we'd be potentially hiding a bug in your code.
Since you are not awaiting the returned Task, it sounds like you really just want an async void method here:
Callable.From(async void () =>
{
    GD.Print("Waiting for delay");
    await ToSignal(GetTree().CreateTimer(1), "timeout");
    GD.Print("Delay complete");
}).Call();

Oh wow; I completely forgot that async void lambdas were supported on .NET 6. Thank you for this solution, it's exactly what this use-case needs.


mdl-layout documentation and examples need clarification (breaks links otherwise)

http://www.getmdl.io/components/index.html#layout-section/layout
I tried to implement this on my site, which is powered by Angular and ngView. I added the transparent layout to my index.html, before the content from my Angular views. All buttons, links, etc, were unusable, except for the link to open up the mdl-layout drawer.
I feel like this may be because my code wasn't encapsulated within:
<main class="mdl-layout__content">
</main>

However, I didn't test to see if that was the case. Either way, the sample code on the website I shared above should be modified to indicate that your content is supposed to go in there. I will submit a pull request for that, as I think it is counter-intuitive to have to scroll all the way to the bottom to use the sample code.
On that note, perhaps these examples should be collapsed upon page load, and the actual information regarding how to use these components should be given higher priority.

The element with the class is shown in each example snippet. The element being necessary is also clearly stated in step 11 of usage plus the configuration table shows it as required.
What exactly should be improved?

I agree the Layout documentation is long and isn't obvious unless you know to look for the Configuration Options section at the bottom of each component. This page has 4 components one right after the other so each component's information blends into the next. I'd recommend adding additional sub-headings/links right to each component's Configuration Options or Examples or Basic Use or whatever... just something at the very top of the page to indicate there's more than just code examples on this page.
But, yes, the required <main> is clearly documented if you figure out how to get to the actual spec/documentation.

<main> actually isn't a requirement, just something with the .mdl-layout__content class. Unless I'm missing something.

Similar discussion at https://github.com/google/material-design-lite/issues/1381#issuecomment-130609508


Update scalafmt-core to 3.0.3

Updates org.scalameta:scalafmt-core from 3.0.2 to 3.0.3.
GitHub Release Notes - Version Diff
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.
Configure Scala Steward for your repository with a .scala-steward.conf file.
Have a fantastic day writing Scala!

Files still referring to the old version number
The following files still refer to the old version number (3.0.2).
You might want to review and update them manually.
build.sbt
rules/src/main/scala/fix/Enrichments.scala

Ignore future updates
Add this to your .scala-steward.conf file to ignore future updates of this dependency:
updates.ignore = [ { groupId = "org.scalameta", artifactId = "scalafmt-core" } ]

labels: library-update, semver-patch, old-version-remains

Superseded by #30.


Create FUNDING.yml

PR Checklist
Please check if your PR fulfills the following requirements:

[ ] The commit message follows our guidelines: https://github.com/graycoreio/daffodil/blob/master/CONTRIBUTING.md#commit
[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features)

PR Type
What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Fixes: N/A
What is the new behavior?
Does this PR introduce a breaking change?
[ ] Yes
[ ] No

Other information

doesn't this not do anything unless you add options? You just want to add the file for the future?

@lderrickable yes, I just want to add this file for the future.


Augment AST before generating module

Some of the AST nodes now have a functionEntry member that
provides a way to lookup symbols defined in parent scopes.
FunctionContext is used for context tracking as well
as looking up symbols beyond local context through the use
of a stack. Function indexing starts from 1, as index 0 is
reserved for the top-level body, which is now treated as a
function.
Also-by: Robert Young<EMAIL_ADDRESS>Signed-off-by: Nazim Uddin Bhuiyan<EMAIL_ADDRESS>

@rwy0717 You're right! Inserting the instructions so that <body> returns 0 fixes the issue with jitting it. Requesting another review.

Thanks! Great work!
A couple of thoughts for future work (some you already know):

We should think about removing the block entry and block context from the frontend
We should remove finding the <body> function by name in b9run, and just execute function zero.
We consider removing the ability to run a specific function from a script. It makes less sense now that symbol lookup is contextual. If we remove this functionality, it probably makes sense to remove the loop option too. After all, you can just call a function in a loop inside the script.
A little hacky, but maybe we should compile the functions backwards, to maximize the impact of direct-call and pass-param.


Limit zooming on minimum value of axes

I'm creating a graph with circles on it, and when clicking a circle the graph zooms to that specific circle centering it. I'm using the scaleExtent and translateExtent to get this done. However I would like to see the zooming be limited to the minimum value of the axes.
In this example, when clicking on the blue circle the graph zooms in but as you can see the x-axis in this case now has a minimum value displayed of -5. The graph should be limited to zero in this case instead of centering the circle.

You can do this, but you’ll need to compute the appropriate transform yourself. We don’t currently enforce constraints on programmatic zooming (so that you have more flexibility).


[Bug] Environment Variables Configuration Seems Ineffective

Describe the bug
Setting Environment Variables seems to be ineffective. The environment variables only exist in the build environment. When the build is completed, the build environment stops running, and the environment variables no longer exist at that point. As a result, it is not possible to read variables such as OPENAI_API_KEY, CODE, BASE_URL, etc., from process.env in the frontend JavaScript.
To Reproduce
Set Environment Variables
Deployment

[ ] Docker
[x] Vercel
[ ] Server

Please redeploy the project in Vercel dashboard to apply the environment variable settings.


Expose access to root hashes

To provide for https://github.com/mafintosh/hyperdb/issues/41 , it seems an API function must be added to retrieve the root hashes that concisely hash the content of the node.
These hashes are more useful than just the signature already provided, because signatures that only verify a portion of the data are not always shared or available, whereas root hashes are.  For discussion see for example https://botbot.me/freenode/dat/2018-01-20/?msg=95975498 .
I believe this could basically be implemented by exposing Feed.prototype._getRootsToVerify .  I imagine the implementation could be the same as Feed.prototype.verify but it would not call crypto.tree nor crypto.verify, rather returning the roots as-is.
Note: last PR I had accepted required spending some time on IRC to draw attention to it, and including tests and documentation updates.

@xloem Here is an attempt at implementing access to root hashes; what is the expected behaviour of verify in this case?

@denim2x thank you so much for this work.
I was originally imagining a new api function, with ‘verify’ maintaining its existing behavior, but I personally think your solution of adding the behavior to ‘verify’ to be great.
Would you be able to add test coverage, submit a pull request, and work with mafintosh to have the change merged?

@xloem I'll do my best 😃


[HOLD for payment 2024-07-10] Android - Composer - Suggestion list appears amid report view after sending multiline text

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 1.4.86-0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail: n/a
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:
Action Performed:

Launch New Expensify app.
Open any chat.
Tap on the composer.
Create many new lines until the composer is scrollable.
Enter @ and select a user from the list.
Send the message.
Enter @.

Expected Result:
The mention list will appear above the cursor.
Actual Result:
The mention list appears in the middle of the report view.
Workaround:
unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[x] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
https://github.com/Expensify/App/assets/38435837/0ca6d03b-b28e-469e-b8d5-2ce96068f428

View all open jobs on GitHub
Issue OwnerCurrent Issue Owner: @MitchExpensify

@grgia FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

We think that this bug might be related to  #vip-vsb

This also seems related to https://github.com/Expensify/App/pull/42630 @perunt @puneetlath

Doming from here https://expensify.slack.com/archives/C035J5C9FAP/p1718916284226989 demoting and @perunt can handle this once online

fixed
will add screen records in a couple of minutes

Thank you @perunt !

Payment summary:

$250 @perunt C (Upwork)
$250 @sobitneupane C+ (NewDot)

Does this look alright to you?

I'm from the Margelo agency, and we get our payments separately, so all good

Cool, closing as the only payment needed is via NewDot

$250 approved for @sobitneupane


Fix typo

1⃣ Is this something that is wanted/needed? Did you create an issue / discussion about it first?
No.
2⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
No.
3⃣ Does it include tests, if possible? (Not a deal-breaker, just a nice-to-have)
No.
4⃣ Please include a thorough description of the improvement and reasons why it's useful.
This PR just to fix a typo for the code snippet.
5⃣ Thanks for contributing! 🙌

Thanks for finding and fixing that. Merging now.


gpio.setup error is not correctly caught by Promise

If the regex match fails on rpi-gpio.js:365, the next line that attempts access match[1] will fail and throw a TypeError that is not caught by the Promise. Instead of .catch being called and handling this in my promise, the program will die.

Thanks for reporting this and for the PR.
Out of interest, did you come across this using real Raspberry Pi hardware? Is there also an issue with the regex?

@JamesBarwell Ah sorry, should have said here. Regex does seem fine, I just liked developing locally as it's quicker than on the Pi =)


nz-transfer move operations disabled state

Reproduction link
https://stackblitz.com/edit/angular-pmeuuz
Steps to reproduce
Context: The nz-transfer must have some disabled checked items

Check another item => the move right button will be highlighted.
Uncheck the same item => the move right button is still highlighted

What is expected?
The move right button should be disabled
What is actually happening?
The move right button is still enabled

Environment
Info

ng-zorro-antd
18.1.1

Browser
Chrome

https://github.com/user-attachments/assets/89dcec2a-f3e5-488a-b6cf-c67978dba223

The issue seems to be related to this function:
https://github.com/NG-ZORRO/ng-zorro-antd/blob/d58c52d21f294983e2da0cc425c153c7f029eae6/components/transfer/transfer.component.ts#L246

Hi @krisztofer-kunc
Thank you for reporting this issue 👍
A fix has been made #8824

Many thanks!


fix(SDK): IBM Cloud SCC Python SDK

:tada: This PR is included in version 0.0.4 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


controllers: Do not verify IBM CSI driver CSV version

IBM CSI driver is backward compatiable with IBM odf-operator and should
be allowed to upgrade at any point of time. Which means we should not
check for specific version of its CSV.
Signed-off-by: Nitin Goyal<EMAIL_ADDRESS>

/cherry-pick release-4.11
/cherry-pick release-4.10


Wrong deprecation message for 'es-archive' option

Describe the bug
The deprecation message for the flag --es-archive.max-num-spans reads:

(deprecated, will be removed in release v1.21.0. Please use es.max-doc-count). The maximum number of spans to fetch at a time per query in Elasticsearch. The lesser of es.max-num-spans and es.max-doc-count will be used if both are set.

It should state that the flag to use is es-archive.max-doc-count instead of es.max-doc-count. Same with all other occurrences of the flag names in the message.
To Reproduce
Steps to reproduce the behavior:

$ podman run -e SPAN_STORAGE_TYPE=elasticsearch -p 16686 jaegertracing/all-in-one:1.20.0 --help | grep max-num-spans

Expected behavior
The es-archive prefix should be used in the message.
Version (please complete the following information):

Jaeger version: 1.20

@albertteoh would you like to take care of this one?

Yup, leave it with me, @jpkrohling! Thanks for picking that up.


Revert "Revert "cherry-pick Fix enable_mow_light_delete default value""

Reverts apache/doris#40359

skip buildall

skip buildall


Compilation & execution instructions missing from the doc

We need a HOWTO in the doc about compiling applications using PDI:

with CMake / a plain Makefile
from C / Fortran

unassigned @ksiero

In GitLab by @ksiero on Nov 20, 2020, 15:44
mentioned in commit 1969477a5ca75b50297dc9760d86ccf0089c4a25


Ability to specify audience during authentication

Looking to see how we can specify an audience during authentication?
Expected behaviour
Able to specify an audience IE:
GET https://xxxxxxxx.com/authorize?
  audience=API_IDENTIFIER&
  scope=SCOPE&
  response_type=code&
  client_id=XXXXXXXXXXXX&
  redirect_uri=https://YOUR_APP/callback&
  state=STATE

Actual behaviour
Cannot see an option for this within the wiki.

I guess we can use OIDCAuthRequestParams but should there be a specific parameter for aud?

audience is a non-standard parameter and would necessarily have to be part of OIDCAuthRequestParams; also, if you're trying to influence the aud setting that is returned in the id_token then there' s no defined way to do that; by spec that value would be filled with the client_id

@zandbelt thanks, makes sense. will close this


fix(openapigateway-to-lambda): refine python example in README based on deployed code

I recently created a python-based CDK app using the openapigateway-to-lambda solutions construct, and noticed a few discrepancies with the example in our README. This PR updates the example code to be correct.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

AWS CodeBuild CI Report

CodeBuild project: githubautobuild-for-cdk-v2
Commit ID: 61858a3dcbe00ed358c35e13ac6278f8357c1fe2
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


Incorrect comment

URL(s) with the issue:
Please provide a link to the documentation entry
Description of the issue:
Small nitpick, need to remove the also removes volumes

Submitting a pull request?
Are you willing to submit a pull request to solve this issue? (Yes/No): Yes

Thanks for identifying this. It's now updated in commit


fix(theme): update danger100 hex color

fix #794

Codecov Report

Merging #801 (3b1ebf0) into dev (e226b85) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #801   +/-   ##
=======================================
  Coverage   86.59%   86.59%
=======================================
  Files         175      175
  Lines        4475     4475
  Branches     1520     1520
=======================================
  Hits         3875     3875
  Misses        296      296
  Partials      304      304

Impacted Files
Coverage Δ

packages/react-magma-dom/src/theme/magma.ts
100.00% <ø> (ø)

Continue to review full report at Codecov.

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


Make brief specialist values match the service specialist keys

DOS services store specialist location and day rate using a camelCase version of the role name: eg agileCoachLocations. Since the brief specialist value is used to build the location key to use for filtering in the API, the role value should match the service key prefix to avoid any rewriting logic in the API.

:+1:


Hotfix/clear activecall xsig when no calls

Fixed issue where activecall xsig would not clear call connected information when there were no calls.
This meant that there was no data change when the far side ended a call.

Note - This is tested and working

I think I'm missing something here.
In the Essentials implementation of the CiscoCodec, calls are removed from the ActiveCalls list based on feedback from the codec, and then the CallStatusChange event is called and the XSig updated based the current ActiveCalls list.
If the ActiveCalls list has no items, the foreach (var call in ActiveCalls) { ... } loop in the UpdateCallStatusXSig won't run, and the while loop will fill up the whole array with digitals set to false and empty string values for each call item.
What part of that process isn't working correctly?


Doesn't work with downloaded buffer

Here is my code:
request = require('request')
lwip = require('lwip');

request.get( { url: 'http://www.bing.com/az/hprichbg/rb/MatterhornVideo_EN-US10200962750_320x180.jpg', encoding: 'binary'}, function ( err, response, body ) {
	console.log(body);
	// obtain an image object:
	lwip.open(body, 'jpeg', function(err, image){
		console.log(err);
	  // check err...
	  // define a batch of manipulations and save to disk as JPEG:
	  image.batch()
	    .resize(32, 18)      // scale to 75%
	    .blur(5)              // Gaussian blur with SD=5
	    .writeFile('output.jpg', function(err){
	      console.log(err);
	    });

	});
})

It throws the following error:
[Error: Path must be a string without null bytes.]
I checked the buffer with console.log, and it prints a garbled string - because it's a binary - so I see binary data is not null.
Could anyone tell me what's wrong?

hey @cihadturhan, try setting encoding: null - that should definitely return a buffer from request.
i am guessing that lwip interprets any String it finds as a filename.

That's right, body needs to be an instance of Buffer.


Updated docs related to electron to 10.4.3 upgrade

Description
Updating the documentation about electron version.
Motivation and Context
If any techie comes to the base page of this repo, they would probably be interested in the version of electron being used (to understand security concerns)
Checklist:

[x] My pull request is properly named
[x] The changes respect the code style of the project ($ npm run lint)
[x] I tested/previewed my changes locally

@vraravam thanks!
@all-contributors please add @vraravam for doc


fix: migrate gravity proposals to text proposals

Description
closes: #XXXX

Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...

[ ] included the correct type prefix in the PR title
[ ] added ! to the type prefix if API or client breaking change
[ ] added appropriate labels to the PR
[ ] targeted the correct branch (see PR Targeting)
[ ] provided a link to the relevant issue or specification
[ ] added a changelog entry to CHANGELOG.md
[ ] included comments for documenting Go code
[ ] updated the relevant documentation or specification
[ ] reviewed "Files changed" and left comments if necessary
[ ] confirmed all CI checks have passed

Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...

[ ] confirmed the correct type prefix in the PR title
[ ] confirmed all author checklist items have been addressed
[ ] reviewed state machine logic
[ ] reviewed API design and naming
[ ] reviewed documentation is accurate
[ ] reviewed tests and test coverage
[ ] manually tested (if applicable)

@robert-zaremba btw in gravity.go there are some functions added to MsgGovUpdateRegistry which I think we need (at least they were needed during my tests).

Closing in favor of #2220


Not taking previously set headers into account

The library doesn't take previously set headers into account (maybe RequestOptions in general). In my app, I have overloaded BaseRequestOptions, which means that each instance of RequestOptions instantiated through the app will use that as base. Resource just ignores it and gets empty headers instead.
Also, I have injected another CustomHttp in place of Http:
{
    provide: Http,
    useFactory: (backend, reqOpts, storage) => new CustomHttp(backend, reqOpts, storage),
    deps: [XHRBackend, RequestOptions, StorageService]
}

which also sets some specific headers. These headers are also ignored, even though the injected instance is indeed CustomHttp (checked it).
P.S. The idea with CustomHttp is to use it in separate places instead of providing it in place of Http, does this library support choosing a different implementation of Http at the moment?

Hi!
For the headers you can specify by setting setHeaders or simply create BaseResource class and override getHeaders method, please check here .
To use custom Http please try to inject it into the resource constructor and add the resource to your providers list manually (add parameter add2Provides to disable automatic adding to providers).
@ResourceParams({
    url:BASE_API+"/users",
    add2Provides: false
})
export class UserResource extends Resource {

    constructor(http: CustomHttp, injector: Injector) {
      super(http, injector);
    }
    @ResourceAction({
        isArray: false
    })
    query: ResourceMethod<IUserQueryInput, IUserShort[]>;
}

Hi, thanks for the explanation!
It would be great if custom http service could be sent as a parameter to ResourceParams, so we could avoid adding providers to the module manually, creating explicit constructors, etc. I may even create a PR for that in the future if I get the time, if it's fine by you.
As for this issue and headers, setting headers manually on a resource is not really something you should do if it works in other places throughout the app without doing it. That's kinda the point of DI.
If you want to affect that resource explicitly, that's fine. But if you want to affect the entire app, then it should be done in one place, and that is actually the purpose of BaseRequestOptions (providing default request options throughout the app).
Injecting RequestOptions somewhere here and taking them into consideration in getHeaders etc. could do the trick. I wanted to do it actually, but after cloning the repo I'm getting ngc errors (and also, angular peer deps need an update as well). ..


Pointer support for all platforms

Check out this SO post for more info: https://stackoverflow.com/a/41238807.
As of now, a traditional computer mouse is all that is supported. Moving forward a universal approach, if possible, would be ideal (mouse, touch panel, & pen support). Although that may be too bloated for one class...

More than mouse support is supported now. Although, I do not think the Pointer class should concern itself with providing helper methods to manage touch panel / pen / mouse  specific code. If anything that would be a job for SmartPointer!


name changes

some names have been changed to reflect 1.16's naming conventions. ctm will not be updated until optifine has an official release for 1.16.

... you do realize there's already a 1.16 branch where most of this was already done... Right???


Calculus Notebook

Can you please inform whether there are any plans to develop Calculus Ipython Notebook?

Hi @ajkn ,
I've been wanting to write this notebook for so long, but I never found the time, so let me just quickly summarize the main ideas here. You don't actually need to know calculus in order to practice Machine Learning, since TensorFlow (or other Deep Learning libraries) take care of that for you, as we will see below.
All you really need is a good intuition of what a derivative is.  In short, you can think of the derivative of a function as the slope of that function at each point. For example, if you consider the function f(x) = 2x, then the slope is constant, it is written f'(x), and it is always equal to 2: f'(x)=2.
Now let's consider a more complicated function, g(x)=x^2-3x+5, the slope keeps changing! In fact, the derivative of g(x) is g'(x)=2x-3. How can you figure this out yourself? Well, let's start by figuring out the slope between any two points x1 and x2 on a function: it is of course equal to the difference on the vertical axis (g(x2) - g(x1)) divided by the difference on the horizontal axis (x2 - x1):
slope_of_g(x1, x2) = (g(x2)-g(x1))/(x2-x1)
If x2 = x1 + e, you can rewrite this equation as:
slope_of_g(x1, x1+e) = (g(x1+e)-g(x1))/e
As x2 approaches x1 (i.e., as e gets smaller and smaller), you get a more and more precise approximation of the slope at the point x1. That's actually the definition of the derivative of g(x) at the point x: it is the limit of the slope between the point at x and the point at x+e as e approaches zero ("limit when e approaches 0" is noted lim e->0):
g'(x) = lim e->0 (g(x) + e) / e
So, let's look at the example above:
g'(x) = lim e->0 (g(x) + e) / e
Let's replace g(x) with its definition:
g'(x) = lim e->0 [(x+e)^2-3*(x+e)+5 - (x^2 - 3x + 5)]/e
Then let's gradually simplify this equation:
g'(x) = lim e->0 [x^2 + 2xe + e^2 - 3x - 3e + 5 - x^2 +3x - 5]/e
g'(x) = lim e->0 [2x*e + e^2 -3e]/e
g'(x) = lim e->0 [2x - 3 + e]
When e approaches zero, of course 2x - 3 + e approaches 2x - 3, so:
g'(x) = 2x - 3
Now in practice, you don't have to go through this process at all, as TensorFlow uses Reverse Mode Autodiff (see appendix D) to compute this for you. For example:
import tensorflow as tf
x = tf.placeholder(dtype=tf.float32)
g = tf.square(x) - 3.0*x + 5.0
[gp] = tf.gradients(g, x)
init = tf.global_variables_initializer()
with tf.Session():
    init.run()
    print("g(6) = 6 * 6 - 3 * 6 + 5  =", g.eval(feed_dict={x: 6.}))
    print("g'(7) = 2 * 7 - 3 =", gp.eval(feed_dict={x: 7.}))

This code outputs the correct result:
g(6) = 6 * 6 - 3 * 6 + 5  = 23.0
g'(7) = 2 * 7 - 3 = 11.0

The point at which the derivative of a function is zero is a point where the slope is horizontal. A function's minimum or maximum is always located at a point where the derivative is zero. If the derivative of a function is zero at a given point, it can be a minimum or maximum (it could be a local minimum/maximum, not necessarily the global minimum), or it could be a plateau, and so on.  So when people search for a function's minimum (analytically, not using gradient descent), they usually compute the function's derivative first, and then try to find the points at which it is zero.
If the function has several inputs instead of one, for example f(x,y)=xy + 3x - 5y + 2, then you must compute the so-called "partial" derivative of the function with regards to x, which is written ∂f/∂x, just like a regular derivative, but keeping all other inputs constant (in this case y stays constant while you try to find the slope (f(x+e,y) - f(x,y))/e as e approaches zero, then you do the same thing for y. If you put all the partial derivatives in a vector, it is called a "gradient vector", noted ∇u f (u is the vector (x,y) and it should be a subscript).
Gradient descent is quite simple: first, estimate the slope of the cost function with regards to the model parameters (i.e., find all the partial derivatives of the cost function with regards to each model parameter). This gives you a vector that points "uphill": if you move the parameters in this direction, the cost will go up.  So instead, you subtract the gradient vector from the model parameter vector in order to reduce the model's error (as measured by the cost function).
Hope this helps.

The calculus notebook is now available: math_differential_calculus.ipynb