Dashboard improvements (8 Oct 2024)

Pull Request Description

:warning: Follow up to #11219. MUST NOT be merged in before that PR.

Changes:

Add optional overlay to Popovers
Add and use useAssetPassiveListener everywhere to get reactive updates to asset state even outside Asset Panel
setItem has been removed in favor of simply waiting for invalidations
Migrate more Modals to Popovers
Migrate more inputs to Forms
Periodically refetch Datalink in Asset Panel
Show optimistic state for asset description (adding this because it is trivial to add)
Remove usages of get* as mutations throughout the entire codebase - replace with fetchQuery

Fixes most of rest of https://github.com/enso-org/cloud-v2/issues/1529

(1) :information_source: fixed in #11219
(2) :x: backend issue
(3) :x: out of scope
(4) :x: backend issue
(5) :x: out of scope
(6) :x: [wontfix]? i think this is intentional, it's not so much slow scrolling and moreso snapped scrolling
(7) :x: backend issue
(8) :information_source: fixed in #11126
(9) :x: out of scope (potentially requires a way to trigger a tooltip on a disabled button)
(10) :x: (will check later) Make sure you are not able to open a project opened by another user: cmd + click is not always working.
(11) Drag from team space to user space should copy asset
(12) :x: (will check later) Drag from user space to team should move (and swap ownership)
(13) :information_source: fixed in #11219
(14) :information_source: fixed somewhere (?)
(15) :information_source: fixed somewhere (?)
(16) Show correct (and up-to-date) description for projects
(17) :information_source: fixed in #11219
(18) :information_source: fixed in #11219

Fix https://github.com/enso-org/cloud-v2/issues/1535

Completely remove optimistic UI for "copy asset"

Fix https://github.com/enso-org/cloud-v2/issues/1541

Make selection brush work again
Unintentionally regressed in https://github.com/enso-org/enso/commit/51733ee87677fb47e7a123c98660bc72abe1a9fd#diff-f3e29bffcda342ab6a9dbafc58dde88ce26638eaecda1f17f40ca7e319c90cc8L89

Important Notes
None
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:

[x] The documentation has been updated, if necessary.
[x] Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
[x] All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
[x] Unit tests have been written where possible.

assigning @MrFlashAccount for QA (not super urgent of course!)

Something broke with datalink modal

Box selection is only working for bottom assets... because it is limited to the asset table I can't select 3 top ones or just the ones in the middle... should we fix / improve this as separate PR?

Can't open a local project...

[box selection] is limited to the asset table

this one is an easy fix, because actually we had such behavior before

@MrFlashAccount i guess this one is awaiting re-CR?


[WIP] clean and lint code

Just removing useless stuff and lint code with black--which is now widely used in the community. To use black, pip install black and black pyomeca/ before pushing to GitHub. Or Pycharm integration is described in repo if needed.
Related to #69

Ready to be merged


Files with missing cell

Just tried to read an xyz file that Ilyes sent me, a dataset of cumulene molecules in vacuum, therefore no cell is defined and the "cell" is missing entirely. Could the package be extended to allow this or is this when pbc = "FFF" and specifiy an automatic cubic cell that includes all particles.?
But maybe it goes against the specification of the format?
Example:
13
Properties=species:S:1:pos:R:3:forces:R:3 energy=-66.79083251953125 pbc="F F F"
C       -5.13553286       0.00000000       0.00000000       0.00006186      -0.00000000       0.00000000
H       -5.72485781      -0.91726500       0.00000000      -0.00001280       0.00000756       0.00000000
H       -5.72485781       0.91726500       0.00000000      -0.00001280      -0.00000756      -0.00000000
C       -3.82464290       0.00000000       0.00000000      -0.00004186       0.00000000       0.00000000
C       -2.55226111       0.00000000       0.00000000       0.00003090      -0.00000000      -0.00000000
C       -1.27494299       0.00000000       0.00000000      -0.00009426       0.00000000      -0.00000000
C        0.00000000       0.00000000       0.00000000       0.00000000      -0.00000000      -0.00000000
C        1.27494299       0.00000000       0.00000000       0.00009426       0.00000000       0.00000000
C        2.55226111       0.00000000       0.00000000      -0.00003090      -0.00000000       0.00000000
C        3.82464290       0.00000000       0.00000000       0.00004186       0.00000000      -0.00000000
H        5.72485781       0.00000000       0.91726500       0.00001280      -0.00000000      -0.00000756
H        5.72485781       0.00000000      -0.91726500       0.00001280       0.00000000       0.00000756
C        5.13553286       0.00000000       0.00000000      -0.00006186      -0.00000000      -0.00000000

btw - completely not urgent, I just fixed the xyz file and can now load it. It just seemed like a sensible default to build in.

This should now be fixed in master: if cell is missing then AtomsBase bounding_box() interface returns an infinite box.


[BUG] Hover effect for "My Second post"

Bug Report
Describe the bug

When we hover onto other headings like "Vyvy-vi's Blogs" and "My First post" everything seems to be perfect but hovering upon the "My Second post" shows some difference.
All others becomes bold black after hovering but the "My Second post" becomes white which is invisible in the light mode at least.

Expected behavior

All the hover styles for the blog titles should be the same to develop a sense of continuity.

Screenshots/ Video
https://user-images.githubusercontent.com/53310847/118592603-d382c300-b7c3-11eb-97b4-9c5f2e0a1f17.mp4

:O That was fast xD
Thanks for opening an issue!!! 🎉
The earlier implementation was buggy and not well-polished, since it relied completely on pelican.
The current version uses Jekyll with custom theme-ing

I am going to close this issue, however, thanks for the opening it.
You also get the 🟩 . I'll keep this in mind while building the updated site with Jekyll

Yeah! sure. :)


Incorrect Display of Skill Magazines

The 'TrackersAlliancePreparednessPrime' series of Magazines always displays SkillMagazine number 4.

Details
The error is that the same nif file is always called up for all 5 magazines. But the nifs, materials and textures are present
These are the wrong formIDs:
00209097: MODL: Items\Magazines\TrackersAlliance_04.nif
must be: Items\Magazines\Magazine_TrackersAlliance_05.nif
00209099: MODL:
Items\Magazines\Magazine_TrackersAlliance_04.nif
must be:
Items\Magazines\Magazine_TrackersAlliance_03.nif
0020909a: MODL:
Items\Magazines\TrackersAlliance_04.nif
must be:
Items\Magazines\Magazine_TrackersAlliance_02.nif
0020909b: MODL:
Items\Magazines\TrackersAlliance_04.nif
must be:
Items\Magazines\Magazine_TrackersAlliance_01.nif
Game Version
<IP_ADDRESS>
Language
English (EN)
New Game
Yes
Reproduction Steps
Collect all TrackersAlliance Magazines and look that all are the same.

This appears to have been intentional. Models 01, 02, 03, and 05 all mention specific weapon types. 04 has a generic "Accelerated Combat Training" cover. When you match the the books with the models they were supposed to have, the perks given by these books don't match the cover anymore.
We could change the perks to match the covers, but that would cross the line of bug fixing.
Because of this, I will veto this one. Even though it stings to see four perfectly good models go to waste. This would be a good change for a separate mod though.


cert expired ;)

https://junegunn.kr/images/fzf-0.17.0.png
This server could not prove that it is junegunn.kr; its security certificate expired in the last day. This may be caused by a misconfiguration or an attacker intercepting your connection. Your computer's clock is currently set to Thursday, 12 August 2021. Does that look right? If not, you should correct your system's clock and then refresh this page.

Thank yout. I had to restart the nginx process. I'd want to avoid doing that manually, but I haven't had time to investigate.


Fix configuration inconsistency in app &  Kubernetes depoyment in '_run/single'

We did some investigation and found a few things

Once the akashd pod is running the container is up but you can't get to it, you just get a 502 Bad gateway response
The akash application is listening on tcp://localhost:26657 not on the wildcard address
Tendermint is creating a default config.toml file since we aren't providing one in the container
Even specifying the command line flag for the TCP port for RPC doesn't seem to override it, this should be fixed since command line switches should always win.

We need to make our configuration consistent & document how we're doing it as well as recommended methods.

Here's the result of my investigation on this so far

I played with setting environmental variables like AKASH_RPC_LADDR on my local machine and they don't do anything that I can see with. I think we may have to call viper.SetEnvKeyReplacer to get it to replace the dot character with underscores when checking the environmental variables for configuration
There appears to be a bug in cosmos SDK when loading the configuration for tendermint, explained below

In cmd /akash/cmd/root.go we install a PersistenPreRunE function into cobra which gets executed and calls server.InterceptConfigsPreRunHandler which is expected from the cosmos SDK. That function is defined in this file
https://github.com/cosmos/cosmos-sdk/blob/9856327d95eba4c3ae2d15d26a76ad3ac17b20b8/server/util.go#L57
That function calls a private function called interceptConfigs which has the following section
	rootDir := rootViper.GetString(flags.FlagHome)
	configPath := filepath.Join(rootDir, "config")
	configFile := filepath.Join(configPath, "config.toml")

	conf := tmcfg.DefaultConfig()

	if _, err := os.Stat(configFile); os.IsNotExist(err) {
		tmcfg.EnsureRoot(rootDir)

		if err = conf.ValidateBasic(); err != nil {
			return nil, fmt.Errorf("error in config file: %v", err)
		}

		conf.RPC.PprofListenAddress = "localhost:6060"
		conf.P2P.RecvRate = 5120000
		conf.P2P.SendRate = 5120000
		conf.Consensus.TimeoutCommit = 5 * time.Second
		tmcfg.WriteConfigFile(configFile, conf)
	} else {
		rootViper.SetConfigType("toml")
		rootViper.SetConfigName("config")
		rootViper.AddConfigPath(configPath)
		if err := rootViper.ReadInConfig(); err != nil {
			return nil, fmt.Errorf("failed to read in app.toml: %w", err)
		}

		if err := rootViper.Unmarshal(conf); err != nil {
			return nil, err
		}
	}

Note that rootViper is created in InterceptConfigsPreRunHandler, passed into this function and does not leave that scope.
In the code above if config.toml cannot be loaded it is created from defaults. This file does not exist presently in container when started in kubernetes. If the file does exist, the file is parsed by called rootViper.ReadInConfig and then rootViper.Unmarshal(confg) is called to update the tendermint configuration with whatever is in rootViper.
rootViper contains the entirety of the configuration set on the command line due to the fact that the flags are bound to it in InterceptConfigsPreRunHandler. So although this section of code appears to read and load a configuration file, it actually picks up the command line flags. However, if config.toml did not exist then the call to rootViper.Unmarshal never happens. This means the command line flags are never used to update the tendermint configuration.
I was able to confirm this by adding to our kubernetes startup script touch /home/config/config.toml to create an empty file before running akash. When I did that and set the command line flag I was able to get the process to listen on <IP_ADDRESS>::22657.  I could then access the process over HTTP using the Ingress declared in Kubernetes.
So to me this is a bug in Cosmos we need to fix or a bug in our usage of Cosmos.

