router

https://gist.github.com/fend25/3619d6d730039c30a34d

https://jsfiddle.net/fend25/e1LLu72c/
better and preferabe variant now,
guys, feel free to comment)

Haha, thought yesterday how great it would be having some views bound to routes. :D
A good basic router implementation would be this:
http://krasimirtsonev.com/blog/article/A-modern-JavaScript-router-in-100-lines-history-api-pushState-hash-url
My approach would use regexes as parameter of a directive:

  User 1

  Back to overview

This would allow to define the urls directly in the template, no additional code would be needed for basic features. But there should definitivly be an API to do redirects, handle url errors etc.

Oleg's reference implementation (ancient one)
http://plnkr.co/edit/QYb53NUnnhIezI5n0JxB?p=preview

Ok, i just build an absolute simple, quick n dirty views implementation:
https://jsfiddle.net/r9t3ropc/
Comments are welcome.

I think it should be separated into two different API's:

Router is for router events and state handling
View rendering route data into template and managing scope.

The first should have unified interface to change current location and update/rewrite state. Also it should be presented in root scope and should broadcast events. Events could be stopped to prevent page reloading when there is unsaved changes.
View should allow to use functions as route value. This functions return async operations to load template and resource specified by route. This is needed to render state dependent routes. By default view could use alight.router to bind routes.
<al-app>
    <al-view al-view.routes = "routes"/>
</al-app>

alight.bootstrap('al-app', {
    routes : {
        // Simple template route
        '/': 'index.html',

        // State dependent route
        '/home': function(route, scope){
            if (scope.user) {
                return {
                    scope: {
                        user: user
                    },
                    template: '/users/home.html'
                }
            } else {
                return '/sign-in.html'
            }
        },
        // Async route
        '/users/:id' : route => {
            var user = users.get(route.id);

            var promise;

            if (! user) {
                promise = loadUser(route.id).then(user => {
                    if (! user) {
                        return {
                            scope: {},
                            template: '/errors/404.html'
                        };
                    } else {
                        return {
                            scope: {
                                user
                            },
                            template: '/users/item.html'
                        }
                    }
                });
            } else {
                promise = Promise.resolve(user);
            }

            return promise;
        }
    }
});

@rumkin Can I use common header and footer for "/users/*" in your approach.
In this example by @fend25 I can do this: https://jsfiddle.net/fend25/e1LLu72c/

Events could be stopped to prevent page reloading when there is unsaved changes or so.

It's @onLoseFocus="showPreventWindow()" in the example.
@onSwitchTo="onSwtichToUser()" could be something like this:
scope.onSwtichToUser = route => {
            var user = users.get(route.id).then

            var promise;

            if (! user) {
                promise = loadUser(route.id).then(user => {
                    if (! user) {
                        // User not found, show 404 error page
                        return '/errors/404.html';
                    } else {
                        // User found, use user page template
                        return {
                            scope: {
                                user
                            },
                            template: '/users/item.html'
                        }
                    }
                });
            } else {
                promise = Promise.resolve(user);
            }

            return promise;
}

What do you think? "Configuring" in html can be easier.

@lega911 Yep, it could be done with several view elements. Like this:
<al-view al-view-routes="routes.head">
<al-view al-view-routes="routes.body">
<al-view al-view-routes="routes.footer">

It's not so elegant as HTML configuration but it simply to manage from code. I think configuring in HTML is the most proper way to do this for MVVM app. But it should be also configurable and controllable from code.
@ character is not a valid HTML attribute name element. Is this some kind of future DSL?

I think it's better to configure router with html and implement fully controllable router and view instances with focus on usability.

@ is not valid HTML attribute name character. Is this some kind of future DSL?

Yes, but we can make it like this (with the event directive):
al-on:on-lose-focus="showPreventWindow()"
or alias
@on-lose-focus="showPreventWindow()"
and "on-lose-focus" is an event (new CustomEvent), which you can catch even with jQuery. https://github.com/lega911/angular-light/issues/123

Maybe it's better to avoid on duplication and bind events to target component to avoid mess of events in the future:
al-router:on-reload-requested
al-router:on-lost-focus

Maybe, but custom events can be more flexible.

May it possible to implement url handling like express.js?
Another point is, that using Views which use "templates" would not be useful, because in this case i could redirect the user to a page which shows the content of this "template".
Views should be used inline, to easily switch between views by a changing url.
I also think it is not useful to bind a view to a scope variable.
My approach uses a global routes object, where regexes are defined.
Using al-route or by adding routes to the object, new routes can be defined.
Also a custom route can be defined in this object, which will be referenced by a given element selector.

alight.routes = {
  '/home': (scope) ->
    return '#home'
  '/players/:id': (scope, id) ->
    scope.id = id
    return '#players'
}
alight.d.al.route = (scope, exp, element, env) ->
  alight.routes[exp] = ->
    return element

What do you think?

My approach uses a global routes object,

I agree

div id="home">

