Fix for #1493 Delayed loading of pubsub and binding components.

Description
Delayed loading of pubsub and binding components
Also delayed loading of actors because they depend on app to be started so that we can load app configuration
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #1493
Checklist
Please make sure you've  completed the relevant tasks for this PR, out of the following list:

[x] Code compiles correctly
[ ] Created/updated tests
[ ] Unit tests passing
[ ] End-to-end tests passing
[ ] Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
[ ] Specification has been updated / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
[ ] Provided sample for the feature / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

@yaron2 Please review this

We can load pubsub and binding components as is, but we need to delay subscribing the topic and bindings.  So we can call beginPubsub and readFromBinding to postpone the subscription after app channel is established.
For pubsub,
https://github.com/dapr/dapr/blob/67c4553aea13acef634540469f4a287b759a288e/pkg/runtime/runtime.go#L359
For input binding,
https://github.com/dapr/dapr/blob/67c4553aea13acef634540469f4a287b759a288e/pkg/runtime/runtime.go#L624
@yaron2 any thought ?

We can load pubsub and binding components as is, but we need to delay subscribing the topic and bindings. So we can call beginPubsub and readFromBinding to postpone the subscription after app channel is established.
For pubsub,
https://github.com/dapr/dapr/blob/67c4553aea13acef634540469f4a287b759a288e/pkg/runtime/runtime.go#L359
For input binding,
https://github.com/dapr/dapr/blob/67c4553aea13acef634540469f4a287b759a288e/pkg/runtime/runtime.go#L624
@yaron2 any thought ?

That makes sense.
Looking at the e2e results, I see that the app channel never gets initialized.

@youngbupark Thanks for the direction will follow that approach. @yaron2 Will check why app channel is not getting initialized and add tests too.

To delay the functionality of read from binding, I need to store parameters for readFromBinding and beginPubSub in DaprRuntime struct in a slice and then call those functions. Is that what you mean @youngbupark

To delay the functionality of read from binding, I need to store parameters for readFromBinding and beginPubSub in DaprRuntime struct in a slice and then call those functions. Is that what you mean @youngbupark

From what I understand, we can simply call readFromBinding and beginPubSub after createAppChannel.

@yaron2 @jigargandhi
I thought that it is very straight-forward. this is the initialization logic. we need to move this part after appchannel is established.
https://github.com/dapr/dapr/blob/d6a7784418fac9a6ce035b73ba3e3965ac711b0c/pkg/runtime/runtime.go#L968-L972
for example, we can remove this part then we can define startSubscribing lie below. then we can call this after appchannel is established. input binding will be similar.
func startSubscribing() error {
        for name, pubsub := range a.pubSubs {
                 if err := a.beginPubSub(name, pubsub); err != nil {
                         // warning message
		}
        }
}

@youngbupark The suggestion does not work. I have created a sample nodejs application that queries the dapr secret store before binding to the port (enhance from hello-world node js samples)
`// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
// ------------------------------------------------------------
const express = require('express');
const bodyParser = require('body-parser');
require('isomorphic-fetch');
const app = express();
app.use(bodyParser.json());
const daprPort = process.env.DAPR_HTTP_PORT || 3500;
const stateStoreName = statestore;
const stateUrl = http://localhost:${daprPort}/v1.0/state/${stateStoreName};
const port = 3000;
app.get('/order', (_req, res) => {
fetch(${stateUrl}/order)
.then((response) => {
if (!response.ok) {
throw "Could not get state.";
}
        return response.text();
    }).then((orders) => {
        res.send(orders);
    }).catch((error) => {
        console.log(error);
        res.status(500).send({message: error});
    });

});
app.post('/neworder', (req, res) => {
console.log(req.body)
const data = req.body.data;
const orderId = data.orderId;
console.log("Got a new order! Order ID: " + orderId);
const state = [{
    key: "order",
    value: data
}];

fetch(stateUrl, {
    method: "POST",
    body: JSON.stringify(state),
    headers: {
        "Content-Type": "application/json"
    }
}).then((response) => {
    if (!response.ok) {
        throw "Failed to persist state.";
    }

    console.log("Successfully persisted state.");
    res.status(200).send();
}).catch((error) => {
    console.log(error);
    res.status(500).send({message: error});
});

});
app.delete('/order/:id', (req, res) => {
const key = req.params.id;
console.log('Invoke Delete for ID ' + key);
const deleteUrl = stateUrl + '/' + key;

fetch(deleteUrl, {
    method: "DELETE",
    headers: {
        "Content-Type": "application/json"
    }
}).then((response) => {
    if (!response.ok) {
        throw "Failed to delete state.";
    }

    console.log("Successfully deleted state.");
    res.status(200).send();
}).catch((error) => {
    console.log(error);
    res.status(500).send({message: error});
});

});
var succeeded = false;
function FetchSecret() {
fetch("http://localhost:3500/v1.0/secrets/envvar-secret-store/PATH").
then((response) => {
succeeded = true;
console.log(response);
app.listen(port, () => {
console.log(Example app listening at http://localhost:${port})
});
});
}
FetchSecret();
(function wait () {
if (!succeeded) {
FetchSecret();
setTimeout(wait, 2000);
}
})();
// app.listen(port, () => {
//     console.log(Example app listening at http://localhost:${port})
// });`
When I do dapr run --app-id mynode --app-port 3000 --dapr-http-port 3500 --log-level debug node app.js
I think that because we are using go routines, the component is not loaded before we are being blocked for app to start. I see the following logs
`  Starting Dapr with id mynode.    This will block until the app is listening on that port."  This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
== APP == (node:21539) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
`

@jigargandhi let me reproduce it on my machine ideally, flushOutstandingComponents will make sure that all components are loaded.

@jigargandhi I quickly prototyped what I am thinking - https://github.com/dapr/dapr/compare/master...youngbupark:youngp/delay-start?expand=1
Please look at this. I see that components are loading correctly. hopefully it will help you.

I see the components are loading but after the app starts executing. The sequence of loading components and starting the server is fine. I also observe that (I might be wrong here) if the app is too fast in connecting to Dapr before the HTTP server is started something wrong happens.
Is this because we just start the dapr from cli and immediately start app.
Shouldn't we wait to start the app until dapr is listening on dapr port.
We need something like
blockUntilDaprisReady() daprRunning <- true
at https://github.com/dapr/cli/blob/master/cmd/run.go#L178

I see the components are loading but after the app starts executing. The sequence of loading components and starting the server is fine. I also observe that (I might be wrong here) if the app is too fast in connecting to Dapr before the Dapr's HTTP server is started something wrong happens.

something wrong happens ? do you mean app fails to call dapr api?

Is this because we just start the dapr from cli and immediately start app.
Shouldn't we wait to start the app until dapr is listening on dapr port.
We need something like
blockUntilDaprisReady()
daprRunning <- true

where
func blockUntilDaprIsReady() {
for {
		conn, _ := net.DialTimeout("tcp", net.JoinHostPort("localhost", fmt.Sprintf("%v", daprPort)), time.Millisecond*500)
		if conn != nil {
			conn.Close()
			break
		}
		// prevents overwhelming the OS with open connections
		time.Sleep(time.Millisecond * 50)
	}

}

at https://github.com/dapr/cli/blob/master/cmd/run.go#L178

I think we already have tracking issue :) @artursouza Don't have the tracking issue for this problem?
@jigargandhi If you think my proposal makes sense, then can you please update the PR and add test to complete the issue ?
Regarding my prototype code, please review it and rewrite as you need because it may have a bug ;)

@jigargandhi we have the tracking issue in SDK side - https://github.com/dapr/java-sdk/pull/363 we tried to handle this issue in sdk side because there is a chance to happen in k8s environment. but checking daprd status in cli is also good approach. Can you please file new issue in cli repo?

@youngbupark #363 is not related to this PR. We had discussed about this problem in the past but we have not created the issues in the SDKs yet. The code for app to block waiting for runtime seems to be a reasonable feature in the SDKs but it is up to the app dev to make sure it does not cause a deadlock. Today, some apps need to invoke dapr to get secrets, for example, while dapr needs the app to initialize pubsub/binding/actor.

I have created an issue on cli project. https://github.com/dapr/cli/issues/497 to block app until daprd is accepting incoming connections.
@yaron2 Is this PR ready to merge, or do we still need to investigate on app channel not initialized issue?

Invocation E2E test is broken, based on the test result, when caller calls callee, callee's appchannel was unavailable. Can you please investigate the e2e test issue ?

The error message in e2e failure is message "app channel is not initialized" which is returned from CallLocal function
https://github.com/dapr/dapr/blob/master/pkg/grpc/api.go#L107
The method description states that this is an internal Dapr grpc api call. That means we should start the internal GRPC server after the app channel is initialized or wait for the appchannel for the app channel to get initialized (like in a loop)
Can @youngbupark  or @yaron2 in my latest commit, I have started the GRPC internal server after the app channel is initialized. Is that enough for the intent? Is there something more to be added (like may be telling placement service that dapr internal grpc server is ready). Let me know if I am missing something or need to go through some docs to understand where I can make some changes.

@jigargandhi I just wanted to let you know that your work on this item is very appreciated, this is not a minor issue. It's both important for Dapr and is a complex piece of work in the heart of the runtime, so thanks and don't worry about this taking time, we'll get through this :)

The error message in e2e failure is message "app channel is not initialized" which is returned from CallLocal function
https://github.com/dapr/dapr/blob/master/pkg/grpc/api.go#L107
The method description states that this is an internal Dapr grpc api call. That means we should start the internal GRPC server after the app channel is initialized or wait for the appchannel for the app channel to get initialized (like in a loop)
Can @youngbupark or @yaron2 in my latest commit, I have started the GRPC internal server after the app channel is initialized. Is that enough for the intent? Is there something more to be added (like may be telling placement service that dapr internal grpc server is ready). Let me know if I am missing something or need to go through some docs to understand where I can make some changes.

@yaron2  thank you for the kind words, that keeps up the motivation, Can you let me know what you think about the quoted message