I did a local checkout of cosmos SDK and moved the call to rootViper.Unmarshal to being after the if / else loop. After that change, things work as expected with respect to command line switches taking priority over the configuration file.
Still doing further investigation on why env. vars. don't work

As a test I shoved in this
 rootViper.SetEnvPrefix("AKASH")
  rootViper.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_"))
  rootViper.AutomaticEnv()

And the env vars work as expected now. So this one situation with rootViper is causing both issues.

Was this fixed in #862 ?

It looks like the fix for this was reverted in https://github.com/ovrclk/akash/commit/dd86936e7473d52e3f2681ddcf43907af80f0f95 because we are no longer using the fork I made of Cosmos


Fix commands overview title in nav bar and page.

This change is

v22.1/cockroach-commands.md, line 2 at r1 (raw file):

---
title: cockroach Commands Overview

So... I get that this is lowercase because it's supposed to be code-formatted, but it can't be formatted because it's in a title.
But... it's lowercaseness here, I'm not sure the authorial intent for it be a doomed homage to code-format really lands, and it kind of just looks like a typo? So it might be better to just leave it capitalized. I could see doing that and still even leaving the side-bar handle as is, with the code format.
The page title could also be "CockroachDB CLI commands". 'cockroach', as in cockroach sql... is the file-handle of the executable that is the CockroachDB CLI, right?  So cockroach commands are CockroachDB CLI commands, nah?
By the same token, I rather dislike "This page introduces the cockroach commands for configuring, starting, and managing a CockroachDB cluster, as well as environment variables that can be used in place of certain flags."
It seems ungrammatical; I don't think English supports code interpolation that way. Code blocks make sense when we say "run code", or refer to like the param parameter, but in that sentence, seems to me it should say "...introduces the CockroachDB CLI commands..."
Just being real nit-picky here.

v22.1/cockroach-commands.md, line 2 at r1 (raw file):

---
title: cockroach Commands Overview

So... I get that this is lowercase because it's supposed to be code-formatted, but it can't be formatted because it's in a title. But... it's lowercaseness here, I'm not sure the authorial intent for it be a doomed homage to code-format really lands, and it kind of just looks like a typo? So it might be better to just leave it capitalized. I could see doing that and still even leaving the side-bar handle as is, with the code format. The page title could also be "CockroachDB CLI commands". 'cockroach', as in cockroach sql... is the file-handle of the executable that is the CockroachDB CLI, right? So cockroach commands are CockroachDB CLI commands, nah?

We need an Overview page to distinguish it from the folder cockroach Commands containing the individual pages and Overview is pretty much our convention for these types of pages.
I think we have to live with the fact that we can't put code-format cockroach in the title.
The page title should match the nav bar title.

By the same token, I rather dislike "This page introduces the cockroach commands for configuring, starting, and managing a CockroachDB cluster, as well as environment variables that can be used in place of certain flags."
It seems ungrammatical; I don't think English supports code interpolation that way. Code blocks make sense when we say "run code", or refer to like the param parameter, but in that sentence, seems to me it should say "...introduces the CockroachDB CLI commands..."
Just being real nit-picky here.

This PR isn't about the page contents, it's  only about the titles in the nav bar and page.


deadlock: page fault handler fails to lock process in readv() syscall.

Solutions:

Map delay-mapped pages in check_(mut_)ptr
Drop lock explicitly
Make lock reentrant

We use No. 2 solution for now.


Sound rendering engine

In the readme.md it indicates that the audio renderer is currently not open sourced. Are there any plans to do so in the future, or prior open source implementations of the bidirectional path tracing algorithm?

@ChrisIck I'm so sorry about the late reply. For the simulation, we are currently not planning to release the code, but are planning to release the binaries early next year. Keep an eye out for that!
As for the intern recruiting, I think it's best to directly contact the audio team. This position might be of interest to you: https://www.facebookcareers.com/jobs/229191922518250. I look forward to seeing and working with you in the future!


Network recreation in one step

We might want to experiment with fabrica-config.json ei. add organizations, peers and recreate network in one command.

resolved


VS Code all of a sudden giving error AL0197 and AL0118

We have a repo another developer pushed some changes, when pulled by me VS Code started showing AL0197 and AL0118 errors. We usually clone and work on repos in Onedrive location. Initially we thought its a UNC Path and we tried cloning the repo on Local Drive and the errors are still there.
The app doesn't have any extension and depends only on
We tried following things and the problem is still there.

We cloned the repo on other machine and checked
We cloned the repo on Local Machine Document folder
Disabled AL language extension in VS Code, restarted the VS Code and enabled the extension again. The errors are still there.
Deleted and re-downloaded the symbols the errors are still there

2. To Reproduce
Can't reproduce.
Any suggestions or place we should look at to resolve this?
Thanks in advance.

Reset to the commit before the pushed changes and reload VS Code

Make sure the rad.json file and the generated *.app files are not in the committed repo; clear them from your working directory before rebuild.
Are you using the same version of the AL extension as your colleague? Eg have you or they loaded a vsix or blocked updates?
Some VSc extensions create cached and temp AL files, make sure you don't have any uncleaned leftovers from them.
Remove any unnecessary APP files from the .alpackages directory. Especially an earlier version of this extension. Also best if these are NOT commited either.
If you have "al.enableCodeAnalysis": true, tuned on make sure you select the scope eg: "al.backgroundCodeAnalysis": "Project",

No version, no clear repro. CLosing.


Entity default attacks

Can you please list out the default attack damage of each boss? This way we know what the modifier in the config is actually modifying
Example
if I use 2.0 for fallen_icon_damage_modifier what is it multiplying by 2?

It modifies every single attack. The damage of each attack differs. 2.0 will double the damage of all it's attacks, while 0.5 will half it.

It modifies every single attack. The damage of each attack differs. 2.0 will double the damage of all it's attacks, while 0.5 will half it.

Yes I understand how multiplication works. I am asking if you can write out each entities original attack damage so I know what I am multiplying to. Does the fallen icon do 5 damage? Or 20?
What is the original damage for each boss

Fallen Icon:

Blinding Light: 20, 15 for projectiles in phase 2
Eruption: 35
Mace Of Spades: 20 first hit, 25 second hit
Obliterate: 50
Moonveil: 50
Moonfall: 30 for each erruption, 65 on the place the sword hits
Thrust: 50
Sword of Light: 25 for both projectiles and sword swipe
Core Beam: 20

If you're interested in seeing the damage for the other bosses, then I suggest going into the ai/goal files yourself and look for when the damage modifier methods are used.

Sure, can you send me the locations of the files?

I will put them all up on the wiki when I can.


readFile error checking

While deploying my express 3.X app the first time to production, I ran into an issue with readFile where it wasn't able to read the locales directory, and also didn't have permissions to write new files.  Relevant code from i18n.js (https://github.com/jeresig/i18n-node-2/blob/master/i18n.js#L268):
try {
    var localeFile = fs.readFileSync(file);
    //... continues
} catch (e) {
    this.writeFile(locale);
}

The end result is that my app complains that it can't find res.locals.__.  Which makes sense - the middleware for express in expressBind calls new i18n(opts) which isn't successful, and thus (silently AFAIK) dies.
I think a reasonable thing to do might be to wrap the writeFile call in a try/catch and log an error.  I can create a PR if that seems reasonable to you.
(PS: I'm not asking for help with my app - I just needed to explicitly pass the directory option)

I can create a PR if that seems reasonable to you.

Please do :smile:

I think https://github.com/jeresig/i18n-node-2/issues/39 closes this issue ?


ENH: repace general Exception with specific exceptions

Description
Allowing any failure to be caught in a try/except loop can create unidentified bugs.
Potential impact

Is the feature related to an existing problem?  There are several try/except loops with generic catches
How critical is this feature to your workflow?  This will allow for unexpected behaviour, especially if dependencies change
How wide of an impact to you anticipate this enhancement having?  Essential for maintenance, potentially hides existing bugs
Would this break any existing functionality? No

Potential solution(s)
Specify allowed exceptions when possible
Alternatives
Leave as is
Additional context
This will likely be cleared up when linting tests pass.

Related to Issue #4 - lint doesn't like the current approach.

Closed in most recent commit.


kube-prometheus: conflicts on existing node-exporter

What did you do?
Deployed kube-prometheur
What did you expect to see?
Everything working
What did you see instead? Under which circumstances?
The node-exporter daemonset of kube-prometheus conflicts with the exporter shipped with Tectonic
Environment
Tectonic v1.5.5 via AWS Terraform

I don't think this is a problem, as this is meant to be from 0 to end-to-end monitoring, all of this will eventually move into being handled by tectonic. Once we move all of kube-prometheus into a helm chart, we could potentially make it configurable whether the node-exporter is deployed at all. Would that be a suitable solution? For the time being, it is probably best to modify the hack script to deploy and just not deploy the node-exporter.

Helm solution https://github.com/coreos/prometheus-operator/pull/757

With helm you can now choose what do deploy of the kube-prometheus stack. Closing this here.


[crashpad] fix targets

Fixes #40415

[x] Changes comply with the maintainer guide.
[x] SHA512s are updated for each updated download.
[x] The "supports" clause reflects platforms that may be fixed by this new version.
[x] Any fixed CI baseline entries are removed from that file.
[x] Any patches that are no longer applied are deleted from the port's directory.
[x] The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
[x] Only one version is added to each modified port's versions file.

@microsoft-github-policy-service agree

Please test https://github.com/microsoft/vcpkg/pull/40519. I applied more, and different, changes. But I don't have MSVC for local tests.


Broker should extract credentials from secret

This change makes it so the broker can handle secrets that are created
by APBs when using the asb_encode_binding module.

Update the broker so that it can handle secrets generated by the APB
when asb_encode_binding module is used from the asb-modules.
Update executor::ExecuteApb to wait for pod to complete, since the
pod is no longer kept alive for credential extraction.
Clean up some of the log messages and code format related to apb
actions.

Fixes #544
Fixes #553
Implements the proposal #550
Depends on the following PRs:

ansible-asb-modules#8 This is how the secret gets generated.
apb-base#7 Remove scripts related to extracting credentials from the containers filesystem.
ansible-playbook-bundle#163 Bump the APB versions so freshly built APBs will pass version validation checks.

Changes Unknown when pulling f25784d11b191dfca6dadaa465801ddb60989877 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling f25784d11b191dfca6dadaa465801ddb60989877 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling bda09b19d28aadba245eccf55fc17e2aa47a82ec on djzager:bind-creds into ** on openshift:master**.

Note to reviewers: This will fail CI until all the deps are merged.

I'm a little concerned with bumping the APB spec version. The versioning doc explains how that version should be bumped. It is meant to track changes to the spec file itself and not meant to track dependency changes.
I've raised the concern with David but I cannot think of a better way to track dependency changes that will break broker functionality unless we strive to be backwards compatible. In that case I would like to see the broker support the old mechanism of execing in if the secret doesn't exist or simply read from the secret if we are using the newest asb_encode_binding.

@dymurray I tend to agree with you that the version is for the spec. This PR isn't changing the spec definition.

At one point I suggested we remain backwards compatible, but we seemed to have dropped that for some reason. @djzager do you remember why we stopped being compatible?

The lesson learned here for me is that I should have included @dymurray on the original proposal. My bad.

The reason we stopped the backwards compatibility train @jmrodri was @shawn-hurley wise intervention about 1-off if statements that change behavior and I think that remains true. I tend to agree with @dymurray's point(s), we'll have to do a meaningful rework of the ExecuteApb architecture and flow to handle backwards compatibility correctly. (Call me out if anything I just said was stupid).

@djzager ok i'm not afraid of some if statements, unless of course they are EVERYWHERE. Then let's re-evaluate the ExecuteApb then to see how to best handle this new requirement.  Thanks for the summary.

Based on comments in scrum I think this would be a good approach:

Keep APB spec version at 1.0.
Add a new label to the Dockerfile for apb-base titled apb-base-version and set it to 1.0.
In the broker, add a new check during spec bootstrap to check apb-base version
If apb-base version exists and is 1.0 then we can assume it has the newer asb_encode_binding module and will look for a secret for the bind creds
if apb-base version does not exist then assume it doesn't have the newer asb_encode_binding module and revert to execing into the pod

Changes Unknown when pulling c70079884fd87ac60d94d73ba6db2ab9f59ea316 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling 072490eed280da8ba43c906a4ecdae2fb13deef3 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling ab5dcc326e263c6b15022f8f286d53c2262b0814 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling 0069ffbe3bd239c81407811dd4074af791fe28db on djzager:bind-creds into ** on openshift:master**.

@dymurray @shawn-hurley @jmrodri @eriknelson
A bunch of changes have been made for the sake of 1) introducing a new APB runtime version (now at version 2) and 2) providing backwards compatibility between the 2 runtime versions, specifically with respect to secrets. I still need to update the proposal document to line up with what was actually done but I've included below my method for testing.
Testing
Setup
NOTE Started with catasb to bring up cluster.