```
should it work like al-if with dependency on url?

What about this one?

Loading...

      No page 404

when url = "/user/:userId", then controller al-ctrl="userItem" is activated, there you can load a user,
a template can be loaded by al-include when a user is loaded

,
and you can see this before user is loaded
Loading...

so, visible html for /user/:userid would be

Loading...

visible html for /user would be

visible html for /login would be

I agree. It looks exciting. And it should to allow to load subroutes with al-include for complex components.

@rumkin al-include is depricated, look at http://angular-light.readthedocs.org/en/latest/directive/html.html , it's more flexible :)

@lega911 I will. But you're using al-include in your example.
Let's focus on modular structure, easy of use and flexibility. And thus it should to use al-html to load subroutes.

should it work like al-if with dependency on url?

Yes, you have a pretty good understanding of what i mean. :-)

al-route:group

I do not understand how this works.
I understand that it groups some requests.
But how is defined that a specific request should be used?
Which request is the first if you are visiting url "/"?
What i also thought about are subroutes:

Which would be matched when accessing:
/user
/user/:id
/user/details
Maybe it would be great to also handle redirects in the global route object:
alight.routes = {
  '/home': '/'
  '/players/:id': (scope, id) ->
    scope.id = id
    return '#players'
}

which would redirect "/home" to "/".

al-route:group
I do not understand how this works.

It's easy, one rule - if there is a route that maches to url, then it should be visible, then all parent DOM should be visible, other route-elements should be hidden.
I changed your example in this style:

for url "/user/details" we will have:

In your approach we have to copy-past header and footer for "/profile".
On the other side, your approach works well for loaded html (al-html).

Hm. This is a bit confusing for me.
The grouping attribute is there to render a specific header an a footer, right?
The content for a route is inside of

My user profile

So when accessing /profile i will see this:

My user profile

Am i right?

So when accessing /profile i will see this:
Am i right?

Yes.
Actually for example above we don't need a "group" route:

In a simple, "group route" just holds all child routes, like al-route="/profile; /user/:id/view; /user/details", but it take them automatically.

The grouping attribute is there to render a specific header an a footer, right?

Not exactly, it renders all DOM, except routes that don't match to URL

Not exactly, it renders all DOM, except routes that don't match to URL

I like the approach. :-)

Prototype: http://plnkr.co/edit/lZkXWXg5xk1J147nb1jX
embeded: http://run.plnkr.co/plunks/lZkXWXg5xk1J147nb1jX/
with no regex yet.

Here an regex router with params and tail capturing:
https://jsfiddle.net/rumkin/y6xLu1sm/

Here an regex router with params and tail capturing:

I took it for my example

You can get arguments using event al-on.route-to

One more example: http://plnkr.co/edit/pQBycumCKE1DJb3oVaPE?p=preview

you can use the same routes a few times in different places (al-route="/user/details" in the example)
default route

          404 - Not found

You can get arguments using event al-on.route-to

Why not providing these variables automatically in a scope variable?
E.g.
scope.$route.name

Parsing from URL shouldn't be so difficult:
"/user/:id/name/:name/view/:action".match(/:\w+/g)

Why not providing these variables automatically in a scope variable?

It can give you collisions for routes "/users/:name", "/users/root" they both will be visible on url="/users/root"

@dev-rke It could be done manually with hooks. And sometimes it could cause type mismatch.
I prefer to assign route params to scope.$route.

It can give you collisions for routes "/users/:name", "/users/root" they both will be visible on url="/users/root"

No, it should not.
When you provide for each route a own scope, e.g.

    {{$route.profileID}}

    {{$route.id}}

    {{$route.id}}

Then i see no problems. :-)

No, it should not.
When you provide for each route a own scope, e.g.

Ok, I don't see normal cases for collision, so $route should be ok.
But I don't want al-route makes a new scope, you can use al-ctrl if you want a new scope, also in this case you don't need @route-to because your controller be called when route is active.

$route is ready http://plnkr.co/edit/pQBycumCKE1DJb3oVaPE?p=preview

http://plnkr.co/edit/pQBycumCKE1DJb3oVaPE?p=preview
embedded: http://run.plnkr.co/plunks/pQBycumCKE1DJb3oVaPE/
what works:
al-route - define route (can be empty)
al-route:default - it's activated on undefined url (for 404)
al-route-out="onOut()" - return true if you want prevent url be changed
al-on.route-to - event when route is activated, you can use al-ctrl instead of it.
scope.$route - contains arguments from url
what else?

what about "href" ?
Login
Login
Login

Also we have js API:
service.location = {
    go(link)  // move to link
    subscribe(fn, [flag])  // subscribe on changes url, on 404 page, out-event
    unsubscribe(fn, [flag])
    getCurrentUrl()
    isDefault()  // returns true if it's 404
}

What about routes that should not be routed and need page reloading? If I wish to serve /blog with another lib or have sub applications?

need page reloading

you can use usual  to reload the page.

If I wish to serve /blog with another lib or have sub applications?
al-route="/blog/**"

Your example works, but router should be only one.
Your sub applications can use API: location.subscribe()

you can use usual  to reload the page.

ok

what about "href" ?

al-href?

al-href?

Why not, this name is not busy.

al-href will collide with :href, at least in mind.

vuejs and A2 seems to use name "link", v-link, router-link

Hm, your examples do not work anymore, maybe due to the last release?

It doesn't work in FF because of wrong transpiler, FF doesn't support sync xhr

I tried it in Chrome, it works fine.
I like the defined behaviour.
What do you think about prefixing normal href attributes using "#" instead of introducing something like al-link?
When a href attribute is prefixed with "#" the attribute gets handled by alight.
If html5 push states are used, the prefix get removed before applying the url.

Do you mean?
Profile

Interesting, what about buttons and other?
<button href="#/user/profile">Profile</button>

Profile

Profile

Shebang is using to navigate across webview. This behaviour will be broken. I think al-href is good enough. It short and clean. It refers to default href which it actually is.

What do you think about prefixing normal href attributes using "#" instead of introducing something like al-link?

it will bring some strange logic to the service.go processor.
But the most important thing is that it will collide and brake specs about hashes in url. And what if I want to use hashes?
I think, special directive is the best way because it won't collide with specs nor with other things in mind.

Interesting, what about buttons and other?
<button href="#/user/profile">Profile</button>

Why? it's totally confusing. And all workaround (like twitter bootstrap) is prepared for

 structure.

@lega911

Interesting, what about buttons and other?

href is meant as compatibility mode. So for using buttons normally one would use a link element and style it like a button (compare twitter bootstrap). Also a button can be used with a form, so the form action also should be handled.
I also think it would not be necessary to prefix links using a hash.
As we have a global routing object, we can check each click event of a link in the current scope, if the url matches to a defined route.
E.g.
Will show /user route element
There is no matching route, so page /profile will load

There should be no problems and it is easy to implement.
Also it would be good for SEO purposes, because one can use prerender.io and all links should be accessible.
@rumkin

Shebang is using to navigate across webview.

Can you please give an example?
I did not understand what the problem is. Maybe its obsolete when we check a clicked link against our routes (see above).
@fend25

But the most important thing is that it will collide and brake specs about hashes in url. And what if I want to use hashes?

Why would it break?
Using views requires to use hashes or html5 push states, because they are bound to the url.
When someone uses a link outside of defined routes, the link would not be handled by alight.
The great benefit would be, that someone can define "hard" links (e.g. for SEO purposes) which are handled by javascript, if the client supports javascript and fallback to normal links, if no javascript is used.

if the client supports javascript and fallback to normal links, if no javascript is used.

What if I need a normal link, but I use javascript.

There is no matching route, so page /profile will load

You can do this with al-route:default

What if I need a normal link, but I use javascript.

Typically an url is unique (especially to prevent duplicate content in search engines).
So when defining a url that links to a view, the url is handled by alight.
When defining a url, that does not match a predefined route, the link will be handled by browser defaults (e.g. by opening the linked page).

You can do this with al-route:default

I do not understand, what exactly you mean.
The concept is, that i define routes.
A route can be defined by al-route or by extending alight.routes object using custom javascript.
If a link (a tag with href attribute) is clicked, alight will try to match the href attribute to a route.
If a route matches the url, alight will handle the url (e.g. by showing a view) and prevent default browser behaviour.
If no route matches the url, alight will not intercept the default browser handling and the browser will handle the url, e.g. by loading the url.

When defining a url, that does not match a predefined route

then "al-route:default" is activated
I think your approach can give us implicit mistakes

I think your approach can give us implicit mistakes

e.g. if I open an application by wrong direct link, it would redirect me somewhere.

then "al-route:default" is activated

When al-route:default is not defined, what happens then?
Nothing? I do not see the benefit of a "default" modifier.
If one needs a default setting, he can define a catchall route (e.g. "/.*") manually.
A default route should not be implicitly defined, it should be defined by the user.
Especially if you want to implement progressive enhancements (https://www.w3.org/wiki/Graceful_degradation_versus_progressive_enhancement#Graceful_degradation_and_progressive_enhancement_in_a_nutshell) on older websites, we need to ensure that the page can be used without modern features / excessive javascript usage.

I think your approach can give us implicit mistakes

Can you please give an example?
Currently I do not see any case where there could be real problems.
The benefit is, that i do not need to define urls twice, when i want to make my application ready for SEO and for screenreaders.
Feature Request: defining a custom base path for the application, e.g. when an application is placed in a subfolder. A simple and not flexible solution would be to read the base tag from head element.
A better way would be to provide a custom interface, e.g. a property, which can be set manually.

If one needs a default setting, he can define a catchall route (e.g. "/.*") manually.

This route will be visible always, route-default is visible only for undefined routes (page 404).

the page can be used without modern features / excessive javascript usage.

I'm not care about old browsers for the router, because it's not a part of core (yet), and probably there will be a few variants of router.

Can you please give an example?
Maybe, if you make a misprint or if there is a mistake in code, the browser will reload page, and you will not see a error.

Also, how will you use "href" for buttons, it will look weird, http://getbootstrap.com/css/#buttons
So al-link/al-href should be anyway, your approach can be additional.

defining a custom base path for the application

ok, it's should be useful.

how will you use "href" for buttons, it will look weird

ok, for bootstrap you can use a for buttons, but I want it works for other elements, images/div/span...

defining a custom base path for the application

done
service.location.setBase(location.pathname);

Added history-list example: http://plnkr.co/edit/pQBycumCKE1DJb3oVaPE?p=preview

This route will be visible always, route-default is visible only for undefined routes (page 404).

So this is like a catchall.
Do you think this is necessary to provide in alight?
A user can define a catchall route manually.

Also, how will you use "href" for buttons

I will NOT use the button tag. I will use an a tag and style it the same as a button.
This is what bootstrap does.
When using a button, the button relates to a form (e.g. as submit or reset button).
And the form needs to be also handled by alight, when a submit event occurs.

but I want it works for other elements, images/div/span

Typically one defines an anchor element around an image or a span element.
A div should not be wrapped by an anchor, because a div is a block element while an anchor is an inline element. What you can do is using an anchor and a span and display them as block or inline-block elements.
So what you are thinking of here is to break pre-defined functionality.
I think about merging existing standards with new functionality.
It also would be even simpler for new users just to define routes instead of changing the full template.
E.g. one already implemented a template and wants to enhance it with some client side features, it would be more simple to add alight when he does not need to modify existing links.
He just needs to define routes and a view which displays the contents.

Hm, what about this:
One defines his application e.g. /users/

CatchAll for /users instead of al-route:default, e.g. when one removes paramters from the url

Redirects to a page /login
Redirects to a page /groups

This would be definitively more flexible than handling urls by using an extra directive.
It's interesting that you were against an own directive in combination with templates, but you prefer a own directive for routing. I am not against an own directive for routing, but i think it is not required.
It would be better to provide a simple API, e.g.:

This would give more flexibility than a directive, because the directive is bound to a specific event, e.g. click event. But what when a view should be shown when something is hovered?

This is a simple preview box of a large image, when the user hovers over a thumbnail image.
The great thing about this is, that each large image has an own url, so one can share this url. :-)

Better API proposal:

you were against an own directive in combination with templates, but you prefer a own directive for routing

Because template directives are all about one thing - inserting templates and I can reuse the code, no double code.
al-link has unique code, and it's shorter than al-route.link

something is hovered?

a good idea, it's done
    User Ubuntu

But you have a limitation here - you should use al-ctrl if you want to have "scope.$route"

But you have a limitation here - you should use al-ctrl if you want to have "scope.$route"

I can place $route in every scope, but I think it's can be bothersome e.g. my timepicker component doesn't need it.
So $route is placed in child scope of al-route (al-route doesn't make a child scope), you can use al-ctrl to make a child scope where $route is appeared.

CatchAll for /users instead of al-route:default

"/user" - is a simple route, need a key-symbol, maybe "/user/*"?

"/user" - is a simple route, need a key-symbol, maybe "/user/*"?

Nope, i explicitly mean /user.
This could be some kind of userlist or something, because it has no id in the url.
Matching against "/user/*" would exclude "/user", so it would not be a default handler.
If you append anything else, it would be matched by "/user/:id".

It seems we talk about different things, your route "/user" works.

It seems we talk about different things

No, we talk about the same.
You want to implement a catchall handler, that catches all urls that are not matched by the given urls in a group, right.
I recommend to make the catchall configurable, by defining custom catchall handlers.
And i would like to see handling normal links.
What do you think about making the al-link directive as additional attribute, e.g. like:
User 1234

If al-link has no parameters, it tries to fetch the link from href attribute or from action attribute (for form elements) or from src attributes (for images). Is this a compromise? :-)

CatchAll for /users instead of al-route:default, e.g. when one removes paramters from the url

I don't like this mark-way, what if I want "/user" as usuall route, it has no sign for this.
So, we can make special sign for this, we can make a special type for routes (e.g. :catch"),:

CatchAll for /users #1

CatchAll for /users #2

CatchAll #3

While al-route works in parallel (a few routes can be visible), al-route:catch depends on an order - only first al-route:catch will be visible, so for "/user" #1 will be visible, for other urls it will be #3
You can propose another sign for this

I don't like this mark-way, what if I want "/user" as usuall route, it has no sign for this.

You can do this. The idea is NOT to define a DEFAULT route using a special directive or directive modifier.
You define your url handling, as it is done e.g. by express.
If you want to handle a 404, you need to specify on which route the 404 should be activated.
E.g. you define

This is your /users route

Catch, when no other route matches /users/(.*)

Another example:

This is the catcher, anything matches anything except users/*

In this example any parameter, that does not match a given route, needs to be handled after other urls. We only need to define a processing priority, to ensure, that urls containing parameters are handled last. This can be done when adding new routes to the global routes object.

While al-route works in parallel (a few routes can be visible), al-route:catch depends on an order - only first al-route:catch will be visible, so for "/users/wrong/link/here" CatchAll for /users #1 will be visible, for other urls it will be CatchAll #3

I understand, what you like to do, but we do not need a directive modifier for this behaviour.
Its always the same handling structure.

If you want to handle a 404, you need to specify on which route the 404 should be activated.
E.g. you define

This is your /users route

Catch, when no other route matches /users/(.*)

It doesn't work now, because all routes work in parallel, e.g. if url = "/users/details/id123" then this DOM will be visible:

Catch, when no other route matches /users/(.*)

to ensure, that urls containing parameters are handled last

which one should be a last? (what if url = "/users/details/details/details")?

I understand, what you like to do, but we do not need a directive modifier for this behaviour.
Its always the same handling structure.

We have two modes for routes: parallel and sequence, and we need to know which  where.
Example:

props

Catch, when no other route matches /users/(.*)

info

For url="/users/root", result DOM: (It's Important case - two blocks are visible.)

url="/users/details/id123", result DOM:

url="/users/props/id123", result DOM:

props

url="/users/info/id123", result DOM:

Catch, when no other route matches /users/(.*)

It's Important case, you have al-route:catch="/users/.*" instead of al-route:catch="/users/info/.*", because of sequence (position in DOM)
What is your Skype? (mine: lega911), it will be easier ;)

which one should be a last? (what if url = "/users/details/details/details")?

/users/details/:id
has precedence before
/users/:id/details
Why? Because static defined routes are more specific than routes containing a variable.
Also the segment length of a given route should be respected:

It's Important case, you have al-route:catch="/users/.*" instead of al-route:catch="/users/info/.*", because of sequence (position in DOM)

Your html:

Catch, when no other route matches /users/(.*)

info

First /users/info/.* would be processed, afterwards /users/.*.
Why? Because /users/info/.* are three  segments ['users', 'info', '.*'] and /users/.* are only two segments ['users', '.*']
We need to sort segments by length and we need to resolve position of all dynamic segments (variables like .* or :id).

What is your Skype? (mine: lega911), it will be easier ;)

Talking wouldn't be that easy for me, because i need some time to think about what the best approach would be. If it is ok for you we can chat, but then we also could use the git chat application that you provided in another issue. :-)

Talking wouldn't be that easy for me

I meant for chatting, it seems no one visits gitter

First /users/info/.* would be processed, afterwards /users/.*.

Ok, I see logic, but using the order of routes is easier, because you don't need to think of "static defined", count of segments etc.
More important - how can you do this:

For url="/users/root", result DOM: (It's Important case - two blocks are visible.)

These two blocks should be visible together for utl "/users/root".
ngRoute has died because it had no this feauter, it had only one ngView

There are problems when adding a Route using JavaScript. So you need to sort the routes before processing them or it wont work.
To your second question: i see no case when two routes should be visible. There is exactly one visible DOM segment.

To your second question: i see no case when two routes should be visible. There is exactly one visible DOM segment.

No, it exactly why ngRouter is dead, there are a lot of cases.
When you change url - top menu should be changed, left menu, body and footer. For this case you need use 4 similar routes, and they should be visible together.

Wouldn't it be more flexible to apply al-if instead of al-route?
E.g.:

But yes, i understand your case.

Wouldn't it be more flexible

It's not easy to implement "confirm on exit" for this.
Also it is fired on every scan loop while al-route is working on url changing only (a better berformance).
I also append al-route-out2 - it is fired on url chaning,
al-route-out is fired when you leave a "group"/route.

Wouldnt it be better to implement custom events?

It's good idea, I'll try it

What about al-route and al-route:catch? Need good names for them.

I'd recommend to prefix framework specific events.
It's easier to understand what the code is about.

What about al-route and al-route:catch? Need good names for them.

The problem is, it should not do a segment less catchall only (e.g. catching / for 404).
It only catches a specific route (e.g. to catch /users without parameters).
I accept al-route:catch.
We also need to discuss al-link vs. using the href attribute.
Would it be possible to enable href attributes using a config flag or by defining an empty al-link attribute?

Would it be possible to enable href attributes using a config flag or by defining an empty al-link attribute?

yes

it should not do a segment less catchall only (e.g. catching / for 404).

Do you mean, al-route:catch="/"  should catch all "/.*"?

Do you mean, al-route:catch="/" should catch all "/.*"?

All, except already defined routes.
We do not need to define variable segments using .*, because it is a "catcher", it has to catch all urls that do not match any defined route.
The good thing is, that we do not need to process .* in any url.
We can always force the user to define a variable name, e.g. "/:id" or "/:anything".

I'd like to push on this, because it will be a great feature.
It is also ok when the first implementation hasn't a final API, because we can introduce this as not stable feature.
The API get's better and better when we can use the unstable API for development and test purposes,
this will show up which other feature are required for productive usage.
Do you need any further information to implement this?
Or do you like to focus on other features?
Then i would implement a quick proposal.

I will append it to package "ext"

Where will it be found? In the regular repository?

In the regular repository?

Yes

Is it ready? I'd like to play with it. :-)

Hello! I was quite busy, published it here: https://github.com/lega911/alight/blob/master/ext/router.es.js
source here: https://github.com/lega911/angular-light/blob/master/src/ext/router.es.js
direct link: https://rawgit.com/lega911/alight/master/ext/router.es.js
example: http://plnkr.co/edit/HpUDXdTJ0QFenprkE5QJ?p=preview

Great to have you back. :-)
Is it contained in the bower package?
I'll give it a try tomorrow :-)

Is it contained in the bower package?

Not yet, but it will be there in next version


Cannot run docker image 0.4.3

Image: epam/ai-dial-chat
Tag: 0.4.3
Dockerhub image of ai-dial-chat execution fails with following log:
[+] Building 0.0s (0/0)                                                                                docker:colima
[+] Building 0.0s (0/0)                                                                                docker:colima

<EMAIL_ADDRESS>start
next start

⚠ Invalid next.config.js options detected:
⚠     The value at .basePath must be a string but it was an object.
⚠ See more info here: https://nextjs.org/docs/messages/invalid-next-config
▲ Next.js 13.5.4

Local:        http://localhost:3000

✓ Ready in 3.4s
npm notice
npm notice New minor version of npm available! 10.2.4 -> 10.4.0
npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.4.0
npm notice Run npm install -g<EMAIL_ADDRESS>to update!
npm notice
npm ERR! path /app
npm ERR! command failed
npm ERR! signal SIGKILL
npm ERR! command sh -c next start
npm ERR! A complete log of this run can be found in: /home/nextjs/.npm/_logs/2024-02-08T14_49_16_970Z-debug-0.log
### Log from dev run:
/app $ npm run start

<EMAIL_ADDRESS>start
next start

⚠ You are using a non-standard "NODE_ENV" value in your environment. This creates inconsistencies in the project and is strongly advised against. Read more: https://nextjs.org/docs/messages/non-standard-node-env
⚠ Invalid next.config.js options detected:
⚠     The value at .basePath must be a string but it was an object.
⚠ See more info here: https://nextjs.org/docs/messages/invalid-next-config
▲ Next.js 13.5.4

Local:        http://localhost:3000

✓ Ready in 3.3s
Killed
/app $
/app $ npm run dev

<EMAIL_ADDRESS>dev
next dev

⚠ Invalid next.config.js options detected:
⚠     The value at .basePath must be a string but it was an object.
⚠ See more info here: https://nextjs.org/docs/messages/invalid-next-config
Error: > Couldn't find any pages or app directory. Please create one under the project root
at findPagesDir (/app/node_modules/next/dist/lib/find-pages-dir.js:42:15)
at initialize (/app/node_modules/next/dist/server/lib/router-server.js:67:69)
at async Server. (/app/node_modules/next/dist/server/lib/start-server.js:219:36)

Hello @AdamKrzysiek
Could you please provide the logs from npm on your first fail?

@mikitabut
0 verbose cli /usr/local/bin/node /usr/local/bin/npm
1 info using<EMAIL_ADDRESS>2 info using<EMAIL_ADDRESS>3 timing npm:load:whichnode Completed in 11ms
4 timing config:load:defaults Completed in 16ms
5 timing config:load:file:/usr/local/lib/node_modules/npm/npmrc Completed in 5ms
6 timing config:load:builtin Completed in 7ms
7 timing config:load:cli Completed in 11ms
8 timing config:load:env Completed in 2ms
9 timing config:load:file:/app/.npmrc Completed in 1ms
10 timing config:load:project Completed in 10ms
11 timing config:load:file:/home/nextjs/.npmrc Completed in 0ms
12 timing config:load:user Completed in 2ms
13 timing config:load:file:/usr/local/etc/npmrc Completed in 0ms
14 timing config:load:global Completed in 2ms
15 timing config:load:setEnvs Completed in 6ms
16 timing config:load Completed in 61ms
17 timing npm:load:configload Completed in 62ms
18 timing config:load:flatten Completed in 14ms
19 timing npm:load:mkdirpcache Completed in 2ms
20 timing npm:load:mkdirplogs Completed in 1ms
21 verbose title npm start
22 verbose argv "start"
23 timing npm:load:setTitle Completed in 4ms
24 timing npm:load:display Completed in 4ms
25 verbose logfile logs-max:10 dir:/home/nextjs/.npm/_logs/2024-02-08T21_40_49_472Z-
26 verbose logfile /home/nextjs/.npm/_logs/2024-02-08T21_40_49_472Z-debug-0.log
27 timing npm:load:logFile Completed in 61ms
28 timing npm:load:timers Completed in 0ms
29 timing npm:load:configScope Completed in 0ms
30 timing npm:load Completed in 264ms
31 silly logfile done cleaning log files
32 verbose stack Error: EACCES: permission denied, mkdir '/home/nextjs/.npm/_cacache'
33 verbose cwd /app
34 verbose Linux 6.1.55-0-virt
35 verbose node v20.11.0
36 verbose npm  v10.2.4
37 error code EACCES
38 error syscall mkdir
39 error path /home/nextjs/.npm/_cacache
40 error errno -13
41 verbose Error: EACCES: permission denied, mkdir '/home/nextjs/.npm/_cacache'
42 error
42 error Your cache folder contains root-owned files, due to a bug in
42 error previous versions of npm which has since been addressed.
42 error
42 error To permanently fix this problem, please run:
42 error   sudo chown -R 1001:65533 "/home/nextjs/.npm"
43 verbose exit -13
44 timing npm Completed in 2472ms
45 verbose unfinished npm timer command:start<PHONE_NUMBER>024
46 verbose unfinished npm timer command:run-script<PHONE_NUMBER>056
47 verbose code -13
48 error A complete log of this run can be found in: /home/nextjs/.npm/_logs/2024-02-08T21_40_49_472Z-debug-0.log


Release and deploy to production

Changes

(PR #68, 2023-03-13) chore: bump mypy from 0.991 to 1.0.1
(PR #71, 2023-03-13) chore: bump actions/cache from 3.2.5 to 3.2.6
(PR #74, 2023-03-14) chore: bump tox from 4.4.5 to 4.4.7
(PR #72, 2023-03-14) chore: bump coverage from 7.1.0 to 7.2.1
(PR #76, 2023-03-29) Decrease Dependabot's open pull request limit to 5
(PR #75, 2023-03-30) Update GitHub Actions Linux runner to Ubuntu 22.04
(PR #85, 2023-04-05) Update code owners for Python dependencies
(PR #82, 2023-04-05) chore(deps): Bump black from 23.1.0 to 23.3.0
(PR #79, 2023-04-14) chore: bump actions/dependency-review-action from 3.0.3 to 3.0.4
(PR #77, 2023-04-14) chore: bump actions/cache from 3.2.6 to 3.3.1
(PR #83, 2023-04-17) chore: bump mypy from 1.0.1 to 1.1.1
(PR #57, 2023-04-26) chore(deps): Bump ipython from 7.34.0 to 8.10.0
(PR #87, 2023-05-05) chore: bump actions/checkout from 3.3.0 to 3.5.2
(PR #94, 2023-05-15) chore: Bump coverage from 7.2.1 to 7.2.5
(PR #93, 2023-05-16) chore: Bump tox from 4.4.7 to 4.5.1
(PR #98, 2023-05-26) Add Codecov to CI workflow

@jtrobles-cdd which version should be set for this release?

@jtrobles-cdd which version should be set for this release?

Let's increase the minor version (1.3.0 --> 1.4.0).


Add functionality to support keys outside of directional arrows and enter button.

The main use case for this functionality is the ability to map a key like the escape button the functionality of a home or back button on a TV remote.

is this working?

is this working?

It works for me.
When the focus is on the search bar, pressing the space key or esc key (called out in my commit) will console log a supported key was pressed.
because the feature code is not merged into dead/react-key-navigation repo npm install on the example writes the original repo files into node_modules. If you pull from my repository build the react-key-navigation code and replace examples/youtube-react-tv/node_modules/react-key-navigation/build/ folder with react-key-navigation/build and run the example you will see the console log as stated above.
Steps to get the custom key example running
clone master
cd to react-key-navigation
npm install
npm run-script build
cd examples/youtube-react-tv/
npm install
replace "examples/youtube-react-tv/node_modules/react-key-navigation/build/" folder with the one you just built "react-key-navigation/build"
npm run start-dev
open browser to view example, when the focus is on the search bar at the top, press space or esc and you will see the console log, (spacebar press does same action as enter, esc only has a console log)

It works for me.
When the focus is on the search bar, pressing the space key or esc key (called out in my commit) will console log a supported key was pressed.
because the feature code is not merged into dead/react-key-navigation repo npm install on the example writes the original repo files into node_modules. If you pull from my repository build the react-key-navigation code and replace examples/youtube-react-tv/node_modules/react-key-navigation/build/ folder with react-key-navigation/build and run the example you will see the console log as stated above.
Steps to get the custom key example running
clone master
cd to react-key-navigation
npm install
npm run-script build
cd examples/youtube-react-tv/
npm install
replace "examples/youtube-react-tv/node_modules/react-key-navigation/build/" folder with the one you just built "react-key-navigation/build"
npm run start-dev
open browser to view example, when the focus is on the search bar at the top, press space or esc and you will see the console log, (spacebar press does same action as enter, esc only has a console log)


Contribution guide: Add coding standards, e2e test and improve code comments

Hey guys, I've improved the contribution guide with the latest changes:

Added a section about the end-to-end tests that points to https://github.com/frontity/frontity/blob/dev/e2e/README.md.
Improved the "Code comments" section with our latest decisions about using TSDoc.
Added a "Coding Standards" section with the first suggestions to maintain the CS of Frontity.

I'm sure we will improve this much more with time, but at least we have something more or less updated to the current way we work now.

@luisherranz I have added some changes to the  README.md with some of the clarifications we've commented in our conversation this morning

Thanks @juanmaguitar!


add a final user documentation for the website builder program

put it in ./site-builder folder

Look the doc in site_builder folder.
done 20230702


Decrease position swap utils

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

Feature
Testing

What is the new behavior?

implemented decrease_position_swap_utils.cairo file

Does this introduce a breaking change?

No

Dont forget to add : "test_decrease_position_swap_utils" in lib.cairo


Homework 1-4 resubmission from new branch, removed execute(qprog)

PR Description

Some notes

[ ] Please make sure that you placed the files in an appropriate folder

[ ] And that the files have indicative names.

[ ] Please note that Classiq runs automatic code linting, which may minorly alter some files.

[ ] If you're familiar with pre-commit, you may run pre-commit install, and then at each commit, your files will be altered in a similar way

Already merged, will be visible after next version


[Feature Request]: Building for ARM64 CPUs

Is there an existing issue for the same feature request?

[X] I have checked the existing issues.

Is your feature request related to a problem?
I'm using M2 MacBook with Docker Desktop.
But I cannot run Infinity in a docker instance because I cannot pull x86_64 images.
It would be better for providing ARM64 built images.

I'm aware that Rosetta would be a solution but I don't have Rosetta on my Mac.

On the other hand.
There are servers running with ARM CPUs I'd like to deploy Infinity to.

Describe the feature you'd like
Publish docker images for ARM64.
Describe implementation you've considered
No response
Documentation, adoption, use case
No response
Additional information
I tried to build from source on an arm64v8/ubuntu:23.10 image but got many errors like:

simde missing. (fixed with apt-get install libsimde-dev)
failed with -msse4.2 and -mfma

Fix by #2082

Well done.
Thanks a lot.


Audit logging

I am making really good progress with light4j. Thank you for putting the good work.
I have a need for storing all CRUD transactions per user in a database for audit purposes to know who and what changed at any give point of time. What is the best way to accomplish this? Is it better to write a custom audit handler or any recommended way to address this?

@raviada I am glad you figure it out on your own. It is a pretty complicated system as it is designed for enterprises. To audit CRUD in a database, you can create your AuditHandler and replace the default one in the request/response chain. One of the banks is doing that. There are a lot of tricky performance tuning considerations I remember. @ddobrin is the best guy to ask specific questions as he did it. I would be so happy if you could open source your handler once it is done.

@raviada - due to some other commitments, I could not follow a number of projects. If this is still an issue you are interested in addressing, just drop a comment here. thank you


Support WiFi SSID containing URLs

It would be useful to use SSID of a WiFi AP to be in a form of URL, or in a form containing a URL. In that way, shops or restaurants can turn their existing APs easily into beacons.
It will be more approachable to people, since they don’t need to have Bluetooth activated, the only thing needed is just a passive WiFi scanning.
A little drawback may be that this approach wouldn’t probably work on iOS.

Supporting wifi is indeed a very interesting idea and we're looking at it. As you say, the biggest downside is that it doesn't work on iOS which is really a drag as we're trying to do something that works for everyone.
The only other issue is that there would be no encoding possibilities so the URLs would need to be fully enumerated so you'd need "https://www." in each one which burns up a bit of space (and we're encouraging https for URLs if at all possible)

For iOS, there is at least a possibility to get the SSID for the network the device is connected to: http://stackoverflow.com/questions/5198716/iphone-get-ssid-without-private-library
Well, the "www." part could be omitted, if you use URL shortener for encoding the original URL.
I created a sample project to demonstrate "WiFi beacon" capabilities. APs can be used entirely in a Bluetooth beacon fashion. You can check it out at: https://github.com/jonasevcik/Android-WiFiBeacon-demo

Very cool! Thanks for sharing. I agree with you that the length issues are fairly trivial. I hadn't thought that you could use signal strength as a rough relative comparison tool.
As to the iOS issue of getting the network you are connected to, I agree of course, that would easily work but it feels very limited. For any public use case, it's not as useful as you wouldn't be connected to anything first.
If we proceed with this we should be clear: we'd be doing this only for Android as that's the only version where it actually works properly. We can make a hack for iOS but it would be a pale shadow of the primary implementation.

Also, I think that choosing the AP we want to connect to on iOS via an app is an other issue.

@dinhviethoa sadly, iOS is overrestrictive in this matter.
@scottjenson even only on Android, it can give an added value to its users. If it makes sense, I can prepare a new branch with its implementation.

How about supporting the recently announced Wi-Fi Aware for beaconing?
They claim it will work on iOS as well
http://www.wi-fi.org/beacon/rolf-de-vegt/wi-fi-aware-a-platform-for-proximity-based-wi-fi-innovation

Any standards based tech that we can use is alway interesting. My concern with Wi-Fi aware is that, at least according to this (http://p2pkit.io/blog.html) it requires a chipset upgrade and changes to the OS. So it's still aways out.
One of the reasons we picked BLE and mDNS (for our initial tech to use) was that they were open, and well supported. We're happy to explore new approaches/technologies but, like any project we need to stay focused. Both wifi aware, and even the original wifi scanning that started this issue, are very much worth looking at, but they aren't as widespread as BLE/mDNS.

@jonasevcik Thanks so for raising this issue. It has spurred quite a discussion within the team. Given that we already support mDNS, we already have a strong solution that works over wifi. It has the added benefit of carrying not only the URL, but TITLE and DESCRIPTION information as well (well... not just yet, we're adding it soon). This is especially important for devices broadcasting a local URL. None of that is really possible with an SSID solution.
An additional concern is that the SSID solution pretty much kills a wifi device from using the wifi radio for outbound data (it can only pretend to be an AP so can only accept connections). This SSID idea is very clever and it allows you to setup wifi beacons easily but with mDNS you can broadcast AND use your wifi to connect the device.
We've discussed this internally quite a bit and we were very impressed that not only did you post the idea but you also did a pull request! Very much appreciated but we're going to keep wifi to mDNS for now.
Scott

@scottjenson I was afraid you would write that:) I guess, we can close this issue now.
Let me know, if I can be of any (hopefully relevant) assistance.
Jonas


impl(bigquery): Applies SafeGetTo function Job response parsing

This PR applies the SafeGetTo() from PR-12199 to Job resource response parsing.

This change is

FYI: There is one additional commit after I sent it for review. In the latest commit I moved the SafeValueOr method to json_utils.h as it will be used by other files as well.
See feedback: https://reviewable.io/reviews/googleapis/google-cloud-cpp/12208#gh-1273040771


add test to confirm that hostname validation is working

we have tests to prove that the TLS hostname verification fails or is ignored, but no positive test to prove that it actually works.

+1


Getting Warning: ViewPagerAndroid has been extracted from react-native core

Which OS ?
Mac Os
Version
Which versions are you using:
react :  16.8.3
react-native :  0.59.1
react-native-swiper : ^1.5.14
Expected behaviour
Actual behaviour
Getting below warning -
Warning: ViewPagerAndroid has been extracted from react-native core and will be removed in a future release. It can now be installed and imported from '@react-native-community/viewpager' instead of 'react-native'. See https://github.com/react-native-community/react-native-viewpager
Steps to reproduce
Run the application using react-native run-android

https://github.com/leecade/react-native-swiper/issues/713

Can confirm having the same issue on clean and fresh project.

我也出现了同样的问题

So, what can we do now?

@afilp
Of course, you have chosen:

https://github.com/leecade/react-native-swiper/issues/713#issuecomment-476687466
Use another more contributable project
Don't use this module and upstar it

我也出现了同样的问题

怎么解决的

If you don't want the warning in your debugger's console log.

Install https://github.com/react-native-community/react-native-viewpager with yarn as mentionned in the Readme
link library as mentionned in the doc
go to .\node_modules\react-native-swiper\src\index.js and change the ViewPagerAndroid component with "ViewPager" (don't forget to import it with : import ViewPager from "@react-native-community/viewpager";)
4 It works for me.
Hope it helps.

Will this problem be corrected in the next release?

no, not a single hint that this module will be developed in the near future

I have replaced this with a stable and good slider - https://github.com/Jacse/react-native-app-intro-slider
Its having similar features

If you don't want the warning in your debugger's console log.

Install https://github.com/react-native-community/react-native-viewpager with yarn as mentionned in the Readme
link library as mentionned in the doc
go to .\node_modules\react-native-swiper\src\index.js and change the ViewPagerAndroid component with "ViewPager" (don't forget to import it with : import ViewPager from "@react-native-community/viewpager";)
4 It works for me.
Hope it helps.

This worked for me. The only downside is that you have redo the process in case of fresh npm install.

Thanks @grean. I hacked together a little postinstall script so this doesn't have to be done after every package install (@Rutheniceye92):
const fs = require('fs');
const path = require('path');

const moduleDir = path.resolve('./node_modules');
const swiperPath = path.join(moduleDir, 'react-native-swiper', 'src', 'index.js');

// Patch react-native-swiper for Android according to https://github.com/leecade/react-native-swiper/issues/951#issuecomment-493684332
if (fs.existsSync(path.join(moduleDir, '@react-native-community', 'viewpager', 'package.json'))) {
  const swiperSource = fs.readFileSync(swiperPath).toString();
  if (!swiperSource.match('@react-native-community/viewpager')) {
    fs.writeFileSync(
      swiperPath,
      swiperSource
        .replace(
          "} from 'react-native'",
          "} from 'react-native'\nimport ViewPager from '@react-native-community/viewpager'",
        )
        .replace('ViewPagerAndroid,\n  ', '')
        .replace(/ViewPagerAndroid/g, 'ViewPager'),
    );
    console.log('react-native-swiper patched using @react-native-community/viewpager');
  } else console.log('react-native-swiper already contains @react-native-community/viewpager');
} else console.log('Please install @react-native-community/viewpager first');

Simply put this in a new JavaScript file (if you don't already have a postinstall script), and add this using node ./postinstall.js to the package.json in the scripts.postinstall section.

Thanks a lot @mfkrause ... your postinstall script was tremendously helpful.

RN V- >60
React-native-swiper 1.5.14
GO TO inside node module  React-native-swiper
open src => index.js
then find ViewPagerAndroid and replace it by ScrollView {from 'react-native}
it will be work... good luck


Get RTTStats from session ?

Is it possible to get congestion.RTTStats from session ? Now it's non-exportable field !
It's very useful )). For example, in my case i want to close connection with high rtt and use only low rtt

It's not possible, and we're not planning to provide an API for that. However, we might this kind of information as port of #1258.

It would be cool

The problem here is that any field we make public is part of our external API, which means we can't change it anymore without breaking it for our users. In particular the RTT / congestion related code is bound to change in the future, so it's currently not a good candidate for the public API.

Ok, thx for explain. Fork will help me ;)


add-optional-id-to-props-iframe

it's is for to do some actions on iframe identified by id.
eg: with jquery
$('iframe#id').load(function() {
callback(this);
});

:+1:


J2oVisitor throws inconsistent exceptions

end() throws IllegalStateException although its header declares ParseException.  The other methods like objectItemValue and arrayItemValue throw ParseException. objectBegin throw both exceptions:
public void objectBegin() throws ParseException {
...
try {
mValue = mType.newInstance();
} catch (Exception e) {
throw new IllegalStateException(e.getMessage(), e);
}
...
ry {
mValue = mType.newInstance();
mWrapper = Wrapper.getWrapper(mType);
} catch (IllegalAccessException | InstantiationException e) {
throw new ParseException(StringUtils.toString(e));
}
}
It is quite confusing.

Would you pls submit a PR to fix?

Here is the pull request:https://github.com/apache/dubbo/pull/12676
As the methods declare that they can throw ParseException, I revised all thrown IllegalStateExceptions to ParseExceptions.

@tohidemyname
Is there any progress?

I am sorry, but I am catching several paper deadlines. I can work on these bugs next month.

This issue should not exist in version 3.x.

I have submitted a pull request to the master branch.
https://github.com/apache/dubbo/pull/12676/commits/13efd64708902bd9ef56071dd8de6ef6cee17187
I also checked 3.2, but this version does not have J2oVisitor.java. This issue does not affect 3.x.


Parametrise lifecycle for aws_autoscaling_schedule

Is your request related to a new offering from AWS?
Is this functionality available in the AWS provider for Terraform? See CHANGELOG.md, too.

Yes ✅: please list the AWS provider version which introduced this functionality ( lifecycles is a built-in Terraform meta-argument)

Is your request related to a problem? Please describe.

When we specify a recurrent schedules for managed node groups, it works as expected. However, as the recurrent schedule updates the start date each time to the next schedule, running Terraform apply next time sees this as a change and tries to revert the next schedule start date to the past date (which is the original start date given the first time). This obviously fails due to a validation error as follows:
Error: creating Auto Scaling Scheduled Action (scale-up): ValidationError: Given start time is in the past
│ 	status code: 400 ...

modules/eks-managed-node-group/main.tf line 455, in resource "aws_autoscaling_schedule" "this":
│  455: resource "aws_autoscaling_schedule" "this"

Therefore, the users have to update the start_time in the Terraform file each time to bypass this issue which is not ideal.
Describe the solution you'd like.

It may help if there is a lifecycle variable to pass through to the aws_autoscaling_schedule resource.
With a lifecycle variable we should be able to ignore the start_date changes as follows:
lifecycle {
        ignore_changes = [start_time]
}

Describe alternatives you've considered.

Creating the aws_autoscaling_schedule without using this terraform-aws-eks module as follows:
resource "aws_autoscaling_schedule" "my_schedule" {
  scheduled_action_name  = "my-schedule-name"
  min_size               = 1
  max_size               = 5
  desired_capacity       = 1
  start_time             = "2023-03-05T00:00:00Z"
  end_time               = "2024-03-05T00:00:00Z"
  time_zone              = "Etc/GMT+0"
  recurrence             = "0 0 * * *"
  autoscaling_group_name = "my-asg-name"
  lifecycle {
    ignore_changes = [start_time]
  }
}

Additional context

This can be reproduced by running an example similar given in this repo and wait the schedule run at least once to update the start date. Then plan and apply again. The plan phase goes ahead with no issue, but when it gets applied the error given above happens.

we are quite cautious and reluctant to ignore arguments because this comes with its own set of challenges -  have you researched if others are encountering this issue, what solutions or workarounds they are using, and any related issues on the AWS provider?

we are quite cautious and reluctant to ignore arguments because this comes with its own set of challenges - have you researched if others are encountering this issue, what solutions or workarounds they are using, and any related issues on the AWS provider?

Thanks for the swift response @bryantbiggs . Indeed, I did a bit of research before opening this issue.
Seeing the scheduled variable is kind of new in this module, I doubt it has mass adoption yet.
However, ignoring an argument is a Terraform feature, I linked this above as well.
This issue is not about asking a new feature from this module per se. We are asking let's not restrict what Terraform provides in this module without a good reason.
If I create a resource outside this module as follows, it works like a charm and as an end user I have the flexibility to control if the "start_time" needs to be ignored or not. Without this exposed in this module as a variable, the users, including myself, will have to do something like below, because it is not really usable in this module for frequent Terraform applies:
    resource "aws_autoscaling_schedule" "my_schedule" {
      scheduled_action_name  = "on_instance"
      min_size                            = 1
      max_size                           = 5
      desired_capacity              = 1
      start_time                          = "2023-03-05T00:00:00Z"
      end_time                            = "2024-03-05T00:00:00Z"
      time_zone                          = "Etc/GMT+0"
      recurrence                         = "0 0 * * *"
      autoscaling_group_name = "my-asg-name"
      lifecycle {
        ignore_changes = [start_time]
      }

There could be some other hacky work-arounds. But what we are asking here is, to not restrict what Terraform already provides. Thanks!

We are asking let's not restrict what Terraform provides in this module without a good reason.

And

If I create a resource outside this module as follows, it works like a charm and as an end user I have the flexibility to control if the "start_time" needs to be ignored or not.

I think this is the crux of the issue - you haven't grasped what you are actually asking for, which is to ignore this value for ALL users, whether they want that or not. Terraform does not support variables within lifecycle blocks so this is an all or nothing type change

Terraform does not support variables within lifecycle blocks so this is an all or nothing type change

That is unfortunately true in general, I am aware of the years old issues around this, i.e. 24188 and 3116.
However, those are the implementation details and should not limit our imaginations to abstract this feature within this module. E.g. when the lifecycle argument is given, this module could create "aws_autoscaling_schedule" resource with the lifecycle enabled, if that argument is not given, then this module could create another "aws_autoscaling_schedule" with not lifecycleset. That is not ideal - it would create two separate copies of aws_autoscaling_schedule resource. But that is probably better than introducing a non-usable feature into this module and expecting the end users will update the start_date each time as part of their daily PRs.

commenting to remove the stale label.

this is not something that we're going to be able to support here - however, you can pass the ASG ARNs created by this module to the autoscaling schedules of your choosing with the desired configuration

this is not something that we're going to be able to support here - however, you can pass the ASG ARNs created by this module to the autoscaling schedules of your choosing with the desired configuration

Frankly, that is a surprising decision. Sure, we can always create and maintain an autoscaling schedule outside this module. This ask was more about making the autoscaling schedule added into this module usable. Unless you guys are going to disable that broken feature, I cannot wrap my head around this decision why to keep a broken feature in this module at all.


Allow users and/or packages to create custom versions of "Wrap with ..." actions

It doesn't seem like there's an issue specifically for this, but there has been some discussion across various other issues (and PRs like https://github.com/Dart-Code/Dart-Code/pull/4232).
There is a desire to have more options in the "Wrap with..." actions list. Sometimes package-specific widgets, and sometimes widgets that users feel they use a lot. Including all of them in the default list could make it unwieldy (and doesn't handle packages). Perhaps analysis server plugins are the best way to handle this, but they might also be a bit complex for simple uses where a user just wants a simple extra wrap for a widget.

I saw this comment here from the PR mentioned above.
I was thinking if this ever becomes a thing, there could be a way for the user to set which assists they'd like to see. Like a list option in vs code configs or something where they tick whatever they want to show. WDYT?
Not really sure how could we make that reflect the options on the analyzer, but maybe not to select what you want but possibly the other way around. Like what you'd like to hide kind of a thing.

I was thinking if this ever becomes a thing, there could be a way for the user to set which assists they'd like to see.

Do you mean allowing the user to also hide some of the existing values (like "Wrap with Padding"), or also hide other kinds of assists (like "Change to double quotes")?
Assuming the former and that the goal is to reduce the number of entries, something that might not be obvious (and doesn't negate the desire for this, but might help in the meantime), is that you can type to jump around the code actions list. For example here, I hit Ctrl+. and then typed column and it jumped to that action in the list:

I meant only hiding the "Wrap with ..." assists. Great help, will use your tip!
However, your comment got me wondering if the user's created "Wrap with" could be added to non-Widget classes for dart packages that add wrappers to their own or Dart base classes. Or even for List and other base classes like ValueNotifier if these assists can be made by users.

your comment got me wondering if the user's created "Wrap with" could be added to non-Widget classes for dart packages that add wrappers to their own or Dart base classes

I think that would be more difficult, because the current assists are specifically written for Widgets with build, and child/children. For non-widget classes, presumably these patterns might not be the same. However, for your own classes, analysis server plugins might be a way to achieve some of this (although at this point they're still a work in progress).

How about Shopify's approach?
Shopify is building ruby lsp (language server for ruby, and it's mainstream in ecosysyem)
As you see this article, Shopify add capability to extending language server's feature. So called ruby lsp addon
https://railsatscale.com/2024-10-03-the-ruby-lsp-addon-system/

Thanks for the reference.
Ruby's approach is very similar to what we refer to as analysis server plugins. (The analysis server is our LSP support.) The initial prototype of plugins for Dart has been around for several years, and we're in the process of updating and simplifying the whole system with a goal of releasing it more formally.
Plugins are one way to provide some extensibility. They would support the "packages" portion of this request, but wouldn't support the "users" portion. For that I think we'd need a different solution.

@bwilkerson
Okay. (If i guested corrrectly), Do you mean, Even though it's referencing package in local path, they would support package's customized behaviour? not in Project's configuration? Like custom lint ruies?

There are two potential ways to provide this kind of capability.
This first is to allow a package author to specify that some of the widgets in that package should have a "Wrap with" action created for them. Plugins would support this use case.
The second is to allow an individual user to specify that they want a "Wrap with" action for a given widget even though the author of the widget didn't specify this. Plugins are too heavy weight, IMO, to be a good way to support this second use case.


No feedback when failing an attempt to send to a lightning address

Using the web wallet on Chrome on Android, I attempt to send to a lightning address by clicking send then put an address in the form<EMAIL_ADDRESS>When I click Continue, the button briefly turns blue, but nothing happens. No explanation what error is occurring. The lightning address stays in the field, all I can do is go back.

Also tried on desktop Linux Chromium. Same behaviour.

It's most likely that that lightning address is malformed, we should definitely present that error to the user though

when pressing Ctrl+Shift+I or right-click => inspect do you see this error?
Uncaught (in promise) Failed to call on the given LNURL.

A-ha, it is a CORS issue. The lnurlp is static content and no specific Access-Control-Allow-Origin header has been set.
This won't be a problem in an app, but will potentially be a problem in a web browser unless the lnurlp specifications state that such resources should have an Access-Control-Allow-Origin: * header set.
I think either way, the promise rejection should trigger a helpful error message.
Access to fetch at '---redacted---'from origin 'https://app.mutinywallet.com' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

Ah yeah, should reach out to the owner of the domain and ask them to add cors

I have added an accept header. That fixed the problem for that domain.
For apache
a2enmod headers
In apache server config eg virtualhost
<Directory /home/user/httpdocs/.well-known/lnurlp>

Header set Access-Control-Allow-Origin "*"

If that header isn't part of the spec (I suspect it isn't), then a work-around could be implemented. A proxying server to relay the request in the event of CORS failure  would be fairly easy. In any case, a helpful message which at least shows the wallet understands the address but there was an error fetching the lnurl resource would be good.


Weirdness when ONNX optimize/exporting and quantizing Llama2 - fails on both, have tried python and CLI approaches

System Info
latest version optimum / onnx / runtime
python 3.10
running on CPU server Debian 11 with apache, 32G RAM, 60G swapram, 320G HDD, 8 threads

Who can help?
@michaelbenayoun, @JingyaHuang, @echarlaix
Hey peeps, been struggling with Llama2 re optimize quantize, and a couple of weird things happening.
Failure to recognise pth filetype

When running from meta/llama2 repository (meta-llama/Llama-2-7b), onnx doesn't recognise the 13GB consolidated.pth file. It just straight up fails to recognise the type. Paraphrased error message = "Please use pytorch or other recognised format"

Failure to optimise correctly or quantize on HF respository files

When running from meta/llama2 HF respository (meta-llama/Llama-2-7b-hf), instead of resulting in an optimized model.onnx file of 13G (which is the total size of the either the 2 safetensor files, OR the 2 pytorch files), I get two files being a 3Meg model.onnx, and a 26G model.onnx_data file. It looks like the optimizer is combining the safetensor and pytorch files
Instead of resulting in the expected model.onnx, it has this onnx_data file

When quantizing

It could be an offshoot of the above issue from optimization, but when I try to quantize the optimised result, I get:
--->
Creating dynamic quantizer: QOperator (mode: IntegerOps, schema: u8/s8, channel-wise: False)
Quantizing model...
WARNING:root:Failed to infer data type of tensor: /model/layers.0/self_attn/Concat_4. Please add data type info for this tensor if your model has customized operators.
Traceback (most recent call last):
File "/usr/local/bin/optimum-cli", line 8, in
sys.exit(main())
File "/usr/local/lib/python3.10/site-packages/optimum/commands/optimum_cli.py", line 163, in main
service.run()
File "/usr/local/lib/python3.10/site-packages/optimum/commands/onnxruntime/quantize.py", line 102, in run
q.quantize(save_dir=save_dir, quantization_config=qconfig)
File "/usr/local/lib/python3.10/site-packages/optimum/onnxruntime/quantization.py", line 416, in quantize
quantizer.quantize_model()
File "/usr/local/lib/python3.10/site-packages/onnxruntime/quantization/onnx_quantizer.py", line 306, in quantize_model
node = self.quantize_node_with_sub_graph(node)  # noqa: PLW2901
File "/usr/local/lib/python3.10/site-packages/onnxruntime/quantization/onnx_quantizer.py", line 217, in quantize_node_with_sub_graph
kv = {attr.name: self.quantize_subgraph(attr.g, f"{node_name}:{attr.name}")}
File "/usr/local/lib/python3.10/site-packages/onnxruntime/quantization/onnx_quantizer.py", line 182, in quantize_subgraph
sub_quantizer = ONNXQuantizer(
File "/usr/local/lib/python3.10/site-packages/onnxruntime/quantization/onnx_quantizer.py", line 115, in init
self.activation_qType = activation_qType.tensor_type
AttributeError: 'int' object has no attribute 'tensor_type'

Information

[X] The official example scripts
[ ] My own modified scripts

Tasks

[ ] An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
[ ] My own task or dataset (give details below)

Reproduction (minimal, reproducible, runnable)
$   nohup sudo optimum-cli export onnx --model /home/admin/LLAMA2HF/Llama-2-7b-hf --task text-generation --optimize O3 /home/admin/LLAMA2HF/optimized
$  nohup sudo optimum-cli onnxruntime quantize --onnx_model /home/admin/LLAMA2HF/optimized --arm64 -o /home/admin/LLAMA2HF/quantized && touch finishedQuant.txt
Expected behavior
Was hoping to crunch the 13G Llama into a 5G optimized model.onnx, then quantize it and get a 1G model.onnx
That would be sweet. :)
Wishful thinking I know.. but at any rate, the optimization and quantization don't seem to be working at all.
I'm going to try another approach, downloading everything to local, deleting / hiding the safetensor files, and running again with only the py.bin files in the folder..
I'll post my results.. to see if this is a viable workaround.

Hi @gidzr thank you for the report! This a bug in ONNX Runtime 1.16.0 https://github.com/microsoft/onnxruntime/pull/17651  (& llama ONNX export that adds subgraphs https://github.com/huggingface/transformers/pull/26162 https://github.com/pytorch/pytorch/issues/109292). The easiest (temporary) solution would be to downgrade to ONNX Runtime 1.15.
By the way, I don't think you will be able to shrink https://huggingface.co/meta-llama/Llama-2-7b/tree/main (25 GB fp32) to 1 GB using ONNX Runtime. ORT would allow you to shrink it down to ~6-7 GB with int8 quantization, but you would have to be careful about the prediction quality of the quantized model.

@fxmarty awesome, thanks for the steer on the downgrade... and absolutely re 6-7GB .. the 1GB target is nice dreamz :P lolz

Hi @fxmarty , just a quick addition tying into a related issue, Llama needs 1.15 to be quantized, but using the Xenova convert script which I need to use for the per_chanel reduce_range flags, has to be 1.16 for inferencing (https://github.com/huggingface/optimum/issues/1385). I think that means having one version for quanting and one version for infer'ing. If the results differ I'll come back and update.

cc @xenova Have you seen the same issue https://github.com/microsoft/onnxruntime/pull/17651 for models with subgraphs?

cc @xenova Have you seen the same issue microsoft/onnxruntime#17651 for models with subgraphs?

I have not... 👀 Unless this was introduced in onnxruntime > 1.14 (as Transformers.js is still using v1.14).

Yes the issue was introduced in 1.16.


[Question] Debian Support

Describe the problem
有计划提供对 Debian （12 bookworm） 的支持吗？提供 Debian DEB 软件包。

@Vonng We will look into this recently.

@Vonng supported by #549, will publish new package at next release.

/close

impressive 👍


Deployment

Deployment
User Story
As a boot camp student,
I want the prework notes to be structured on a webpage,
So that I can easily find and read the information
Acceptance Criteria
GIVEN a Prework Study Guide website,
WHEN I visit the website using the URL,
THEN I can access my website from any browser

Created sharable page using git hub


File permission update

Details
On dev my permission of the file was like so,
 chirag  ~/Expensidev/App/scripts   main <B>  ls -l
total 32
-rwxr-xr-x  1 chirag  staff  714 Apr 20 18:34 build-desktop.sh
-rwxr-xr-x  1 chirag  staff  287 Apr 11 07:27 react-native-web.sh
-rw-r--r--  1 chirag  staff  901 May  3 10:56 set-pusher-suffix.sh
-rw-r--r--  1 chirag  staff  486 Apr 11 07:27 shellUtils.sh

So when i ran npm i it would fail with this error. Updated the file permission in this PR to 777.
Fixed Issues
https://expensify.slack.com/archives/C3BGN04K1/p1651604264005359
Tests

On dev in App cd into /scripts. Run ls -l and confirm you see,

-rwxrwxrwx  1 chirag  staff  901 May  3 10:56 set-pusher-suffix.sh

QA Steps
None

[ ] Verify that no errors appear in the JS console

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Thanks @chiragsalian - looks like @kidroca did the same fix here: https://github.com/Expensify/App/pull/8862


Review My Project

Hi My Dear:
Please Review My Project

Hi @SaboorSohaib 👋
Congratulations on your project! Your code looks clean and follows best practices.
Highlights
✔No Linter errors.
✔Descriptive Readme file.
✔proper use of HTML semantics in the index.html.
✔You followed DRY, KISS, and YAGNI principles correctly.
Things to look out for:♻

In your addAndRemove.js file you should consider consistently declaring functions in the ES6 format
https://github.com/SaboorSohaib/to-do-list/blob/a1a3228e5f0eb4cae1f022826fe76daa57abcbb1/src/module/addAndRemove.js#L98
In your CSS file, I suggest you avoid repeating yourself and keep your code DRY by using classes for similar styling.

Happy Coding 💕


Restore new versions of Django and Wagtail Flags

Now that Django-Flags 4.1.2 is released and fixes the critical bug that broke our database flags, this change restores the latest versions.
This reverts the reversion in #5100.
Checklist

[x] PR has an informative and human-readable title
[x] Changes are limited to a single goal (no scope creep)
[x] Code can be automatically merged (no conflicts)
[ ] Code follows the standards laid out in the CFPB development guidelines
[ ] Passes all existing automated tests
[ ] Any change in functionality is tested
[ ] New functions are documented (with a description, list of inputs, and expected output)
[ ] Placeholder code is flagged / future todos are captured in comments
[ ] Visually tested in supported browsers and devices (see checklist below :point_down:)
[ ] Project documentation has been updated
[x] Reviewers requested with the Reviewers tool :arrow_right:

Thanks @chosak! I agree about "optional," I'll make that change in the next beta of wagtail-flags.


[webapi] Remove audio and video block tests

These tests is bloked because of some events failed to invoke, and these check points are not good for w3c, so remove thest tests.
Related PR:https://github.com/crosswalk-project/crosswalk-test-suite/pull/1993

Impacted TCs num(approved): New 0, Update 0, Delete 7
Unit test Platform: Crosswalk Project for Android 15.43.347
Unit test result summary: Pass 0, Fail 0, Blocked 0
BUG=https://crosswalk-project.org/jira/browse/XWALK-3646

You are trying to remove all tests against wav PCM audio clip. Does it mean the platform doesn't support that audio format? If yes, that is strange.

The issue has been discussed in #1993. This PR is just integrate https://github.com/crosswalk-project/crosswalk-test-suite/pull/1993 to Crosswalk-13 branch.
If change the execution type to manual, these cases can be pass.

The issue has been discussed in #1993. This PR is just integrate https://github.com/crosswalk-project/crosswalk-test-suite/pull/1993 to Crosswalk-13 branch.
If change the execution type to manual, these cases can be pass.

The issue has been discussed in #1993. This PR is just integrate https://github.com/crosswalk-project/crosswalk-test-suite/pull/1993 to Crosswalk-13 branch.
If change the execution type to manual, these cases can be pass.

The issue has been discussed in #1993. This PR is just integrate https://github.com/crosswalk-project/crosswalk-test-suite/pull/1993 to Crosswalk-13 branch.
If change the execution type to manual, these cases can be pass.


use svg for edit icon button instead of material-ui icon

Fixes #10583

@oliviertassinari Why is it done differently for Edit icon only?
In the commit, I made the Edit icon implementation consistent with Add and Delete icon.

Why is it done differently for Edit icon only?

@chiragmongia It's here to make sure both version are working. Let's not hide the problem.

yea, the inconsistency was intentional
and also I'm a fan of the <Icon>edit</Icon> way  :)

@caub Thanks for taking the time to open the bug report! So, what about adding overflow: hidden on the .MuiIcon-root classname?

@oliviertassinari yes it works https://codesandbox.io/s/5yq0wk7j3p, can it be added directly on Button component though?

can it be added directly on the core Button component though?

@caub I don't think so, it's a bold move.

ok it fixes it on Icon: https://github.com/mui-org/material-ui/pull/10594


Security filter abstraction for turbine server

Added TurbineSecurityFilter abstraction
Provides a default unsecured implementation
Fixed netty version
Fixed broken TurbineStreamTests test

Unpolished PR. Consider current state for review only

I'm sure this is obsolete now that this has moved to webflux. Feel free to open a new PR.


The ringpop.Interface was not in sync with the implementation.

@dansimau I updated the interface to have the same signature as Ringpop, added a test to make sure you can assign a Ringpop instance to its interface. And as a bonus, updated the examples with the swim.Bootstrap change.

Sweet, good catch.


Prerequisites / cross platform compatibility

Hey,
when trying to install bau (or rather scriptcs) on an Linux or OSX machine you'll most likely run into some compatibility issues because some system packages are simply missing.
Let's say I'm executing bau.sh on my OSX 10.10 machine it'll error out because:

wget is not pre-installed on OSX
unzip is not pre-installed on OSX

While it's rather safe to assume that wget is installed on Linux it is much safer to use curl to download the scriptcs NuGet package.
So line 17 of bau.sh should look like this:
curl -O "https://github.com/bau-build/bau-blob/raw/master/scriptcs-0.10.0-alpha.140530.nupkg"

This is one problem fixed. The next problem is a bit more tricky as unzip isn't pre-installed on any popular distribution as far as I'm aware of. One simply can't execute apt-get install unzip as that doesn't work on OSX. On OSX you'd need to use homebrew (http://brew.sh/):
Now, I'm wondering what you wanna do:

Add documentations so people need to manually install it themselves before using any library like bau that requires on scriptcs?
Create an prerequisites.sh (or anything like that) that checks whether everything is installed and optionally installs it for you?

Cheers

What is the status of this? I am trying to get started on a Mac and I'm not sure how or if I can do a build.

For reference I was able to complete a successful build by changing the current wget line in bau.sh to
curl -L -o ScriptCs.0.13.3.nupkg "https://chocolatey.org/api/v2/package/ScriptCs/0.13.3/"

Apparently I already had unzip installed.

Now that scriptcs has a reasonable installation path on both Mac and OSX, I think we can just get rid of this scripcs.exe bootstrapping altogether from bau.sh and change the build instructions to state that scriptcs should be installed as a pre-requisite.
scriptcs can be installed on Mac via homebrew or on both Mac or Linux via svm (https://github.com/scriptcs-contrib/svm/wiki/Installing-svm).

@cH40z-Lord @eatdrinksleepcode please see the latest build instructions https://github.com/bau-build/bau/blob/dev/how_to_build.md

@adamralph looks good! Thanks for taking care of it.


Fixed closing of already closed http_listener.

I observed a crash of the nmos-cpp library when the nmos::server is closed and the http_listeners vector contains already closed listeners.
http_listener::close() returns a default created pplx::task from cpprest (2.10.17). This leads to a crash in pplx::when_all() under Linux CentOS8.1. I did not investigate the reason for this crash further, however, default created pplx::task behave different. For example they throw on wait or get calls.
To solve this issue I propose the following fix which only adds non default created tasks to the vector.
The provided fix is tested in my application where the problem occurs when a nmos::server is closed that comprises invalid http_listeners due to wrong socket configurations, like for example an already existing port number.

Hm, that doesn't sound good.
Can you explain how http_listener::close returns a default-constructed task? I expect it to return e.g. pplx::task_from_result() (a completed task) sometimes but not a default-constructed one.
But I do agree when_all does not seem to play nicely with default-constructed tasks. I see several accesses in the implementation of when_all which do not seem to account for default constructed tasks:
https://github.com/microsoft/cpprestsdk/blob/07cf589108f50ee40e56c56dc43bfab1022604e0/Release/include/pplx/pplxtasks.h#L6591
What we probably expected to happen is this... pplx::when_all to return a task that would throw the same pplx::invalid_operation exception as the default constructed task.
This would be handled by nmos::server::close by completing other operations and then returning that task. In this way, nmos::server::close would pass on the exception from the underlying http_listeners. This seems more appropriate than effectively hiding the problem? (In the example nmos-cpp-node main.cpp the contained exception would be thrown by the node_server_guard destructor, and handled by the catch-all block. It'd be nice if there was a specific catch (pplx::invalid_operation& e) block.)
We could submit a test case and patch to cpprestsdk but given it's in maintenance mode, I agree it might be simpler to handle the issue in this repo, though there are several instances of when_all and when_any to which any fix ought to be implied.
But first, I'd prefer to understand the first point, how http_listener::close returns a default-constructed task.

I found the implementation of close() here.
The function open returns m_close_task which is defined as  pplx::task_from_result() in the default constructor http_listener_impl::http_listener_impl(). But the other two constructors,
etails::http_listener_impl::http_listener_impl(http::uri address) and details::http_listener_impl::http_listener_impl(http::uri address, http_listener_config config) do not define m_close_task as pplx::task_from_result(). Thus, it is default when using one of the other constructors, which is done in the nmos-cpp code.
So basically you are right, this bug should better be fixed in cpprest. But I'm not sure, how easy it is to provide a patch for cpprest.

I quickly added microsoft/cpprestsdk#1700 and microsoft/cpprestsdk#1701. Patches for both issues should be possible. However, to avoid having to wait for that to happen, maybe we could replace instances of pplx::when_all(tasks.begin(), tasks.end()) in nmos-cpp with a helper that does something like:
pplx::ranges::when_all(tasks | boost::adaptors::transformed([](pplx::task<X> task)
{
    try { task.wait(); return task; }
    catch (const pplx::invalid_operation& e) { return pplx::task_from_exception<X>(e); }
    catch (...) { return task; }
})

(where pplx::ranges::when_all would be just a wrapper function, taking a range argument,  for the pplx::when_any function that  takes an iterator pair)

Thanks for adding the issues at cpprest.
I'm not an cpprest expert. Could you please explain if you like to write a new function pplx::range::when_all() that is used instead of pplx::when_all provided by the cpprest library or does the pplx::range::when_all() already exist in cpprest?
I am also a bit confused by the pplx::when_any in your last sentence. Did you mean pply::when_all() or did I miss something about  pplx::when_any()?
Nevertheless, I think it is a very good idea to fix the when_all() call instead of sorting out the default tasks as proposed by me.

Yes, sorry, typo fixed!
Those pplx::ranges functions don't exist now. They could be submitted to cpprestsdk, but same is true of e.g. pplx::complete_after found in nmos-cpp pplx/pplx_utils.{h,cpp}, and I propose these new functions could be implemented there too.

Okay thanks, now it is more clear to me.
Basically, we need to write a new when_all() function like this:
template<class RandomAccessRange>
pplx::task<void> when_all( const RandomAccessRange& rng )
{
    return pplx::when_all( rng.begin(), rng.end() ).then( [rng]( pplx::task<void> finally )
    {
        for( auto task : rng ) details::wait_nothrow( task );
        finally.wait();
    } );
}

This function is then called by your code and should then only handle valid tasks, right?
Just one question to your code. Are you sure that a default task throws if task.is_done() has been called?

Just one question to your code. Are you sure that a default task throws if task.is_done() has been called?

https://github.com/microsoft/cpprestsdk/blob/07cf589108f50ee40e56c56dc43bfab1022604e0/Release/include/pplx/pplxtasks.h#L3642-L3647
If we don't want to rely on that, we could do as I originally wrote:
    try { task.wait(); return task; }
    catch (const pplx::invalid_operation& e) { return pplx::task_from_exception<X>(e); }
    catch (...) { return task; }

The possible issue that made me rewrite there is that the tasks which throw other exceptions have then been "observed", which could mask a programming error in the caller.

Basically, we need to write a new when_all() function like this:
[...]
This function is then called by your code and should then only handle valid tasks, right?

Yes, so I think the following may be sufficiently composable, leaving the job of wait_nothrow for all tasks, etc. to the caller.
template <typename InputRange>
pplx::task<void> when_all(InputRange&& tasks)
{
    return pplx::when_all(tasks.begin(), tasks.end());
}

Okay I agree that is_done should work as well and is the better choice as the wait shall be done in the original when_all.
However, I'd think that we still need the wait_nothrow if we keep the is_done. Because if I've got it correctly, the original when_all() stops on exceptions so that it won't wait for all tasks after the exception, which is desired in our case right?
https://github.com/microsoft/cpprestsdk/blob/07cf589108f50ee40e56c56dc43bfab1022604e0/Release/include/pplx/pplxtasks.h#L6863-L6867

Yes, wait_nothrow is still required, I just meant that it should remain in the calling code's continuation not the implementation of pplx::ranges::when_all.

Okay, that's fine. I'm currently testing the code changes with a small unit test that adds some http_listeners into the nmos::server and directly closes them. However, currently the original when_all still crashes with an unhandled exception.
I'm not absolutely sure but I think that the created task from pplx::task_from_exception<X>(e) might throw again in when_all(). Is this really what we want or shall we better generate a task from pplx::task_from_result()?

Sorry, I'm away from compiler right now. In my tests, pplx::task_from_exception<X>(e) was handled as I'd expect by pplx::when_all, with no crash. Wonder why our test cases are different?🤔
In calling code, we definitely still need the for loop over all the tasks in the continuation to ensure all exception-containing tasks are observed.
In total I was expecting something like this to work...
return pplx::ranges::when_all(tasks | boost::adaptors::transformed([](pplx::task<X> task)
{
    try { task.is_done(); return task; }
    catch (const pplx::invalid_operation& e) { return pplx::task_from_exception<X>(e); }
}).then([tasks](pplx::task<void> finally)
{
   for (auto& task : tasks) details::wait_nothrow(task);
   finally.wait();
});

Maybe you're right and the filter should be factored into pplx::ranges::when_all itself... and maybe we need a version of pplx::observe_exception that takes a range of tasks and does the no-throw wait, so that we could simplify calling code to...
return pplx::ranges::when_all(tasks)
    .then(pplx::observe_exception<void>(tasks));

I've implemented exactly the same code but it crashes with an unhandled exception. I did my tests under Windows using Visual Studio 2017 and ccprest 2.10.18.
It does not crash when I use pplx::task_from_result() instead of pplx::task_from_exception<void>(e). But I agree that it should  also work the other way around as the documentation of when_all() states that it could handle exceptions within the tasks.
The crash happens in the destructor of the task in~_ExceptionHolder.
 ~_ExceptionHolder()
  {
      if (_M_exceptionObserved == 0)
      {
          // If you are trapped here, it means an exception thrown in task chain didn't get handled.
          // Please add task-based continuation to handle all exceptions coming from tasks.
          // this->_M_stackTrace keeps the creation callstack of the task generates this exception.
          _REPORT_PPLTASK_UNOBSERVED_EXCEPTION();
      }
  }

Here is the call stack stored in this->_M_stackTrace:
0x00007ff786172896 {nmos-cpp-test.exe!Concurrency::task_from_exception<void,Concurrency::invalid_operation>(Concurrency::invalid_operation _Exception, const Concurrency::task_options & _TaskOptions), Line 7081}
0x00007ff786ae7340 {nmos-cpp-test.exe!`Concurrency::task<void> <lambda>(Concurrency::task<void>)'::`1'::catch$4(void), Line 103}
vcruntime140d.dll!0x00007ffbf21a1030 (load symbols for additional information)
vcruntime140d.dll!0x00007ffbf21a74da (load symbols for additional information)
ntdll.dll!0x00007ffc219d1426 (load symbols for additional information)
0x00007ff7860d2b42 {nmos-cpp-test.exe!nmos::server::close_listeners::__l2::Concurrency::task<void> <lambda>(void)::__l2::<lambda>(Concurrency::task<void> task), Line 102}
0x00007ff786035204 {nmos-cpp-test.exe!boost::range_detail::default_constructible_unary_fn_wrapper<Concurrency::task<void> <lambda>(Concurrency::task<void>),Concurrency::task<void> >::operator()<Concurrency::task<void> >(Concurrency::task<void> & arg), Line 62}
0x00007ff786072713 {nmos-cpp-test.exe!boost::iterators::transform_iterator<boost::range_detail::default_constructible_unary_fn_wrapper<Concurrency::task<void> <lambda>(Concurrency::task<void>),Concurrency::task<void> >,std::_Vector_iterator<std::_Vector_val<std::_Simple_types<Concurrency::task<void> > > >,boost::use_default,boost::use_default>::dereference(void), Line 126}
0x00007ff78604d5ba {nmos-cpp-test.exe!boost::iterators::iterator_core_access::dereference<boost::iterators::transform_iterator<boost::range_detail::default_constructible_unary_fn_wrapper<Concurrency::task<void> <lambda>(Concurrency::task<void>),Concurrency::task<void> >,std::_Vector_iterator<std::_Vector_val<std::_Simple_types<Concurrency::task<void> > > >,boost::use_default,boost::use_default> >(const boost::iterators::transform_iterator<boost::range_detail::default_constructible_unary_fn_wrapper<Concurrency::task<void> <lambda>(Concurrency::task<void>),Concurrency::task<void> >,std::_Vector_iterator<std::_Vector_val<std::_Simple_types<Concurrency::task<void> > > >,boost::use_default,boost::use_default> & f), Line 550}
0x00007ff78649df62 {nmos-cpp-test.exe!boost::iterators::detail::iterator_facade_base<boost::iterators::transform_iterator<boost::range_detail::default_constructible_unary_fn_wrapper<Concurrency::task<void> <lambda>(Concurrency::task<void>),Concurrency::task<void> >,std::_Vector_iterator<std::_Vector_val<std::_Simple_types<Concurrency::task<void> > > >,boost::use_default,boost::use_default>,Concurrency::task<void>,boost::iterators::random_access_traversal_tag,Concurrency::task<void>,__int64,0,0>::operator*(void), Line 656}

