How to build istio/proxy

(NOTE: This is used to report product bugs:
To report a security vulnerability, please visit https://istio.io/about/security-vulnerabilities/
To ask questions about how to use Istio, please visit https://discuss.istio.io
)
Bug description
I checked out istio/proxy, and trying to build with make build or make, it would fail.
root@ip-172-31-45-102:~/istio/proxy# make build
export PATH=/usr/lib/llvm-9/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin CC=clang CXX=clang++ && bazel  build  --config=libc++ //...
INFO: Analyzed 175 targets (834 packages loaded, 51063 targets configured).
INFO: Found 175 targets...
ERROR: /root/istio/proxy/src/istio/utils/BUILD:77:1: C++ compilation of rule '//src/istio/utils:attribute_names_lib' failed (Exit 1) clang failed: error executing command /usr/bin/clang -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer '-stdlib=libc++' -MD -MF ... (remaining 21 argument(s) skipped)
Use --sandbox_debug to see verbose messages from the sandbox
In file included from src/istio/utils/attribute_names.cc:16:
./include/istio/utils/attribute_names.h:19:10: fatal error: 'string' file not found
#include <string>
         ^~~~~~~~
1 error generated.
INFO: Elapsed time: 68.531s, Critical Path: 0.44s
INFO: 1 process: 1 linux-sandbox.
FAILED: Build did NOT complete successfully
make: *** [Makefile.core.mk:68: build] Error 1

root@ip-172-31-45-102:~/istio/proxy# make build_envoy
export PATH=/usr/lib/llvm-9/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin CC=clang CXX=clang++ && bazel  build  --config=libc++ --config=release //src/envoy:envoy
INFO: Build options --compilation_mode and --strip have changed, discarding analysis cache.
INFO: Analyzed target //src/envoy:envoy (5 packages loaded, 36487 targets configured).
INFO: Found 1 target...
ERROR: /root/.cache/bazel/_bazel_root/7403c55f491a61a6e198902a394e10ff/external/com_google_protobuf/BUILD:117:1: C++ compilation of rule '@com_google_protobuf//:protobuf_lite' failed (Exit 1) clang failed: error executing command /usr/bin/clang -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 31 argument(s) skipped)
Use --sandbox_debug to see verbose messages from the sandbox
In file included from external/com_google_protobuf/src/google/protobuf/arena.cc:31:
external/com_google_protobuf/src/google/protobuf/arena.h:36:10: fatal error: 'limits' file not found
#include <limits>
         ^~~~~~~~
1 error generated.
Target //src/envoy:envoy failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 3.704s, Critical Path: 0.56s
INFO: 2 processes: 2 linux-sandbox.
FAILED: Build did NOT complete successfully
make: *** [Makefile.core.mk:71: build_envoy] Error 1