Build and tag the image:

$ make build-image
$ docker tag docker.io/ansibleplaybookbundle/origin-ansible-service-broker:latest \
   docker.io/djzager/origin-ansible-service-broker:latest

Update the broker-config to include djzager org:

$ oc edit configmap broker-config
+      - type: "dockerhub"
+        name: "dz"
+        url: "docker.io"
+        org: "djzager"
+        tag: "latest"
+        white_list:
+          - ".*rhscl-postgresql-apb$"

Update the broker deployment config to deploy the new broker

$ oc edit dc asb
-        image: docker.io/ansibleplaybookbundle/origin-ansible-service-broker:latest
+        image: docker.io/djzager/origin-ansible-service-broker:latest

Test

Provision PostgreSQL into oldproject
Provision MediaWiki into oldproject
Bind PostgreSQL to MediaWiki in oldproject
Verify MediaWiki is installed correctly
Unbind PostgreSQL
Deprovision PostgreSQL && MediaWiki
Provision PostgreSQL V2 (from djzager) into newproject
Provision MediaWiki into newproject
Bind PostgreSQL to MediaWiki in newproject
Verify MediaWiki is installed correctly
Unbind PostgreSQL
Deprovision PostgreSQL && MediaWiki
:sunglasses:

Changes Unknown when pulling e5d681b347274bffc47c8d8838dd562ad872ff4e on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling 934a5bc0a245d97d53ae224b7bcb20681cee5a36 on djzager:bind-creds into ** on openshift:master**.

Note to reviewers: This will fail CI until all the deps are merged.

Backwards compatibility for using exec is in this PR.  If the gate is green, it's testing using exec until the deps are merged.  Once this gets rebased and passes CI again, we can merged the deps and let CI run again to confirm that using a secret in the gate also works.

Changes Unknown when pulling 2eb545aa4bc50e85a9ce211ac2b656523a3a2be0 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling 0f5e9cd8a447e4654bd7d62c2f259bd914a89953 on djzager:bind-creds into ** on openshift:master**.

 Improve Ansible Broker's Bind Credential Extraction Process

Changes Unknown when pulling ac468b5477c6bf74f0d49e1a756845193bbd6f76 on djzager:bind-creds into ** on openshift:master**.

@shawn-hurley I've got the feeling that I missed that discussion about breaking up method declarations.
Before I start, let me just say this...I may be a little OCD (just ask me about scratches on my sunglasses) and may need to just get over myself. Personally, I would prefer either break them up in the way I had done it or shove it all on one line regardless of length. I say that because one dangling parameter + return list + { makes me a sad :panda_face:
I'll do whatever is agreed on.

I need to read the actual go style guide.
We should probably document where we diverge from that style so newcomers to the project can use that as reference.

Changes Unknown when pulling 55cf1cc6e269545a12beff29e9851ebeffe7800a on djzager:bind-creds into ** on openshift:master**.

I'm fine with either.  I leave it up to the writer to choose as long as the function wouldn't normally fit
on one line.
I don't think we need to block on this since we haven't really established a style here. And we would
have to change it everywhere to match the style we go with.  We can discuss in a separate issue if
we want to pursue it.

Changes Unknown when pulling e14bdda8b4c98e9fd9cbc2a29ad0a1cda4993104 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling 91fd2349bd8e8ad15d325e97041721cbd1f9d516 on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling 34fe506ff2d78db9595a1c8f47fed66b3242852b on djzager:bind-creds into ** on openshift:master**.

Changes Unknown when pulling 34fe506ff2d78db9595a1c8f47fed66b3242852b on djzager:bind-creds into ** on openshift:master**.


Fix events not triggered when scrolling past boundaries

Instead of returning when scrolling past the container's boundaries,
override value to the max allowed and let the other custom events
fire if needed.

Hi there, I've just had the chance to review your code.
Just wanting to double check… you've named this PR Fix events not triggered when scrolling past boundaries.
When testing your code, events are actually not triggered when scrolling past, instead of being triggered, as your fix would imply.
In which scenario would your code cause events to be triggered, which in the past were not being triggered?

Hey, thanks for taking a look! :)
Sorry this wasn't clear in the commit message: I'm not referring to the reach-(x|y)-(start|end) events, but to the scroll-(x|y) and scroll-(up|down|left|right) events. Those return statements that I removed where preventing the other conditionals further down from being checked. Does that make sense?

Oh, now I understand. So you're addressing a scenario where for example an upward scroll reaches the top.
In my previous code we'd only receive the event that it has reached the top, but thanks to your code we'll receive both that we reached the top, as well as that there was an upwards scroll.
Let me still double-check re. performance, as now we're going to go through the whole function, and previously we were only going through the function till we hit the first return.

Yeah that's it. No worries, please do!

Thanks for the great work @axelboc. To my pleasant surprise I didn't notice any performance issues.

Happy new year @DanielApt! Thanks for merging this in. By any chance, would you have time to do another release some time soon?

Apologies for the delay. I need @noraesae to do a release.

Sorry for the late response. Will release a new version in a while.

Thanks! :)

Just wanted to say, that this pull-request can change the order in which the events are firing. For example in this code:
$el = $('.foo');
$el.on('ps-x-reach-end', function() { $('.bar').addClass('reached-end'); });
$el.on('ps-scroll-x', function() { $('.bar').removeClass('reached-end'); });

scrollbar.initialize($el[0]);

When scrolling to the end, it was triggering the ps-x-reach-end event as the last event. But with this pull-request the 'ps-scroll-x' event is triggerd as the last event.
In my case I want to add a class to another element if the scrollbar hits the end, and it should remove the class if the scrollbar is scrolling. But now the class is removed even if the scrollbar hits the end, because the scroll event is fired last.
Would it be possible to change the order of the events inside update-scroll.js, so that the xEndEvent and yEndEvent are fired last?

When ps-x-reach-end gets triggered, it means that the user has scrolled towards the right. So could you listen to ps-scroll-left instead of ps-scroll-x? By doing so, the class would be removed only when the user scrolls left. And you could do the same on the other side with ps-x-reach-start and ps-scroll-right.

That's a good idea!
I solved it yesterday by saving the scrollLeft property on reach-end and checking in the scroll-x event handler if the scrollLeft property is different. But thanks for the suggestion :+1:

I think @axelboc's suggestion is more elegant. Your logic should depend on the events that take place, and not per-se the order of the events.


Link the button in news page to medium

Load more post button links to the home page. Make it link to medium.

Issue fixed. Hence, closing it.


fix: Use correct otel-collector distro

There is a potential breaking change that requires this change during the dep installation (just for testing, nothing to users)
https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-collector/UPGRADING.md#0880-to-0890
Checklist

[x] Commits are signed with Developer Certificate of Origin (DCO - learn more)

/run-e2e sequential


Fix txindex=0 on new node with no previous block/chainstate data

When starting an empty node with :
prune=1000
txindex=0
It would fail to validate the txindex value hence would not be able to start with a pruning node from scratch, this fixes it

Going to close as it breaks staking implementation


Move to GitHub Actions

I assume you're using some kind of script?
Do you intend to do all the repos? Just curious. https://github.com/search?q=user:sindresorhus+filename:travis.yml&type=code

I assume you're using some kind of script?

Yes, I bodged together a script that takes repo names and does everything else automatically.
About 99% of your repos don't use complicated things in the build script so I'm planning on covering those.

Great! Thanks again. This is saving me from a lot of grief.


Pod outage Chaos scenario

Pod outage chaos scenario blocks traffic at pod level irrespective of the network policy used. With the current network policies, it is not possible to explicitly block ports which are enabled by allowed network policy rule. This chaos scenario addresses this issue by using OVS flow rules to block ports related to the pod. It supports OpenShiftSDN and OVNKubernetes based networks.
Below example config blocks access to openshift console.
- id: pod_outage
  config:
    namespace: openshift-console
    direction:
        - ingress
    ingress_ports:
        - 8443
    label_selector: 'component=ui'

test

/funtest

/funtest

Do we need to also edit the plugin.schema.json file here? I  am getting this error when trying to edit the scenario file

/funtest