Thus, the problem seems to be that when_all() is not able to handle task_from_exception tasks.
I'd propose to implement it that way:
return pplx::range::when_all( tasks | boost::adaptors::transformed( []( pplx::task<void> task )
{
    try { task.is_done(); return task; }
    catch( const pplx::invalid_operation& e ) { return pplx::task_from_result(); }
} ) ).then( [tasks]( pplx::task<void> finally )
{
    for( auto task : tasks ) details::wait_nothrow( task );
    finally.wait();
} );

Because I would assume that the task returned from http_listener::close() should be of type pplx::task_from_result() when the listener is already closed, which would already be the case for default constructed http_listener instances.

@garethsb I looked into your code which is fine for me so far. I think, it is a good idea to have our own implementation for when_all and when_any.
However, there is one thing I'm not really sure about. Maybe the patched code is not asynchronous anymore as the wait is already called in the transform adaptor.
pplx::ranges::details::when_all(tasks | boost::adaptors::transformed(pplx::details::workaround_default_task<ReturnType>()));
I'd expect that the when_all does not wait at all anymore. For closing the listeners this might be acceptable but in other cases  we may want run other code in parallel while when_all is waiting for the tasks to be finished.

Okay sure! Sorry, now I've got it.
We check for an empty task and call wait only on empty ones. This will throw, which creates a new task of type pplx::task_from_exception<ReturnType>(e). Then we wait for the new task and handle its exception because this cannot be done by when_all.
Finally the struct observe_exceptions is replacing the code
.then( [tasks]( pplx::task<void> finally )
{
    for( auto task : tasks ) details::wait_nothrow( task );
    finally.wait();
} );

Thanks a lot! I learned a lot about cpprest and boost by this. I think we can close this pull request. Okay?

Thanks very much for working this through with me. I've made PR #260 from the branch, so if that works for you, hopefully @lo-simon will review and merge, and this one can be closed.


build: use gts 1.x on master

This temorary fix pins us to the master branch of gts, which switches from clang to prettier.

Node 6 is spitting out this error
npm ERR! enoent ENOENT: no such file or directory, chmod '/tmpfs/src/github/nodejs-spanner/node_modules/gts/build/src/cli.js'
``

Yeah, this isn't going to work until node 6 goes away.