Affected product area (please put an X in all that apply)
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure
Affected features (please put an X in all that apply)
[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane
Expected behavior
Steps to reproduce the bug
Version (include the output of istioctl version --remote and kubectl version --short and helm version if you used Helm)
1.7.4
How was Istio installed?
Environment where bug was observed (cloud vendor, OS, etc)
UBUNTU
Additionally, please consider attaching a cluster state archive by attaching
the dump file to this issue.

Anyone could suggest what's the best way to build istio/proxy?

cd /root/go/src/istio.io/
git clone https://github.com/istio/proxy.git

cd proxy
docker run -it -w /work -v $PWD:/work gcr.io/istio-testing/build-tools-proxy:master-latest bash
make

have a try ...

Or trying
docker run \
   -e=TEST_TMPDIR=/tmp/bazel \
   -v=/tmp/bazel:/tmp/bazel \
   -v=${PWD}:/go/src/istio/proxy \
   -w=/go/src/istio/proxy \
   docker pull ghcr.io/querycap/istio/istio-envoy-build-env:0.1.0 make build_envoy

it will be simpler and clean than build-tools-proxy
Dockerfile: https://github.com/querycap/istio/blob/master/build/istio-envoy-build-env/Dockerfile.istio-envoy-build-env
This used for multi arch support.
https://github.com/istio/istio/issues/26652#issuecomment-714858976

@istio/wg-networking-maintainers-data-plane it would be great to have a doc on this. I have struggled as well

@howardjohn put doc under istio repo or proxy repo?

Thank you all for the suggestion.  I am trying the way @morlay suggest.
@morlay more context to you, the reason I am starting to build the istio envoy is I need to do some hacks on the envoy binary. Actually I started from the official envoyproxy/envoy, and built the binary with my hack(http_parser source code change) and shipped to istio-proxy image, but it can not work as expected. https://github.com/istio/istio/issues/28390@howardjohn suggested in slack that I need to build it from istio/proxy repo, since there is some extensions or istio specific customization on top of the original envoy.
So is it still possible I inject my hack on the envoy?

Was the documentation created somewhere?


Unified pattern for instrumentation dependencies

This issue is about dependencies in instrumentation package.jsons, discussed here
The current code base includes many instrumentations that have different dependencies patterns on @opentelemetry/* packages.
I would like to get to a consensus about the correct way to do it, and then apply the decision on all pacakges
Peer dependency on @opentelemetry/api
Should be set to minimum version that satisfies the instrumentation, with a caret. This will usually be:
  "peerDependencies": {
    "@opentelemetry/api": "^1.0.0"
  },

This change is covered in #984  by @Flarna
Dev depenency on @opentelemetry/api
Currently, each instrumentation uses a different version (probably the latest one available when it was created).
This dependency is used for build and test.
I guess we want to keep it pinned to enforce consistency, but the question is if it should be pinned to least supported version (1.0.0), or latest version (1.1.0 and bump on new releases).
A suggestion is to use test-all-versions to verify tests are passing with various api versions, but we will still need a single version in package.json dependencies for all other tasks.
Dependencies on @opentelemetry/* packages
Should instrumentation state the minimum supported version (e.g. ^1.0.0), or the latest version (^1.2.0). I guess both are OK, even if user is using old SDK version, instrumentation will just pull it's own copy version into another branch in node_modules and will function correctly.
Currently, there is inconsistency in these dependencies which I support in straightening and documenting the "right" template.
@Flarna - you always have good insights on these topics. What do you recommend doing?

If we state compatibility with API 1.0.0 we should test it somewhere. Not sure if using test-all-versions is really needed, maybe some smaller setup which just installs deps and transpiles is already enough.
Same applies more or less on SDK.

I guess both are OK, even if user is using old SDK version, instrumentation will just pull it's own copy version into another branch in node_modules and will function correctly.

It will most likely work correct but typescript will likely complain - see https://github.com/open-telemetry/opentelemetry-js-contrib/issues/983 Long term this could be fixed by replacing all classes in exported APIs by interfaces but I doubt this will happen soon. Not sure if this is even possible because I think we even export abstract classes at a few places.
Also duplication of API can be problematic. If an instrumentation has a copy of API 1.1.0 in it's subtree and app uses 1.0.x the instrumentation won't create traces because the global installed TracerProvider of 1.0.0 gets a request from 1.1.0 API it can't fulfill. The other way around (instrumentation has 1.0.0 pinned, app uses 1.1.0) should be ok.

If we state compatibility with API 1.0.0 we should test it somewhere. Not sure if using test-all-versions is really needed, maybe some smaller setup which just installs deps and transpiles is already enough.
Same applies more or less on SDK.

We can configure tav to run only the compile script and not tests. I think we can also define a matrix to test multiple combinations of api and SDKs.

It will most likely work correct but typescript will likely complain - see #983 Long term this could be fixed by replacing all classes in exported APIs by interfaces but I doubt this will happen soon. Not sure if this is even possible because I think we even export abstract classes at a few places.

I think this also depends on the exact combination of usages. Instrumentation mostly depends on the API and @opentelemetry/instrumentation libraries, so as long as they don't have conflicts, the situation should be relatively ok I assume. But did not check it in depth.

Also duplication of API can be problematic. If an instrumentation has a copy of API 1.1.0 in it's subtree and app uses 1.0.x the instrumentation won't create traces because the global installed TracerProvider of 1.0.0 gets a request from 1.1.0 API it can't fulfill. The other way around (instrumentation has 1.0.0 pinned, app uses 1.1.0) should be ok.

We talked about it a few weeks ago at the SIG, and understood this is unaviodable.
For instrumentations, this is simpler as one can just use a compatible version of the instrumentation package until the SDK is upgraded to support a newer API. For packages that embed OTEL, this can be an issue as users will want to upgrade the package version for it's new functionality or bug fixes and it could bump the supported version of @opentelemetry/api which will break their app. For these cases, we talked about recommending always using the minimum supported version in dependency, e.g<EMAIL_ADDRESS>unless some new feature is required. That will increase flexability of npm in choosing the right version and less chance for issues.

I think this also depends on the exact combination of usages. Instrumentation mostly depends on the API and @opentelemetry/instrumentation libraries, so as long as they don't have conflicts, the situation should be relatively ok I assume. But did not check it in depth.

Even if the same version is in both folders it wont work. And I think this has a good reason. Even if the class has the same source and therefore the same functionality an instanceof check would fail as the class with the same name exists twice and this is what instanceof compares.


[FSunstone] VM speed graphs always incrementing

Description

Graphs showing speed are always incrementing

Disk read and write IOPS
Network download and upload speed

Wrong heading for "Disk write IOPS" graph. It is now: "Disk write bytes"

To Reproduce
Deploy VM and check graphs
Expected behavior
Graphs should show correct values (up and down slopes).
Details

Affected Component: FireEdge Sunstone
Hypervisor: KVM
Version: 6.6.3 enterprise

After a quick analysis it seems like this might be core related and not an issue directly in Fireedge. The values sent to Fireedge from the core seem to be cumulative across timestamps, thus producing these kinds of graphs.

After a quick analysis it seems like this might be core related and not an issue directly in Fireedge. The values sent to Fireedge from the core seem to be cumulative across timestamps, thus producing these kinds of graphs.

The legacy Sunstone shows the same graphs correctly. I would guess that it receives values from the same source?


Add some aceptance tests for the pipeline resource

This resource had no acceptance tests, and we were nervous about the risk of regressions.
This adds a few initial test cases:

creating a pipeline with only the required attributes set
creating a pipeline with all attributes set
updating a pipeline
importing an existing pipeline

Pipeline deletion is tested implicitly when the terraform testing sdk cleans up after a test.
We assume there's potential for more tests that exercise specific combinations (for example, exposing this teams related bug in pipeline creation #84), but this is a good start

I've run the full acceptance test suitre locally and confirmed it is green :+1:


proc.require()

Alternative to .completed(proc=> proc.start()) that runs the script in the node instance used by fuse (debugging if fuse is debugged)
.completed(proc=> {
	proc.require().then(exps=> {
		// exps is the main module' exports
	});
});

There are several ways to express how to close the deprecated instance once the bundle has been re-transpiled, when it is started again, depending on the AEFL (Automatically Executed File on Load)
If the bundle has NO AEFL, a close function taking no argument can be given as argument of require
proc.require(function() {
	console.log('unloaded');
});

If the bundle HAS an AEFL, the first of these choice that exists/is defined will be used :
A- Give a close function to the require call, who takes an argument containing the exports of the AEFL
proc.require(exps => exps.kill())

B- export a close function in the AEFL that takes no argument
// server.js
[ ... ]
export function close() {
	myResources.$destroy();
}
[ ... ]

C- Export directly in exports or in default an object that hase a close function (without parameters)
// server.js
[ ... ]
export default app.listen(config.port);
[ ... ]

OR
// server.js
[ ... ]
export = app.listen(config.port);
[ ... ]

PROMISES:
If the close function that is called returns a Promise, the module will be re-started on that promise' resolution.
proc.require(function() {
	return Promise((resolve, reject)=> setTimeout(()=> resolve(true), 60000));
});

This will wait a minute before re-requireing the module.
note: This promise' resolution value is ignored
OBVIOUS NOTE:
The close function is not called the first time the module is launched. It is called just after re-transpilation and before re-launching the module.
RETURN VALUE:
The function require returns a Promise that resolves to the exports of the AEFL

Awesome!
We need a documentation for this feature with examples, you think you could do that?

For the documentation, I did it mainly in the description I gave here - now I could perhaps write in your documentation directly.
For examples - I would need ideas or some bone to bite, perhaps something in https://github.com/fuse-box/fuse-box-examples ?

@eddow yeah, that repo. I will dwell on the code a little bit longer and will merge soon enough ;-)

Don't rush - I come again on little modifications I could bring indeed, I don't know if I can still modify it after you pull it

@eddow just lemme know where you are ready. But it's looking cool, I like it. But we need a solid documentation on that which answers the questions why motivation and how ;-)

I'll wait for #522 as I think it is the reason I had to hack with getMainExport.
For the doc, I just get you a md file I guess? I'm noob in these too but I can give it a shot even if I feel I exposed the main technical points in the description I made here

@eddow that will be this file
https://github.com/fuse-box/fuse-box/blob/master/docs/bundle.md#executing-a-bundle
=)

I was going to finish the doc and wait for #522 to redo the getMainExport stuff - so if you happen to change the branch by the time, keep me informed


[Bug]: Could not pass event AsyncPlayerPreLoginEvent to ProSkillAPI v<IP_ADDRESS>-SNAPSHOT

Is there an existing issue for this?

[X] I have searched the existing issues

Have you tried to reproduce this without OP?

[X] I have tested without OP

Current Behavior
https://pastebin.com/EnKQcPXM
it happens when 1 player joins the server
Expected Behavior
No response
Steps To Reproduce
No response
Environment
- Server Software: paper 1.16.5
- ProSkillAPI Version: ProSkillAPI v<IP_ADDRESS>-SNAPSHOT
- ProMCCore Version: promccore-<IP_ADDRESS>-1150257
- Plugins:  Aliases*, BattlePass, BlockEffects, BossBarHealth, Citizens, ClearLag, CustomDrops, DeluxeCoinflip, DeluxeMenus, EconomyShopGUI, EliteLootbox, Essentials, EssentialsSpawn, EvenMoreFish, ExtraStorage, GiftCodes*, ItemJoin, KiteBoard, LiteBans, LuckPerms, MMOItems, MyPet, MythicLib, MythicMobs, Oregen3, PlaceholderAPI, PlayerData*, PlayerKits, PlayerPoints, PlayerVaults, PlugManX (PlugMan), ProMCCore, ProSkillAPI, ProSkillAPIParties, ProtocolLib, PuerkasChat*, SkinsRestorer, SuperiorSkyblock2, SupperSeason, TAB, TimeIsMoney*, Vault, ViaBackwards, ViaRewind, ViaVersion, Vouchers, WorldEdit, WorldGuard, XPBoost*

latest.log
latest.log

# paste log here...

Anything else?
No response

Is this just one player, or is this any player that joins?

Many people too

It looks like your ProMCCore build is outdated. Could you try grabbing the newest builds of both ProSkillAPI and ProMCCore from our Discord and see if you have the same problem?

I have updated the version promccore-<IP_ADDRESS>-20221110.064223-9
it's still like that
but
i deleted everything in the playerdata section it was gone, when i log out and rejoin the server it shows up again

Have you tried turning the server off, deleting the data, then starting it up again and see if the error is still there?

I just did that, the first time I joined the server it was normal but the second time it showed up

Can you try the new build that'll be posted to Discord in a few minutes and tell me if that fixes it?

sorry for the late reply, seems to be no longer the problem


interpolation-messageformat depends on global MessageFormat

Even though the module is exposed using UMD (yay) it is actually using the global MessageFormat object here. This is forcing me to either expose MessageFormat onto window (which I would prefer not to do), or make a workaround with webpack (which is also cumbersome). The real solution is to use UMD correctly and not depend on globals but rather require things correctly.
Here's what the UMD format looks like now:
(function (root, factory) {
  if (typeof define === 'function' && define.amd) {
    // AMD. Register as an anonymous module unless amdModuleId is set
    define([], function () {
      return (factory());
    });
  } else if (typeof exports === 'object') {
    // Node. Does not work with strict CommonJS, but
    // only CommonJS-like environments that support module.exports,
    // like Node.
    module.exports = factory();
  } else {
    factory();
  }
}(this, function () {

// interpolation-messageformat code that uses the global MessageFormat variable

}));

Here's what it should look like:
(function (root, factory) {
  if (typeof define === 'function' && define.amd) {
    // AMD. Register as an anonymous module unless amdModuleId is set
    define(['messageformat'], function (MessageFormat) { // <-- changed line
      return (factory(MessageFormat)); // <-- changed line
    });
  } else if (typeof exports === 'object') {
    // Node. Does not work with strict CommonJS, but
    // only CommonJS-like environments that support module.exports,
    // like Node.
    module.exports = factory(require('messageformat')); // <-- changed line
  } else {
    factory(root.MessageFormat); // <-- changed line
  }
}(this, function (MessageFormat) { // <-- changed line

// interpolation-messageformat code that uses the global MessageFormat variable

}));

Thanks!

@kentcdodds Thanks for the hint. We accidentally didn't care about this.
Do we know that AMD and CJS modules' name would be messageformat and not MessageFormat?

I know that for CommonJS it is messageformat, with AMD, I checked and it is not exposed with a name. So people would have to shim it themselves. You would essentially be deciding the shim for people, but that's better than not providing anything at all (like the current scenario). Thanks for being willing to fix this issue :-)

Okay, that will be the new one:
(function (root, factory) {
  if (typeof define === 'function' && define.amd) {
    // AMD. Register as an anonymous module unless amdModuleId is set
    define(["messageformat"], function (a0) {
      return (factory(a0));
    });
  } else if (typeof exports === 'object') {
    // Node. Does not work with strict CommonJS, but
    // only CommonJS-like environments that support module.exports,
    // like Node.
    module.exports = factory(require("messageformat"));
  } else {
    factory(MessageFormat);
  }
}(this, function (MessageFormat) {

Added in f5e039c - thank you.

Thanks for your fast response and all of your work on this library :-)

Do you mind letting me know when this gets released so I know when I can pull out my workaround? Thanks!

@kentcdodds I recommend following @knalli on twitter, I'm sure he'll send a tweet about it :)

:+1:


[move/next-vm] Fix main

Description:
next-vm execution layer cut (#14874) raced with removal of scripts (#14870), introduced a build issue.
Fixed by replicating the fix applied to v0 from the scrip removal PR.
Test Plan:
external-crates/move/move-execution/next-vm$ cargo nextest run

No problem, we just need a merge queue!

If this is the "next" vm, shouldn't this be replicating the change on latest, not v0?

No, I don't think so.  The immediate fix here, i.e. to stop main from being broken, is to get things building again, and since the next-vm branch doesn't include the removal of scripts, it got the fix from v0 whose frames also potentially include scripts.
It may be that you want to apply the script removal PR to next-vm (or recut next-vm from latest), but that's really a semantic concern about this next-vm cut, and can be addressed later.


Make Chromedriver truly headless

When I use Suspenders' configuration on a new project, JavaScript tests open up the Chrome app in a real window (i.e. not headless).
When using the configuration in this commit, Chrome quietly bounces once, in the taskbar (on macOS), and does not create a window.

I just started a new project and was frustrated to see my tests open up a non-headless chrome. I was excited to see this PR, but unfortunately the change had no effect for me. Was there anything else you needed to do to get this working?

Maybe the version of Chrome matters?
I'm using:

Chrome 75.0.3770.100 (Official Build) (64-bit) (taken from chrome://version/)
macOS Mojave 10.14.5

There is technically slightly more stuff in my spec/support/chromedriver.rb, e.g the changes from #981  and #983, though I think that doesn't affect it. Regardless, here is my full spec/support/chromedriver.rb:
require "selenium/webdriver"

Capybara.register_driver :chrome do |app|
  Capybara::Selenium::Driver.new(app, browser: :chrome)
end

Capybara.register_driver :headless_chrome do |app|
  options = ::Selenium::WebDriver::Chrome::Options.new
  options.headless!
  # Set a big (15" MBP retina resolution) window size so that
  # `save_and_open_screenshot` captures the whole page.
  options.add_argument '--window-size=1680,1050'

  capabilities = Selenium::WebDriver::Remote::Capabilities.chrome(
    chromeOptions: { args: %w(headless disable-gpu) },
    loggingPrefs: {
      browser: 'ALL'
    }
  )

  Capybara::Selenium::Driver.new app,
    browser: :chrome,
    options: options,
    desired_capabilities: capabilities
end

Capybara.javascript_driver = :headless_chrome

# Silence Puma's startup message in specs
# https://github.com/rspec/rspec-rails/issues/1897
Capybara.server = :puma, { Silent: true }

I can confirm that this PR makes my tests go headless. Anyway, I remember I was able to run them headless without this PR, but now I need this PR.
So maybe it depends on a newever Chrome version?
I'm wondering what headless means in  chromeOptions: { args: %w(headless disable-gpu) },. Do we need it?

Google Chrome
75.0.3770.100 (Build ufficiale) (a 64 bit)

Revisione
cd0b15c8b6a4e70c44e27f35c37a4029bad3e3b0-refs/branch-heads/3770@{#1033}

Sistema operativo
Mac OS X 10.14.5

OK, I updated all the things and this is working for me now. After a bit of digging, I am not convinced the chromeOptions: { args: %w(headless disable-gpu) }, is doing anything anymore. It looks like the key actually changed to goog:chromeOptions in https://github.com/SeleniumHQ/selenium/commit/0ba8188b1a26ff3587f08afa6b6182c32479e980#diff-6d36ad83184bcee9c97af166e99e3f4b.
I tried changing that key without including any of the changes in this PR and it did make chromedriver headless.
It may also be fine to lose the disable-gpu options. I am not sure exactly why that option was there in the first place, but Selenium was using it as a workaround for windows and removed it in https://github.com/SeleniumHQ/selenium/pull/6943.

I confirm that on macOS, with Chrome 75, using just headless! runs the tests in actual headless Chrome. I've updated this PR appropriately, in 8f33976da2ea38402805f2d6c60f6fbc2d0fc345.

Thanks!


sriov-network-operator should list nodes based on label filters to decide whether to skip drain

Currently in sriov-network-operator it is determining whether a cluster is a single worker node cluster by checking node count without any filtering https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/pkg/utils/cluster.go#L78 . If it is a single worker node cluster then it marks as "skip drain" https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/main.go#L249-L263 and then daemon will decide whether to "skip drain" in https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/pkg/daemon/daemon.go#L519 .
Now when we are using sriov-network-operator in a cluster where there is only one "worker node" where we deploy data plane pods on (including sriov-device-plugin and sriov-network-config-daemon, we make those pods to go to worker-node only by adding configDaemonNodeSelector  in SriovOperatorConfig , see https://cloud.google.com/anthos/clusters/docs/bare-metal/latest/how-to/sriov#configure_the_sr-iov_operator), and control plane is used for kube-apiserver, the worker-node will stuck in SchedulingDisabled after applying SriovNetworkNodePolicy:
$ k get nodes
NAME                                  STATUS                     ROLES           AGE   VERSION
control-plane-0                       Ready                      control-plane   41d   v1.26.2-gke.1001
control-plane-1                       Ready                      control-plane   41d   v1.26.2-gke.1001
control-plane-2                       Ready                      control-plane   41d   v1.26.2-gke.1001
worker-node               Ready,SchedulingDisabled       <none>          41d   v1.25.5-gke.1001

The reason behind is that we have PodDisruptionBudget for some pods on worker-node:
$ k get PodDisruptionBudget/istio-ingress -n gke-system
NAME            MIN AVAILABLE   MAX UNAVAILABLE   ALLOWED DISRUPTIONS   AGE
istio-ingress   1               N/A               0                     41d

$ k get PodDisruptionBudget/istiod -n gke-system
NAME     MIN AVAILABLE   MAX UNAVAILABLE   ALLOWED DISRUPTIONS   AGE
istiod   1               N/A               0                     41d

and those pods are supposed to be scheduled on work-nodes only as control-plane nodes have taints:
taints:
  - effect: NoSchedule
    key: node-role.kubernetes.io/control-plane

which those pods don't tolerate. So when sriov-network-config-daemon try to drain the node and evict those pods, they don't have any other nodes to go and show the following error in sriov-network-config-daemon
2023-06-26T18:57:43.787766559Z stderr F I0626 18:57:43.787737   11036 writer.go:132] setNodeStateStatus(): syncStatus: InProgress, lastSyncError:
2023-06-26T18:57:44.764460102Z stderr F I0626 18:57:44.764395   11036 daemon.go:133] evicting pod gke-system/istiod-665ccd8cfb-dtcc9
2023-06-26T18:57:44.765505368Z stderr F I0626 18:57:44.765470   11036 daemon.go:133] evicting pod gke-system/istio-ingress-77cbf5d986-8cvhc
2023-06-26T18:57:44.783806543Z stderr F E0626 18:57:44.783775   11036 daemon.go:133] error when evicting pods/"istiod-665ccd8cfb-dtcc9" -n "gke-system" (will retry after 5s): Cannot evict pod as it would violate the pod's disruption budget.
2023-06-26T18:57:44.784855495Z stderr F E0626 18:57:44.784818   11036 daemon.go:133] error when evicting pods/"istio-ingress-77cbf5d986-8cvhc" -n "gke-system" (will retry after 5s): Cannot evict pod as it would violate the pod's disruption budget.

and it just keeps retrying so node drain will never succeed. I think for such kind of cluster settings we should just skip node drain as it will never succeed.
Given that configDaemonNodeSelector  in SriovOperatorConfig  is already deciding which nodes to deploy sriov daemon for configs and for potential drains, I think it makes more sense to just decide whether there is only one "node" in cluster by listing node according to labels listed in configDaemonNodeSelector . I am thinking of the following 2 solutions:

In pkg/utils/cluster.go we modify the node list to be based on label filtering in ns os.Getenv("NAMESPACE") sriovnetworkv1.SriovOperatorConfig named default. Then in controllers/sriovoperatorconfig_controller.go it will add DisableDrain based on whether there is only one node.
We don't make the controller to manipulate SriovOperatorConfig  CR spec by adding DisableDrain, instead when there is DisableDrain in operator config, we just follow that, when there is no DisableDrain in operator config, we modify the code to still decide whether it is a single node cluster based on node label filtering, but we need to expose this if it is a single node cluster somewhere through CR I suppose, would like to hear people's ideas to see where we should put this information to show status of single node cluster determination.

@SchSeba Hey Sebastian can you comment more on what we discussed on Jul, 17, 2023's meeting?
We are trying to determine node count by filtering based on node labels in default SriovOperatorConfig .Spec.ConfigDaemonNodeSelector if they are provided. (Since only nodes with those labels will have sriov daemon deployed.) If user specifies disableDrain then we follow that, otherwise after node filtering if it is single node cluster we will disable drain underneath.
Would like to get more clarifications on whether this is a feasible change as I think this is a bug when counting nodes. Thanks!

we don't want to do it automatically when you have more than 1 node because the user needs to understand the implications of that.
meaning when you have one node you need to handle the pods reset after a configuration. when you have multiple nodes the user must manually configure the skipDrain and understand that the implication his that he will need to reset the workloads

doing some housekeeping closing this one please free to reopen if needed


ActionList.Item: Bug fix to support both inline and block description at the same time

Thanks to ton to @joshblack for his help with the types!! ✨
Found a regression that was introduced in 6 months ago, but hasn't been reported yet :)
To fix that, added a backward compatible "test condition" to slots config, so that the component can be in two different based on props. Only ActionList.Item has this use case with inline and block description.
Example:

const [slots, childrenWithoutSlots] = useSlots(props.children, {
      leadingVisual: LeadingVisual,
      trailingVisual: TrailingVisual,
-     description: Description,
+     blockDescription: [Description, props => props.variant === 'block'],
+     inlineDescription: [Description, props => props.variant !== 'block'],
    })

Show code for preview
<ActionList.Item>
  Only I
  <ActionList.Description>inline description</ActionList.Description>
</ActionList.Item>
<ActionList.Item>
  Only B
  <ActionList.Description variant="block">block description</ActionList.Description>
</ActionList.Item>
<ActionList.Item>
  I + B
  <ActionList.Description variant="inline">inline description</ActionList.Description>
  <ActionList.Description variant="block">block description</ActionList.Description>
</ActionList.Item>
<ActionList.Item>
  The everything bagel
  <ActionList.LeadingVisual><StarIcon/></ActionList.LeadingVisual>
  <ActionList.Description variant="inline">inline description</ActionList.Description>
  <ActionList.Description variant="block">block description</ActionList.Description>
  <ActionList.TrailingVisual><StarIcon/></ActionList.TrailingVisual>
</ActionList.Item>

Before
After

Hey @siddharthkp! Before I respond to the margin question, can you clarify how this is a regression? Its my understanding that its one or the other: inline or block. They shouldn't co-exist.. is there a use case I'm not aware of?

Hey @siddharthkp! Before I respond to the margin question, can you clarify how this is a regression? Its my understanding that its one or the other: inline or block. They shouldn't co-exist.. is there a use case I'm not aware of?

When Vini did the initial inventory, we saw a few use cases for it, example in the reviewer selection:


InitialRating cannot be zero?

Hi, I just tested your plugin and it works perfectly fine.
I just have one concern about the initial rating. Can we set the initial rating to zero? Or all the stars are not highlighted if the rating is zero?
Thanks

1
2
3
4
5


Add a blocking get stats interface

Hi - I am working on an application that consumes the dockerclient api! (woot) Specifically I am working with the stats API. The implementation of the stats API in this library and great and seems well suited for short lived client application. I am writing a daemon that will monitor the stats and have some additional requirements that don't seem to work well with the current implementation.

When an error occurs that causes the go routine to exit, I need to know the error and the container ID that it is related to.
I need to be able to ask a specific stats monitor (go routine) to stop politely
I would prefer to use channels over callbacks for ``stat``` processing.

Thoughts?

Yeah I'm not sure either.  When I added the original implementation I was thinking along the lines of the event stream which handles things that way (callback).
+1 for stopping events for a single container.

@pmorton @ehazlett #71 handles the 3 requests that pmorton listed above - could you take a look at it? it has slightly more complex logic, but it's been tested and it works


fix(modal): fix bug on iOS content when modal is open

Current coverage is 100% (diff: 100%)

Merging #158 into master will not change coverage

@@           master   #158   diff @@
====================================
  Files          19     19
  Lines         323    323
  Methods         0      0
  Messages        0      0
  Branches        0      0
====================================
  Hits          323    323
  Misses          0      0
  Partials        0      0

Powered by Codecov. Last update 515f471...c07e1ca


Upgrades

Consul

Upgrades consul version to v1.0.7
Removes the need to patch consul when building
Add -bind=<IP_ADDRESS> which is necessary to be able to bind on machines with multiple NIC's
Make consul a service in the snap

Generic Snapcraft

Use source-depth: 1 for appropriate parts - speeds up build time, especially for the go part
Add needed wget package for parts
Remove libzmq part and just use the deb package
Replace all instances of build/install keywords with new override-build. See DN8 and DN9.
Add as of yet unused packages for glide on armhf and i386.

Mongodb

Make mongo a service

Overall the changes look good, although I'd appreciate it if you could break this PR into four separate PRs that handle:

consul startup bind change

general build improvements (e.g. .gitignore, install vs. override build, ...)

convert mongo & consul to services

upgrade consul to 1.0.7

I may wait on merging the consul version upgrade till I get a reply from Jim as to whether the project will do this for California or not.
Please update the top-level README to mention that consul and mongo are now handled as services too!
Finally I saw that you grabbed the start-mongo script from the docker-edgex-mongo.  If we're going to use this, I'd prefer that we fix the infinite look per:
https://github.com/edgexfoundry/docker-edgex-mongo/issues/8

I can open this up as multiple PR's no problem.
Re: the mongo start script, that sounds good, a re-try limit would be easy to implement, I will integrate that as part of the mongo + consul PR's.


Fix Crash When Changing Levels with Menu Open

When changing levels or ending PIE with a menu open, the for loop within CloseAll would crash, as it would be calling functions on Menus marked as unreachable. This fix prevents this by checking if the menu is valid before calling RemoveFromStack on it.

Thanks!


[WIP] Statviz frontend refactoring

Actions before merge:

[ ]  Deps nicht mehr in root package.json
[ ]  backend deps where updated
[ ]  fix front es-lint errors

@HaGuesto
the most importand issues are fixed and this PR can now be merged.
Note that this merge request is for merging into the shared-components-package-research branch.
You can either merge the shared-components-package-research first and change this PR to merge into master or first merge this PR as it is and them merge the shared-components-package-research branch into master


Reduce required pydantic version

This helps with other integration compatibility.

Codecov Report

Merging #48 (8c5d05e) into main (778094f) will not change coverage.
The diff coverage is n/a.

:exclamation: Current head 8c5d05e differs from pull request most recent head 7560be4. Consider uploading reports for the commit 7560be4 to get more accurate results

@@           Coverage Diff           @@
##             main      #48   +/-   ##
=======================================
  Coverage   73.48%   73.48%
=======================================
  Files          11       11
  Lines        1373     1373
=======================================
  Hits         1009     1009
  Misses        364      364

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 778094f...7560be4. Read the comment docs.


Plugin service accounts

Description
This PR is part of addressing https://github.com/opensearch-project/security/issues/2941.
It introduces the Application interface as well as a ServiceAccountManager interface which is implemented by IdentityPlugins. With this new structure, when a plugin is installed, it is assigned a new ServiceAccount principal which can be used to reference the plugin. The service account principal is then assigned a set of permissions (to be implemented at a later date per https://github.com/opensearch-project/security/issues/2943) which it can use to operate without impersonating the undefined user context.
Here is a diagram explaining the design idea:

Zooming in on the installation process:

And finally on the request flow:

Related Issues
This PR is part of addressing https://github.com/opensearch-project/security/issues/2941.
Check List

[x] New functionality includes testing.

[x] All tests pass

[x] New functionality has been documented.

[x] New functionality has javadoc added

[x] Commits are signed per the DCO using --signoff
[x] Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Remaining questions related to this design are:

How do we determine when impersonation is required or not?
How do we inform OpenSearch of the context that an application is operating under?
How do we define FGAC?

This branch is going to show the spike for working on impersonation cases: https://github.com/scrawfor99/OpenSearch/tree/pluginImpersonation.

@peternied this is the service accounts for Plugins PR you and I discussed if you end up taking a look while I am oncall.

Hi @peternied, I agree that this PR is incomplete. I am mostly looking for feedback on the design at this time. I am hoping to get input on the mechanisms suggested in the diagrams before implementing the entire set of changes it will require. I also suspect that this will be the version of things that we ultimately settle on.
The changes in the Scopes PR are not adequately supportive of the final use cases. My apologies for not being more clear when I mentioned looking for feedback on my outstanding PRs. This one is very much in progress. Thank you for taking the time to review it.


Deduplicate entries in cargo --list

Fixes #6088

r? @ehuss
(rust-highfive has picked a reviewer for you, use r? to override)

@bors: r+
Thanks!
I suspect this will conflict with another PR I just r+'d as well, but let's see what bors says.

:pushpin: Commit 4d7aee133fb4baadddd968a9c32e2dd21d6dd31a has been approved by alexcrichton

:lock: Merge conflict
This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.
How do I rebase?
Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout cargo_list_dupe (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self cargo_list_dupe --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.
Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.
Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
Auto-merging tests/testsuite/cargo_command.rs
CONFLICT (content): Merge conflict in tests/testsuite/cargo_command.rs
Auto-merging src/cargo/util/command_prelude.rs
CONFLICT (content): Merge conflict in src/cargo/util/command_prelude.rs
Auto-merging src/bin/cargo/main.rs
CONFLICT (content): Merge conflict in src/bin/cargo/main.rs
Auto-merging src/bin/cargo/cli.rs
Automatic merge failed; fix conflicts and then commit the result.

:umbrella: The latest upstream changes (presumably #9764) made this pull request unmergeable. Please resolve the merge conflicts.

All good. I'm familiar with the conflicting diff (I also reviewed it since I noticed it was in the same area and I had context).
Just rebased it.

@bors: r+

:pushpin: Commit 0ab79d7aafe8fa25142e13d03a3bb16fd4699a04 has been approved by alexcrichton

:hourglass: Testing commit 0ab79d7aafe8fa25142e13d03a3bb16fd4699a04 with merge b51439fd8b505d4800a257acfecf3c69f81e35cf...

:sunny: Test successful - checks-actions
Approved by: alexcrichton
Pushing b51439fd8b505d4800a257acfecf3c69f81e35cf to master...


how to reset graph to initial state?

(forgive my psuedocode)
Is there a way to set a the-graph instance back to the initial state?
We have this structure :
<our-editor>
  <the-editor>
    graph
    nofloGraph
    <the-graph graph={{nofloGraph}}>

We have a button that is supposed to reset our editor back to its initial state, and we do that by :
<our-editor>.reset()
  <the-editor>.graph = {};

However, when reset() as above, and then load another graph, it is clear that there is still some 'state' left over from the previous graph - if a node in the new graph has the same key name as one in the old graph, then the new one inherits the ports from the old one. This is only obvious momentarily, so, for example, if the old node has an input port, and the new one has an output port, then the user can see that the 'new' node is added with an input port, but that then is removed and replaced with an output port.
The end result is correct, but we'd like to be sure that the 'reset()' is actually thorough. Is there any way to avoid this glitch?

State is a bit of a mess here, for which I take responsibility :bow:
Would scrapping the whole <the-graph> element and mounting a new one be an acceptable workaround for now?

I'm not sure of your terminology - what is 'scrapping' and 'mounting'? The latter sounds like a React thing - I've really not delved much into React. Do you mean something React-ish?
My first thought about your suggestion was it was referring to the element itself, much like any other element, it can be deleted and recreated in javascript. I guess it is possible, but we have it inside another polymer element that is referenced in many places as this.$.graph, and recreating instances by javascript would mean we have to wrap that element in a containver div and use this.$.container.querySelector('the-graph') everywhere. A bit ugly, but we could give it a try.
Let me know if I've misunderstood your advice.

We don't have a <the-graph> element in our code - we use a <the-graph-editor> element.
It seems to be that it is infeasible to reach down into <the-graph-editor> and remove its <the-graph> element. <the-graph-editor> has many references to this.$.graph and I'd not only have to re-wire those (perhaps through a getter get graph() { this.shadowRoot.querySelector('#graph'); }), but also attempt to do a boat load of setup for the new element, and who knows what other components get hooks from that first instance of <the-graph>.
Any thoughts?

What about making a new instance of the-graph-editor? Pseudo:
<your-editor>.reset: function(){
  this.removeChild( this.editor )
  this.editor = new <the-graph-editor>
  this.appendChild( this.editor )
}

No, the editor is all wired into the rest of the app with signals and what-not (methods that set and get various structures deep within). I actually spent most of the day trying to make this work, but there is far too much that is done at startup that would need to be re-done if I were to remove the editor (and therefore the graph).


Urijs updates

The first commit moves everything into a uri namespace. This is a breaking change but was done to mirror other libraries like decimal.js and moment.js. I also wasn't sure how else to support the URI('') and new URI('') syntax that this allows for.
jquery support was moved into a new file since it's not required. If this doesn't make sense then I can move it back in.
The rest of the commits start to add missing functionality from newer versions. I can submit a new PR for these if they'd be better on their own.

urijs/URI.d.ts
to authors(@RodneyJT). could you review this PR?
:+1: or :-1:?
check list

[ ] pass the Travic-CI test?

urijs/jquery.URI.d.ts
check list

[ ] is collect naming convention?

https://www.npmjs.com/package/urijs
http://bower.io/search/?q=urijs
others?

[X] has a test file? (urijs/jquery.URI-tests.ts or others)
[ ] pass the Travic-CI test?

@xt0rted thank you for your contributions.
please rename urijs/jquery.URI.d.ts to URIjs/URIjs.d.ts.
https://www.npmjs.com/package/URIjs is exists.

@vvakame I got rid of the jquery file and renamed them to URIjs to match the npm package

@xt0rted thanks mate!

Why was everything moved into a namespace? Am I supposed to do type URI = uri.URI; in every file now?


Expert availability profile ui and search ui

Experts should have the ability to indicate projects and community availability, as in:

Determine where the data will persist in fcrepo/elastic.
Also the search page needs to be extended to filter by the various open to preferences.

@Vensberg
I was just chatting with @wrenaria regarding the ui for the open-to/availability feature we're adding to AE. Are each of these preferences opt-in, so that by default nothing is enabled/checked?

@UcDust , they will be fed by Availability label data in Elements. It's new data.

In terms of updates, there is existing generic user profile update code at userprofile This is used for example at updateUserPriviacyLevel.  We need to see if updates to the availability follow this pattern, of if they have their own endpoint.

@Vensberg looking at production it appears like we ended up creating a new Availability label scheme.  I thought we were going to use a subset of the existing availability labels.  Was that incorrect?
If we are adding a new one, we should probably think about one set of labels for all aggie experts parts.  I kinda like using the baked in one better for this.

We needed to add community partnerships. We have a stakeholder request for that.

@UcDust, OK @Vensberg has let me know we aren't interested in collaborating with CDL about a common availability controlled vocabulary, and we are going to use a ucd specific one.  So, I've updated fcrepo-dev to include these.  I have added our own hasAvailability flag, so a user looks like:
"@graph": [
{
"@id": "expert/LDdgBTXN",
...,
"hasAvailability": [
{
"@id": "ark:/87287/d7nh2m/keyword/c-ucd-avail/Join%20a%20web%20conference%20as%20a%20panellist%20or%20speaker",
"@type": "Concept",
"availabilityOf": "expert/LDdgBTXN",
"inScheme": "ark:/87287/d7nh2m/keyword/c-ucd-avail/",
"prefLabel": "Join a web conference as a panellist or speaker"
},
{
"@id": "ark:/87287/d7nh2m/keyword/c-ucd-avail/Teaching%20provision",
"@type": "Concept",
"availabilityOf": "expert/LDdgBTXN",
"inScheme": "ark:/87287/d7nh2m/keyword/c-ucd-avail/",
"prefLabel": "Teaching provision"
},
{
"@id": "ark:/87287/d7nh2m/keyword/c-ucd-avail/Community-engaged%20projects",
"@type": "Concept",
"availabilityOf": "expert/LDdgBTXN",
"inScheme": "ark:/87287/d7nh2m/keyword/c-ucd-avail/",
"prefLabel": "Community-engaged projects"
},
{
"@id": "ark:/87287/d7nh2m/keyword/c-ucd-avail/Collaborative%20projects",
"@type": "Concept",
"availabilityOf": "expert/LDdgBTXN",
"inScheme": "ark:/87287/d7nh2m/keyword/c-ucd-avail/",
"prefLabel": "Collaborative projects"
},

For display, only include the ones that have
"inScheme": "ark:/87287/d7nh2m/keyword/c-ucd-avail/",

@UcDust Need to be in the qh-availability branch for this to display properly

@qjhart @Vensberg
It looks like the list of availabilities in the api response are these:
"Join a web conference as a panellist or speaker"
"Teaching provision"
"Community-engaged projects"
"Collaborative projects"
"Media enquiries"
"Mentoring (long-term)"
"Career advice"
"Technical support"
"Masters Research or PhD student supervision"
"Mentoring (short-term)"
"Membership of an advisory committee"
"Industry Projects"

Does that look right? Also, is it likely that this data could change over time?

I changed "Community-engaged projects" to "Community partnerships" to be identical with the Aggie Experts wording.
We will be using only four of these Elements labels in Aggie Experts:

Elements label
Aggie Experts label

"Community partnerships"
Community Partnerships

"Collaborative projects"
Collaborative Projects

"Media enquiries"
Media Interviews

"Industry Projects"
Industry Projects

@Vensberg:

I changed "Community-engaged projects" to "Community partnerships" to be identical with the Aggie Experts wording.

Related to this, would you like to change "media interviews" to "media enquiries" in Aggie Experts so that we match the Elements label? Enquiries and interviews seem like equally fine words, so the consistency might be nice.
@UcDust:
Since it's preferred that users make all desired availability selections prior to saving, the design has been changed to a modal solution. Tooltip hover on the Edit button should say "Edit availability."
In Figma, these pages are updated to reflect the change:

Profile User Admin
Experts "Open to" Feature

Screenshots from Figma:

Display
Profile View
Editing Modal

Desktop

Mobile

@Vensberg
When updating the availability labels, it's possible (hopefully not likely) that the update to CDL could fail.
If that happens, should we show a warning modal like we have a in a few other places? Would verbiage like this work for you?:
Updating expert availability labels could not be done through Aggie Experts right now. Please, try again later, or make changes directly in the <a href="https://oapolicy.universityofcalifornia.edu/">UC Publication Management System.

That works, but can it redirect the user to to the editing interface of the Elements user profile in a new tab?

@Vensberg Accessibility best practices is that links always open in the same window when possible to avoid loss of context. If you want links to open in a new tab, it needs to be explicitly stated that the link will open in a tab in order to avoid confusion. This is especially important for vision impaired users.
If users want to use a new tab, they can make that decision manually (right-click, open in new tab or window).

@wrenaria , I meant for it to work similarly to the Introduction and Links editing workflow, where the pop-up window that explains the process shows up on the same page, but the edit button from there opens Elements in a new tab.

@Vensberg The fact that it is already being done elsewhere doesn't change the reality that opening links in a new tab without warning is an accessibility problem; it means we need to adjust either the link text or behavior across the whole site. I missed that this link behavior was happening, or I would have said something earlier.
https://www.w3.org/TR/WCAG20-TECHS/G200.html
Best practice is to not open links in a new tab. Second best is to warn them using text associated with the link itself. Minimum compliance would be to include the warning in the modal text prior to the yellow "edit" button that appears on most modals -- for example, change You will be redirected to this system to You will be redirected to this system in a new tab.

@wrenaria , I like the second option: "You will be redirected to this system in a new tab"

@Vensberg I've created a new issue for this topic, please sign off on my proposed solution there -- issue #543

@UcDust , Elements has been updated with the shortened version of UCD labels.


BREAKING CHANGE: Fix State

Description
This changes how state is defined and works across the application to be more compatible with state store components.
Actors

Actor state now needs to extend ActorState.
The ActorState attribute needs to be removed from actors.
The actor must define the type of state in the constructor.

See the changes to the docs
Transactionless State

Must have a StateStore attribute.
Instantiate the state and call State::load_state($my_state) to set the properties from the state store.
Save state with State::save_state($my_state) to store the properties to the state store.

See the changes to the docs
Transaction State

Must have a StateStore attribute.
Extends TransactionalState
May be used with State::load_state()
Must call ->begin() to start a transaction
Must call ->commit() to commit a transaction.

See the changes to the docs
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close:
fixes #15
fixes #10
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:

[x] Code compiles correctly
[x] Created/updated tests
[x] Extended the documentation

Codecov Report

Merging #16 (36277e7) into main (a636891) will decrease coverage by 3.91%.
The diff coverage is 61.00%.

@@             Coverage Diff              @@
##               main      #16      +/-   ##
============================================
- Coverage     61.77%   57.85%   -3.92%
+ Complexity      342      336       -6
============================================
  Files            30       31       +1
  Lines          1138     1101      -37
============================================
- Hits            703      637      -66
- Misses          435      464      +29

Impacted Files
Coverage Δ
Complexity Δ

src/index.php
0.00% <0.00%> (ø)
10.00 <1.00> (+1.00)

src/lib/Actors/ActorState.php
37.73% <37.73%> (-62.27%)
20.00 <20.00> (+19.00)
:arrow_down:

src/lib/State/TransactionalState.php
75.00% <73.17%> (-14.91%)
14.00 <12.00> (-19.00)

src/lib/State/Internal/StateHelpers.php
75.00% <75.00%> (ø)
2.00 <2.00> (?)

src/lib/State/State.php
92.85% <92.30%> (-2.02%)
11.00 <11.00> (-11.00)

src/lib/Actors/ActorRuntime.php
74.75% <100.00%> (+5.23%)
29.00 <5.00> (+2.00)

src/lib/State/Attributes/StateStore.php
100.00% <100.00%> (ø)
1.00 <1.00> (?)

src/lib/State/Internal/Transaction.php
100.00% <100.00%> (ø)
4.00 <4.00> (?)

src/lib/consistency/StrongFirstWrite.php
0.00% <0.00%> (-100.00%)
2.00% <0.00%> (ø%)

... and 2 more

Continue to review full report at Codecov.

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

Codecov Report

Merging #16 (36277e7) into main (a636891) will decrease coverage by 3.91%.
The diff coverage is 61.00%.

@@             Coverage Diff              @@
##               main      #16      +/-   ##
============================================
- Coverage     61.77%   57.85%   -3.92%
+ Complexity      342      336       -6
============================================
  Files            30       31       +1
  Lines          1138     1101      -37
============================================
- Hits            703      637      -66
- Misses          435      464      +29

Impacted Files
Coverage Δ
Complexity Δ

src/index.php
0.00% <0.00%> (ø)
10.00 <1.00> (+1.00)

src/lib/Actors/ActorState.php
37.73% <37.73%> (-62.27%)
20.00 <20.00> (+19.00)
:arrow_down:

src/lib/State/TransactionalState.php
75.00% <73.17%> (-14.91%)
14.00 <12.00> (-19.00)

src/lib/State/Internal/StateHelpers.php
75.00% <75.00%> (ø)
2.00 <2.00> (?)

src/lib/State/State.php
92.85% <92.30%> (-2.02%)
11.00 <11.00> (-11.00)

src/lib/Actors/ActorRuntime.php
74.75% <100.00%> (+5.23%)
29.00 <5.00> (+2.00)

src/lib/State/Attributes/StateStore.php
100.00% <100.00%> (ø)
1.00 <1.00> (?)

src/lib/State/Internal/Transaction.php
100.00% <100.00%> (ø)
4.00 <4.00> (?)

src/lib/consistency/StrongFirstWrite.php
0.00% <0.00%> (-100.00%)
2.00% <0.00%> (ø%)

... and 2 more

Continue to review full report at Codecov.

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

Will add more tests to increase coverage

Will add more tests to increase coverage


Disallow the creation of text exercises with negative maxPoints

Checklist

[x] I tested all changes and all related features with different users (student, tutor, instructor, admin) on the test server https://artemistest.ase.in.tum.de locally.
[x] Server: I followed the coding and design guidelines.
[x] Client: I followed the coding and design guidelines.

Motivation and Context
This PR disallows the creation of text exercises with negative points. The save-Button was just never disabled on the client. Also the server just checked the maximum points for being equal to 0, not less or equal.
Description

Disabled the save-Button or form errors for text exercises
Validating the exercise against negative points on the server.

Steps for Testing

Try creating a text exercise with a negative score -> not possible.
Try creating a text exercise with valid settings -> save-Button is active.

Disabling the save button is done in #3262 so this PR is a one-liner now.

I added the validation of the bonus points based on the IncludedInOverallScore by using an enum-method. I'm not sure if this is the best way for doing this so I'd like to hear your feedback.

One thing I noticed: Does the following selector even work?

Looks like a regression from the strict template changes, I filed #3265.

Opend #3279 on an internal branch.


Isuue with messages stram

Hi! I need a clarification.
  Stream<List<types.Message>> messages(types.Room room) {
It always returns all the messages from the room, which means that parsing all over the messages occurs on each new message, right? Don't you think it's time consuming?
Should the stream return only the updated document?
What do you think?

 We will need to split between the load and update otherwise?

In my implementation, I first of all get all the messages currently on firestore and then listens to the updates:
Check this stackoverflow question I have asked

Yeah, but this is like for the last added element. What about history or updates?

Well, on my implementation I first off all get all the messages and then the stream listens to updates, use "updatedAt" instead of createdAt to get updated messages. For deleted - I don't have a solution

Yeah, forgot about deleted ones.. So basically this was done as a proof of concept of the chat and planned as something that will work (and I need all updates so it works). Never planned further optimisations, because making and describing to different people and different use cases how it works - it does not make sense. Every case will be unique and there will never be a perfect library. So I think it should at least work as intended - as an early stage proof of concept, where you can show a working chat to your client and save some money on an initial implementation and then do a complete solution when client has more money to spend :)

Friend you did an excellent job!
If you were in Israel, we would hire you, definitely.


Add Python 3.12 to CI

Looks like the version manifest file was not updated yet.

Codecov Report

Merging #80 (8b6cd68) into main (24c31b5) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #80   +/-   ##
=========================================
  Coverage   100.00%   100.00%
=========================================
  Files            2         2
  Lines          433       433
  Branches        21        21
=========================================
  Hits           433       433

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

Meh, closing until the first beta.


Full-on crash

If you are filing a bug report, please answer these questions. If your issue is not a bug report, you do not need to use this template. Either way, please consider donating if we've helped you. Thanks!
1. What version of Caddy are you running (caddy -version)?
0.8.3
2. What are you trying to do?
Host web sites?
 What is your entire Caddyfile?
jacobgadikian.com www.jacobgadikian.com {
gzip
tls<EMAIL_ADDRESS>root            /jacobgadikian/public
basicauth       /admin ******************(redacted)
hugo            /jacobgadikian/
}

kloudery.com www.kloudery.com {
gzip
tls<EMAIL_ADDRESS>root            /kloudsweb
}

blog.kloudery.com {
tls<EMAIL_ADDRESS>proxy           localhost:32768 blog.kloudery.com
gzip
}

4. How did you run Caddy (give the full command and describe the execution environment)?
systemctl restart caddy
and here's the unit file:
[Unit]
Description=Caddy Web Server
[Service]
ExecStart=/usr/bin/caddy -conf /root/Caddyfile
[Install]
WantedBy=multi-user.target
5. What did you expect to see?
Well, what I wasn't expecting was (see below) anyway...
6. What did you see instead (give full error messages and/or log)?
● caddy.service - Caddy Web Server
Loaded: loaded (/etc/systemd/system/caddy.service; enabled; vendor preset: enabled)
Active: failed (Result: exit-code) since Sat 2016-05-21 07:16:12 UTC; 7s ago
Process: 30212 ExecStart=/usr/bin/caddy -conf /root/Caddyfile (code=exited, status=2)
Main PID: 30212 (code=exited, status=2)
May 21 07:16:12 fuzzbox2 caddy[30212]: /home/caddy/go/src/github.com/hacdias/caddy-hugo/hugo.go:54 +0x3ff
May 21 07:16:12 fuzzbox2 caddy[30212]: github.com/mholt/caddy/caddy.loadConfigs(0x7ffcca5f2f8c, 0x9, 0x7fa0cbf5f4e0, 0xc820226480, 0x0, 0x0, 0x0, 0x0, 0x0)
May 21 07:16:12 fuzzbox2 caddy[30212]: /tmp/custombuild_1635_943691114/src/github.com/mholt/caddy/caddy/config.go:166 +0xa15
May 21 07:16:12 fuzzbox2 caddy[30212]: github.com/mholt/caddy/caddy.Start(0x7fa0cbf23a10, 0xc820226450, 0x0, 0x0)
May 21 07:16:12 fuzzbox2 caddy[30212]: /tmp/custombuild_1635_943691114/src/github.com/mholt/caddy/caddy/caddy.go:145 +0x25a
May 21 07:16:12 fuzzbox2 caddy[30212]: main.main()
May 21 07:16:12 fuzzbox2 caddy[30212]: /tmp/custombuild_1635_943691114/src/github.com/mholt/caddy/main.go:102 +0x9c6
May 21 07:16:12 fuzzbox2 systemd[1]: caddy.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
May 21 07:16:12 fuzzbox2 systemd[1]: caddy.service: Unit entered failed state.
May 21 07:16:12 fuzzbox2 systemd[1]: caddy.service: Failed with result 'exit-code'.
7. How can someone who is starting from scratch reproduce this behavior as minimally as possible?
I do not know.

Tried directions in #840, did not resolve the crash.

Here's my systemd unit:
[Unit]
Description=Caddy web server
After=network.target

[Service]
; an empty value clears the original (and preceding) settings
ExecStart=
ExecStart=/usr/bin/caddy -conf="/root/Caddyfile"

[Install]
WantedBy=multi-user.target

This doesn't seem to be related to systemd. Does Caddy/Hugo panic when started without an init daemon?

let me check.

I'd had my working directory for hugo misconfigured.  Thanks @wmark!


"Loading events" hangs (fiber not defined)

I'm getting errors due to undefined fibers. Debugger is pointing specifically at:
https://github.com/lahmatiy/react-render-tracker/blob/a3794feb16f2a7b330b24208d1b775e7800678ba/src/data/process-events.ts#L203-L206
Other cases seem to be solving this by first checking getFiberById.has(fiberId) and continuing if not. However, I'm guessing this would create knock-on effects downstream. Not to mention that the missing fiber is probably a symptom of an issue further upstream. Will investigate further with a local / unminified build and see what I can find.
Please let me know if there's any further detail I can provide.

After some (very) high-level debugging I was able to apply a band-aid solution to get rendering working on our unwieldy app, with the following steps:

Check for the existence of the fiber in process-events.ts > processEvents > case "update":

export function processEvents(
    [...]
    case "update":
        if (!fiberById.has(event.fiberId)) {
          continue;
        }
    [...]

Default to the closest fiber in fiber-maps.tsx > useFiberChildren

export const useFiberChildren = (
  fiberId: number,
  groupByParent = false,
  includeUnmounted = false
) => {
  const { selectTree } = useFiberMaps();
  const tree = selectTree(groupByParent, includeUnmounted);
  let leaf = tree.getOrCreate(fiberId);

  // fiber {0} invalid? find the next one that works
  if (!fiberId && leaf?.fiber == null && ) {
    for (const [activeFiberId, { fiber, firstChild }] of tree.nodes) {
      if (fiber && firstChild) {
        leaf = tree.getOrCreate(activeFiberId);
        break;
      }
    }
  }
  [...]

This solution obviously doesn't take into account the root cause. I'm also not aware of any knock-on effects at this time, though I'm sure they exist.
Am really appreciating the project. Keen to help find a solution that can make its way upstream. Any more insight / pointers in the regard would be much appreciated.

@theprojectsomething Please, try a new version 0.7.1 released today. Does problem persist?
Your proposition is fixing consequences. I believe, it should not be fixed that way. I did some refactoring in the last release, it might help. Otherwise, in your code some fiber type is used that is not covered by RRT properly. That's the most luckily a root cause. Just ignoring an event can lead to more errors.

Thanks @lahmatiy ... appreciate the quick response! Yes, the fix here is only working around consequences, though we're keen to dig further into it if you're able to provide any pointers. Is there a straightforward way to debug fiber types within the tree map / event processor?
As far as I know we're not doing anything radical in our environment. Currently running react 18.0.34. Could it potentially be a plugin or even custom elements?
Regards latest updates, the build we've based our testing off here (and in the fork) is the latest from the main branch, which included your updates from a couple of days ago. I've just rebased now to confirm, but yes the problem remains without the workaround in place.
I put up a placeholder PR #33  with our fix in place, but there is no intention to have that merged. Just as a discussion starter.

I should note that the useFiberChildren fix only applies to the root node (fiberId===0), so there would appear to be no other effected leafs / trees. Does the root node always start at index 0?

Please try 0.7.3, it may works

Thanks @lahmatiy I've just tried it but unfortunately no luck with 0.7.3. The errors look similar, if not the same, as the previous builds (i.e. attempting to directly access properties of undefined fibers within the update case of  processEvents).
I will try and find some time to debug the latest version (0.7.3) this week. And potentially to step through the whole data flow so I have a better mental model of what might be going wrong upstream, in our specific scenario.
If you're interested, we're attempting to implement the library on the (non-public) development version of abc.net.au/news. If you have a way to inject a script prior to page load (e.g. tampermonkey) then you should be able to see the errors we're seeing at that URL.
Thanks again!


dockerfile-add-scanner: Update containerd and buildkit

dependabot flagged (but could not generate a PR) an issue in dockerfile-add-scanner. This issue is not security-relevant since dockerfile-add-scanner is just a tool we run during the build and not at runtime, but updating this removes a source of noice. The underlying issue in containerd is:
/sys/devices/virtual/powercap accessible by default to containers
Intel's RAPL (Running Average Power Limit) feature, introduced by the Sandy Bridge microarchitecture, provides software insights into hardware energy consumption. To facilitate this, Intel introduced the powercap framework in Linux kernel 3.13, which reads values via relevant MSRs (model specific registers) and provides unprivileged userspace access via sysfs. As RAPL is an interface to access a hardware feature, it is only available when running on bare metal with the module compiled into the kernel.
By 2019, it was realized that in some cases unprivileged access to RAPL readings could be exploited as a power-based side-channel against security features including AES-NI (potentially inside a SGX enclave) and KASLR (kernel address space layout randomization). Also known as the PLATYPUS attack, Intel assigned CVE-2020-8694 and CVE-2020-8695, and AMD assigned CVE-2020-12912.

Superceeded by #3746 3746


gomobile doesn't support go1.13

What version of Go are you using (go version)?

$ go version
go version go1.13 darwin/amd64

Does this issue reproduce with the latest release?
yess
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE="off"
GOARCH="amd64"
GOBIN="/Users/fredlee/Documents/develop/go/workspace/bin"
GOCACHE="/Users/fredlee/Library/Caches/go-build"
GOENV="/Users/fredlee/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/fredlee/Documents/develop/go/workspace"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/local/go"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/34/z2z_vp1573g5014m8k7wys100000gn/T/go-build483921249=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.13 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.13
uname -v: Darwin Kernel Version 18.2.0: Fri Oct  5 19:41:49 PDT 2018; root:xnu-4903.221.2~2/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.14.1
BuildVersion:	18B75
lldb --version: lldb-1<IP_ADDRESS>
  Swift-4.2

What did you do?
/Users/fredlee/Documents/develop/go/workspace/src/app.sdk/lightControl
macbookpro:lightControl fredlee$ gomobile bind -o lightControl.aar -target=android app.sdk/lightControl
gomobile: /Users/fredlee/Documents/develop/go/workspace/bin/gobind -lang=go,java -outdir=/var/folders/34/z2z_vp1573g5014m8k7wys100000gn/T/gomobile-work-981232532 app.sdk/lightControl failed: exit status 1
type-checking package "app.sdk/lightControl" failed (/Users/fredlee/Documents/develop/go/workspace/src/app.sdk/lightControl/addr.go:4:2: could not import bytes (type-checking package "bytes" failed (/usr/local/go/src/bytes/buffer.go:12:2: could not import unicode/utf8 (/usr/local/go/src/unicode/utf8/utf8.go:28:8: expected ';', found b00000000 (and 10 more errors)))))
What did you expect to see?
gomobile bind -o lightControl.aar -target=android app.sdk/lightControl
it success

It looks like you 1.13 installation is somehow messed up. Did this start immediately after upgrading to 1.13 ? Can you run other programs using 1.13 ?

yes, i run gomobile after upgrading to 1.13,  and i can run my other go code using 1.13 without wrong info, only runing gomobile it failed.

Please try go install golang.org/x/mobile/cmd/... to rebuild the gomobile (and gobind) tool.

thanks, i rebuild the gomobile (and gobind) tool, it workssss

Thanks for following up.


rafctor LookupCluster

Please provide a description of this PR:

remove duplicate code
introduce new metric for lookup fail

@zirain: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name
Commit
Details
Required
Rerun command

release-notes_istio
695f083bb78b12c807626a2d5e8873858e302768
link
true
/test release-notes

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

@zirain: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name
Commit
Details
Required
Rerun command

release-notes_istio
e6d4dc88016ac720beb0e6abf43f5cd9f9f36b07
link
true
/test release-notes

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

@zirain: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name
Commit
Details
Required
Rerun command

release-notes_istio
e6d4dc88016ac720beb0e6abf43f5cd9f9f36b07
link
true
/test release-notes

lint_istio
e6d4dc88016ac720beb0e6abf43f5cd9f9f36b07
link
true
/test lint

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

@zirain: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name
Commit
Details
Required
Rerun command

release-notes_istio
e6d4dc88016ac720beb0e6abf43f5cd9f9f36b07
link
true
/test release-notes

lint_istio
e6d4dc88016ac720beb0e6abf43f5cd9f9f36b07
link
true
/test lint

gencheck_istio
e6d4dc88016ac720beb0e6abf43f5cd9f9f36b07
link
true
/test gencheck

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


Ensure the messages have no precedding tabs

fixes #69
I needed to require 'JSON' in my local specs to get them to run. I wonder if circle/travis are on a different ruby to me.

Sorry about that 😰

Nothing was "released", so all is :+1:


Set up Pyodide in the top level scope

This ensures that Pyodide instantiation and package loading happens inside of the enterStartupJs limitEnforcer. In workerd, we can't yet load packages outside of an IoContext because we make actual requests to load the packages. So in workerd, only run loadPyodide at top level and run setupPackages at first request. In the internal runtime, we can do the whole thing at top level.
As a side effect, we have to disable hash randomization because normally the hash comes from crypto.getRandomValues but this crashes at top level. Likewise, Python initializes the random module's random seed using crypto.getRandomValues. We replace crypto.getRandomValues with a function that tries to call crypto.getRandomValues but if it throws an Error saying "Disallowed operation called within global scope" then we fall back to doing nothing. We reseed the rng in the first request context. Note however that hash randomization can only happen at startup, if we don't have access to crypographic rng at startup we can't enable hash randomization later.

Supposedly at top level the time shows 0 so you can't get entropy that way. You can use Math.random to get non cryptographic rng. But I think reseeding later with cryptographic rng is the right thing to do. Otoh any source of startup rng would be good for the hash seed. On the other other hand, memory snapshots imply a fixed hash seed anyways so we can't really win there unless we periodically regenerate the memory snapshots. This will have to be a longer term discussion I think.


add python2.7 environment on CI test

python2.7 test is failed, it will be fixed after merged #4

👌


Namespacing

I am a bit worried about our namespaces. It seems like we have three things: models, tests, analyses, and they all have their own namespace tree. Seems weird. I wonder if top level shouldn't be pardot and then within that there are tests, models, analysis. There could potentially be other elements as well.
Curious to know whether this has occurred to you / if you see it as a problem. Might be a useful cleanup.

@jthandy i like this idea! I don't think we should mandate a folder structure, but i do think there should be an opinionated way of doing things. I think that opinion should be something like you mentioned:
model_name/
  - tests/
  - models/
  - analysis/

Need to give this some more thought...

Right now, all of the stuff within the old analytics repo has been split into its own repositories. This creates a bunch of really simple repos with narrow focus, which solves much of this problem.
I do think that this is going to have its own challenges once we begin to build out more extensive analytical repos--I could imagine rjmetrics/analytics as a repo for all of our internal analytics, and at that point I would think you'd want modules that contain their own folder structures.
Regardless, at this point I don't feel the pain around this because of package management. I'm going to close this for now and we can reevaluate later as needed.


Fix non-ascii char in readme

What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
Review checklist

[ ] PR has a meaningful title or PR has the no-changelog label attached
[ ] Feature or bugfix has tests
[ ] Git history is clean
[ ] If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo

Additional Notes
Anything else we should know when reviewing?

Hello,
What exactly does this fix? Like, what was the "weird char", just out of curiosity? :)

I believe it was an unbreakable space

yeah I think it would be better. I thought it would be a one time thing but apparently not

Also PR descriptions!

I strongly agree with @gzussa in that we should instead make the pipeline support unicode.

This PR can be merged since the other one covers it.


tutorial04 - No module named 'forcebalance'

Hi! I have an issue while running tutorial04. While trying "forcebalance.evaluator_io import Evaluator_SMIRNOFF" I get "No module named 'forcebalance'" error.

You probably don't have it installed - try mamba install forcebalance -c conda-forge