Do we need to also edit the plugin.schema.json file here? I am getting this error when trying to edit the scenario file
Regenerated the plugin.schema.json file, it should be resolved now.

@yogananth-subramanian unit tests are failing which is blocking the PR merge: https://github.com/redhat-chaos/krkn/actions/runs/4946862575/jobs/8845486738?pr=398.


Suppress alias warnings on stairsplus overrides.

Overriding stairsplus nodes in other mods resulted in warnings like:

WARNING[Main]: Not registering alias, item with same name is already defined: default:micro_ice_bottom -> default:micro_ice

This commit resolves the warnings by forcing the alias.

Dead.


Debug start view: Cannot activate debugger extension

Testing #91143
mkdir test
code-insiders test

Go to Debug View
Click "create a launch.json file"

This is up to the Javascript Debugger extension. Forwarding to @connor4312
And he can assign back to me if it is a vscode issue (which I believe it is not)

/duplicate https://github.com/microsoft/vscode/issues/91407


FR: Allow customization of FUIAuthProvider buttons

Welcome to FirebaseUI and thanks for submitting an issue!
Please take a look at open issues, as well as resolved issues, to see if your issue is either already being addressed, or has been solved by someone else.
If your issue concerns the CocoaPods error, "...transitive dependencies that include static binaries", please take a look at any of the following issues and see if your problem is solved: #48, #47, #46, #34, #23.
If not, please feel free to fill in the following info so we can help faster!
Step 1: Are you in the right place?
Yes, I am.
Step 2: Describe your environment

Objective C or Swift: Swift
iOS version: iOS 13.5
Firebase SDK version: 6.27.0
FirebaseUI version: 8.4.2
CocoaPods Version: n/a

Step 3: Describe the problem:
Steps to reproduce:

Attempt to customize FUIEmailAuth button background color, text color, image, etc.

Observed Results:
The relevant properties are readonly, seemingly unnecessarily.
Expected Results:
The ability to customize those buttons' appearance.
Relevant Code:
n/a

This feature request is now internally tracked as b/160627635.

I currently can't submit my app to the app store because the Sign in with Apple button is white instead of black.

I have the same question. Have you find any solution?

I have the same question. Have you find any solution?

I have the same question. Have you found any solution?

Solution for me was unfortunately to ditch all providers for login except email, and write my own email login. Took me like a day. They don't maintain this anymore although they claim they do ....
On Fri, May 31, 2024, at 3:59 PM, Konstantinos Apostolakis wrote:

I have the same question. Have you find any solution?
—
Reply to this email directly, view it on GitHub https://github.com/firebase/FirebaseUI-iOS/issues/878#issuecomment-2143077985, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJNVFJFDPATZTAGWQRQPPS3ZFD6E3AVCNFSM4OOAIEI2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJUGMYDONZZHA2Q.
You are receiving this because you commented.Message ID: @.***>


Fix getLocations that was ignoring failOnCompositeError flag

Fixes gh-2110

Thanks @sergioasantiago!
Could you submit this against the 3.1.x branch?  I will then merge it forward to main.

Done!

Thanks @sergioasantiago!
Could you submit this against the 3.1.x branch? I will then merge it forward to main.

Done here


use /dev/urandom for weblogic:12.1.3-dev image

The dockerfile used for building the image oracle/weblogic:12.1.3-dev defines but doesn't use the env var CONFIG_JVM_ARGS. The result is that weblogic takes 10 mins to start (in my machine) if we use the image weblogic:12.1.3-dev as the base image.
I propose adding the definition of the env var to the .bashrc file using something like that in the Dockerfile:
echo "export CONFIG_JVM_ARGS=$CONFIG_JVM_ARGS" >> ~/.bashrc

Thanks @Djelibeybi for calling my attention to this one. I'll be reviewing by the end of this week. Likely to merge into the sample code since it looks to be a reasonable configuration. Will test though.
Thanks @talberto for your suggestion.

I actually had to also set the same configuration when starting the domain (after domain creation), otherwise the startup of the domain was taking way too long (+10 mins).
In order to do that I added the following in the Dockerfile for domain creation (in samples/12c-domain):
ENV JAVA_OPTIONS -Djava.security.egd=file:///dev/./urandom

Also at same place I have added remote debugging information like -Xdebug -Xnoagent...

I will add the JAVA_OPTIONS but I will leave the debugging paramaters out since this is a sample and users should come up with their own.
Thanks @ovanekem


npm start command doesn't work

I've tried to run npm start command and faced an error:
npm ERR! Darwin 15.4.0
npm ERR! argv "/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin/node" "/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin/npm" "start"
npm ERR! node v5.10.1
npm ERR! npm  v3.8.3

npm ERR! missing script: start
npm ERR!
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

npm ERR! Please include the following file with any support request:
npm ERR!     /Users/ivanyurchenko/npm-debug.log

I've found a solution on StackOverflow which tells to add a line to the package.json file:
http://stackoverflow.com/a/31976870
Now I'm starting to run npm start and getting the error (which is pretty logically as I didn't create any your-script.js files).
log:
0 info it worked if it ends with ok
1 verbose cli [ '/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin/node',
1 verbose cli   '/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin/npm',
1 verbose cli   'start' ]
2 info using<EMAIL_ADDRESS>3 info using<EMAIL_ADDRESS>4 verbose config Skipping project config: /Users/ivanyurchenko/.npmrc. (matches userconfig)
5 verbose run-script [ 'prestart', 'start', 'poststart' ]
6 info lifecycle<EMAIL_ADDRESS><EMAIL_ADDRESS>7 silly lifecycle<EMAIL_ADDRESS>no script for prestart, continuing
8 info lifecycle<EMAIL_ADDRESS><EMAIL_ADDRESS>9 verbose lifecycle<EMAIL_ADDRESS>unsafe-perm in lifecycle true
10 verbose lifecycle<EMAIL_ADDRESS>PATH: /Users/ivanyurchenko/.nvm/versions/node/v5.10.1/lib/node_modules/npm/bin/node-gyp-bin:/Users/ivanyurchenko/node_modules/.bin:/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin:/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin:/usr/local/share/npm/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin
11 verbose lifecycle<EMAIL_ADDRESS>CWD: /Users/ivanyurchenko
12 silly lifecycle<EMAIL_ADDRESS>Args: [ '-c', 'node your-script.js' ]
13 silly lifecycle<EMAIL_ADDRESS>Returned: code: 1  signal: null
14 info lifecycle<EMAIL_ADDRESS>Failed to exec start script
15 verbose stack Error<EMAIL_ADDRESS>start: `node your-script.js`
15 verbose stack Exit status 1
15 verbose stack     at EventEmitter.<anonymous> (/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/lib/node_modules/npm/lib/utils/lifecycle.js:239:16)
15 verbose stack     at emitTwo (events.js:100:13)
15 verbose stack     at EventEmitter.emit (events.js:185:7)
15 verbose stack     at ChildProcess.<anonymous> (/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/lib/node_modules/npm/lib/utils/spawn.js:24:14)
15 verbose stack     at emitTwo (events.js:100:13)
15 verbose stack     at ChildProcess.emit (events.js:185:7)
15 verbose stack     at maybeClose (internal/child_process.js:850:16)
15 verbose stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:215:5)
16 verbose pkgid<EMAIL_ADDRESS>17 verbose cwd /Users/ivanyurchenko
18 error Darwin 15.4.0
19 error argv "/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin/node" "/Users/ivanyurchenko/.nvm/versions/node/v5.10.1/bin/npm" "start"
20 error node v5.10.1
21 error npm  v3.8.3
22 error code ELIFECYCLE
23 error<EMAIL_ADDRESS>start: `node your-script.js`
23 error Exit status 1
24 error Failed at the<EMAIL_ADDRESS>start script 'node your-script.js'.
24 error Make sure you have the latest version of node.js and npm installed.
24 error If you do, this is most likely a problem with the ivanyurchenko package,
24 error not with npm itself.
24 error Tell the author that this fails on your system:
24 error     node your-script.js
24 error You can get information on how to open an issue for this project with:
24 error     npm bugs ivanyurchenko
24 error Or if that isn't available, you can get their info via:
24 error     npm owner ls ivanyurchenko
24 error There is likely additional logging output above.
25 verbose exit [ 1, true ]

The problem is I can't run npm start. I've tried reinstalling it, updating, upgrading and clearing cache. Also restarted everything include my Mac - didn't work either.

That really doesn't make any sense because there is a start command. I don't have anything else useful to add at the moment, but I'll keep thinking about it...

From the logs it seems that you are not in the project directory, but in the home dir.

Running from a wrong directory was a problem in this case, thanks. I think it would be cool to see an informative exception that would tell user to switch the folder. Anyway, at least I've got what caused the issue so I'm closing it.


chore(ssr): PoC server-side rendering React + Headless

SSR works well generally, but there are some fixes to make.
Headless

An error on the server related to fetch is not defined when using the AnalyticsFetchClient.
The facet id auto-generation mechanism does not work with SSR. Either the facetId option should become required, or the mechanism should be simplified to not generate ids.

React Samples

We need to adjust to register controllers before componentDidMount (e.g. in the constructor).

Another thing to put in the bucket of "things to do eventually": Have a small e2e test suite to ensure that SSR is working.

We need to adjust to register controllers before componentDidMount (e.g. in the constructor).

Is there a way around this? As far as I recall, registering controllers in the constructor meant we couldn't access the React context, which was necessary to retrieve the engine.


Chrisda to Master

Docs Build status updates of commit 232fcee:
:clock10: Full build: the average full build time is 14 min(s) 15 sec(s), this is based on the last 123 build(s) for this repository.
:warning: This is a FULL build due to the incremental build being disabled in configuration file (.openpublishing.publish.config.json in 'live' branch). For more detailed info, please click here.

Docs Build status updates of commit 232fcee:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

exchange/exchange-ps/exchange/mail-flow/Get-TransportServer.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/mail-flow/Get-TransportService.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/mail-flow/Set-TransportServer.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/mail-flow/Set-TransportService.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/mailboxes/Set-CalendarProcessing.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/policy-and-compliance-dlp/Get-DlpSiDetectionsReport.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/unified-messaging/Get-UMService.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/unified-messaging/Get-UmServer.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/unified-messaging/Set-UMService.md
:white_check_mark:Succeeded
View (exchange-ps)

exchange/exchange-ps/exchange/unified-messaging/Set-UmServer.md
:white_check_mark:Succeeded
View (exchange-ps)

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.


Project scope?

Hey man good idea of making a new stripe library.
I was wondering do you see the future of the library be a replacement for tipsi-stripe if yes I would love to help in any way. even though my knowledge is mostly ts.
Maybe it would be good if you added stuff in the projects tab so people could see what you are working on