The error message in e2e failure is message "app channel is not initialized" which is returned from CallLocal function
https://github.com/dapr/dapr/blob/master/pkg/grpc/api.go#L107
The method description states that this is an internal Dapr grpc api call. That means we should start the internal GRPC server after the app channel is initialized or wait for the appchannel for the app channel to get initialized (like in a loop)
Can @youngbupark or @yaron2 in my latest commit, I have started the GRPC internal server after the app channel is initialized. Is that enough for the intent? Is there something more to be added (like may be telling placement service that dapr internal grpc server is ready). Let me know if I am missing something or need to go through some docs to understand where I can make some changes.

@yaron2 thank you for the kind words, that keeps up the motivation, Can you let me know what you think about the quoted message

We will look into this.

@youngbupark @mukundansundar can you assist here?

@jigargandhi
We need three functions:
SetAppChannel
SetActorRuntime
SetDirectMessaging
I tried updating the code from one of your previous commits. With this change, I was able to get E2E tests passing in AKS when I ran it manually.
Diff seen here: https://github.com/dapr/dapr/compare/master...mukundansundar:fix-1493#diff-ce3f68305f0eb28b6b8911c281186536c80d8cac9dff3cece2c8bdf3dd955315R44-R49

@jigargandhi  The bindings E2E test are failing.
https://github.com/dapr/dapr/pull/2215/files#diff-e56f9bb947923c6f8bb4c995bf4b9469a0ed117276223d16b177dcb04399a4bcL735-L742
In initInputBinding, there is a check done to see if the App is subscribed to binding. Based on the code flow, appChannel is not initialized when initInputBinding is called.
That needs to be moved to before starting  to read from binding. probably to the function
func (a *DaprRuntime) readFromBinding(name string, binding bindings.InputBinding) error {

Code to be moved:
        if a.appChannel == nil {
		return errors.New("app channel not initialized")
	}
	subscribed := a.isAppSubscribedToBinding(c.ObjectMeta.Name)
	if !subscribed {
		return nil
	}

@jigargandhi, we verified that the changes proposed by @mukundansundar will have e2e tests pass.

@yaron2 Found one issue with this change ...

@yaron2 Found one issue with this PR....
The runtime_init e2e test creates an init pod e2e-runtime-int, which fails to create the first time and creates correctly on restart.
Error message seen from daprd container:
time="2020-11-06T21:38:27.438419773Z" level=error msg="error reading from input binding runtime-bindings-http: app channel not initialized" app_id=runtime-init instance=runtime-init-788f64849d-t4t5d scope=dapr.runtime type=log ver=edge
time="2020-11-06T21:38:27.438457275Z" level=error msg="error reading from input binding test-topic: app channel not initialized" app_id=runtime-init instance=runtime-init-788f64849d-t4t5d scope=dapr.runtime type=log ver=edge

daprd is trying to read from appChannel but I believe, the appChannel is not initialized since there is no app to create an app channel to.
runtime_init container:
2020/11/06 21:36:18 Error publishing messages to pubsub: Post "http://localhost:3500/v1.0/publish/runtime-pubsub-http": dial tcp <IP_ADDRESS>:3500: connect: connection refused

This I believe is because daprd is not fully initialized when the app starts. So app cannot send something immediately to daprd container.
This is one case where the app needs to wait for daprd to be completely initialized before sending anything to it.
All of the other test apps generally starts a HttpServer waiting for request so that I can then send requests to daprd sidecar.

On the other hand, ran the runtime_init test manually and observed the same behavior.

I think this PR is ready to be merged

/ok-to-test

@youngbupark can you please approve? We can't merge

/ok-to-test


Add prototypeItem to ReordableList (and ReordableListView)

Use case
I have a ListView with items of fixed height. I want to use an advantage of this by using prototypeItem, though ReordableList (and ReordableListView) is missing it.
(A continuation of #81290 and #81372 discussion)
Proposal
Add prototypeItem to ReordableList (and ReordableListView)

CC @darrenaustin


feat: crate for retry with backoff strategy

crate to do retries with different backoff strategies as an iterator for any async function.

This coverage report is wrong, it is checking doc comments and marking it as not covered even though I have doc tests.
   Doc-tests backoff

running 1 test
test backoff/src/lib.rs - (line 7) ... ok

we can do that in a follow up PR


Input size for F1 Score

System information

OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Windows 10
TensorFlow version and how it was installed (source or binary): binary
TensorFlow-Addons version and how it was installed (source or binary): binary
Python version: 3.7
Is GPU used? (yes/no): no

Describe the bug
Hi, I wanted to ask what input shape is allowed for F1 Score metric in the library. Should the y_true and y_pred vectors be one hot encoded, e.g. [batch_size, no_labels], or contain label of the prediciton, e.g. [batch_size]? Can I make it work with raw softmax values as well? Can't find any reference to this in docs.

cc @AakashKumarNain who might understand metrics more.

Thanks @WindQAQ
@KacperKubara It should work in both the cases, i.e. OHE as well as with the softmax scores. If something isn't working, please let us know. We will also update the documentation for F1 score in the next version.  Thank you for the inputs

That's good to know, thanks @AakashKumarNain. I've had some issues with F1 score but it seems that it was because of my misunderstanding. If I will encounter any bugs, I will open another issue.
I am happy to hear that docs will updated for F1 scores. On the sidenote, while the TFA seems to be a great addition to TF, the lack of docs (especially for metrics) refrained me from actually using it and looking for alternatives. The implementation code for metrics is also quite complicated and uses a lot of inheritance which makes it quite difficult to figure out what's going on by yourself. I will close this issue for now.

@KacperKubara thanks for the feedback. We will take this into account and will try our best to improve the end-user experience. On a side not, if you think that the documentation of particular functionality in TFA isn't proper, please feel free to open an issue and we will look into it.


Key should never be @undefined

const crypto = require("crypto");
const secretStack = require("secret-stack");

const appKey = crypto.randomBytes(32).toString("base64");

const stack = secretStack({ appKey });

const app = stack();

console.log(app.id); // => "@undefined"

Note to self:
We should require (TS) config.keys and/or emit a (JS) warning when it's missing.

I took a swing at this based on your note @staltz  , https://github.com/ssb-js/secret-stack/pull/67. A new error is logged when no keys are passed in along with the config and updated the Config type in types.d.ts. Not sure how I feel about the log since it's going to make a lot of tests throughout the ecosystem very noisy. Thoughts?

@austinfrey Thanks for the PR! :)
What mix said over there is what I would say too


using with rails 7 without webpacker

Thank you for this amazing project however are there any workaround for rails 7?

@anilonrails Did you get into some errors? If you can post those then it will be easier to debug and provide you with  guidance in the right direction.
I am using this project in Rails 7 and newer versions already and generators work as intended. If you are getting issues then it most probably might be for a specific generator with some issues we have not caught.


Change Font Color

Hi,
I'm creating a blog with your Hugo Theme and I love it.
I'm trying to change the icon colors in the sass file but setting the $primary color to a new one, but I can't do it.
I'm a R/Python programmer and I don't know how to program in .css.
How can I accomplish it?
Thank you.

Currently the sass file must first be compiled to css.
There's a new version of the theme in progress (#66) that will remove this requirement with the use of Hugo's new Resource transformation functions. Stay tuned! :)


Review Authentication

The initial goal is to improve the loading time (bundle) by not loading the authentication code until the user wants to log in.
To be Discussed:

Shouldn't we redirect to /account/my-lists when the user login? It is strange that the user login and stays on the same page or even redirect to the default page.
The accessToken is only requested if a requestToken is present in the localStorage in the AuthEffect. But the AuthEffect is only loaded when the account-menu.component is loaded. So a tiny glitch is happening when hovering over the menu.
How do we manage the renewal of an expired accessToken from the localStorage? Usually, by receiving a 401 from the interceptor we should decide if we log in again or reset to the read accessToken
shouldn't we protect the /account/my-lists if not logged in or redirect to the login page?
not sure it is improving the loading perf...

Issue with origin, another PR on https://github.com/tastejs/angular-movies/pull/193


Ability to analyze request before allowing CORS

I'd like to be able to pass a callback which allows/forbids CORS for this specific request, similar to how http-proxy-middleware allows to pass a custom matcher https://github.com/chimurai/http-proxy-middleware#context-matching
Example usage: an API may require a specific header to be set IF the request is cross-domain.
To perform additional checks when deciding on CORS may be an antipattern - I'm not sure. But I personally find it pretty useful.

Hi @me-andre ! This sounds like a standard middleware pattern: you want to decide, based on something (the request here) if you want the middleware (cors) to perform it's action. This is pretty straight-forward for any Express middleware:
app.use(runIf((req) => req.header.secret === 'shhh', cors(/ * your cors config */)))

function runIf (fn, mw) {
  return (req, res, next) => fn(req, res) ? mw(req, res, next) : next()
}

Let me know if this works for your use-case or not.

It does work, thanks! I think the issue can be closed.
(Maybe posting your recipe in the README might help someone).

Awesome, glad to hear that @me-andre ! As far as the README goes, that is not specific to this module and is among dozens of different "recipes" for using middlewares -- replicating these in every middleware module would make them so long as to be untenable. Some kind of central location should be created by someone so inclined which could be referenced.


Move language code into a subpackage

This PR was created via: https://gist.github.com/dhermes/e239691aa584bd56a5352e34aad27cf3
export PROJECT_DIR="${HOME}/google-cloud-python"
export READMES_DIR="${HOME}/i-wrote-some-readmes-for-2357"
cd ${PROJECT_DIR}
git worktree add -b make-language-subpackage ../hotfix official/master
python make_commits.py \
  --git-root "${PROJECT_DIR}/../hotfix" \
  --package language \
  --package-name "Google Cloud Natural Language" \
  --readme "${READMES_DIR}/language/README.rst"

For some reason this didn't kick off a new Travis build when I rebased on top of #2428. Maybe the "approval" stops it?

Here is the orignal (green) PR build: https://travis-ci.org/GoogleCloudPlatform/google-cloud-python/builds/162895936


A11y_Azure Data Studio_Query Plan (Preview)_ScreenReader: NVDA is not announcing tooltip present at any node.