Stripe will soon release their official react-native library.

Source?

@gax97  any thoughts about what's the minimum time it'll take them to release the lib?


WIP add eeglab event reader, 3rd try

EEGLAB reader is missing the ability to read events, making it basically worthless as is. This adds the ability to read events (with a syntax for handling non-ints previously discussed with @agramfort , @teonlamont and @jasmainak).
WIP. It doesn't crash and it returns something, that's it for now.
Sorry for taking so long, I just really don't get file IO.
Thanks @jaeilepp for tips.
No test yet. Do we even have raw data with events?

Sorry for taking so long, I just really don't get file IO.

IO is hard...

It seems _get_info currently returns byte objects for the channel names in the test file, not strings. I'm not sure how to deal with this - decode on demand? @jasmainak

I would simply read in the events and crop the array when preload=False so that you have as many events as much as data that has been loaded

I'm not sure I could implement that. The incremental IO really confuses me.
Is this actually better than what's going on currently?

I'm not sure I could implement that. The incremental IO really confuses me.
Is this actually better than what's going on currently?

Maybe I didn't understand your question. I thought you were stuck with preload=False ?

Nope, with how Jaakko remodeled incremental reading, it works.

... I think. I'm not entirely sure what's going on.

maybe @jaeilepp can help then

Some, but not all comments addressed.
There is a problem with the channel types still. The info repr sometimes doesn't work.

just an observation, our constructor isn't uniform. it should be init(self, input_fname, montage, eog, misc, ..., preload, verbose)

can we fix this too? we should maintain the standard structure across readers

Okay I give up. I just don't get it.
It basically works (if I manually load the example data, epoching and plotting shows it's basically doing things correctly), but I'm still confused about incremental reading.
======================================================================
ERROR: Test importing EEGLAB .set files
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python27-conda64\envs\testenv\lib\site-packages\nose\case.py", line 197, in runTest
    self.test(*self.arg)
  File "C:\Python27-conda64\envs\testenv\lib\site-packages\numpy\testing\decorators.py", line 146, in skipper_func
    return f(*args, **kwargs)
  File "C:\Python27-conda64\envs\testenv\lib\site-packages\numpy\testing\decorators.py", line 146, in skipper_func
    return f(*args, **kwargs)
  File "C:\projects\mne-python\mne\io\eeglab\tests\test_eeglab.py", line 38, in test_io_set
    montage=montage)
  File "C:\projects\mne-python\mne\io\tests\test_raw.py", line 53, in _test_raw_reader
    data1, times1 = raw[picks, sl_time]
  File "C:\projects\mne-python\mne\io\base.py", line 549, in __getitem__
    data = self._data[sel, start:stop]
IndexError: index 32 is out of bounds for axis 0 with size 32

@jaeilepp can you take a look?

The problem seems to be that the trigger channel is not included when preload=True. This can be fixed by adding data = np.vstack((data, self._event_ch)) to https://github.com/mne-tools/mne-python/pull/2975/files#diff-8e938f98848641c5f536c613fb90387aR329
This causes a failure in the other test because the events are not read correctly. I'm not quite sure how this reader should function, so I'll leave that to you.

Thanks @jaeilepp , it works now!
Can I have reviews?

besides, LGTM

I'll give it a try today / tomorrow. Don't merge yet ...

Otherwise LGTM

@Eric89GXL does the circleci time-out tell me anything I should take care off on my end? It seems unrelated.

No, don't worry about it

thanks for the rearrangement. to be consistent, the constructor should also handle miscellaneous channels if people have them.

Test failures seem false alarms ...
@Eric89GXL can you check if I did what you wanted in https://github.com/mne-tools/mne-python/pull/2975/files#diff-8e938f98848641c5f536c613fb90387aR345 ?

The tests fail because you import warn twice. Just remove it from line 6 in eeglab.py

okay but now :) any more reviews?

travis is not happy.
that's it for me. I'll let our eeglab experts confirm that it works as expected.
thx @jona-sassenhagen

@jona-sassenhagen something is weird with the triggers.  Why not use that instead of throwing a dozen warnings?

Are we okay with this much verbosity for a file read?
In [37]: raw = read_raw_eeglab('eeg_recording_1.set')
/home/mainak/anaconda2/bin/ipython:1: RuntimeWarning: Events like the following will be dropped entirely: [u'boundary'], 1 in total
  #!/home/mainak/anaconda2/bin/python
Events like the following will be dropped entirely: [u'boundary'], 1 in total
/home/mainak/anaconda2/bin/ipython:1: RuntimeWarning: Some event codes could not be mapped to integers. Use the `event_id` parameter to map such events to integers manually.
  #!/home/mainak/anaconda2/bin/python
Some event codes could not be mapped to integers. Use the `event_id` parameter to map such events to integers manually.
/home/mainak/anaconda2/bin/ipython:1: RuntimeWarning: Data will be preloaded. preload=False or a string preload is not supported when the data is stored in the .set file
  #!/home/mainak/anaconda2/bin/python
Data will be preloaded. preload=False or a string preload is not supported when the data is stored in the .set file

okay that's it for me. It works smoothly I think. No errors at least :) thanks for the efforts @jona-sassenhagen !

Why are they triangular shaped?

Looks like a plotting artifact due to single sample. Plot with stem or
stairs and it should go away.

@jona-sassenhagen something is weird with the triggers. Why are they triangular shaped?

How else do you expect the plot to look like?  Look at the raw values in the stim channel, if you see anything strange, can you post a reproducible example? It looks perfectly normal whatever I try.
 Why not use that instead of throwing a dozen warnings?

I don't know about raw.annotations, but I think there should be a workflow in line with the other stuff.
I'm okay with there also being a hook-up to annotations later, but for now I think it'd be cool to have working continuous EEGLAB IO.

Are we okay with this much verbosity for a file read?

Since the alternative is not raising warnings, yes. And it's much less than e.g. baselining used to do.
I rarely see the single-file set format though.

Do we actually need two separate functions for event_id and event_id_func. I don't want to repeat any of the old discussion. Maybe @jona-sassenhagen can you remind me why we chose to have two different arguments instead of event_id : dict | callable | None?

We've discussed this over and over again. It all comes down to arbitrary string triggers, and what people use for them.

sometimes, event codes will be ints with a regular (int) component. In that case, you want a mapping to int, with a function (as in BV).
sometimes, you don't, in which case you want a dict (as in BV).
sometimes, you have both (as in BV).

You could in principle have a dict inside of the callable and let the user re-implement everything provided here. I am against that though.

okay cool. Thanks for the clarification. Regarding the triangular shaped triggers, I was imagining rectangular pulses. I am actually surprised that mne.find_events does not throw any error ...

something like this: http://www.electronics-tutorials.ws/waveforms/tim10.gif?81223b

Travis is unrelated I think.
Btw, for a real-life case of such triggers, see #2778. Generally, whenever you have EEGLAB files originally from Brainvision data.
But since everyone seems confused by my API, I guess I'll definitely have to make an example ...
@jasmainak , what about mne.viz.plot_events ?

Hey, just a quick question - with the way event reading works now boundary events would be dropped. In eeglab boundary events that are not on the beginning/end of the file contain important information - that signal is not continuous in that place. EEGlab filter functions respect these boundaries and do not cross them.
I am not aware if there is similar functionality in mne, but if not maybe another warning might be useful? Otherwise users may not be aware that dropping boundary events may be dangerous. :skull:

Good point.
We don't have anything in MNE to deal with boundary events, do we? I think EEGLAB inherently considers them when e.g. filtering or epoching.

Also, just to clarify: you could at least add triggers for boundary events by doing
event_id = {'boundary':9999}
raw = read_raw_eeglab(fname, event_id=event_id, ...

Oh yes, I am aware these can be added this way. I was just wondering whether something should be  done automatically when boundaries are not at file edges. These should be rare cases and I think an additional warning that the signal is not continuous would be enough.
When it comes to eeglab behavior - boundaries are not crossed when filtering and epochs that contain boundaries are dropped.

Oh yes, I am aware these can be added this way. I was just wondering whether something should be done automatically when boundaries are not at file edges. These should be rare cases and I think an additional warning that the signal is not continuous would be enough.

Maybe for now just a mental note to do this later .

I think both CI failures are still unrelated, there are no errors relating to the actual PR.

can you put MRG in title if ready for merge then?

Anybody wanna push the button? Travis is unrelated.

@jona-sassenhagen how long do the tests take on master vs this branch?

no objection to merge.

On my MBA,
python ~/tools/mne-python/mne/io/eeglab/tests/test_eeglab.py
...
time: 4 sec
Total: 1 tests
• 4 sec (4 sec for test_io_set)
• Peak memory -1 MB (test_io_set)

Hold off merge, still a problem here.

Oh God what happened to Travis.
The EEGLAB test passes on my local machine, and none of the errors I see on Travis are related, but maybe we should still wait until the CIs recover.

Something is really weird with Travis these days, wondering if we have some exceptionally conditional bug somewhere.

@jona-sassenhagen in this case it seems that the CIs hate the doc string formatting, consistently.

Travis has been bad about dependencies in the past, and this is another
manifestation of that problem. I hope to find time to fix it this week if
nobody beats me to it.

It works for me. Until tests come back, can anyone pull and see if tests pass for them? Thank you!

Tests pass over here.

is there some data I could play with? with a tiny gist to start?

Can somebody restart Travis and see if it passes with #3000? @Eric89GXL @agramfort

@jona-sassenhagen Travis restarted :)

@jona-sassenhagen I noticed a merge commit. You can remove it by doing a rebase. Let me know if you need a hand.

Thanks @jasmainak ... let's see if we get some green for a change.

Okay 5 of 6 Travis checks work and the last is unrelated. I'll see if I can fix up the preload line and the minor stuff and rename to MRG once I've done so.
See if I can get around to a gist later.

@agramfort you can try http://nbviewer.jupyter.org/gist/jona-sassenhagen/daebba0e986f172147e8

Finally.

It's a bit long for this functionality. It would be good to shorten if
possible but not strictly necessary :)

congratulations on travis passing :)

can you also remove the merge commit? thanks!

congratulations on travis passing :)

#3000, best commit

can you also remove the merge commit? thanks!

So I do
git rebase -i F
and unselect c35d79c, then commit and push?

Travis error is unrelated (download failure).

Comments addressed @jaeilepp

LGTM, +1 for merge once the CIs are happy. Any other reviewer ready to give +1?
Thanks @jona-sassenhagen for slogging through the unification steps. They are a bit annoying but helpful in the long run

I think there's a bit of consolidation to be done on the readers in general. But in another PR :)

that's it for me

that's my 2c

needs rebase

Coverage decreased (-0.1%) to 90.645% when pulling 0179077ca4b7270de78eebdc2bb00b8995e979af on jona-sassenhagen:eeglab-events into 1f362c53ee87648ebacaeb9c3e4a6bab97c88cf2 on mne-tools:master.

Most comments addressed. The ones I haven't I'm not sure about ...

Coverage decreased (-0.09%) to 90.675% when pulling d9bf40b121575a9f3050d810576f9d60b8fe9eb4 on jona-sassenhagen:eeglab-events into 1f362c53ee87648ebacaeb9c3e4a6bab97c88cf2 on mne-tools:master.

@jaeilepp Adressed comment.

@jona-sassenhagen just get rid fo the misc param then. It's better not to have it than to have it wrong because ignored

Ok I will, thanks.

Then +1 for merge from you @agramfort?

yes

Coverage decreased (-8.8%) to 81.956% when pulling 040cb725a0a11ea9d3fd5f8afcc2b9113a927d3f on jona-sassenhagen:eeglab-events into 1f362c53ee87648ebacaeb9c3e4a6bab97c88cf2 on mne-tools:master.

misc removed, travis still passing.

Are we good?

Sounds like we've converged. @jona-sassenhagen I'll go ahead and rebase locally and push to master so you don't have to endure another rebase :)

Ah, I'm beginning to get the hang of it :) Thanks Eric, thanks everyone else for reviews.

Thanks @jona-sassenhagen!

(merged by rebase)

Thanks @jona-sassenhagen !

wait, I don't understand why the misc parameter was removed. It breaks the consistent eeg signature pattern. it is very easy to handle it internally, we have a function for mapping channels to different types cf. mne.io.Raw.set_channel_types. All it does is mark auxiliary channels that you give in the constructor to MISC XXX with the proper channel properties. this the correct way to do the channel type remapping,  for both eog and misc (and other types of channels)

My understand from the (many, many) previous comments was that it would be easiest to deal with unification of those things in a subsequent PR.

Let's have an issue to take note of the fact that the eeglab reader currently doesn't take MISC. also, I think the epochs reader should actually be extended to allow the event_id_func mechanism ...

@jona-sassenhagen can you open the issue? You probably have the best handle on what remains to be done

Ok I will, once I step off this train.

@Eric89GXL I tried to do some cleanup and this one reads:

Closed with unmerged commits
This pull request is closed, but the jona-sassenhagen:eeglab-events branch has unmerged commits.

Is that in any way significant?

It shouldn't matter, I merged by rebase. That means all your commits got new numbers because I rewrote history, so it doesn't think of these as having been merged.

Ok thanks.


Add extra parameters to FindCommandParser to parse

Describe your changes

Add extra parameters for FindCommandParser.parse()
Parse using CLI Syntax
Create separate ContainsKeywordsPredicate classes for simplicity
Updated Testcases

Issue ticket number and link
closes #104
Checklist before requesting a review

[X] I have performed a self-review of my code.
[X] If it is a core feature, I have added thorough tests.

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

Files with missing lines
Patch %
Lines

...seedu/address/model/person/PredicateContainer.java
55.55%
8 Missing and 20 partials :warning:

Files with missing lines
Coverage Δ
Complexity Δ

...java/seedu/address/logic/commands/FindCommand.java
100.00% <100.00%> (ø)
8.00 <3.00> (+1.00)

...du/address/logic/commands/FindEmployeeCommand.java
100.00% <100.00%> (ø)
6.00 <5.00> (+4.00)

...u/address/logic/commands/FindPotentialCommand.java
100.00% <100.00%> (ø)
6.00 <5.00> (+4.00)

.../seedu/address/logic/parser/FindCommandParser.java
93.75% <100.00%> (-6.25%)
15.00 <14.00> (+8.00)
:arrow_down:

...el/person/DepartmentContainsKeywordsPredicate.java
100.00% <100.00%> (ø)
7.00 <7.00> (?)

...s/model/person/EmailContainsKeywordsPredicate.java
100.00% <100.00%> (ø)
7.00 <7.00> (?)

...s/model/person/PhoneContainsKeywordsPredicate.java
100.00% <100.00%> (ø)
7.00 <7.00> (?)

...ss/model/person/RoleContainsKeywordsPredicate.java
100.00% <100.00%> (ø)
7.00 <7.00> (?)

...seedu/address/model/person/PredicateContainer.java
55.55% <55.55%> (ø)
25.00 <25.00> (?)

... and 10 files with indirect coverage changes


⚠ Bofrost.lu has degraded performance

In 684ebf6, Bofrost.lu (https://www.bofrost.lu) experienced degraded performance:

HTTP code: 200
Response time: 1042 ms

Resolved: Bofrost.lu performance has improved in 4dc0757 after 13 minutes.


Allow ID to be non-integer hash (same format as gravatar)

Right now we only allow id's to be integers - ideally we would allow this to be a non-integer hash of an email address too.

closed by a8d074f9dea33bb5e4f87c863cb315cdd58fcbf1


Create Dynamic Basemap Gallery sample

Description
Addition of a new sample demonstrating how to implement a dynamic Basemap Gallery using the new BasemapStylesService and associated classes. These new API will be released in 200.5.
Type of change

[x] New sample implementation

Platforms tested on:

[x] Windows
[ ] Android
[ ] Linux
[ ] macOS
[ ] iOS

Checklist

[ ] Runs and compiles on all active platforms as a standalone sample
[ ] Runs and compiles in the sample viewer(s)
[ ] Branch is up to date with the latest main/v.next
[ ] All merge conflicts have been resolved
[ ] Self-review of changes
[ ] There are no warnings related to changes
[ ] No unrelated changes have been made to any other code or project files
[ ] Code is commented with correct formatting (CTRL+i)
[ ] All variable and method names are camel case
[ ] There is no leftover commented code
[ ] Screenshots are correct size and display in description tab (500px by 500px, platform agnostic)
[ ] If adding a new sample, it is added to the sample viewer
[ ] Cherry-picked to Main branch (if applicable)

@lsmallwood - please could you review the sample implementation? I'm still working on the README etc, which is why the checks are failing, but the implementation is ready for review.

@ldanzinger and @tanneryould - please could you review this PR?
One consideration that I've not been able to spend much time on is how this sample will render on a mobile device. Please let me know if you have any thoughts on this, or the UI in general.

Thanks for the suggestion of using the QML Popup @ldanzinger. I've modified the implementation to use the QML Popup as suggested. Using the popup means the UI is compatible across a range of screen sizes. I've tried to demonstrate this in the video below, in which the popup size is shown across a range of window sizes:
https://github.com/Esri/arcgis-maps-sdk-samples-qt/assets/86665727/e814f825-2452-45e6-8b25-1e863ef835a7
What are your thoughts?

@AndrewBladon nice! Here are a few things to consider:

A different icon for the button other than hamburger icon - maybe a settings icon? Or would a basemap picker symbol make sense?
The combo boxes (and button) being left justified with lots of open space on the right looks a little off. Maybe center? Or maybe decrease the default overall size and have the combo boxes span the width?

Thanks @ldanzinger. I've updated the UI as suggested. These are shown in the updated video below:
https://github.com/Esri/arcgis-maps-sdk-samples-qt/assets/86665727/b133650c-926b-4cc7-93a4-8bd1ca8a238d
Let me know if there is anything else that should be changed or if you're happy for me to merge the PR.

Thanks for chatting with me on Friday @ldanzinger. As suggested, I've updated the sample to use a QML Flow element to make the customisation input options dynamically resize according to the width available in the Popup element. Here's a demo of the updated UI:
https://github.com/Esri/arcgis-maps-sdk-samples-qt/assets/86665727/ef512656-5a15-4db6-84c1-2f66a1d32a24
In https://github.com/Esri/arcgis-maps-sdk-samples-qt/pull/1714/commits/3c1af0dda9ea1333a5e067eb084e0f20523bdad0, I also merged this branch into the latest v.next branch to incorporate the folder restructuring.

@AndrewBladon please take another look. This broke the build.
https://github.com/Esri/arcgis-maps-sdk-samples-qt/pull/1718


consensus: ExecCommitBlock wastes time calculating proposer priorities

Feature Request
Summary
Eliminate unnecessary recalculation of proposer priorities.
Problem Definition

Replaying blocks calls state.ExecCommitBlock, which calls:

state.buildLastCommitInfo
state.dbStore.LoadValidators
(*types.ValidatorSet).IncrementProposerPriority

LoadValidators starts from the last stored height and increments from there to reach the proposer priority at the given block height.
As a result, IncrementProposerPriority is called with times = height % 100,000 if there are no changes to the validator set, since the validator set checkpoint interval is 100,000.

This can measurably slow down block replaying. Since the calculations are not memoized in any way beyond the validator set checkpoint interval, the amount of overhead has a sawtooth pattern, increasing with the block height until it reaches a multiple of 100,000. On a network with 15-20 validators, commenting out the call to IncrementProposerPriority improved the replay time of empty blocks by almost an order of magnitude in some cases.
Proposal
buildLastCommitInfo doesn't use the proposer priority - all it uses is the validator address and voting power. I propose either:

Add a method to Store, e.g. LoadValidatorsFast, that skips IncrementProposerPriority; or
Add a method to Store, e.g. LoadAbciValidators, that returns []abci/types.Validator instead of *types.ValidatorSet, which would allow it to safely skip IncrementProposerPriority since abci/types.Validator does not include the proposer priority.

Looks like a great proposal to me 👍 Thanks @firelizzard18
Would you be willing to work on this?

On a network with 15-20 validators, commenting out the call to IncrementProposerPriority improved the replay time of empty blocks by almost an order of magnitude in some cases.

Do you have any concrete numbers?

Would you be willing to work on this?

Yes, I'm happy to contribute :smiley:

Do you have any concrete numbers?

No, this is just something I ran across while debugging issues with the network I'm working on. Our database structure does not have any way of rolling back a block (we aren't using any part of Cosmos except CometBFT/Tendermint) so I've had to delete and rebuild the app database a number of times. I'll try to put together a simple key value app to test it.

As far as setting up a test or a test network to get some concrete numbers, what's the best way to generate history? This issue is most apparent when the block number approaches the checkpoint interval (modulo the interval), so to see the most impact I'd need ~100,000 blocks. With the default ~1 second block time, that would take a day. Which I could do, but it would be a lot more convenient if I could generate the test setup more quickly. Of course I could change the timeouts to get shorter blocks, but I'm not sure how well that would work. Is there a way to run an accelerated simulation, or artificially generate history?

Of course I could change the timeouts to get shorter blocks, but I'm not sure how well that would work

This should work just fine.

Is there a way to run an accelerated simulation, or artificially generate history?