“Check out Accessibility Insights! - Identify accessibility bugs before check-in and make bug fixing faster and easier.”
GitHubTags:
#A11yMAS; #A11yTCS; #A11ySev2; #SQL Azure Data Studio; #SH-SQL Azure Data Studio-Win32-OCT2022; #DesktopApp; #ScreenReader; #WCAG1.3.1; #FTP; #NVDA; #A11YCAIOH
Environment Details:
Application Name: Azure Data Studio
Windows Version: Windows 11
Repro Steps:

Launch the "Azure data studio" application.
Connect with the provided SQL server.
TAB to "New Query" button and hit ENTER key.
Enter the SQL query and TAB to "Estimated Plan" button and hit ENTER key.
"Query Plan (Preview) screen appear.
Observe that NVDA is not announcing tooltip present at any node.

Actual:
NVDA is not announcing tooltip present at any node.
Expected Result:
NVDA should announce tooltip present at any node.
Attachment:

@Rama8345, this issue is fixed in the lates insiders. Now, when you click on the node the tooltip is shown and announced. Can you please verify and close? Thanks!

@aasimkhan30
Verified the bug in the latest insiders build, but we are getting below error.

Verified the issue in the latest insiders build. The issue is fixed. So closing the bug. Please find attachment for reference.


Screen Recording for Black Shark 4

Preparation

[X] I read the Troubleshooting Guide
[X] My question isn't answered in any other issues (even the closed ones)

Your question
FGA not able to record the write dimension of the screen for Black Shark 4 phone.
I can see the rectangles in the wrong spot on the debugging mode.
When trying to set the manual area, I am unsure what numbers I need as it doesn't seem obvious on how much they change.
So wondering if you guys can help me out.
Thanks

Please provide a screenshot.

It does scroll now, as well as change tabs.
But it never selects anyone. The red rectangle seems to off place to check
the skills.
On Wed, 27 Jul 2022, 02:17 reconman, @.***> wrote:

Please provide a screenshot.
—
Reply to this email directly, view it on GitHub
https://github.com/Fate-Grand-Automata/FGA/issues/1257#issuecomment-1195824065,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AFAANQ5MUVN5ONDS6LPUZ4TVWATR5ANCNFSM54V573MQ
.
You are receiving this because you authored the thread.Message ID:
@.***>

You ignored my comment, please provide a screenshot.

Also, have you ever encountered the problem reported in #1301? Do you know the solution?

Closing because of inactivity.


Lint markdown files on CI

Add markdownlint  to the CI
Lint docs
Fix linter issues
Disable issues related to https://pkg.go.dev/github.com/spf13/cobra/doc, which would require a custom doc generator to be fixed (see https://github.com/spf13/cobra/blob/bcfcff729ecdeb4072ef6f2a9c0b5e4ca1853206/doc/md_docs.go#L57)

Closes #268

Looks great! Could you add an entry to the Makefile as well? Something like make md-lint.

Sure, I completely forgot it! 😅


Atlas public dashboards improvements

This PR

improves a bunch of things :( Let me split it a bit

Checklist

[ ] Update changelog in CHANGELOG.md in an end-user friendly language.

@QuantumEnigmaa this one should be reviewable

It's a bit better indeed :sweat_smile:


Integrate AccessKit

Objective
UIs created for Bevy cannot currently be made accessible. This PR aims to address that.
Solution
Integrate AccessKit as a dependency, adding accessibility support to existing bevy_ui widgets.
Open questions
At the moment this PR is a draft which I'd like to merge into 0.10. To that end, there are a few issues with the current implementation that I'd like guidance on:

The accesskit dependency is currently re-exported from bevy_winit. This seems cleaner than adding it as a dependency in several places. It also ensures that any plugins that implement UIs always use the version of AccessKit appropriate to the Bevy engine version. If there's a better way or if this was the wrong call, please do let me know.
Non-container widgets aren't currently handled because there's no clear way to determine whether a given entity is a widget that should be exposed, or additional detail for a widget that was already handled. I.e. I can't tell whether text is a label for a button, or a standalone widget as seen in examples/ui. We may need an additional Widget marker component, but I'm open to other suggestions.
Likewise, please tell me if there's anything else that might be handled differently. This is my largest Bevy contribution to date and I'm also new to AccessKit, so there are lots of things I could have gotten wrong. :)

Changelog

This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

What changed as a result of this PR?
If applicable, organize changes under "Added", "Changed", or "Fixed" sub-headings
Stick to one or two sentences. If more detail is needed for a particular change, consider adding it to the "Solution" section

If you can't summarize the work, your change may be unreasonably large / unrelated. Consider splitting your PR to make it easier to review and merge!

Migration Guide

This section is optional. If there are no breaking changes, you can delete this section.

If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes
Simply adding new functionality is not a breaking change.
Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change.

@ndarilek this is awesome!

The accesskit dependency is currently re-exported from bevy_winit. This seems cleaner than adding it as a dependency in several places. It also ensures that any plugins that implement UIs always use the version of AccessKit appropriate to the Bevy engine version. If there's a better way or if this was the wrong call, please do let me know.

I do think that we shouldn't use bevy_winit to pull in accesskit, see: https://github.com/bevyengine/bevy/pull/6874#discussion_r1041561352

I've introduced a bevy_a11y crate for the AccessKit dependency and non-Winit code, decoupled bevy_ui from bevy_winit, and tried to clean up focus-handling so AccessKit always gets a focus as needed. I've also given the Adapters/Handlers structs less generic names to indicate their use in AccessKit.
The integration now aggressively pushes all changes to AccessKit each frame. I removed change-detection just to get something working, but this is preliminary to making AccessKit lazily poll for changes so it's only temporary.
I'd like to get action handling work next, so access technologies can tell bevy_ui to focus/click widgets. I think I'm miscalculating widget bounding boxes, and would appreciate any help fixing that. I believe Narrator has a tool to draw visual rectangles around where it thinks widgets are, but since I can't see those, I'm not sure if there's a way for me to use that tool independently. Help very welcome.
Also, there's probably additional plumbing needed to ensure bevy_a11y is more completely integrated into CI/releases. Help with that is welcome too, particularly if we can address or silence the CI warnings.

I'm removing the draft status on this, even though it isn't fully ready, because I'd like a bit more attention on it.
I don't want to go too far down the rabbit hole of modifying bevy_ui because it needs a lot of work in general, but one limitation of my current approach is that all AccessibilityNodes are treated as parents of the root window and there is no nesting. Right now the bevy_ui integration tries to generate AccessibilityNodes for widgets, but it's challenging to determine whether something like a piece of text is a label associated with a button vs. something like a standalone text label, and lots of the examples just generate and position widgets without enough context to make a usable accessibility tree. I included a Label widget/marker to get the simpler examples working but at some point we'll need to brainstorm a strategy to, say, mark some things as containers so Transform calculations are done correctly and nodes are correctly nested.
It may be that this PR has to ship with a reasonably sound AccessKit integration that others can integrate with, but a broken bevy_ui accessibility implementation, and future UI work will have to mold itself to the AccessKit integration and vice versa. But with AccessKit support in the newly-released egui, I'd really like to ship something that bevy_egui and other UI experiments can hook into. Just wanted to prepare folks for the possibility that bevy_ui may be broken for accessibility until it becomes a focus area.

Trying to merge latest main, I get the following from cargo run --example ui:
     Running `target\debug\examples\ui.exe`