One possibility is to use a snapshot, but we don't provide that currently. This is probably something we should do, though.


main - package.json

Hello Branch
it seems there is an error in the npm package
your are specifying a "main": "dist/web/build.min.js" inside your package.json that does not exist when you install the package through npm, thus webpack and probably other package manager fail to install the dependency

@pgherveou We just broke this repo down into 3 different ones: titanium, web and cordova. It's definitely still got a few kinks. I believe the correct path is "dist/build.min.js". Let me fix the repo!

I think I've cleaned it up now - try now?

I can confirm that this issue is solved on 3819cbf7ff1c1002222620c059922fd84a8aaa2d
Did this by:
bower install 'https://github.com/BranchMetrics/Smart-App-Banner-Deep-Linking-Web-SDK.git#3819cbf7ff'
Nevertheless documentation on https://github.com/BranchMetrics/Smart-App-Banner-Deep-Linking-Web-SDK/blob/master/WEB_GUIDE.md
'Common.JS and RequireJS compatibility' section should be updated to contained the correct require name which seems to be 'branch-sdk'
@pgherveou is it solved on your end?

cool testing it now

repo seems to be fixed but can you push a new version on npm
https://www.npmjs.com/package/branch-sdk says the version is 4 days old

my build works fine when I copy the file at the right location so I think its just a matter of publishing the package now

Hey @pgherveou they will probably batch this with other fixes.
@aaustin at your convenience let us know when we can expect this fix in a release.

Hey team - we deployed the fixed version last night! Just FYI. Hope we're out of the woods now.

Confirmed.
Version 1.6.6 work as intended.
Thanks @aaustin

good stuff - thanks for the heads up guys


Test ingestion of repository files

Test ingestion using ingest script with SHA1. Read files from the /data/in folder in the test system.

Created an entry in the config file to choose between zip file or pypreservica which uses the upload API to create the package.

Successfully ingested folders in my laptop.


gh-1045 samples dependencies management

https://github.com/spring-projects-experimental/spring-native/issues/1045

I close this PR without merging for the reasons explained in https://github.com/spring-projects-experimental/spring-native/issues/1045#issuecomment-926500100, thanks for bringing those issues to our attention.


Need consistent printing for Name, Namespace, etc

From #3729
We should land on one syntax for printing namespace and name (for all sorts of objects) "namespace/name" vs "name.namespace" vs other.
namepsace/name has an advantage when both name and namespace are DNS subdomains: "foo.bar/quux.zorb" is clear, while "quux.zorb.foo.bar" is ambiguous.
If we all agree, we should start enforcing it in code reviews and fixing it up when we find it.

Vote for namespace/name. Once we enforce this, the logging could be a little bit less verbose.

In what context? User-visible messages?
What about partial URLs?

Context is primarily logs and maybe error messages
On Jan 23, 2015 1:32 PM, "Brian Grant"<EMAIL_ADDRESS>wrote:

In what context? User-visible messages?
What about partial URLs?
--
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/issues/3731#issuecomment-71269484
.

Namespace/name works for me for logs and error messages.
Even better if we have a utility that returns that for me so we can just enforce its usage.
Sent from my iPhone

On Jan 23, 2015, at 5:42 PM, Tim Hockin<EMAIL_ADDRESS>wrote:
Context is primarily logs and maybe error messages
On Jan 23, 2015 1:32 PM, "Brian Grant"<EMAIL_ADDRESS>wrote:

In what context? User-visible messages?
What about partial URLs?
--
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/issues/3731#issuecomment-71269484
.

—
Reply to this email directly or view it on GitHub.

In the other issue I commented this should be in api/meta.  For object refs we want to go a step further and print additional ref info (uid resource version) which in some log contexts is also useful

PR #5192 provides a possible canonical implementation of "namespace/name" format in pkg/types/valid/namespacedname.go.
Do we want to standardize on using that?

Most people seem to be in favor of namespace/name. I will point out, however, that that's not sufficient to identify an object. One also needs at least the kind or resource path. Since names and namespaces exclude /, we could support both 2- and 3-part short names: namespace/name and namespace/resourcepath/name.

namespace/resourcepath/name seems a bit confusing (because I would assume resource path dominates namespace), but our API does take that pattern.

Yes, I'm trying to avoid introduction of yet another naming schema for fully qualified names. While it's less concise, I'd be happy with URL suffixes (omitting apiserver address, path root, and api version). Actually, it's even debatable whether we can omit api version, since not all objects exist in all versions, at least not at the same paths.

We at least (today) use the provided api version to convert resource types from the cli to the internal type.  So clients can control that via --api-version.

On Mar 17, 2015, at 5:11 PM, Brian Grant<EMAIL_ADDRESS>wrote:
Yes, I'm trying to avoid introduction of yet another naming schema for fully qualified names. While it's less concise, I'd be happy with URL suffixes (omitting apiserver address, path root, and api version). Actually, it's even debatable whether we can omit api version, since not all objects exist in all versions, at least not at the same paths.
—
Reply to this email directly or view it on GitHub.

I'm ok with namespace/resource/name, until we need to introduce namespace// selectors :)

On Mar 17, 2015, at 5:11 PM, Brian Grant<EMAIL_ADDRESS>wrote:
Yes, I'm trying to avoid introduction of yet another naming schema for fully qualified names. While it's less concise, I'd be happy with URL suffixes (omitting apiserver address, path root, and api version). Actually, it's even debatable whether we can omit api version, since not all objects exist in all versions, at least not at the same paths.
—
Reply to this email directly or view it on GitHub.

Using #8225 for format in logs. Using this for format in kubectl output.

See also #10626

And #10591

Also relevant: #10009 and #7111


reorganize schema section

For the schema, there is one obscure link in the website that points to the README of the schema. I think this README should either be made WAY more obvious. Note that a more user friendly rendering of this README is available on the BIDS schema tools RTD. In the spirit of avoiding having information scattered Another option would be to render file as part of the spec itself around this page. @effigies what do you think?

from https://github.com/bids-standard/bids-website/issues/518

The README in the website is just an accident of mkdocs rendering any markdown it finds. It is intended to be descriptive and not at all make statements that we need to retain backwards compatibility, so it would probably be better to remove it from the specification website, if mkdocs allows us to exclude files.

Sorry, just realized this is the website. Yes, if we're linking to the README, we should switch to the bidsschematools RTD.

by the way would it not be better to render this schema page (https://bidsschematools.readthedocs.io/en/latest/description.html) in the bids spec website rather than on the doc of bidschematools
cannot remember the rationale for rendering it there


Add topic availability safety check to controlled shutdown

The controller will not allow a broker to initiate controlled shutdown unless the shutdown is unlikely to lead to a topic-partition ISR shrinking below the min.insync.replicas setting for that topic.
This new functionality is optional and disabled by default. The following new configs have been added:

controlled.shutdown.safety.check.enable: Enable this safety check for any ControlledShutdownRequest handled by the controller.
controlled.shutdown.safety.check.redundancy.factor: Additional replicas above min.insync.replicas that are required for any topic-partition to allow shutdown of a broker. Default is 2, so if a topic has min.insync.replicas=2, then the ISR must have 4 replicas to allow shutdown.
controlled.shutdown.topic.cache.size: In order to implement this safety check the controller needs access to topic configuration. Looking this up requires a read from ZooKeeper, and these configs change infrequently, so caching this will vastly reduce the amount of time required to perform this safety check on large clusters with tens of thousands of topics.
controlled.shutdown.topic.config.cache.ttl.ms: The number of milliseconds to cache topic configs for this safety check. Default is one minute.

By default a broker attempting shutdown will only send ControlledShutdownRequest to the controller 3 times with a 5 second sleep in between. If it is desired to never skip controlled shutdown, controlled.shutdown.max.retries can be increased to a much larger value than 3. It can be set to<PHONE_NUMBER> (Integer.MAX_VALUE) to effectively retry forever.
A controller integration test has been added which exercises this code with a simulated three node cluster with a topic configured with min.insync.replicas=2, allowing only one broker to shut down at a time.
org.apache.kafka.common.cache.BoundedTimedCache was added to implement a time-based bounded cache for topic configuration in the controller.
In ControllerContext, instead of maintaining a list of broker ids that have begun shutting down called shuttingDownBrokers, a map of brokerId -> brokerEpoch. This will make it easier to keep this map and broker presence notifications (via zookeeper) in sync.
When a ControlledShutdown request is allowed to begin, it is recorded in zookeeper under /brokers/shutdown/, with the value of the broker epoch. On controller initialization this is loaded into controllerContext.shuttingDownBrokers. When a broker has restarted, it will come back with a new broker epoch which will take precedence over the epoch in controllerContext.shuttingDownBrokers.

This functionality is intended to replace the shutdown safety check we have in our internal Kafka deployment. In that system, we:

Acquire a zookeeper lock for the current rack (any other member of that rack will also hold this lock so they can all shut down concurrently)
Gather topic metadata to determine partition assignments
For each local topic, get topic config to learn min.insync.replicas setting
If shutting down will not leave any topic-partitions with ISR smaller than min.insync.replicas, proceed by calling KafkaServer#shutdown

I haven't done a lot of testing with this yet. I'm putting it up here as a RFC for now.

One thing I can think of that is missing here is the ability to tell a broker to bypass controlled shutdown at runtime. That is something our internal shutdown check allows and it is required from time to time.

Thanks for reviewing guys. I have done some testing locally and on one of our test clusters with a snapshot and this seems to work correctly from my testing.

Another important thing to note: This patch depends on interbroker-protocol-version 2.4, since that's the version that added brokerEpoch to ControlledShutdownRequest.
With an earlier IBP version the brokerEpoch is -1 which breaks this code. I guess it could be set to the current known broker epoch of that brokerId instead.
It looks like we don't use 2.4 everywhere in production yet so it is probably best to make that a prerequisite of this patch.

I updated this to deal with IBP < 2.4 by just using the broker epoch from liveBrokerIdAndEpochs (with a log message).

The one thing not addressed by this patch is how to skip the shutdown check. In our internal one we can do it via JMX. I'd like to have a better, more secure way to do it here.
I think if we change KafkaServer#doControlledShutdown slightly then we can just do a dynamic broker config instead.
diff --git a/core/src/main/scala/kafka/server/KafkaServer.scala b/core/src/main/scala/kafka/server/KafkaServer.scala
index cfcca2d4f271..910a1909a59a 100755
--- a/core/src/main/scala/kafka/server/KafkaServer.scala
+++ b/core/src/main/scala/kafka/server/KafkaServer.scala
@@ -471,7 +471,7 @@ class KafkaServer(val config: KafkaConfig, time: Time = Time.SYSTEM, threadNameP

     val socketTimeoutMs = config.controllerSocketTimeoutMs

-    def doControlledShutdown(retries: Int): Boolean = {
+    def doControlledShutdown(): Boolean = {
       val metadataUpdater = new ManualMetadataUpdater()
       val networkClient = {
         val channelBuilder = ChannelBuilders.clientChannelBuilder(
@@ -514,12 +514,12 @@ class KafkaServer(val config: KafkaConfig, time: Time = Time.SYSTEM, threadNameP

       try {

-        var remainingRetries = retries
+        var attempts = 0
         var prevController: Broker = null
         var ioException = false

-        while (!shutdownSucceeded && remainingRetries > 0) {
-          remainingRetries = remainingRetries - 1
+        while (!shutdownSucceeded && attempts < config.controlledShutdownMaxRetries) {
+          attempts += 1

           // 1. Find the controller and establish a connection to it.

@@ -617,7 +617,7 @@ class KafkaServer(val config: KafkaConfig, time: Time = Time.SYSTEM, threadNameP

       brokerState.newState(PendingControlledShutdown)

-      val shutdownSucceeded = doControlledShutdown(config.controlledShutdownMaxRetries.intValue)
+      val shutdownSucceeded = doControlledShutdown()

       if (!shutdownSucceeded)
         warn("Proceeding to do an unclean shutdown as all the controlled shutdown attempts failed")

Then to skip shutdown on broker 0, we'd just do
$ bin/kafka-configs --bootstrap-server ... --entity-type brokers --entity-name 0 --alter --add-config controlled.shutdown.max.retries=0

I'll test this after lunch today.

The one thing not addressed by this patch is how to skip the shutdown check. In our internal one we can do it via JMX. I'd like to have a better, more secure way to do it here.
...
I'll test this after lunch today.

Turns out that only works if we convert the config to a dynamic one. I see how it works now. So maybe "controlled.shutdown.enable" could be made into a dynamic broker config.
An alternative would be to do something to get the controller to just skip the safety check. We could make controlled.shutdown.safety.check.enable into a dynamic config. But ideally we would make it so that we can tell the cluster to skip the check for an individual broker. So it might be better to inject a command into the controller telling it to skip for a certain (brokerId, epoch).
I think telling the controller to skip shutdown safety check for (brokerId, epoch) is the best thing to do, but it would be pretty lame to introduce a new request type just for this. And it would be even lamer to make twiddling znodes the way for the admin to do it.

The one thing not addressed by this patch is how to skip the shutdown check. In our internal one we can do it via JMX. I'd like to have a better, more secure way to do it here.
...
I'll test this after lunch today.

Turns out that only works if we convert the config to a dynamic one. I see how it works now. So maybe "controlled.shutdown.enable" could be made into a dynamic broker config.
An alternative would be to do something to get the controller to just skip the safety check. We could make controlled.shutdown.safety.check.enable into a dynamic config. But ideally we would make it so that we can tell the cluster to skip the check for an individual broker. So it might be better to inject a command into the controller telling it to skip for a certain (brokerId, epoch).
I think telling the controller to skip shutdown safety check for (brokerId, epoch) is the best thing to do, but it would be pretty lame to introduce a new request type just for this. And it would be even lamer to make twiddling znodes the way for the admin to do it.

I like the idea that we add a dynamic broker-level config controlled.shutdown.safety.check.enable or controlled.shutdown.safety.check.disable to influence the flag on specific brokers:
./bin/kafka-configs.sh --bootstrap-server localhost:9092 --command-config ssl_configs.properties --alter --entity-type brokers --entity app0.linkedin.com --add-config 'controlled.shutdown.safety.check.disable=true'
And during shutdown check, each broker encodes its local flag into the ControlleredShutdownRequest before sending it to the controller. What do you think?

I just pushed a change that will address the remaining issue, which is to allow admins to skip the safety check.
Lucas and I discussed this a bit and decided that it's best to just add a new RPC request type instead of modifying ControlledShutdownRequest. This is mainly just because it may take some time for upstream to accept a patch like this, and if we have to carry it for a while, merging changes to ControlledShutdownRequest from upstream may be much more difficult in terms of wire format compatibility then it will be to maintain additional request types in our fork.

I just pushed a change that will address the remaining issue, which is to allow admins to skip the safety check.
Lucas and I discussed this a bit and decided that it's best to just add a new RPC request type instead of modifying ControlledShutdownRequest. This is mainly just because it may take some time for upstream to accept a patch like this, and if we have to carry it for a while, merging changes to ControlledShutdownRequest from upstream may be much more difficult in terms of wire format compatibility then it will be to maintain additional request types in our fork.


kubernetes-master uses deprecated --admission-control

Note: --admission-control was deprecated in 1.10 and replaced with --enable-admission-plugins.

Source: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#is-there-a-recommended-set-of-admission-controllers-to-use
The kubernetes-master charm still uses admission-control for 1.10. We should update it to use enable-admission-plugins for 1.10+.

+1 for this


Need new logo for dna.js open source project

Logo Design Contest
The current project logo for dna.js is pretty uninspiring and is just a placeholder until a real graphic artist creates an inspiring logo.

dnajs-logo.png
Logo Goals:

Clean, simple, and modern
Scalable so it looks good at different sizes
Incorporation of an animal mascot would be cool (but not required)
Slots nicely into the dna.js website: dnajs.org

Some words for design inspiration:
Clone, template, DNA, data, easy, clean, fast, JavaScript, HTML, jQuery
The artist who's logo is used for the project will get recognized on the dna.js website with a link to your portfolio (and if you can get to San Francisco, a drink of your choice plus an Apple Store Gift Card for a hundred dollars).
Attach your artwork to this GitHub issue.  Also, feel free to post any questions or comments below.
Important Note:
dna.js is an open source project and graphics contributed to the project are licensed under MIT/GPL.

I like the DNA symbol.

Woweee, you will recognize the designer on your website!!!
 Do you ask for a free meal at restaurants too?

I hope your company fails, and at the looks of it. It will.

@quiteawake It's 100% open source and there is exactly zero revenue for this project.

Exactly. When you shop at the dollar store, don't expect anything of quality.

dna.js spawns its own new logo:
dna.clone('circle', circles, { callback: addStyle });

http://dnajs.org/logo.html

For inspiration, here are some cool logos:
https://www.23andme.com
http://blueprintjs.com
https://bower.io

Add a comment if you have a better logo to contribute... and we'll just reopen this issue.


chore(main): release 5.1.0

:robot: I have created a release beep boop
5.1.0 (2024-10-02)
Features

editor: streamline system prompt (1c12cab)

Bug Fixes

openai: check for choices (9eb573e)
slash: use loadfile for user's own commands (6c349c8)

This PR was generated with Release Please. See documentation.

:robot: Created releases:

v5.1.0
:sunflower:


There's no request backpressure

I assumed Ace, when you use handle_data does backpressure - doesn't read off the socket until each data chunk is handled (using :gen_tcp's :passive or {:active, :once} modes), give or take some buffer.
To my surprise taking any time when handling data chunks causes a message queue to build up for the handler.
A sample module to reproduce the issue:
defmodule FileBeam.WWW.Upload do
  use Raxx.Server

  @impl Raxx.Server
  def handle_head(request = %{path: _path}, _state) do
    IO.inspect(request)
    {[], :state}
  end

  @impl Raxx.Server
  def handle_data(chunk, _state) do
    IO.puts("received chunk")
    IO.puts("chunk size: #{byte_size(chunk) / 1024} KB")
    Process.sleep(100)
    {[], :state}
  end

  @impl Raxx.Server
  def handle_tail(_trailers, _state) do
    response(:no_content)
  end
end

The end result is the mentioned message buildup, as per the screenshot from wobserver:

When I was testing it with some local files, I think the whole file got uploaded as quickly as the connection allowed, but 204 wasn't received until all the data chunks were processed.
I'd be happy to work on a fix, but I didn't have the chance to look into the implementation too much, I'd be happy to be pointed in the right direction.

Interesting. I think the server does make use of active: :once However it looks like it only then needs backpressure when sending the message to the worker.
https://github.com/CrowdHailer/Ace/blob/master/lib/ace/http1/endpoint.ex#L53-L57
Essentially the worker needs to ack the server process when it is ready to accept more message parts. However because there is backpressure when the worker sends messages to the server. you can't use a Gen.call. call in both directions between two processes can deadlock.
Also did you want to send the 204 before all the data was received?
Finally was this a HTTP/1 or HTTP/2 endpoint. can't tell because the worker is the same in both

Forgot to mention, this was HTTP/1.

Also did you want to send the 204 before all the data was received?

Sorry, I didn't mean to leave that impression, I was just trying to pain the picture of what was happening. What I'm trying to do is to do some processing as the file is being uploaded and only receive as fast as the file is being processed. I want to send 204 after the whole file is uploaded and processed.

Essentially the worker needs to ack the server process when it is ready to accept more message parts. However because there is backpressure when the worker sends messages to the server. you can't use a Gen.call. call in both directions between two processes can deadlock.

I see. I don't think doing it the naive way (sticking a receive in a loop before https://github.com/CrowdHailer/Ace/blob/master/lib/ace/http1/endpoint.ex#L59 ) would be robust enough - it would probably need to be another handle_info to set the socket to active. I'll play around with it when I have the chance and see if I can come up with with a reasonably clean approach and to see how it would fit in with the HTTP/2 side of things - I haven't looked at that yet.
BTW, I'm not entirely clear on what the abstraction is - what the exact responsibilities of Channel, Endpoint, Server and Worker are supposed to be.

Endpoint and Sever are the same process. Endpoint is just code that is useful to both a server and a client.
Worker is the abstraction that deals with translating one request to one response. It is the same process regardless of if the server is for a HTTP/1 or HTTP/2 connection.
A channel is just a reference the worker has back to the connection. it contains the pid of the server process but also stream identifier in the HTTP/2 case. The idea is that the worker can send data to the channel and not know if it is backed by a HTTP/1 or HTTP/2 endpoint/server

Thanks for the clarification, it's very helpful.
After looking into it some more, I don't think this backpressure would make much sense in context of HTTP/2 - the client should be able to initiate a new stream even if all the current ones haven't caught up with processing, so we can't block on tcp in a similar way.
I'm thinking of having the Worker send the acks to the Endpoint process conditionally, if the negotiated protocol isn't http2. The Endpoint will block on tcp if the amount of request chunks unhandled by the client is >= a certain buffer/msgQ size (1 or more).
Let me know what you think or brace for a PR ;)

HTTP/2 has a concept of backpressure in the spec.
https://tools.ietf.org/html/rfc7540#section-5.2
In the future this should probably be exposed in the API somehow. though I have no idea.
Probably best to start with the simplest thing, which is just and ack and add more as need.
I will stay braced for PR