2023-01-23T19:03:28.331739Z  INFO bevy_winit::system: Creating new window "Bevy App" (0v0)
2023-01-23T19:03:29.329104Z  INFO bevy_render::renderer: AdapterInfo { name: "Intel(R) HD Graphics 630", vendor: 32902, device: 22802, device_type: IntegratedGpu, driver: "Intel Corporation", driver_info: "Intel driver", backend: Vulkan }
2023-01-23T19:03:29.954204Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows 10 Pro", kernel: "19044", cpu: "Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz", core_count: "4", memory: "31.9 GiB" }
thread 'main' panicked at 'assertion failed: component_info.is_send_and_sync()', C:\Users\Nolan\src\bevy\crates\bevy_ecs\src\storage\resource.rs:265:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\examples\ui.exe` (exit code: 101)

Any help welcome. Hoping to address remaining feedback and submit this as a candidate for 0.10.
Thanks!

In case it's unclear, it looks like init_resource now requires that the resource type is Send and Sync but WinitActionHandlers seems to fail that check.

@ndarilek you're initially using WinitActionHandlers with NonSendMut, as if it was not Send + Sync here (that's probably where it also gets implicitly initialized, and then registered as non-send) and then you're trying to access it with ResMut here. There may be more places where NonSendMut is used, I haven't checked. Given that WinitActionHandlers is Send + Sync I would just use ResMut everywhere.

This is closing in on being done, and I'd appreciate getting more attention on it.
I debated separating out the bevy_ui bits for a separate PR, but I think leaving them in is a great preview for some of the pain points we're likely to hit building out bevy_ui in general, but in particular for accessibility. In particular, I think we'll need to know whether a given entity represents the leaf of a widget tree and should spawn an AccessibilityNode, vs. a piece of text or data associated with another widget. See my handling of UiImage for example:
Query<(Entity, &Children), (Changed<UiImage>, Without<Button>)>
That seems necessary because there is no other way of distinguishing between images and buttons. Likewise, text is sometimes associated with another widget (I.e. a button label) and other times a standalone piece of text. Those need to be distinct for accessibility purposes.
Still though, the bevy_ui integration is useful for proving out the basic concept, and this PR paves the way for directly integrating bevy_egui and other UI toolkits.

a few things, looking at CI:

AccessKit doesn't compile in wasm. That means everything added in this PR is going to have to be platform flagged
AccessKit enables both x11 and wayland features of winit on linux, without possibility to choose, which we decided to not do on Bevy side. That removes the choice from us
AccessKit adds dependencies that are unmaintained and with known panics / overflow cases. It's unlikely to be exploitable from Bevy, but it still means every project built on Bevy checking for security advisories will have one more

The second concern in the previous comment, and I believe the third one as well, are addressed by AccessKit/accesskit#209.
Note that AccessKit just updated to target winit 0.28, so depending on Bevy's plans for updating its winit dependency, I may need to also apply this fix to a branch that still targets winit 0.27.

The second concern in the previous comment, and I believe the third one as well, are addressed by AccessKit/accesskit#209.

This should address the second point. The third one is... more painful. winit now depends on a wayland crate even with the feature not enabled for a build script, so it seems the security advisory through winit is unavoidable.
There is another use of the unmaintained crate when adding AccessKit as a dependency though, it's accesskit_unix / zbus / serde-xml-rs / xml-rs.

Note that AccessKit just updated to target winit 0.28, so depending on Bevy's plans for updating its winit dependency, I may need to also apply this fix to a branch that still targets winit 0.27.

We'll try updating Bevy to winit 0.28 too: https://github.com/bevyengine/bevy/pull/7480

The easiest way to test this is probably to use a tool like https://accessibilityinsights.io/docs/windows/overview/. Haven't tried this myself but intend to soon. I've made sure the button and ui examples at least work partially:
cargo run --example button
cargo run --example ui

Screen readers like Narrator and NVDA also work of course, but because there's no concept of keyboard focus and navigation in bevy_ui, you'd need to learn to use NVDA's object navigation or Narrator's equivalent, which is probably more complex than just using an accessibility tree visualizer like the one linked above.
Note that the ui example fakes a list by styling a bunch of text nodes. I attached custom AccessibilityNode components from the example to present the list as a list, though more work needs to be done to for instance indicate how many children there are, etc. Again, I'd rather not go too far down the "bevy_ui is incomplete" rabbit hole in my examples, but hopefully this shows how one might eventually do something like attach a custom accessibility component to an entity and make a tilemap present as an accessible grid.
One area that almost certainly doesn't work correctly is bounds calculation of accessibility nodes. This is useful for magnification, mouse pointer routing, etc. I believe screen readers like Narrator can draw visual bounding boxes around where it thinks accessible nodes are located, but I can't see these to correct whatever offset likely exists. I'm sure it's an easy fix to the newly-added calc_bounds system, but even just knowing the direction the box is offset from the widget would help. I'm still a spatial reasoner, and while I often try to imagine the numbers without the visuals, sometimes my brain just locks up when it tries to reason about vectors in the abstract. :)

@mockersf Once the fix for the winit x11 and Wayland features lands, the only transitive dependency on xml-rs should be a build dependency via the atspi-macros crate. Neither accesskit_unix nor atspi depends on the xml feature of zbus at runtime. Would it be feasible to add an exemption for what should be a purely build-time dependency on xml-rs? If not, maybe I can convince the atspi maintainers to switch from proc macros to ahead-of-time code generation.

I think it's OK adding the exception, I added it in #7480. There are a few places that crate is used that would make it hard to root out quickly, but I'll try to open PRs on related projects to get rid of it for the next update cycle

Thanks for all the reviews!
I believe I've addressed all the feedback for me on this PR. I assertively closed conversations I felt were resolved to make working with this a bit easier for me, but please feel free to re-open something if my change didn't address your concern.
I'm also watching the winit 0.28 PR. When that merges, I'll upgrade AccessKit which should hopefully fix at least some of our CI failures.

What specific command do I need to run to cause these CI failures locally?
GitHub's CI interface is an absolute accessibility nightmare, as illustrated by me chasing these down one by one by one. :)

There are git conflicts, so a merge would be useful first. After that

cargo run -p ci should give you most of the checks (check-doc and ci)
cargo check --target wasm32-unknown-unknown should give you the failures in WASM
you can ignore the dependencies / check-bans for now

WASM failures are due to this PR and must be fixed. I opened a PR on your PR to fix the build: https://github.com/ndarilek/bevy/pull/1

a new crate should also be added to the script https://github.com/bevyengine/bevy/blob/main/tools/publish.sh at the correct place for dependencies. This ones needs to be before bevy_ui and bevy_winit, and after bevy_app, bevy_ecs and bevy_derive. I think just before bevy_ui should be good 👍

I think I've addressed everything that was brought up.

I think I've addressed all outstanding feedback.
The one issue I'm fairly sure this PR still has is that the bounds of accessible nodes are probably wrong. Right now I'm feeding their transforms directly to AccessKit, and I likely have to adjust those to take world/viewport coordinates into account. This should be a fairly easy change, but I can't visually see where the accessibility API thinks the components are to correct that offset. And I'd really like to do that before this merges.

Is there a good way to get that information? I'd initially thought to use cargo-bloat but it only supports bin, cdylib and dylib projects.

Most of the dependencies that AccessKit introduces on Linux come through zbus. I see three possible options for shrinking that dependency count:

Put pressure on the zbus maintainers to shrink their dependency count. But I don't want to just complain, particularly about something we got for free.
Use a different Rust D-Bus implementation, like the dbus crate, which uses the libdbus C library. I don't remember all the reasons why the primary author of the AccessKit Unix adapter decided to go with zbus instead, though we discussed it at the time, and I obviously approved the decision. But requiring a C library instead of being a pure-Rust implementation is obviously a downside of this option.
Write an alternative pure-Rust D-Bus implementation that's specifically designed to be small-footprint and low on dependencies. I don't have time to do this now. I might have time to do it later if someone can pay for it.

The AccessKit Unix adapter, and again zbus in particular, also has a noticeable effect on binary size. I measured about 1 MB when optimizing for size.
The impact on dependency count and binary size is much less dramatic on other platforms.

I don't have time to look into this in detail (I am merely a blind lurker who wants the work), but I did glance at docs.rs for zbus and it is listing a lot of feature flags.  I suspect most of them are off by default based on what I'm seeing here, but someone might want to check.
(that said I highly doubt accesskit needs virtio sockets, but doesn't look like that's an optional dep at first glance on their side).

Thanks for looking into this a bit. I'm only seeing a 10 dep increase on windows builds, which to be clear is still a good chunk of deps, but thats much more stomach-able. So this does largely appear to be a linux problem.
I'm very much on the "accessible by default" side. But I'm also on the "we need to keep our dependencies and binary sizes in check" side. I consider a 60 dep increase and 1MB binary size increase "too much" to enable this by default.
I strongly suspect we can improve the dependency / binary size situation, either by trimming down zbus, moving to dbus, etc. If we can't, then we can start making hard calls. But for now I see no reason to hold back this PR, or to compromise on dep counts / binary size, when we can instead just make this feature opt-in. I know thats not satisfying as we all want Bevy UI to be accessible by default. But dep counts / binary size is a line we try to hold everywhere and it has wide and far reaching consequences that affect everyone. Its also a genie thats hard to put back in the bottle once we let it out.

I'm told that Linux support only works with either a hacked version of Orca, or with a currently in-development screen reader not ready for production use.
If we platform-gated this to not be included under Linux and worked on the dependency situation there, would that be acceptable?
A counterpoint is that if developers have to explicitly enable accessibility, they almost certainly won't regardless of how easy it may be. If it absolutely has to be a feature then we can make it one, but I'd like that to be a last resort.
Also, getting some sort of dependency target to aim for would be helpful. I'd also ask that accessibility be measured relative to other core functionalities when coming up with that target (I.e. 20 dependencies may be a lot for a trivial feature, but might not be so bad for something that is foundational. If that is agreed, I'd ask that accessibility be considered foundational in the way rendering, audio, etc. are.)
Thanks.

If we platform-gated this to not be included under Linux and worked on the dependency situation there, would that be acceptable?

Definitely more acceptable. I'd want a full accounting of dependency count, compile time, and binary size changes on the platforms its enabled on prior to merging so we can make a final judgement call. Sadly we can't enable features by default on specific platforms, so it would need to be expressed as direct platform checks in code. It might also be possible to add an "opt in to linux support" feature.
I'm happy to block this PR on that conversation if you want. The opt-in feature requires no conversation and would allow us to have the conversation on its own time.

A counterpoint is that if developers have to explicitly enable accessibility, they almost certainly won't regardless of how easy it may be. If it absolutely has to be a feature then we can make it one, but I'd like that to be a last resort.

Yup this would absolutely be a temporary measure while we sort out the dependency stuff. I fully understood the implications of making it non-default when I wrote my previous message.

Also, getting some sort of dependency target to aim for would be helpful. I'd also ask that accessibility be measured relative to other core functionalities when coming up with that target (I.e. 20 dependencies may be a lot for a trivial feature, but might not be so bad for something that is foundational. If that is agreed, I'd ask that accessibility be considered foundational in the way rendering, audio, etc. are.)

Yup this is a reasonable take. Its important to note that dep counts on their own are just a heuristic. What really matters is:

How many parities are involved? Do we trust them now? Can we trust them forever? More parties, even if we trust them today, increases the cost of verifying we can still trust them after a period of time. Repo owners change over time. People change (good actors can become bad actors).
How many "releases" happen? More deps == more releases (probably), which each has a chance of breaking semver (and therefore, our users). This already happens about once a release cycle and is a real problem for everyone.
How do the deps affect compile times? One big dependency can be worse than many small deps. The reverse can also be true.
How do the deps affect binary sizes? One big dependency can be worse than many small deps. The reverse can also be true.

60 new deps is what I would call a "scary number". Very likely to introduce significant problems in the dimensions above. If someone added an audio dependency with 60 new deps I would put my foot down in the same way. I emphasize "new" here because we're including a lot of deps that have a lot of "dependency overlap" in our tree and are considered to be "ecosystem standard". I consider including deps like syn, quote, cfg-if, parking_lot, futures-x, etc "the cost of doing business" if you want to work collaboratively in the rust ecosystem.
Disabling bevy_audio (and all related default features) removes 17 dependencies from the tree (291 -> 274). I consider 17 a worthwhile cost for what we get (although ideally we can still drive that down).
Its also worth calling out that just like "audio" isn't one thing, "accessibility" isn't one thing either. We already disable some audio formats by default to save on default compile times/costs. We enable the bare minimum by default that unlocks the most value (those 17 deps get us pretty much every piece we need to build out the majority of our audio system). Not all accessibility features will be created equal. Each one will require its own cost/benefit analysis. If we're spending 20 deps on something, we should be very confident that its buying us a "high percentage of one of our core pillars".
I do acknowledge that thinking about accessibility in terms of "cost/benefit" is fraught with issues: people often attempt to justify not including any accessibility features in their apps as a "cost / benefit" / "we don't have the resources for this and this is a small subset of our users" problem. But at the same time, we are building a very complicated piece of software that needs to serve the needs of many people. And being both slim and trustable is a core part of serving those needs. "Accessible by default" is something I fully support. But that doesn't change the cost of bandwidth, the value of peoples' time (waiting for compiles and downloads), the ecological cost of compiling software en-masse, or the risks of downloading unverified software. There is a balance to be struck here.

Thanks for this analysis and clarity. A few questions/clarifications on my end--sorry, I can't figure out how GitHub's quoting works but I'll do my best:

I'm happy to block this PR on that conversation if you want. The opt-in feature requires no conversation and would allow us to have the conversation on its own time.

Just so I know how I should spend my time in the coming days, are we still considering this for the 0.10 milestone? I wouldn't mind it if so since it'd permit integrations with bevy_egui and such, but if not then I'll step back a bit.

Its also worth calling out that just like "audio" isn't one thing, "accessibility" isn't one thing either. We already disable some audio formats by default to save on default compile times/costs. We enable the bare minimum by default that unlocks the most value (those 17 deps get us pretty much every piece we need to build out the majority of our audio system). Not all accessibility features will be created equal. Each one will require its own cost/benefit analysis. If we're spending 20 deps on something, we should be very confident that its buying us a "high percentage of one of our core pillars".

@mwcampbell Please correct me if I'm wrong, but WRT this PR, accessibility kind of is one thing. That is, by bridging back to the platform accessibility APIs, you get screen reader/magnifier support for free. This holds true for any assistive technology written against the platform APIs, so it isn't just vision. There may need to be extra work done, but from Bevy's end that's just updating components and shouldn't involve new dependencies. All the heavy lifting is done by whatever assistive technology the user is running, and I don't think this is like audio in that a few years from now we'll have some new accessibility codec or innovation requiring a bunch of new dependencies in AccessKit. The only thing I could imagine adding to this would be an in-process consumer that essentially acts as a screen reader/magnifier on platforms without native accessibility APIs, but I'm perfectly fine with that being a third-party plugin--would prefer that, actually.
Also, FWIW, I can vouch for Matt as someone who a) cares intensely about all of this and b) will be a good steward of a project like AccessKit. I'm also not too worried that some sort of AccessKit competitor will win hearts and minds in a few years and force a refactor.
Anyhow, I'm willing to try feature-gating Linux support behind an access_kit_linux opt-in. If this can potentially land in 0.10, I'll prioritize either having it complete by Monday, or being unable to move forward without help and kicking it back to 0.11. I'll also see if I can poke folks about scaling back the Linux dependencies, and will see what (if anything) I can do now that I have WSLG. But that's for 0.11 for certain.
Thanks again.

Your quotes worked perfectly FYI: a > before each line is all that it wants.

Just so I know how I should spend my time in the coming days, are we still considering this for the 0.10 milestone?

Correct, that's my understanding. Personally, I'd like to ship something, even if it's incomplete or opt-in. Get it in the hands of developers and users (and our CI) to start testing and refining it.

I can also vouch for Matt.  He's one of probably 100 people in the world to have "owned" a screen reader and other AT for a decade or something like that.  Easily one of the top accessibility coders in the world.  He's also getting contracts to do Accesskit (source, is from Google) so there is funding behind it of some form.
To add some color here.  AT goes in the opposite direction of audio.  Rather than consuming some long list of formats apps which are exposing accessibility expose one standard format per platform.  This is an IPC mechanism.  A screen reader, magnifier, whatever, sits at the far side of that pipe and consumes the format.  The important point is that these formats almost never break.  All major screen readers support software from over 20 years ago to this day.
Eventually accesskit is "finished" in that while it may change, it exposes all of that, and then even if Matt walked away tomorrow it'd still work for basically the rest of time in whatever form it was left.

https://github.com/ndarilek/bevy/tree/accesskit_linux is an attempt at making an accesskit_linux feature that only matters if a) the platform is Linux and b) the feature is specifically enabled. Otherwise, accesskit is required everywhere else.
As of now, it seems to conditionally not use the accesskit dependency under Linux.
Unfortunately it still seems to download and compile AccessKit and all its dependencies, even if it doesn't actually include them in the crate build.
I know how to make a dependency platform-specific, or feature-specific. I'm not clear on how to make a dependency apply on all platforms other than the target, and only on the target when the given feature is specified. I'd hoped I could use the [target.'cfg(...)'.dependencies] syntax with a more complex cfg but either that syntax is pretty limited or I did it wrong. And regular features don't seem to let you include something based on the specific platform.
Also, I hate Rust's cfg syntax for anything more complex. It's information-dense and my brain can't parse it beyond simple stuff. All that is to say, this is probably where I'm leaving it prior to Bevy 0.10's release as I have other things to get to this weekend and early next week. If anyone can figure out how to get these dependencies not compiling when they shouldn't, give this a bit of testing, and submit a PR against this branch, I can at least merge that PR into mine in time for 0.10.
Thanks.

To clarify, there are accessibility requirements that won't be automatically satisfied by using AccessKit. Here are the ones that come to mind:

Using a high-contrast color scheme, or more generally, using the user's chosen color scheme
Implementing keyboard focus
A reduced-motion setting for people that are prone to seizures and the like
Control over font sizes: whether this one is truly needed is a gray area; both Nolan and Austin mentioned screen magnification, in which a portion of the original display is magnified, and the magnifier shows different parts of the screen based on the accessible focus or the location of the insertion point. So that's one solution. But some low-vision users, particularly those with more modest magnification requirements, may prefer to have the application display text in a larger font, with the layout being adjusted to compensate. I know that Windows has a setting for this, but support for that setting in actual Windows apps is uneven at best.

I just want to make sure we're not overselling AccessKit here. In particular, accessibility accommodations that require a modification to how the application displays its UI or handles input are not currently covered by AccessKit.

I just opened a PR to feature-gate the AccessKit Unix adapter in the accesskit_winit crate: https://github.com/AccessKit/accesskit/pull/214 I figure this is the cleanest possible way to work around the problem.

To clarify, there are accessibility requirements that won't be automatically satisfied by using AccessKit.

Sure, I never meant to imply that their weren't. Aside from the ones you've mentioned, games would likely need their own bespoke systems like, for instance, those found in TLOU/Ragnarok. But those are all application-specific, and the fundamentals addressed in this PR aren't likely to shift or scope-creep anytime soon.

Looks like the new accesskit_winit release should feature-gate the Unix backend.
I'll update this branch to use it early on Monday.

@cart You asked for an accounting of compile time and binary size on the platforms supported by AccessKit. What optimization settings do you want me to use when measuring binary size?

I'd like to see LTO binary sizes: that's what devs should be shipping to end users.

Agreed. But optimized for size or speed? And which panic behavior? I suppose optimizing for speed, with the default unwinding panic behavior, would lead to the biggest impact on binary size, and is therefore what I should advertise.

I suppose optimizing for speed, with the default unwinding panic behavior, would lead to the biggest impact on binary size, and is therefore what I should advertise.

Yep, this is what I'd recommend.

@cart How did you measure the increase in dependencies? I compiled the button example, once from the head of this branch, and once from the commit in the main branch that this branch is currently based on. I counted 40 new "Compiling" lines in the output of cargo. Is that not the best way to measure the number of dependencies? I'll grant that 40 dependencies may still be a scary number, but I just want to make sure I do my own measurements the right way.
The binary size increase is indeed significant when optimizing for speed with the unwinding panic behavior, even with LTO enabled. With AccessKit, the Linux binary size for the button example is 22849937 bytes. Without AccessKit, it's 19952196 bytes. That's a nearly 3 MB footprint with those optimization settings. For full transparency, this is what I added to the top-level Cargo.toml in my working copy before compiling the example:
[profile.release]
codegen-units = 1
lto = "fat"

Assuming those settings are OK, I'll do the same comparison on Windows and macOS.

@james7132 @inodentry, opinions on those build settings?

@ndarilek

Just so I know how I should spend my time in the coming days, are we still considering this for the 0.10 milestone? I wouldn't mind it if so since it'd permit integrations with bevy_egui and such, but if not then I'll step back a bit.

I would very much like this to be in 0.10 if we can. I consider making it opt-in an sure-fire way to make that happen, provided we don't wait too much longer.
If we can prove non-linux platforms aren't seriously regressed in the dimensions mentioned above (and platform selection gets implemented in time), then I think we can probably squeeze default-on-those-platforms in too. I personally think implementing opt-in first and getting this merged is the right path, as that way we can include this in 0.10 no matter what. But you're welcome to take whichever path works best for you. I also think phrasing this as a disable-able feature is probably the way to go anyway (even though we're definitely planning on it to being default).
@mwcampbell

What optimization settings do you want me to use when measuring binary size?

In practice I think the defaults matter a lot as many people will just use those. I think defaults and then LTO fat are both good to test.

How did you measure the increase in dependencies?

When I run a build, it will show how many dependencies are in the build. This is the global total for the current build configuration, independent of how many things have already been compiled / cached:

OK, if we're going to use the cargo status line to measure the number of dependencies (even though I'm not sure what it's actually counting), then the impact of AccessKit on Linux is even worse when doing a release build. Up from 293 to 415. When doing a debug build, the numbers I get are 292 without AccessKit, and 354 with AccessKit, which is similar to what @cart is reporting, but not exactly the same. I've run all combinations on Linux multiple times now, and the numbers I get are consistent. But since they're not consistent with what @cart got, and I don't know what that cargo status line is actually counting, I'm not sure we should be using it as a benchmark. But then, bevy users will see that status line, and I agree we don't want to make bevy look bad by making that number go way up.

I think running a build script and then building a package count twice in the Cargo status bar, if that's helpful.  I'm not 100% sure on that.  As far as I know it's not documented.
I'm pretty sure It's also inconsistent if you don't do a cargo clean first.
What you actually want is cargo tree I think, though getting machine-consumable output out of that isn't exactly easy.

Maybe try:
cargo tree --prefix none -e normal | grep -ve '*' | wc -l

I believe I've addressed all the feedback on this I can. There are some rough edges in bevy_ui regarding multi-camera and multi-window that I'd consider out-of-scope for this, though please do let me know if I've missed something. The accesskit_unix adapter is now feature-gated and off by default, This seems to reduce dependency size in Linux builds under WSL. It's also my understanding that a PR recently merged into a transitive dependency might reduce binary sizes even further under Linux and eliminate the banned xmlrs dependency entirely, though we're still waiting on upstream to publish that change. If it publishes before 0.10 is released, I'll rush in a dependency bump PR and see if we can get that in in time.

It's possible that CI might now succeed on this branch, but damned if I can find the Rerun button to do it myself. Thanks, GitHub, for your commitment to accessibility.

@ndarilek we just released new versions of the AccessKit crates. If you still have the time to update a few lines, you'd get binary size decreases on Unix.

Yeah, I believe we're on the latest versions of everything, I just can't force a CI rebuild for the bump of accesskit_winit to 0.12.1.
Thanks for all your hard work on this.

bors try

All the checks on this appear to pass now, BTW.

@cart Here are my numbers for this PR's impact on binary size and number of build tasks (the same rough proxy for dependency count that you were using). In all cases, I did a release build of the button example, with the build settings I quoted earlier.

Linux/GNU x86-64:

with this PR and the accesskit_unix feature: 23,828,902 bytes, 354 build tasks
with this PR but without the accesskit_unix feature (that is, with the null backend in accesskit_winit): 21,113,333 bytes, 298 build tasks
without this PR: 20,930,505 bytes, 295 build tasks

Windows x86-64:

with this PR: 23,258,624 bytes, 315 build tasks
without this PR: 22,853,120 bytes, 305 build tasks

macOS ARM64:

with this PR: 16,628,920 bytes, 327 build tasks
without this PR: 16,343,448 bytes, 322 build tasks

macOS x86-64:

with this PR: 17,674,440 bytes, 327 build tasks
without this PR: 17,341,928 bytes, 322 build tasks

Someone else can measure build times; I had to do these builds on three different machines.

This is good to merge, right?

Oh, no it isn't, more conflicts. I'll fix those again, sorry I didn't look first.
I'd like to get this merged or closed this week if possible. There's more work to do but we do need this as a foundation to do it. Thank you.

bors try

bors r+

bors r+

Excellent, looking forward to building on this!

Definitely cool with this as an initial step, but I think its very important to resolve this in the near future.

Agreed. I'd like to suggest that we adopt a subset of AccessKit's schema into Bevy's UI component structs, using it as the basis for our widget source of truth where possible. Essentially, the closer bevy_ui can come to creating NodeBuilders from its components, the fewer explicit accessibility examples we'll need, I feel like the accessibility schema covers much of what you'd need for a UI definition. At the very least, it'd be a solid start.
Thanks again for merging this.

Most of the dependencies that AccessKit introduces on Linux come through zbus. I see three possible options for shrinking that dependency count:

Before you folks go doing something you might later regret, it would be great if there was some objective criteria of how many dependencies are too many. Crates that depend on other smaller crates (mainly smol-rs ones in case of zbus unless tokio is enabled) instead of re-implementing everything internally, also bring some advantages, such as faster parallel build, significantly reduced hot build time, lower binary size in general (depends on how (un)lucky you are), and most importantly IMO, just being a good Open Source citizen.
So just looking at the number of deps alone is not a good idea. A better criteria would be the total binary size of all the deps combined.

1. Put pressure on the zbus maintainers to shrink their dependency count. But I don't want to just complain, particularly about something we got for free.

I appreciate that you don't want to complain but instead of putting pressure, maybe another option would be to work with zbus maintainers (currently it's just me, working on it in my spare time) to address the issues at hand?

2. Use a different Rust D-Bus implementation, like the [dbus](https://crates.io/crates/dbus) crate, which uses the libdbus C library. I don't remember all the reasons why the primary author of the AccessKit Unix adapter decided to go with zbus instead, though we discussed it at the time, and I obviously approved the decision. But requiring a C library instead of being a pure-Rust implementation is obviously a downside of this option.

If dbus-rs was a great API, I wouldn't have gone down the rabbit hole of writing zbus. If that works better for you, by all means go for it! Just keep in mind that portability wouldn't be as easy then.

3. Write an alternative pure-Rust D-Bus implementation that's specifically designed to be small-footprint and low on dependencies. I don't have time to do this now. I might have time to do it later if someone can pay for it.

Let me tell you a bit of a story. After I gave presentation on zbus (before the first release) in late 2019, a lot of people got this idea that D-Bus can't be that hard somehow (even though I said the opposite) and many D-Bus crates popped up. Many of them didn't go anywhere at all but some of them got to some point where they were just good enough for the very specific needs of the author. However, slowly they all just died out. Never got to the point of being generally useful.
So if that's the route you want to take, that's entirely up to you of course but keep in mind, it's going to be a lot of work. You can reduce the work immensely if you use zvariant but I believe if the output of  cargo bloat is to be trusted, the generics there are a significant part of the binary size.

The AccessKit Unix adapter, and again zbus in particular, also has a noticeable effect on binary size. I measured about 1 MB when optimizing for size.

That's w/o optimizations, isn't it? With some optimizations, it can be reduced by half.

The impact on dependency count and binary size is much less dramatic on other platforms.

Have you found a more reliable method that cargo bloat to measure btw? cargo bloat itself says to take its output with a grain of salt.

another option would be to work with zbus maintainers (currently it's just me, working on it in my spare time) to address the issues at hand?

@zeenix You're right, that's a better way for me to approach this. And I appreciate all the work you've done on zbus in your spare time.
I want to take a closer look at the state of dbus-rs though. AccessKit doesn't have to statically link the platforms' IPC mechanisms on Windows or macOS. So I think the only way to achieve a comparable (lack of) bloat for the free desktops is to use the D-Bus C library, as we use the equivalent C or Objective-C APIs on Windows or macOS. The portability complications that come with using a C library are definitely something to consider, though. What would the Bevy project prefer?

AccessKit doesn't have to statically link the platforms' IPC mechanisms on Windows or macOS. So I think the only way to achieve a comparable (lack of) bloat for the free desktops is to use the D-Bus C library

Fair enough but if avoiding bloat through dynamically linking to libs is what you're aiming for, you may want to reconsider using Cargo at all tbh.

Avoiding bloat as an app developer with cargo can be difficult, but avoiding bloat as a lib developer is entirely doable.  The difference Matt is alluding to however is that on OSX, Windows, iOS and I believe also Android, the accessibility mechanisms are IPC built into the OS fundamentally.  Linux is the weird outlier since it doesn't even necessarily have a desktop environment, "native" widgets are only a thing by convention, and accessibility isn't really a super high priority beyond specific circles.
I personally think bevy should opt to not care and turn it on by default for Linux, but I am blind so I guess I'm biased, and starting optional at least gets it out the door.  I think the more important point is that no one is going to turn accessibility on unless someone figures out how to get as far as the devs and also the app is still maintained the first time someone with insert-disability needs to use it.  Since this is off at the "we didn't compile it in" level, there's nothing to be done for any app that doesn't fulfill the full set of conditions.  A megabyte or so increased binary size isn't great but to me it's a lot better than subsets of users who could otherwise have used a thing without intervention needing to advocate on a per-app basis.

@cart (and the rest of the Bevy core team): How much do we need to reduce the binary footprint of the AccessKit Unix adapter before it can be enabled by default, or ideally not feature-gated at all? If the answer is that the Unix adapter shouldn't be much larger than the Windows or macOS adapter, then I think the only way we can achieve that is by using C libraries (e.g. libdbus or GLib) where we currently use pure Rust (zbus). Which is the lesser evil, increased binary size or more C dependencies? And for the added C dependencies to meaningfully reduce binary size, they have to be installed system-wide.

For me, "added C dependencies" is much much worse. My personal feeling is that "as reduced as reasonably possible" is the bar here: the goal is to reduce footprint where there's low hanging fruit.

Just to make one thing very clear as I think some people have missed aspects of the discussion above due to GitHub pull request UI being really good at hiding discussions:
My end criteria for this effort is for accessible UI to be enabled by default on every platform. The only question at this point is how. The current "feature flag on linux" is a short term middle ground solution while we sort out the best way to handle this situation.

Before you folks go doing something you might later regret, it would be great if there was some objective criteria of how many dependencies are too many. Crates that depend on other smaller crates (mainly smol-rs ones in case of zbus unless tokio is enabled) instead of re-implementing everything internally, also bring some advantages, such as faster parallel build, significantly reduced hot build time, lower binary size in general (depends on how (un)lucky you are), and most importantly IMO, just being a good Open Source citizen.

I've already outlined my stance on this above and touched on the same pros and cons you discussed here. I can't really be more specific than "60 new deps is almost certainly way too much" and "17 is closer to being an acceptable number" without really diving into the details. I agree that dep count is a complicated topic with many dimensions to it. I'll call out that with the alternative being discussed (binding to a c lib), binary size and build time would both go way down. So I don't see "increased parallelism due to high dep count" as a particularly good selling point in this context when the alternative is "4 new tiny deps".
I'll also note that we do value being a Good Open Source Citizen in Bevy land. We aren't dependency averse to the extent of some other projects in the space (such as Macroquad ... but I'll note that they have signficantly better build times and binary sizes than we do because of their rigorous dep-aversion / lack-of-ecosystem-participation). We take targeted ecosystem dependencies in the interest of sharing effort / being good ecosystem players. But we strike a balance with keeping our developer UX in check. See my stance linked above on this. If we take a laissez-faire attitude to deps, this will bite us in a multitude of ways. To summarize again: binary size (in many cases), compile times (in many cases), increased chance of "illegal" semver breakage, increased chance of mismatched dep versions, increased threat surface, decreased user perception of "slimness" ... for better or worse people will look at dep counts when forming their opinions, and increased licensing compliance burden / aggregate license file size for deployed apps.

I appreciate that you don't want to complain but instead of putting pressure, maybe another option would be to work with zbus maintainers (currently it's just me, working on it in my spare time) to address the issues at hand?

I'm very open to this! Some thoughts:

Reworking zbus to cut down on deps would likely be a larger / longer term project. People are clearly itching to get this enabled by default asap, so we might need to make a different call in the short term. But I also don't know off the top of my head how the "slim down zlib" work would compare to porting accesskit to a c dbus lib.
Reworking zbus will yield "unclear wins" at this point. I can't promise that even after finishing a rework effort we would take the dep.
Note that Bevy has a relatively unique (and strict) set of requirements. If we don't choose to use zbus here, that doesn't mean I wouldn't endorse the project generally or use it in my other projects.

Just keep in mind that portability wouldn't be as easy then.

Linking to c libs isn't inherently bad for portability, it just can be bad. We already link to a huge number of them via deps like wgpu, winit, cpal, gilrs, etc. I think this really depends on the specifics of library availablility on systems. I don't have an answer to this problem space.
I'll be honest, if its not clear yet: I'm pretty biased toward pursuing the c lib linking route here (within the context of Bevy specifically).
Alternatively, if anyone wants to do a breakdown of present or future zlib impls that demonstrate that the "dep cost"  (in the dimensions I listed above) is competitive with whatever c-lib-linking-impl we would use, I'm on board. Make your case with numbers and I'll listen.
I'm also curious to see how hard it would be to make accesskit support either zbus or dbus-rs / some other clib option (via cargo config). That would make comparisons easier and it would make it easier to migrate whenever we're happy with tradeoffs. Before making any final decision we need to compare the actual "dep-related concern" numbers and gather data on system compatibility.

I personally think bevy should opt to not care and turn it on by default for Linux, but I am blind so I guess I'm biased, and starting optional at least gets it out the door.

This is certainly a hard call, but from my perspective if we enabled the current impl by default right now we would:

Immediately compromise Bevy in the dimensions discussed above. Every user of Bevy would "pay the price"
Remove pretty much all incentive from the parties involved here to "do this in the way that I think Bevy needs it to be done".

I don't think that would be the correct "optimal solve" for this situation. I'm confident that if we continue to hold our requirement bar reasonably high, that we can meet those requirements in relatively short order. I really don't want to be the "bad guy" here, but I consider myself responsible to all parties in the Bevy community, not just the ones in this thread currently. I'm doing that in the best way I know how. Please let me know if you think I'm not doing that job effectively.

To summarize again: binary size (in many cases), compile times (in many cases), increased chance of "illegal" semver breakage, increased chance of mismatched dep versions, increased threat surface, decreased user perception of "slimness" ... for better or worse people will look at dep counts when forming their opinions, and increased licensing compliance burden / aggregate license file size for deployed apps.

I can't stop people in general making the mistake of shallow assessments but if we want to make a fair assessment, we need to pay attention to specific details. E.g we needed a broadcast channel API in zbus. I could have just implemented it in zbus itself and that would have looked like a win from your perspective as it would have meant one less dependency. Instead I decided to revive an unfinished smol-rs crate so others can use it too. Another example of the same is ordered-stream crate. We didn't really need to split it into another crate.
Many of ours deps are part of the same project, which are split into multiple smaller crates so people can only link the API they need. This means (among other things) there is no increased license compliance burden as all the crates under the same project are under the same license and no increased risk of MSRV bumps etc.
BTW, I noticed that you also seem to like splitting your project into multiple smaller crates rather than keeping a monolith? People can raise the same point against bevy use (directly or indirectly) if they decide based primarily on number of dependencies added.

I'll call out that with the alternative being discussed (binding to a c lib), binary size and build time would both go way down. So I don't see "increased parallelism due to high dep count" as a particularly good selling point in this context when the alternative is "4 new tiny deps".

I wasn't saying that in the context of zbus vs dbus-rs, that would be apple and oranges in this case. There are other reasons (that I mentioned) to prefer zbus over dbus-rs.
My point was that if zbus had fewer deps by 1. implementing things internally and 2. depending on monolithic crates, we'd be seen as heroes to people who just look at the number of deps but it would have meant (among other things), decreased build parallelism.

Just keep in mind that portability wouldn't be as easy then.

Linking to c libs isn't inherently bad for portability, it just can be bad.

I did not say that it's inherently bad. All I can tell you is that I've had multiple people come to me about being unable to cross build their zbus-using code and then it turns out it was because they forgot to remove dbus-rs from their Cargo.toml after porting to zbus.
Don't know how much you know about D-Bus at all but the C library in question here is notoriously bad and undermaintained. I don't know any C app/lib that actually uses it. They all use gio (which is what inspired zbus coincidentally).

Anyway. I think I've put enough of my energy here. It's your project so leave it in your capable hands to make the decision. I provided all the information I could provide so my job here is done. I will now unsubscribe from this but if you have any zbus (or D-Bus) questions, you're more than welcome to contact me either through our our matrix channel or the issue tracker but please don't @ me so I don't get re-subscribed. Thanks. :pray:

Given the state of the libdbus C library, which was also made clear to me in a chat in the GNOME Accessibility matrix room last weekend, I'm ruling out that option for AccessKit.
According to my measurements, the executable footprint of zbus increased sharply with zbus 2, when the implementation was converted from blocking to async. Of course, this change also added several new dependencies.
So I'm thinking about forking zbus 1 to start a new project with the explicit goal of keeping executable size and dependency count low. Of course, this is going to lead to some duplicated effort to reimplement bug fixes and possibly some features that were already implemented in zbus 2.x and 3.x. But it may be the quickest way out of the current situation.

I'm thinking about forking zbus 1 to start a new project with the explicit goal of keeping executable size and dependency count low.

This would backfire and have the opposite result for many applications which would still need zbus for other purposes.

There's one thing Zeeshan Ali said about the libdbus C library (used by the dbus crate) that's not entirely accurate. It is in fact used by some applications. In particular, the Orca screen reader uses it (via the libatspi library), and all GTK 3 applications use it when accessibility is enabled (again via libatspi, and atk-bridge). While the GNOME accessibility stack doesn't exactly have a reputation for robustness, I don't know if any actual GNOME accessibility bugs or general instability are due to problems with libdbus.
So I'm reconsidering the option of using libdbus via the dbus crate.

Another complication with the option to use libdbus via the dbus crate: The D-Bus reference implementation, including libdbus, is dual-licensed under GPL 2 and Academic Free License (AFL) 2.1. My understanding is that AFL 2.1 is a permissive license similar to the Apache license, but still, it's another license. And this info isn't currently reflected in the metadata of the dbus or libdbus-sys crate.


Specify root domain for email links

Is there any way to programmatically specify the root domain which should be used when creating links in emails?
(Something similar to what's specified here in Rails ActionMailer http://guides.rubyonrails.org/action_mailer_basics.html#generating-urls-in-action-mailer-views)

@shawnbro Bamboo uses Phoenix.View.render_to_string function to render Phoenix views.
So you have two options: you either specify the full URL on the template, or you create your own link helper which gets the root url from somewhere (if Phoenix link helper doens't do this already, I am not familiarised enough with Phoenix to know that, but a with a quick search should be something easy to find).
In any way, Bamboo's responsibility is to provide you with a simple interface to manage emails, so what you were asking is not possible to do on the Bamboo side.
@paulcsmith Am I right ?

Yes I believe this is beyond the scope of Bamboo, and is part of Phoenix if that's what you want. You can use the regular link helpers and pass your endpoint to the url helper.
<%= link "Users!", to: users_url(MyApp.Endpoint, :index) %>

Just make sure you set the correct host for the endpoint in config/prod.exs :)
If you're not using Phoenix then I would make a function that generates URLs with the domain you want. I'm going to close this, but feel free to leave a comment if you need further clarification.
Thanks for the comment @jbernardo95!


Support returning the address lines to DOM nodes other than form fields

e.g. a <div> rather than a <input>

Coverage decreased (-0.2%) to 18.139% when pulling 9967f19a9ab1bef8bfd482104e4669ae09dfa23e on jbate:return-data-to-non-form-fields into 8608e48d6a706f0a3df2c86604b704b335b0ff66 on experiandataquality:master.


Random generation of initial states

This introduces two changes:
initialState :: Gen state instead of initialState :: state and a new callback setup :: state -> m () to RunModel.
Checklist:

[x] Check source-code formatting is consistent

This PR is precisely meant to address this clunkyness in the current design. Almost everyone ends up needing to do what you describe and it's always ugly one way or another.
The API in the PR on the other hand imposes minimal overhead if you don't use it (just write pure MyState{..} instead of MyState{..} in one place) and avoids all this clunkyness of having to encode the "have I gone through the setup stage yet" in the precondition and nextState for every action in the model.

There is one design decision left here that we need to discuss and that's how to deal with initial states in DL. Currently the low-level DL functionality punts on this and has you provide an initial state in your property while the monadic forAllDL interface generates an initial state for you from the initialState generator in the state model. Ideally we would change the interface to require you to provide an initial state everywhere, but at the same time that seems clunky. Should we have the "use the built-in generator" and the "run in this particular state" interfaces side-by-side perhaps?

we would change the interface to require you to provide an initial state everywhere

If one defines an initialState generator then why would you want to use a specific initial state for some DL formula and not all? What would be the use case?

The best example would be "when the initial state is drawn from this distribution, I expect this property to hold". E.g. when the protocol parameter for some delay is 0, we expect something to always happen immediately. That would be nice to express as a DL property I think?

But then, it seems to me I could rather express that directly in the DL formula and have a special action initialising the system with some specific values.

Indeed, but mixing that with a random initialState would be messy!

Hence why I am starting to have doubts about the value of this change. WDYT @bwbush?

The initializing action is clunky in one direction, this is clunky in a different direction. I tend to prefer this design but let's think more carefully about what we want to be able to do on the DL side.
You're saying that you want to be able to say action $ Init ... but what if there was something like init :: state -> DL state () that only worked in the beginning of a DL run (i.e. implementing what you would have to implement by hand in Init's precondition)?

Would you need to construct and pass explicitly state to init? Or is init not a combinator from the DL language? How would that look like? I think we came here because of some need that surfaced from Peras project, hence why I would be interested in @bwbush's insights

Yes, you would. However, you would turn:
instance StateModel (Maybe MyState)
  initialState = Nothing

someDLThing :: DL (Maybe MyState) ()
someDLThing = do
  action $ Init ExplicitlyConstructedState{...}
  ...

into
instance StateModel MyState
  initialState = myStateGenerator

someDLThing :: DL MyState ()
someDLThing = do
  init ExplicitlyConstructedState{...}
  ...

Or some variant of this where the original model doesn't have the explicit Maybe part that John is talking about.
Anyway, note that the design in the PR can incorporate both of these techniques (given that we implement init) while the old design forces the clunky Init action way of doing things.
This is not a Peras-only problem. It appears in Peras, but the Init :: MyState -> Action MyState () pattern of variants of it have appeared in lots of places before.

I agree the pattern of having an Init :: ... specific action appeared elsewhere. Actually, I myself have used it in 100% of the q-d models I have written :) However, is this really a problem? Having initialState :: Gen State is symmetric to data Actions State a where Init :: State -> Action State () and for the user to recover control when they want, we need to introduce an additional init combinator to DL language.
I am not sure what we really gain, hence why I would be interested in having more users chime in.
That said, if you think there's value in moving in that direction, please go ahead :)

It's not quite symmetric for two reasons:

With the Gen State approach you can still recover the old approach if you want to, while the opposite is not quite true because
The Init approach requires your state to have some flag for "I have not yet been initialised" that you either have to check in every actions precondition or generator, which you don't need to check in the Gen State approach because that's taken care of for you.

Fair enough


Do not accelerate platform siding

@Kenny-Hui I revised your code to use a canAccelerate flag instead, please take a look and perhaps test it out :)

Now the train is basically stuck at a very slow speed when they stopped right infront of the platform rail due to traffic jam, as they are not allowed to accelerate on the platform rail (So they can only accelerate a tad bit before entering the Platform rail), hence I enforced a 20km/h minimum speed originally to make sure they won't just slowly glide to the platform.
Either way thanks for looking into this


refactor: simplify code generated by Ignite CLI

Description
Ignite CLI sometimes generates unnecessary or superfluous code. This PR is about simplifying this code.

Remove unnamed variables in CLI and key files like var _ = strconv.Itoa(0), those are generated by Ignite CLI to prevent unused import errors but are unnecessary
Group arguments like AddCommand or RegisterImplementation when variadic functions are used
Remove unused DefaultRelativePacketTimeoutTimestamp

Closes:
Type of change

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

[ ] Tested CCTX in localnet
[ ] Tested in development environment
[ ] Go unit tests
[ ] Go integration tests
[ ] Tested via GitHub Actions

Checklist:

[ ] I have added unit tests that prove my fix feature works

Looking into the test failure


'group deployment' is deprecated

The given example suggests we use group deployment, however, it appears this command has been marked as deprecated
This command is implicitly deprecated because command group 'group deployment' is deprecated and will be removed in a future release. Use 'deployment group' instead.
Suggest the documentation be updated to utilize group deployment instead.

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

ID: aa28dc85-7097-6005-1a5b-6cfe8a7e9528
Version Independent ID: 842a431e-cdd8-c695-70eb-78294acd80f3
Content: Create a Log Analytics workspace using Azure CLI - Azure Monitor
Content Source: articles/azure-monitor/learn/quick-create-workspace-cli.md
Service: azure-monitor
Sub-service: logs
GitHub Login: @bwren
Microsoft Alias: bwren

@wahyuen Thanks for your comment. We will review the issue and get back to you shortly.

I ran this one top down and there are some syntax idiosyncrasies, to be certain. Thanks for pointing this out to us @wahyuen!

@rbschneider  Can you please provide an update on the document enhancement?

@SwathiDhanwada-MSFT I tested it last week and it worked both ways, meaning 'group deployment' and 'deployment group'. I will contact the dev team right now to discover what the correct syntax should be and update the doc accordingly.

@SwathiDhanwada-MSFT
This particular area is outside of my teams purview and this would represent a rather profound change to the documentation set. I will reach out and see who the subject matter expert is for the Monitoring

We will be reviewing this issue further this week. Thank you to @wahyuen for reminding us about this.
@SwathiDhanwada-MSFT You can close this issue as this is now part of a larger internal documentation update effort.
#please-close


hare: disable atx grading

Motivation
hare is using a malfeasance proof received later than the proposal's activeset creation time.
disable it for now.

bors try


Allow use of LLRT runtime

AWS released an experimental runtime called LLRT (Low Latency Runtime) which promises to be faster at cold boots than even some of the compiled runtimes containers like Go. The full details are here: https://github.com/awslabs/llrt
As an experiment, sst should support using the LLRT runtimes. The best place to start would likely be with the FunctionConstruct, and then if it's stable and workable apply the changes to SsrConstruct as well.
The runtime should be an opt-in only runtime for the foreseeable future, and I expect there will be annoying bugs and limited support until the runtime hits v1, so this should be considered an extremely dumb thing to do in a production environment.

Thanks for taking a crack at it!

We are transition this repo to https://github.com/sst/v2 and are closing this issue for now. If it is still relevant please reopen it there - sorry for the trouble


Generalization of the fetching process and P. Breheny datasets

This PR :

Includes P. Breheny large-scale datasets (using rpy2 to handle .rds files)

Generalizes the fetching process of a dataset in an AbstractDataset class. This class can then be overloaded regarding on the database (LIBSVM, P. Breheny datasets, ...). This represents big changes in the package but may allow to include many more databases easily into the package.

@TheoGuyard thanks heaps for all the work. It's a big PR so I'd try to split it into smaller ones:

a refactoring with AbstractDataset (without adding Breheny) and keeping the tests as in main
a subsequent PR to add Breheny datasets

WDYT?

I close this PR and I'll make two new ones with the abstraction level for fetching datasets and the adding of P. Breheny datasets.


binding/SkiaSharp.iOS project build fails with error MSB4044 in VS 2017 on Win10

Description
Trying to build SkiaSharp.iOS project fails with:
Error	MSB4044	The "BTouch" task was not given a value for the required parameter "BTouchToolPath".	SkiaSharp.iOS	C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\MSBuild\Xamarin\iOS\Xamarin.iOS.ObjCBinding.CSharp.targets	60
Code

follow the build direction for Windows and macOS.
on macOS, copy output/native/ios files to Windows
Launch binding\SkiaSharp.sln
Set "SkiaSharp.iOS" as the startup project: Debug, Any CPU
Set configuration manager to select only SkiaSharp.iOS

Expected Behavior
No error in the build.
[I was able to build Desktop, Android and UWP projects without any error]
Actual Behavior
Error MSB4044. [see details above]
Basic Information

Version with issue:  cloned yesterday Dec 11, 2018 [using git bash window].

Last known good version:  ?

IDE:  Visual Studio 2017, 15.9.3

Platform Target Frameworks:

iOS:  xamarinios1.0 [copied from .csproj file]

Target Devices:

N/A

Screenshots
N/A
Reproduction Link
[see steps above]

I'm closing this since I didn't understand how the projects work.  It used to be shared library but now the files are included into the project directly with include and wildcards.


Use Keyboard navigation instead of Voice Over for System Settings navigation

What's changed?
I was experimenting with the vanilla Sequoia template here, and noticed that it kept crashing the moment it tried to enable Voice Over.
As a workaround — and possibly a better option — the "Keyboard navigation" setting can be enabled with a defaults command, allowing the navigation of the UI with the keyboard.
So this PR changes from using Voice Over to enabling Keyboard navigation instead. It also includes a couple other unrelated changes which I'll explain inline.

Thank you! Sound like a more error prone way to do it. Will let @edigaryev to decide as the original inventor of the boot commnads and voice over hack.

Thanks!
This should probably nail it for now, but hopefully we'll be able to simplify this back again once Apple fixes this weird crash.
For future reference, the com.apple.Virtualization.VirtualMachine seems to crash with the following error:
FIXME: "Unhandled report type 01" line 324


Update sys-pdw-nodes-column-store-row-groups-transact-sql.md

The first query example does not handle the division by zero on total_rows
It would good to either filter it (total_rows>0) or warn about it in the paragraph describing it.
There are cases where the response time is very long, and finish due to this error.

@lionelpenuchot : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@ronortloff - Would you review this pull request? Thanks.

Instead of this public repo PR #5193, I am encouraging we Merge private repo PR 16459.

https://github.com/MicrosoftDocs/sql-docs-pr/pull/16459

GeneMi (= MightyPen)

Closing permanently this public repo PR #5193 now, because private repo PR #16459 has been merged.
cc: @lionelpenuchot


testdata collides with testData

some leftover duplicates? the directories seems to have the same content? for example: https://github.com/knative/func/tree/main/pkg/builders/s2i
From macos:
git clone https://github.com/knative/func.git
Cloning into 'func'...
remote: Enumerating objects: 32174, done.
remote: Counting objects: 100% (614/614), done.
remote: Compressing objects: 100% (440/440), done.
remote: Total 32174 (delta 186), reused 520 (delta 135), pack-reused 31560
Receiving objects: 100% (32174/32174), 96.47 MiB | 7.35 MiB/s, done.
Resolving deltas: 100% (15366/15366), done.
Updating files: 100% (9464/9464), done.
warning: the following paths have collided (e.g. case-sensitive paths
on a case-insensitive filesystem) and only one from the same
colliding group is in the working tree:

  'pkg/builders/s2i/testData/builder.tar'
  'pkg/builders/s2i/testdata/builder.tar'
  'pkg/docker/testData/image.tar'
  'pkg/docker/testdata/image.tar'
  'pkg/k8s/testData/content.tar'
  'pkg/k8s/testdata/content.tar'
  'pkg/pipelines/tekton/testData/fn-src/.gitignore'
  'pkg/pipelines/tekton/testdata/fn-src/.gitignore'
  'pkg/pipelines/tekton/testData/fn-src/hello.txt'
  'pkg/pipelines/tekton/testdata/fn-src/hello.txt'
  'pkg/pipelines/tekton/testData/fn-src/hello.txt.lnk'
  'pkg/pipelines/tekton/testdata/fn-src/hello.txt.lnk'

My guess is that the recent PR which renamed it got combined with the other PR which reorganized the repo layout, and both ended up kept.


format fixes in AMPL.go

Pull Request Test Coverage Report for Build 362189610

0 of 0   changed or added relevant lines in 0 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-0.06%) to 36.618%

Files with Coverage Reduction
New Missed Lines
%

pkg/tracker/ampl.go
1
77.97%

Totals

Change from base Build 361740367:
-0.06%

Covered Lines:
1598

Relevant Lines:
4364

💛  - Coveralls