Handle upgrading from location when in use to location always on iOS

Summary

Closes https://github.com/zoontek/react-native-permissions/issues/490
Fixes an issue on iOS where you could not ask for LOCATION_ALWAYS permission if you already had LOCATION_WHEN_IN_USE permission.
This only impacts the LocationAlways pod.

Implementation Changes

For check:

If the current authorization status is AuthorizedWhenInUse, and we have not requested LOCATION_ALWAYS in the past, we return NotDetermined.
If the current authorization status is AuthorizedWhenInUse and we have requested LOCATION_ALWAYS in the past, we return Denied, since we're not allowed to prompt for this permission twice.

For request:

If the current authorization status is AuthorizedWhenInUse and we have not requested LOCATION_ALWAYS in the past, we call requestAlwaysAuthorization and flag the permission as requested.
If the current authorization status is AuthorizedWhenInUse and we have requested LOCATION_ALWAYS in the past, then we return the current authorization result, since we're not allowed to prompt for this permission twice.

Test Plan

Request and grant LOCATION_WHEN_IN_USE permission.
Then request LOCATION_ALWAYS permission.

Before
After

Compatibility

OS
Implemented

iOS
✅

Android
N/A

Checklist

[X] I have tested this on a device and a simulator
[X] I added the documentation in README.md
[X] I added a sample use of the API in the example project (example/App.tsx)

It looks like the promise resolve to early:
https://user-images.githubusercontent.com/1902323/191220250-7a79fc18-9071-47a2-9e79-e4595931c421.MP4
https://user-images.githubusercontent.com/1902323/191220759-a91890ec-4fd6-4a89-bc0d-9339e45d418e.MP4

@zoontek that led me down a rabbit hole -- and the solution was a bit unorthodox but from my testing this seems to cover every situation!

Unfortunately, it still fails in some cases. Allowing once, then requesting always doesn't resolve the first time:
https://user-images.githubusercontent.com/1902323/194712865-be084088-866a-47ef-b209-14c206a1c383.MP4

@zoontek if you're talking about requesting LOCATION_ALWAYS, selecting "Allow Once", and then requesting again -- that appears to be the intended behaviour.
Here's what I see:

On requesting LOCATION_ALWAYS, and selecting "Allow Once" -- LOCATION_WHEN_IN_USE is granted until the app is closed and opened again, at which point any location permission is removed, but is requestable again:

https://user-images.githubusercontent.com/15325890/194762856-3dc50e03-3ec8-44b1-b489-a2514c8b3b06.mp4

On requesting LOCATION_ALWAYS and selecting "Allow When In Use" -- the app receives provisional always authorization (described here: https://developer.apple.com/videos/play/wwdc2019/705/?time=195):

https://user-images.githubusercontent.com/15325890/194763064-d666e72c-02c6-43e4-a93a-bff0413e8b3e.mp4

On requesting LOCATION_WHEN_IN_USE, granting it, and then requesting an LOCATION_ALWAYS -- the app correctly prompts the user to upgrade the when-in-use permission to always:

https://user-images.githubusercontent.com/15325890/194763170-350bd4d7-82c1-40f1-a685-ecf90e832df8.mp4
In all cases, LOCATION_ALWAYS can only be requested once as described here: https://developer.apple.com/documentation/bundleresources/information_property_list/protected_resources/choosing_the_location_services_authorization_to_request?language=objc

I'm also not seeing the case where allowing once, then requesting always does not resolve.
Here it is on the simulator:
https://user-images.githubusercontent.com/15325890/194763450-85806259-7b1d-495e-a9dd-48ce56332a81.mp4
And on a real device:
https://user-images.githubusercontent.com/15325890/194763731-9d7506db-5a3f-4f2b-9be8-dabf880e8381.MP4

I am new to using the library, but if it helps at all, I was able to test this successfully on an Iphone SE 2020 running iOS 15.6 and on simulator running iOS 16.  So far it has resolved on the first time as expected, I'll be testing more today and tomorrow though.  React native version 0.67.2

I was able to get this to work on iOS 15 (simulator) and 16 (real device). However, for iOS 14.5 and 12.4 on the simulator, the always prompt briefly appears and then disappears without user action.
https://user-images.githubusercontent.com/1326873/199366773-bcc6e40b-315a-40a0-9fb2-dff2f1df699b.mp4

Just a message to say that we are experiencing the same issue.
I also join @tallpants opinion that iOS allows us asking for LOCATION_ALWAYS only once, so asking twice for the permission will not work, and that's the expected behaviour. (from user experience point of view, this can be enhanced by checking the status and explaining why it will not prompt for the permission.)
There is another way to resolve the issue, by calling the permission request on the main thread, inside a block
dispatch_async(dispatch_get_main_queue(), ^{
    // code to request the permission here
});

@zoontek What do you think of @ag-drivequant solution?

@ag-drivequant @ddarren @tallpants @zoontek to fix the issue with iOS < 15 you just need to modify/add this code:
CLLocationManager *locationManager;

...

locationManager = [CLLocationManager new];
[locationManager requestAlwaysAuthorization];
[RNPermissions flagAsRequested:[[self class] handlerUniqueId]];

The fix is explained here https://stackoverflow.com/a/9474095
Any chance to merge this PR?

any updates about merging this PR?

can we get this one merged? I tried a patch by its failing :/

@woodybury The current state of this feature doesn't work correctly, so I can't merge it (never resolving Promise)
But good news, I can work on it (see https://github.com/zoontek/react-native-permissions/pull/808)! If your company really needs it, contact me 🙂

thanks @zoontek got it to compile and "work" (meaning I can request user to change to always) with this simple patch:
diff --git a/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m b/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
index 719e6be..241134d 100644
--- a/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
+++ b/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
@@ -48,9 +48,10 @@ - (void)requestWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
     if (![CLLocationManager locationServicesEnabled]) {
       return resolve(RNPermissionStatusNotAvailable);
     }
-    if ([CLLocationManager authorizationStatus] != kCLAuthorizationStatusNotDetermined) {
-      return [self checkWithResolver:resolve rejecter:reject];
-    }
+// Removing the authorizationStatus check here
+//     if ([CLLocationManager authorizationStatus] != kCLAuthorizationStatusNotDetermined) {
+//       return [self checkWithResolver:resolve rejecter:reject];
+//     }

     self->_resolve = resolve;
     self->_reject = reject;

but obviously I need the checks to work too :/
definitely +1 for this feature request. If I have the capacity to fix the checks I'll open a PR. Keep me posted - thanks!

@zoontek @woodybury I have created a patch also to handle this case:
`diff --git a/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m b/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
index e20d4fe..56a986e 100644
--- a/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
+++ b/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
@@ -3,9 +3,8 @@
@import CoreLocation;
@import UIKit;
-@interface RNPermissionHandlerLocationAlways()
+@interface RNPermissionHandlerLocationAlways()
-@property (nonatomic, strong) CLLocationManager *locationManager;
@property (nonatomic, strong) void (^resolve)(RNPermissionStatus status);
@property (nonatomic, strong) void (^reject)(NSError *error);
@@ -13,6 +12,9 @@ @interface RNPermissionHandlerLocationAlways()
@implementation RNPermissionHandlerLocationAlways
+static NSString* SETTING_KEY = @"@RNPermissions:Requested";
+CLLocationManager *locationManager;
+

(NSArray<NSString *> * _Nonnull)usageDescriptionKeys {
return @[@"NSLocationAlwaysAndWhenInUseUsageDescription"];
}
@@ -28,7 +30,13 @@ - (void)checkWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
return resolve(RNPermissionStatusNotDetermined);
case kCLAuthorizationStatusRestricted:
return resolve(RNPermissionStatusRestricted);

case kCLAuthorizationStatusAuthorizedWhenInUse:

 case kCLAuthorizationStatusAuthorizedWhenInUse: {

     BOOL requestedBefore = [self isFlaggedAsRequested:[[self class] handlerUniqueId]];

     if (requestedBefore) {

         return resolve(RNPermissionStatusDenied);

     }

     return resolve(RNPermissionStatusNotDetermined);

 }

case kCLAuthorizationStatusDenied:
return resolve(RNPermissionStatusDenied);
case kCLAuthorizationStatusAuthorizedAlways:
@@ -38,22 +46,67 @@ - (void)checkWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve

(void)requestWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
rejecter:(void (^ _Nonnull)(NSError * _Nonnull))reject {
if ([CLLocationManager authorizationStatus] != kCLAuthorizationStatusNotDetermined) {
return [self checkWithResolver:resolve rejecter:reject];
}

CLAuthorizationStatus authorizationStatus = [CLLocationManager authorizationStatus];
BOOL requestedBefore = [self isFlaggedAsRequested:[[self class] handlerUniqueId]];
if (authorizationStatus != kCLAuthorizationStatusNotDetermined && !(authorizationStatus == kCLAuthorizationStatusAuthorizedWhenInUse && !requestedBefore)) {

   return [self checkWithResolver:resolve rejecter:reject];

}

_resolve = resolve;
_reject = reject;

_resolve = resolve;
_reject = reject;

// When we request location always permission, if the user selects "Keep Only While Using", iOS
// won't trigger the locationManager:didChangeAuthorizationStatus: delegate method. This means we
// can't know when the user has responded to the permission prompt directly.
//
// We can get around this by listening for the UIApplicationDidBecomeActiveNotification event which posts
// when the application regains focus from the permission prompt. When this happens we'll
// trigger the applicationDidBecomeActive method on this class, and we'll check the authorization status and
// resolve the promise there -- letting us stay consistent with our promise-based API.
//
// References:
// ===========
// CLLocationManager requestAlwaysAuthorization:
// https://developer.apple.com/documentation/corelocation/cllocationmanager/1620551-requestalwaysauthorization?language=objc
//
// NSNotificationCenter addObserver:
// https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415360-addobserver
//
// UIApplicationDidBecomeActiveNotification:
// https://developer.apple.com/documentation/uikit/uiapplicationdidbecomeactivenotification
[[NSNotificationCenter defaultCenter] addObserver:self

                                        selector:@selector(applicationDidBecomeActive)

                                            name:UIApplicationDidBecomeActiveNotification

                                          object:nil];

_locationManager = [CLLocationManager new];
[_locationManager setDelegate:self];
[_locationManager requestAlwaysAuthorization];

locationManager = [CLLocationManager new];
[locationManager requestAlwaysAuthorization];
[self flagAsRequested:[[self class] handlerUniqueId]];
}

-- (void)locationManager:(CLLocationManager *)manager didChangeAuthorizationStatus:(CLAuthorizationStatus)status {

if (status != kCLAuthorizationStatusNotDetermined) {
[_locationManager setDelegate:nil];
[self checkWithResolver:_resolve rejecter:_reject];
+- (void)applicationDidBecomeActive {

[self checkWithResolver:_resolve rejecter:_reject];
[[NSNotificationCenter defaultCenter] removeObserver:self

                                             name:UIApplicationDidBecomeActiveNotification

                                           object:nil];}

+- (bool)isFlaggedAsRequested:(NSString * _Nonnull)handlerId {

NSArray<NSString *> *requested = [[NSUserDefaults standardUserDefaults] arrayForKey:SETTING_KEY];
return requested == nil ? false : [requested containsObject:handlerId];
+}

+- (void)flagAsRequested:(NSString * _Nonnull)handlerId {

NSUserDefaults *userDefaults = [NSUserDefaults standardUserDefaults];
NSMutableArray *requested = [[userDefaults arrayForKey:SETTING_KEY] mutableCopy];

if (requested == nil) {
requested = [NSMutableArray new];
}

if (![requested containsObject:handlerId]) {
[requested addObject:handlerId];
[userDefaults setObject:requested forKey:SETTING_KEY];
[userDefaults synchronize];
}
}

`

@zoontek, First off thank you for your hard work on this project. After reading through the discussion, it seems like this PR would not be an acceptable approach according to your criteria because of the observer's possible never ending promise. I have done surface level testing with @tallpants/@alessioemireni's patch and it works!
Seems like you are expecting a different approach than to listen to the notification change. If so, should we close this PR? Or are you willing to consider this approach. From my bit of testing, the observer seems work well because the native popup can't be dismissed unless the phone is locked which seems to set it "Keep While Using app", so I don't see a case where the promise would be never ending (I could be completely wrong, but just trying to learn more native code). @tallpants @alessioemireni please chime in. I would love your feedback and thoughts. Thank you for your contributions.

So after my testing of @alessioemireni's code. I had some great results! However, I did find one condition where if the user had selected "Allow once" then "Allow always" was requested the notification would not be posted and the promise would hang. No worries, I have a solution. I created a listener to track if the notification is posted (via UIApplicationWillResignActiveNotification). If after 0.25 seconds the notification has not been posted, then checkWithResolver is called and the appropriate response is resolved.
Would love to get some feedback.
@zoontek @tallpants @alessioemireni
diff --git a/node_modules/react-native-permissions/ios/.DS_Store b/node_modules/react-native-permissions/ios/.DS_Store
new file mode 100644
index 0000000..85e3a0c
Binary files /dev/null and b/node_modules/react-native-permissions/ios/.DS_Store differ
diff --git a/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m b/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
index e20d4fe..f706695 100644
--- a/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
+++ b/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
@@ -3,9 +3,11 @@
 @import CoreLocation;
 @import UIKit;

-@interface RNPermissionHandlerLocationAlways() <CLLocationManagerDelegate>
+@interface RNPermissionHandlerLocationAlways()
+{
+  BOOL notified;
+}

-@property (nonatomic, strong) CLLocationManager *locationManager;
 @property (nonatomic, strong) void (^resolve)(RNPermissionStatus status);
 @property (nonatomic, strong) void (^reject)(NSError *error);

@@ -13,6 +15,9 @@ @interface RNPermissionHandlerLocationAlways() <CLLocationManagerDelegate>

 @implementation RNPermissionHandlerLocationAlways

+static NSString* SETTING_KEY = @"@RNPermissions:Requested";
+CLLocationManager *locationManager;
+
 + (NSArray<NSString *> * _Nonnull)usageDescriptionKeys {
   return @[@"NSLocationAlwaysAndWhenInUseUsageDescription"];
 }
@@ -28,7 +33,13 @@ - (void)checkWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
       return resolve(RNPermissionStatusNotDetermined);
     case kCLAuthorizationStatusRestricted:
       return resolve(RNPermissionStatusRestricted);
-    case kCLAuthorizationStatusAuthorizedWhenInUse:
+      case kCLAuthorizationStatusAuthorizedWhenInUse: {
+          BOOL requestedBefore = [self isFlaggedAsRequested:[[self class] handlerUniqueId]];
+          if (requestedBefore) {
+              return resolve(RNPermissionStatusDenied);
+          }
+          return resolve(RNPermissionStatusNotDetermined);
+      }
     case kCLAuthorizationStatusDenied:
       return resolve(RNPermissionStatusDenied);
     case kCLAuthorizationStatusAuthorizedAlways:
@@ -38,21 +49,92 @@ - (void)checkWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve

 - (void)requestWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
                    rejecter:(void (^ _Nonnull)(NSError * _Nonnull))reject {
-  if ([CLLocationManager authorizationStatus] != kCLAuthorizationStatusNotDetermined) {
-    return [self checkWithResolver:resolve rejecter:reject];
-  }
+    CLAuthorizationStatus authorizationStatus = [CLLocationManager authorizationStatus];
+    BOOL requestedBefore = [self isFlaggedAsRequested:[[self class] handlerUniqueId]];
+    if (authorizationStatus != kCLAuthorizationStatusNotDetermined && !(authorizationStatus == kCLAuthorizationStatusAuthorizedWhenInUse && !requestedBefore)) {
+        return [self checkWithResolver:resolve rejecter:reject];
+    }
+
+    _resolve = resolve;
+    _reject = reject;

-  _resolve = resolve;
-  _reject = reject;
+    // When we request location always permission, if the user selects "Keep Only While Using", iOS
+    // won't trigger the locationManager:didChangeAuthorizationStatus: delegate method. This means we
+    // can't know when the user has responded to the permission prompt directly.
+    //
+    // We can get around this by listening for the UIApplicationDidBecomeActiveNotification event which posts
+    // when the application regains focus from the permission prompt. When this happens we'll
+    // trigger the applicationDidBecomeActive method on this class, and we'll check the authorization status and
+    // resolve the promise there -- letting us stay consistent with our promise-based API.
+    //
+    // In addition, we'll also set a timeout of 0.25 seconds to resolve the promise if the notification fails to occur.
+    // We check by listening to UIApplicationWillResignActiveNotification and setting a flag if the notification occurs.
+    // This is to handle the case where the user has selected "Allow once" and cannot be prompted to "Allow Always"
+    // which results in no notification being posted.
+    //
+    // References:
+    // ===========
+    // CLLocationManager requestAlwaysAuthorization:
+    // https://developer.apple.com/documentation/corelocation/cllocationmanager/1620551-requestalwaysauthorization?language=objc
+    //
+    // NSNotificationCenter addObserver:
+    // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415360-addobserver
+    //
+    // UIApplicationDidBecomeActiveNotification:
+    // https://developer.apple.com/documentation/uikit/uiapplicationdidbecomeactivenotification
+    //
+    // UIApplicationWillResignActiveNotification:
+    // https://developer.apple.com/documentation/uikit/uiapplicationwillresignactivenotification
+    [[NSNotificationCenter defaultCenter] addObserver:self
+                                             selector:@selector(notificationOccurred:)
+                                                 name:UIApplicationWillResignActiveNotification
+                                               object:nil];
+    [[NSNotificationCenter defaultCenter] addObserver:self
+                                             selector:@selector(applicationDidBecomeActive)
+                                                 name:UIApplicationDidBecomeActiveNotification
+                                               object:nil];
+    [self performSelector:@selector(onRequestTimeout) withObject:nil afterDelay:0.25];
+
+    locationManager = [CLLocationManager new];
+    [locationManager requestAlwaysAuthorization];
+    [self flagAsRequested:[[self class] handlerUniqueId]];
+}
+
+- (void)notificationOccurred:(NSNotification *)notification {
+  notified = YES;
+}

-  _locationManager = [CLLocationManager new];
-  [_locationManager setDelegate:self];
-  [_locationManager requestAlwaysAuthorization];
+- (void)applicationDidBecomeActive {
+  [self checkWithResolver:_resolve rejecter:_reject];
+  [[NSNotificationCenter defaultCenter] removeObserver:self
+                                                  name:UIApplicationDidBecomeActiveNotification
+                                                object:nil];}
+
+- (bool)isFlaggedAsRequested:(NSString * _Nonnull)handlerId {
+  NSArray<NSString *> *requested = [[NSUserDefaults standardUserDefaults] arrayForKey:SETTING_KEY];
+  return requested == nil ? false : [requested containsObject:handlerId];
+}
+
+- (void)flagAsRequested:(NSString * _Nonnull)handlerId {
+  NSUserDefaults *userDefaults = [NSUserDefaults standardUserDefaults];
+  NSMutableArray *requested = [[userDefaults arrayForKey:SETTING_KEY] mutableCopy];
+
+  if (requested == nil) {
+    requested = [NSMutableArray new];
+  }
+
+  if (![requested containsObject:handlerId]) {
+    [requested addObject:handlerId];
+    [userDefaults setObject:requested forKey:SETTING_KEY];
+    [userDefaults synchronize];
+  }
 }

-- (void)locationManager:(CLLocationManager *)manager didChangeAuthorizationStatus:(CLAuthorizationStatus)status {
-  if (status != kCLAuthorizationStatusNotDetermined) {
-    [_locationManager setDelegate:nil];
+- (void)onRequestTimeout {
+  if (!notified) {
+    [[NSNotificationCenter defaultCenter] removeObserver:self
+                                                    name:UIApplicationDidBecomeActiveNotification
+                                                  object:nil];
     [self checkWithResolver:_resolve rejecter:_reject];
   }
 }

@alexkev This might be a good solution 👍
I'm currently working on next major version (as iOS 18 and Android 15 are around the corner, I doubt they will not add breaking changes to permissions again 😅), if it works correctly I will include it in the beta.

The feature landed in 5.0.0-beta.3, please try it (I personally checked all flows on my both iPhones - iOS 18 and iOS 15).
The idea is kinda similar to solutions proposed here, but without relying on storage (flagging permission as requested in [NSUserDefaults standardUserDefaults]). Check the code
It also comes with an improved documentation about the different flows


Error when attempting to use

Hi, awesome plugin, however it's recently stopped working...
Getting this error:
error: Autoprefixer
Error:
/Users/admin/Library/Application Support/Sublime Text 3/Packages/Autoprefixer/node_modules/postcss/lib/lazy-result.js:152
this.processing = new Promise(function (resolve, reject) {
^
ReferenceError: Promise is not defined
at LazyResult.async (/Users/admin/Library/Application Support/Sublime Text 3/Packages/Autoprefixer/node_modules/postcss/lib/lazy-result.js:152:31)
at LazyResult.then (/Users/admin/Library/Application Support/Sublime Text 3/Packages/Autoprefixer/node_modules/postcss/lib/lazy-result.js:75:21)
at /Users/admin/Library/Application Support/Sublime Text 3/Packages/Autoprefixer/autoprefixer.js:13:3
at Socket. (/Users/admin/Library/Application Support/Sublime Text 3/Packages/Autoprefixer/node_modules/get-stdin/index.js:23:3)
at Socket.emit (events.js:117:20)
at _stream_readable.js:943:16
at process._tickCallback (node.js:419:13)
Pops up and it's in console.
Sublime Text 3, Mac OS Yosemite.

Turns out I just needed to update Node...


Fix missing quote in code block

Q
A

Bug fix?
no

New feature?
no

Tickets
none

License
MIT

Just a missing quote in a code block

Good catch, thanks @allejo.


Doesn't load template page in IE11

I'm submitting a bug report

Library Version:
2.0.0-rc.8

Please tell us about your environment:

Operating System:
Windows 10

Node Version:
8.12.0

NPM Version:
6.4.1

JSPM OR Webpack AND Version
webpack 4.41.1

Browser:
IE 11

Language:
ESNext

Current behavior:
Just a white blank page when I open the template app in IE11.
Steps to reproduce:
√ Please enter a name for your new project: · aurelia-app
√ Would you like to use the default setup or customize your choices? · Custom App
√ Which bundler would you like to use? · Webpack
√ Which HTTP Protocol do you wish the outputted Webpack bundle to be optimised for? · HTTP/2
√ What platform are you targeting? · Web
√ What transpiler would you like to use? · Babel
√ How would you like to setup your HTML template? · None
√ What css preprocessor would you like to use? · None
√ Do you want to add PostCSS processing · None
√ Which unit test runner would you like to use? · Jest
√ Would you like to configure integration testing? · None
√ What is your default code editor? · None
√ Which features do you want scaffolded into your project? · Navigation App
√ Would you like to add a Dockerfile? · No
Then install npm dependencies
npm install aurelia-dialog --save
Then add aurelia.use.plugin(PLATFORM.moduleName('aurelia-dialog')); to main.js
Open internet explorer 11, type localhost:8080 and observe you're not seeing the template.
Works well on chrome, firefox, edge.

What is the expected behavior?
Be able to see the template, load the welcome page in IE11

Note: Version 1.1.0 works.

I'm going to assume this is similar to my issue.
More specifically this is 'Promise' is undefined. Might I suggest changing the title?
I have a similar setup but I'm using
Library Version:  2.0.0

Here is a converstaion on Discourse
It seems as if Webpack Runtime gets to a point where the promise-polyfill seem to be no longer present. The irony though is that when the error is thrown it is caught by code in the promise-polyfill. At first I thought this was due to loading async chunks in WebPack. Once I got WebPack to bundle all of aurlie-dialog into a single chunk there was still a Promise undefined error just in a different place.
I have tried everything to get the promise-polyfill loaded before the run time. Move it to entry, create vendor entry, have it as a plugin and an entry, put import 'promise-polyfill' everywhere.
But the question is why at times the Promise is defined and other times it is not.  But still puzzled as to why I would need to do to get promise-polyfill loaded first if the stack trace returns back to promise-polyfill. If it returns back to there then it had to be loaded before the run time in order to start from there. You then think Promise would be defined. IDK grasping for straws.
A solution is to put the following line in the index.ejs but that seems to be against the grain.
<script<EMAIL_ADDRESS>In aurelia-dialog this is where it starts
dialog/src/ dialog-configuration.ts
  private _apply(): Promise<void> {
    const renderer = this.renderer;
    const cssText = this.cssText;
    return Promise
      .all([
---->        typeof renderer === 'string' ? RENDERRERS[renderer]() : renderer,
        cssText
          ? typeof cssText === 'string'
            ? cssText
            : cssText()
          : ''
.
.
.

Which then calls ux: () => from the RENDERRERS.
const RENDERRERS: Record<string, () => Promise<RendererStatic>> = {
  ux: () => import('./renderers/ux-dialog-renderer').then(m => m.DialogRenderer) as Promise<RendererStatic>,
.
.

The above is compiled to this
ux: function () { return __webpack_require__.e(/*! import() */ "vendors.async~3e799143").then(__webpack_require__.bind(null, /*! ./ux-dialog-renderer.js */ "yfWE")).then(function (m) { return m.DialogRenderer; }); },

Which then calls the WebPack run time to resolve vendors.async~3e799143
__webpack_require__.e = function requireEnsure(chunkId) {
/******/ 		var promises = [];
/******/
/******/
/******/ 		// JSONP chunk loading for javascript
/******/
/******/ 		var installedChunkData = installedChunks[chunkId];
/******/ 		if(installedChunkData !== 0) { // 0 means "already installed".
/******/
/******/ 			// a Promise means "currently loading".
/******/ 			if(installedChunkData) {
/******/ 				promises.push(installedChunkData[2]);
/******/ 			} else {
/******/ 				// setup Promise in chunk cache
/******/ 				var promise = new Promise(function(resolve, reject) {
/******/ 					installedChunkData = installedChunks[chunkId] = [resolve, reject];
/******/ 				});
/******/ 				promises.push(installedChunkData[2] = promise);

It dies at // setup Promise in chunk cache when creating a new Promise
var promise = new Promise(function(resolve, reject) ....
But then you continue and you end up in the promise-polyfill try … catch
   try {
      ret = cb(self._value);
    } catch (e) {
      reject(deferred.promise, e);
      return

Thanks to @3cp we have a solution!
To me this is a documentation bug. Some where it needs to be noted that these changes need to be made in order for aurelia-dialog 2.0 to work with IE11.
To fix this issue you need to add the following alias under the Resolve section of webpack.config.js
'aurelia-dialog': path.resolve(__dirname, 'node_modules/aurelia-dialog/dist/umb/dialog-aurelia.js')
Here is an example of what the Resolve section should look like after a clean setup.
  resolve: {
    extensions: ['.ts', '.js'],
    modules: [srcDir, 'node_modules'],
    // Enforce single aurelia-binding, to avoid v1/v2 duplication due to
    // out-of-date dependencies on 3rd party aurelia plugins
    alias: { 'aurelia-binding': path.resolve(__dirname, 'node_modules/aurelia-binding') },
    alias: { 'aurelia-dialog': path.resolve(__dirname, 'node_modules/aurelia-dialog/dist/umb/dialog-aurelia.js') }
  },

Your two aliases should be just one alias with two keys. I am surprised nodejs didn't complain about the duplicated object key (alias) in your webpack config.

Good catch something like this
alias: {
  'aurelia-binding': path.resolve(__dirname, 'node_modules/aurelia-binding'),
  'aurelia-dialog': path.resolve(__dirname, 'node_modules/aurelia-dialog/dist/umb/dialog-aurelia.js')
}

aurelia-dialog v1 uses aurelia-loader (the abstration layer normalized module loaders) to load dynamic resources.
v2 switched to native dynamic import() statement to simplify the code. This is indeed a breaking change for IE11.
However the dialog UMD build did normalize import() to plain promise. By default, it should work on IE11.
But our aurelia-webpack-plugin tries to load all aurelia-* packages native-modules build in order to take advantage of some webpack features. For example, webpack can tree-shake esm format, but cannot tree-shake js code in other format. So it did not pick the good old aurelia-dialog UMD build.
In summary, IE should die :-)

Please review the above cli PR (will be released in couple of days).
This one can be closed.

I just did a review and found that the alias is not needed. Going back to what we did earlier when we put the promise-polyfill in index.ejs with a <script> tag it worked.
I'm going to assume that the changes to the promise-polyfill in webpack.config.js resulted in something similar to putting the polyfill into index.ejs.
I did a check by keeping the comments on the alias and it works. I also see in the IE11 debugger the native modules being loaded.
My only other suggestion is to keep the messaging consistent. One section it is commented out stating to Uncomment if using IE11 and the other section is not commented stating if you don't use IE11 to comment. Not that it matters because the alias is not needed I would suggest to just comment out the promise polyfill and have the message Uncomment if using IE11.
If this works with the promise-polyfill moved in the webpack.config.js then the alias and my PR for document changes are not needed.

Yes, it worked without the alias, but I saw many strange WDS error in IE11 console. That could be due to the webpack transpiled import() is not compatible with polyfilled promise. Please double check that.

Either way it will work. I have been working with my app with just the promise-polyfill change without any issues. I think the [WDS] Disconnect! error is a webpack-dev-server issue.  The only time I have seen them in IE11 is when I stop the webpack-dev-server but leave the browser open. Then some times when I start WDS up the error continues to show in IE11. Furthermore I don't see them when I run the app using IIS Express or Kestral (My app is .Net) obviously because I'm not using webpack-dev-server at that time. I still stick with the idea that the alias is not needed and documentation does not need to change, just the way promise-polyfill is loaded.

Thanks for the inside. I thought those WDS errors was caused by import() chunks.
Let's keep the alias in CLI, it's by default behind comment (doesn't hurt much), so that users can still have something to try if they have other problems in IE11. This is just to be cautious, the import() might cause user some other issue if they adjust code split settings in webpack (but I am not a webpack expert to be certain).

is not needed I would suggest to just comment out the promise polyfill and have the message Uncomment if using IE11.

I will do that, that sounds a good default.

It was funny because I was thinking "What is this WDS". When I started to type Webpack-Dev-Server the light bulb went on. That and doing searching for WDS all results pointed to Webpack which I found odd at first until I realized what WDS is.
Always good to be cautions especially when it comes to dated browsers.


"/v1/chat/completions" tokenization issue

Context
The "/v1/chat/completions" endpoint uses the apply_chat_template method of the HF tokenizers. It seems to us that these templates take care of adding special tokens (cf. this line from Llama's default template). However, tokenization in vLLM also seems to add special token(s) if this is the tokenizer's default behavior - in particular, the Llama tokenizer adds a BOS token at the start of its tokenization.
There are therefore configurations in which the final tokenization will contain more special tokens than necessary.
Repro
In a terminal, launch a vLLM server. For example:
python -m vllm.entrypoints.openai.api_server --model TheBloke/Llama-2-7B-Chat-AWQ

In another terminal, request this server:
from openai import OpenAI

# Set OpenAI's API key and API base to use vLLM's API server.
openai_api_key = "None"
openai_api_base = f"http://{FILL_ME}/v1"

client = OpenAI(
    api_key=openai_api_key,
    base_url=openai_api_base,
)

chat_response = client.chat.completions.create(
    model="TheBloke/Llama-2-7B-Chat-AWQ",
    messages=[
        {"role": "user", "content": "Tell me a joke."},
        {
            "role": "assistant",
            "content": " Ah, a moment of levity you seek! Very well. Pray, allow me to regale you with this humorous anecdote:\n\nWhy don't historians play cricket?\n\nBecause they prefer to leave their past in the archives!\n\nAnd now, if you'll excuse me, I must return to my scholarly pursuits. Although, I must admit, it is rather refreshing to engage in such frivolous banter from time to time.",
        },
        {"role": "user", "content": "Another one."},
    ],
)
print("Chat response:", chat_response)

Output:
async_llm_engine.py:379]
Received request cmpl-cca85113d5af4178b3c93fb2c2b72578:
prompt: "<s>[INST] Tell me a joke. [/INST] Ah, a moment of levity you seek! Very well. Pray, allow me to regale you with this humorous anecdote:\n\nWhy don't historians play cricket?\n\nBecause they prefer to leave their past in the archives!\n\nAnd now, if you'll excuse me, I must return to my scholarly pursuits. Although, I must admit, it is rather refreshing to engage in such frivolous banter from time to time. </s><s>[INST] Another one.

We can see that the prompt token ids start with two 1s instead of one.
This issue also impacts the new mistralai/Mixtral-8x7B-Instruct-v0.1 model added in the PR https://github.com/vllm-project/vllm/pull/2011

I did document this issue with the Mistral template in one of the PRs: https://github.com/vllm-project/vllm/pull/1493#issuecomment-1809779694

The issue seems to be that add_special_token=False is not passed below, since add_special_tokens=True is the default.
https://github.com/vllm-project/vllm/blob/f375ec844036104cbae36477ce3541588d026829/vllm/engine/llm_engine.py#L280
However, like mentioned above, the apply_chat_template call below already inserts those tokens:
https://github.com/vllm-project/vllm/blob/f375ec844036104cbae36477ce3541588d026829/vllm/entrypoints/openai/api_server.py#L234

I don't see any use in having add_special_token=True there, or am I missing something?

Note that this not only impacts the "/v1/chat/completions" endpoint, but also when using an embedded LLMEngine which gets prompts passed in which have been formatted using apply_chat_template.

I just want to note that the special tokens being in the template or not is totally upto the author of the model/template. Most templates don't add special tokens that i've seen. But maybe that is a mistake, and the templates really should be forced to handle the special tokens. I wonder what the best way forward is...

I just want to note that the special tokens being in the template or not is totally upto the author of the model/template.

True! For the model I was working with (OpenChat 3.5), they had add_bos_token = True but also had the bos_token in the prompt template explicitly. Same with mistralai/Mistral-7B-Instruct-v0.1, and a few others I checked. So this is most likely also an error on their side.
However, the reason I think "it just works" with vanilla HF is because they explicitly set add_special_tokens=False when tokenizing using apply_chat_template:
https://github.com/huggingface/transformers/blob/17506d1256c1780efc9e2a5898a828c10ad4ea69/src/transformers/tokenization_utils_base.py#L1750
So to be compatible with HF we would have to do something similar...

It appears that the completion endpoint, v1/completions also adds BOS and EOS. I'm applying a chat template to an Instruct-tuned model myself, and using this endpoint, and I was getting fishy results. The reason I found was that this endpoint automatically adds <s> ... </s>.
The remedy for me was to use the tokenize endpoint myself and pass the tokens directly.


NIFI-5950 Refactored shared logic for input and output port update

@kevdoran fixed a bug in port name updating logic for NIFI-5950 in PR 3301, and I did some refactoring which wasn't reviewed & merged at that time. Just capturing those changes here for review when time is available.
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:

[x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?

[x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

[x] Has your PR been rebased against the latest commit within the target branch (typically master)?

[x] Is your initial contribution a single, squashed commit?

For code changes:

[x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
[ ] Have you written or updated unit tests to verify your changes?
[ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
[ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
[ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
[ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

[ ] Have you ensured that format looks appropriate for the output in which it is rendered?

Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

Recommend hiding whitespace changes for this PR (setting under "Diff Settings" or via link here).

Closing this PR for now.


Is there a way to have wait() wait on a Meteor.call? In general could it be made more flexible?

Really love the router, however I find the wait() method to be a quite limiting. Is there a way to extend its functionality so that it could be used with for example a Meteor.call response? Perhaps something similar to how gulp allows asynchronous execution by passing a callback and triggering it when done?

Hi Micha,
Because Iron-Router accomplishes waiting by attaching the ready() state of
a subscription to a dependency computation, I think that it should be
rather simple to implement your own function or class which would make a
method call and return a handle with a reactive ready() function which upon
completion of the method call would reactively run and return true and thus
bringing Iron-Router into a ready state.  Hope this helps
On Wed, Feb 4, 2015 at 12:51 PM, Micah Blu<EMAIL_ADDRESS>wrote:

Really love the router, however I find the wait() method to be a quite
limiting. Is there a way to extend its functionality so that it could be
used with for example a Meteor.call response? Perhaps something similar to
how gulp allows asynchronous execution by passing a callback and triggering
it when done?
—
Reply to this email directly or view it on GitHub
https://github.com/EventedMind/iron-router/issues/1220.

@copleykj that worked brilliantly! Thanks :)

My pleasure :-)
On Feb 4, 2015 3:20 PM, "Micah Blu"<EMAIL_ADDRESS>wrote:

@copleykj https://github.com/copleykj that worked brilliantly! Thanks :)
—
Reply to this email directly or view it on GitHub
https://github.com/EventedMind/iron-router/issues/1220#issuecomment-72930859
.


Remove count_missing_closing_parens()

The span of ast::NestedMetaItem used to be missing closing parens (cc #1668).
This is now fixed via https://github.com/rust-lang/rust/pull/43933 and hence we need not use count_missing_closing_parens() anymore.

Thanks!


using max_items parameter to limit the number of displayed jobs

This PR enable the use of max_items parameter. This is useful when you need to get the response in few seconds and your system is running a huge number of jobs. If you have Cloudflare you usually get a timeout error.
Four small buttons ( 20 50 100 All) are added on to of the jobs table.

Note: I was not able to add a proper specs to add 150 jobs and test the number of rows inside the table. If you can show me an example how to test this i'll follow your examples and add all the specs needed to cover all cases.

Hello @kenaniah ,
thank you for your feedback. I've followed your comments and pushed some changes.
With this call you can
´´´
Sidekiq::Status::Web.number_of_items_options=[10, 15, 30]
´´´
with this line you could set the default value
´´´
Sidekiq::Status::Web.default_max_items=10
´´´

@scicco Thanks for the quick turnaround! I'll work on getting this ready for merge once we get the 0.7.0 version of the gem published. One of the features I'd like to add in the 0.8 release is proper job pagination support, so I might keep this PR open for a little bit until we come up with a good plan for the UI to support both features. Thanks again :-)

Closed in favor of #119


Lazily import pkg_resources for better import performance.

Initial import performance is a consideration for our application of raven.  We found that we were able more than halve the import time by moving the import of the pkg_resources module into the functions that need them.
I believe my changes are covered by existing tests.

Thx!

@MHendricks @ashwoods we've noticed a bit of slowdown in the raven library since updating to 6.8.0 -- reading through the changelog since 6.7, this seems to be the only change which would affect us. If I'm reading this correctly, it looks like import pkg_resources is now called on each message sent to Sentry.
I wonder if it might both solve @MHendricks 's use-case and avoid repeatedly reimporting the package if we use a singleton-ish lazy load system. Something like:
if pkg_resources is None:
    import pkg_resources

Or, even better, maybe we should use a lazy import library. Result from Google, I have not used this.

Python is pretty good with handling reimporting a module. After the first call its essentially just the cost of a dictionary lookup. While I try to avoid importing in a function, it takes a lot of import calls to add up. On my system it took 10,000 calls for time.time() to even register that it took time.
import time
def test(count):
	t = time.time()
	for i in range(count):
		import pkg_resources
	return time.time() - t

>>> t = time.time(); import pkg_resources; print time.time() - t # First import
0.550999879837
>>> test(1000)
0.0
>>> test(10000)
0.00899982452393
>>> test(10000)
0.0090000629425
>>> test(10000)
0.0090000629425
>>> test(10000)
0.0090000629425

As for the change I made, I couldn't get raven to even try loading pkg_resources when using raven.Client.captureException and raven.conf.setup_logging. Do you know if that code is being called? If so, can you show me how to replicate it? I'm somewhat new to raven.
The lazy import library seems useful, but is probably overkill for ravens needs. pkg_resources is only used in two locations and I think a simple try\except could handle that. Something along the lines of this, but I would need to test it first:
pkg_resources = None
def get_version_from_app(module_name, app):
    global pkg_resources
    ...
    try:
        return pkg_resources.get_distribution(module_name).version
    except AttributeError: # Should only get triggered the first time this function is called
        try:
            import pkg_resources # Import and assign the global variable to the module.
            return pkg_resources.get_distribution(module_name).version
        except ImportError:
            ...


Osorensen issue 10

Created new course based on issue 10 by Brian Buffet

Closing this one and creating one single pull request for the last 3 courses


What's about RxJava2

Do you plan to support RxJava2?

Yes, I will try implement it in the near future

🔥


How to load balance '/'?

Hello!
I've been trying to evaluate fabio as a reverse proxy but am having trouble configuring it for our use case: to load balance all traffic (specifically / in this case) to multiple backends. I've gone over the documentation a number of times and spent about half a day trying to configure it for this case with no luck and was hoping you could provide some guidance.
I'm running consul in dev mode with./consul agent -dev -config-dir consul.d with the contents of consul.d/web.json being three test backends all serving a static document at / (Please ignore the silly checks for now)
stevej@proxy-test-16:~/consul.d$ ls
web.json
stevej@proxy-test-16:~/consul.d$ cat web.json
{
  "services": [
    {
      "name": "web",
      "id": "perf-target-1",
      "tags": ["web"],
      "address": "<IP_ADDRESS>",
      "port": 4140,
      "checks":[{"script": "/bin/true"}]
    },
    {
      "name": "web",
      "tags": ["web"],
      "id": "perf-target-2",
      "address": "<IP_ADDRESS>",
      "port": 4140,
      "checks":[{"script": "/bin/true"}]
    },
    {
      "name": "web",
      "tags": ["web"],
      "id": "perf-target-3",
      "address": "<IP_ADDRESS>",
      "port": 4140,
      "checks": [{"script": "/bin/true"}]
    }
  ]
}

I'm running fabio using the instructions in the README and running as bin/fabio with no properties.
When I send a request to / or /web I receive a 404.
stevej@proxy-test-16:~/go$ curl -vvvv http://localhost:9999/web
* Hostname was NOT found in DNS cache
*   Trying <IP_ADDRESS>...
* Connected to localhost (<IP_ADDRESS>) port 9999 (#0)
> GET /web HTTP/1.1
> User-Agent: curl/7.38.0
> Host: localhost:9999
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Fri, 15 Apr 2016 16:37:36 GMT
< Content-Length: 0
< Content-Type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact

I've confirmed that fabio is bound to that port with netstat -anp
tcp6       0      0 :::9999                 :::*                    LISTEN      3815/fabio

Thanks in advance for any help.

Have a look at the Quickstart: https://github.com/eBay/fabio#quickstart
You need to add a urlprefix-/ tag to all service definitions if they handle /. The idea behind fabio is that services announce which endpoints they handle during service registration and they do that by adding a urlprefix-host/path tag for every endpoint they accept.

Also, the checks are vital since fabio will only add services with a passing health check to the routing table. So it is good that you have them. :) Another common mistake are non-unique service IDs for different instances of the same service.

Hi, thanks for the reply!
I went ahead and added urlprefix-/ tags but am still getting a 404 and an error No route for web/ from fabio's stdout. Are my tags still malformed?
$ cat consul.d/web.json
{
  "services": [
    {
      "name": "web",
      "id": "perf-target-1",
      "tags": ["urlprefix-web-/"],
      "address": "<IP_ADDRESS>",
      "port": 4140,
      "checks":[{"script": "/bin/true"}]
    },
    {
      "name": "web",
      "tags": ["urlprefix-web-/"],
      "id": "perf-target-2",
      "address": "<IP_ADDRESS>",
      "port": 4140,
      "checks":[{"script": "/bin/true"}]
    },
    {
      "name": "web",
      "tags": ["urlprefix-web-/"],
      "id": "perf-target-3",
      "address": "<IP_ADDRESS>",
      "port": 4140,
      "checks": [{"script": "/bin/true"}]
    }
  ]
}
stevej@proxy-test-16:~$ curl -H "Host: web" -vvv localhost:9999
* Rebuilt URL to: localhost:9999/
* Hostname was NOT found in DNS cache
*   Trying <IP_ADDRESS>...
* Connected to localhost (<IP_ADDRESS>) port 9999 (#0)
> GET / HTTP/1.1
> User-Agent: curl/7.38.0
> Accept: */*
> Host: web
>
< HTTP/1.1 404 Not Found
< Date: Fri, 15 Apr 2016 17:18:26 GMT
< Content-Length: 0
< Content-Type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact

urlprefix-/

Unfortunately, I get the same 404 with urlprefix-/
Here is some output from fabio's stdout in case it's useful.
2016/04/15 19:01:21 [INFO] Setting GOGC=800
2016/04/15 19:01:21 [INFO] Setting GOMAXPROCS=16
2016/04/15 19:01:21 [INFO] Metrics disabled
2016/04/15 19:01:21 [INFO] consul: Connecting to "localhost:8500" in datacenter "dc1"
2016/04/15 19:01:21 [INFO] consul: Registered fabio as "fabio-proxy-test-16-9998" with address ":9998" and health check to "http://[<IP_ADDRESS>]:9998/health"
2016/04/15 19:01:21 [INFO] Admin server listening on ":9998"
2016/04/15 19:01:21 [INFO] consul: Using dynamic routes
2016/04/15 19:01:21 [INFO] consul: Using tag prefix "urlprefix-"
2016/04/15 19:01:21 [INFO] consul: Watching KV path "/fabio/config"
2016/04/15 19:01:21 [INFO] Using routing strategy "rnd"
2016/04/15 19:01:21 [INFO] HTTP proxy listening on :9999
2016/04/15 19:01:21 [INFO] consul: Health changed to #8
2016/04/15 19:01:21 [INFO] consul: Manual config changed to #1
2016/04/15 19:01:21 [INFO] Updated config to
2016/04/15 19:01:21 [INFO] consul: Health changed to #9
2016/04/15 19:01:25 [WARN] No route for web/

I don't see a route add coomand in the log. Therefore, your service is not included. Can you replace your health check with an http check instead of the script? Did you reload consul after the change? Does consul show the service green ?

What does curl localhost:8500/v1/health/state/any?pretty say?

Switching to http-based health checks brought the cluster up. Sorry for the consul noobishness, it's not something I use on a regular basis.

I have this working now so I'm going to close this ticket. The numbers i'm seeing are better than advertised: 28k qps using 6 cores on a 16-core GCE vm instance using about 45mb of RAM. p90 was 1ms, p95 was 5, p99 was 600.
Thank you for all your help!

No problem. Glad you got it working. Let me know if there are other issues.


Add further plots and their code to benchmarking`

There are more approaches and plots to benchmark a model.
In especially, it is actually quite easy to set up a good framework for evaluating the influence of different parameters on the performance. This could work quite model agnostic only based on the protobuf files. One could even build an automatic parameter tuner.
I would refrain from adding too much functionality here as the parameters can quickly change, but at least having some basic framework would be quite helpful and not too difficult. Maybe even integrate it with the log analyzer?

This kind of plots should be added as they are super useful if one want to compare multiple metrics between two different models/approaches. For example, you could see the trade-offs between quality and runtime between two different approaches. If dealing with a complex multi-objective (as is often the case for real problems), one can use them to explain the client trade-offs as unfortunately we can not always improve on all objectives but sometimes have to make sacrifices.

This kind of benchmarking is not yet discussed in the primer but I think it is very useful. I actually learned abut this kind from some video by nextmv in the context of "shadow models" that are run in parallel to test how the dev-version would have performed.

I wrote some code for this and prepared an example, but I am not yet sure where to put such a section. It does not really fit into the current Benchmarking section because that has a different story line. This is rather about iteratively improving a model.


Convert output and analysis tables to assets

Description
Currently we provide access to more human-readable denormalized outputs using software routines. This adds a layer of complexity and requires users to use Python. It's also kind of slow. Instead, for simple derived values and denormalized tables we can provide this type of output by defining views (stored queries) inside the databases we generate and distribute.
Once we have a working SQLite IO Manager #1972, we can easily create a DAG of output and analysis table assets. Dagster allows you to create asset dependencies by just specifying the asset name in the asset function parameters. For example:
# This will pull the `plants_eia860` and `utilities_eia860` from the pudl.sqlite database for the current run and create a new table called `plants_utils_eia860`.
@asset
def plants_utils_eia860(plants_eia860: pd.DataFrame, utilities_eia860: pd.DataFrame):
      # Glue them together
      return plants_utils_eia860

We can begin this Epic once the ETL has been converted to use Dagster concepts. See #1570.
Motivation

We want to maximize the ability of users to access useful outputs while minimizing dependence on particular software and platforms.
Reduce the complexity of the system that users have to learn in order to access the data. Just downloading the database, vs. downloading the database and then needing to run our software or write their own SQL queries to construct a readable table with commonly used data.
Provide faster access to these outputs -- using SQL inside the DB rather than Pandas to construct the denormalized tables.

How does it align with our cooperative goals or grant goals?
Can you think of any reasons why we should not pursue or prioritize this project?
Is this project blocking something?
Who is this for?

In Scope

Database views which replace all the PudlTabl output methods corresponding to individual data tables like fuel_receipts_costs_eia923 or plants_steam_ferc1.
Unaggregated, as well monthly and annual aggregations of our data tables, as currently provided by the PudlTabl class.
Joining of entity attributes with data tables based on well defined foreign key relationships.
Calculation of simple derived values using arithmetic that we can easily perform in SQL (e.g. multiplying fuel heat content per unit and number of units to get a total heat content).
Adjustment of more complex derived value calculations (e.g. heat rate estimates, net generation allocations) or value imputations done in software to use the new database views.
Convert output and analysis functions that are more complex than simple joins to dagster assets.

Breaking API Changes

Initially we can modify the PudlTabl class to access the database views directly rather than doing its own calculations and joins, but in the long run as we move to providing access via Intake catalogs or the DB directly, we will probably want to deprecate this access method.

Are you thinking of this as a prelude to or instead of defining the simple denormalized tables as views? Or replacing these functions with functions that create the views e.g. using SQLAlchemy?

As a first pass, each output asset can be created using the existing python code. Then we can go through and replace the pandas code with python creates views using SQLAlchemy. The assets that create SQL views won't return anything so we'll use the default IO manager, so empty tables aren't written to the database.

Should this issue supersede #1178?
I made a comment in your output table spreadshee (which seems like a kind of tracking document that could fit well into the new GitHub Projects architecture) about where the dividing line should be between creating a view (using SQL) and defining an asset in Python.
There seem to be a fair number of tables that are mostly simple enough to recreate using just SQL, but where we've done some backfilling or other imputation for a handful of columns where we need better coverage than the original data provides. Would it be simpler to separate out the few columns that need more work into a much smaller software defined asset that has the same primary keys as the table they'll ultimate be joined with in a view?
E.g. we have the generators_entity_eia table, which gets joined with the generators_eia860 table plus some information from the plants and utilities tables to produce a more usable denormalized view. But if there are some columns in the generators_eia860 table that need imputing / backfilling, maybe there's an asset dedicated to producing just those columns, say filling in prime_mover_code and energy_source_code which could be combined with generators_eia860 in a view.  And another asset that generates FIPS county codes for plants based on their lat/lon or county & state names, which is combined with the plants_entity_eia table in a view  And then those generator & plant views which are compositions of big original EIA tables + skinny imputed value tables are what get brought together in an expansive generators view?  Or alternatively the skinny imputed value tables could just be combined with all of the other tables at the same time when creating the expansive generators table.
It just seems little weird to take a whole giant table and keep it in Python just because a single column within it requires more complex work. With that pattern it seems like eventually more and more tables would end up having to stay in Python, as we do more and more work to fill in missing values in various places.
I guess we'll also want to implement some kind of flagging column to annotate the imputed value columns, indicating whether the value is original or filled, and if it's filled what method was used to fill it.

I'm ignorant about dagster assets, but is the jist of this that we decorate each pudl output function with something like:
def materialize_view(table_name: str, engine=pudl_engine, schema='data_mart') -> callable:
    # no schemas in sqlite so do it via namespace
    table_name = schema + table_name
    def decorator(func) -> callable:
        @functools.wraps(func)
        def wrapper(*args, **kwargs):
            table = func(*args, **kwargs)
            table.to_sql(table_name, pudl_engine)
            return table
        return wrapper
    return decorator

@materialize_view('our_awesome_table_of_plants')
def plants_eia860(...):
    ...

Caching could be implemented in that decorator ("if table_name exists return pd.read_sql(table_name)") or in a separate one for modularity

It just seems little weird to take a whole giant table and keep it in Python just because a single column within it requires more complex work.
This seems like one of those things that is obscenely inefficient in a relative sense ("one column out of 25?!") but in absolute terms only wastes like 10 MB of disk space (out of 900+) and 500ms run time (out of 2+ hours). If we get PUDL to the point where those are our most pressing problems, we are doing very well!

@zaneselvans I like the idea of splitting some output tables into two assets if a majority of the output table is just simple joins.
@TrentonBush the decorator you created is kind of what the asset decorator does! The asset decorator writes the dataframe returned by the function to the database. Check out the example output assets in the Dagster PR:
https://github.com/catalyst-cooperative/pudl/blob/0a9ed54cd8eec717941affa71e2efc08647d51c9/src/pudl/etl/output_assets.py#L35-L42

@TrentonBush It's not disk space that I'm concerned about here. I'm more interested in having a single source of truth for the values people are seeing in the DB. Otherwise I think we'll have to deal with bugs where for whatever reason the values in the two tables (original and merged) diverge.
I think separating out different imputations / calculations into different functions / assets will make it clearer where each operation is happening, and let us explicitly define the dependencies between them which will make those dependencies clearer, hopefully narrower in scope, and potentially allow more things to run in parallel.
I'm hoping it will also provide a more general and uniform pattern for how we add calculated / imputed values, rather than having some big functions where there's like 10 different things happening and maybe getting entangled with each other.
It might also be nice to have richer annotations about how a calculated / imputed column was generated in its "home" table (several columns of flags) rather than having a bunch of that kind of data provenance column in the denormalized table where everything is merged together. (not sure about this though, maybe it would better to pull all those annotations into the big table too).

How do we want to handle the output tables that can be aggregated at different frequencies using the freq parameter? Should we only create the unaggregated version of the tables or create the AS and MS aggregates as well?

Also, I created discussion #2275 to flesh out how we want to organize all of our new tables.

If I'm interpreting correctly, the @asset decorator handles the materialization but not the caching. It's the caching that makes things backwards compatible (except for the function args like freq), It sounds like a lot of design decisions (and scope) here will depend on our commitment to backward compatibility. Did we make that decision yet?

@TrentonBush I'm not sure I understand what you're saying about the caching. If the tables are stored in the DB with all of the difficult calculations already done, then the DB effectively becomes the cache, and reading tables from the DB is fast. Not as fast as a dataframe that's already in memory, but way faster than performing the calculations. So if the same PudlTabl methods are preserved, but pull tables from the DB rather than from memory, hopefully they'll mostly just work? Though the methods that have arguments that control what exactly is done to the data will no longer have that flexibility. The underlying functions will still be available in the codebase though, and could be run with other arguments if need be.
I don't think we know to what extent other folks are using the PudlTabl objects, or how much use of the customized parameters is happening. I doubt we'll be able to ensure nothing breaks. Hopefully we can get @gschivley and @grgmiller to test drive some of these changes.
@bendnorman I had been imagining that we would store monthly and yearly aggregated versions of the tables in the DB -- with the DB effectively acting as a cache for all of these derived values. In a lot of the data tables there are only one or two time varying data columns that need to be aggregated, alongside the primary key values that are used for the aggregation (report date, plant ID, generator ID, boiler ID, maybe categoricals like energy source code). To the extent possible it seems like we want to keep the attributes that are associated with the IDs separate, and let them be merged with the aggregated data as need be for other calculations, or just pull a few useful stock columns for legibility in a denormalized view of the aggregated data tables. Otherwise any timeseries data that's reported by generator-month is going to have 100+ columns for all the attributes associated with generators and plants.
The aggregations get complicated (and slow, I think) when there are many different columns being aggregated in different ways -- sums, weighted averages, homogeneous categoricals, number of records in a group. I think the MCOE functions do a bunch of this.
We should probably make a list of what data columns are being aggregated in what tables now, and how.

Oh I think I misinterpreted the @asset decorator as writing the tables to the db but not reading them later (the other half of caching). Which makes no sense. Nevermind!

To the extent possible it seems like we want to keep the attributes that are associated with the IDs separate, and let them be merged with the aggregated data as need be for other calculations, or just pull a few useful stock columns for legibility in a denormalized view of the aggregated data tables. Otherwise any timeseries data that's reported by generator-month is going to have 100+ columns for all the attributes associated with generators and plants.

@zaneselvans this seems like the kind of refactoring that should be a separate issue/process. The output tables will contain duplicate info from the data warehouse -- that's the nature of denormalization. I agree its inefficient and overly complex but it's legacy code. If it works today it is good enough to cache.
The problem we are trying to solve here is data access without python. If we include big refactors into the scope this is going to take weeks/months instead of days and soak up grant budget that could be spent on other things. There are a zillion legacy code issues we could fix; is this number one?

I don't think we know to what extent other folks are using the PudlTabl objects
I think this is one of the compelling reasons why we shouldn't be investing heavily in refactoring here.

I don't think we know to what extent other folks are using the PudlTabl objects, or how much use of the customized parameters is happening. I doubt we'll be able to ensure nothing breaks. Hopefully we can get @gschivley and @grgmiller to test drive some of these changes.

We use pudltabl objects quite extensively in OGE: https://github.com/singularity-energy/open-grid-emissions/search?l=Python&q=pudl_out

@grgmiller From those search results it looks like it would be pretty doable (and ultimately faster) to replace those pudl_out method calls with a pd.read_sql() selecting the necessary data, if it were already available directly within the DB. Does that seem right to you?  Though I see you have fill_tech_desc=False in your initialize_pudl_out() function. Was there something wrong with that process?

My primary use of PudlTabl is calculating heat rates. For other reasons I've been planning to cache my own outputs. I'd love to see a table of heat rates in PUDL but it's also difficult given the number of different timeseries that it could be calculated over.

@grgmiller From those search results it looks like it would be pretty doable (and ultimately faster) to replace those pudl_out method calls with a pd.read_sql() selecting the necessary data, if it were already available directly within the DB. Does that seem right to you? Though I see you have fill_tech_desc=False in your initialize_pudl_out() function. Was there something wrong with that process?

Yes, I think it would be pretty easy to change how we access the data, as long as it is consistent and has the same output/structure as the pudl_out tables.
I think fill_tech_desc was just set to False by default, I actually don't remember what this parameter even does.

It backfills the generator technology description, which only started getting reported in 2013, but which is almost static, and provides more detail about what kind of generator it is (e.g. supercritical pulverized coal, CCNG with heat recovery, etc.)
For now the plan is just to load these "output" tables into the database so they can be accessed directly without needing to use all the Python stuff, so there shouldn't be any structural or contents changes. To get a particular year or years worth of data you'd want to do a SELECT based on report_date but that's easy.

Given that we are rapidly approaching the end of the quarter and are behind on this, I am wondering if we might want to experiment with just writing our existing dataframe outputs into the DB, so we can at least have some version of the derived values available in the most easily accessible place. I don't know for sure that this would be much easier, but if it were much easier, maybe we could run through and do it as a rough first cut, and then go through gradually replacing the denormalized tables with views over time?

Welp, that only took a year! 😬


Remove source build of generate_parameter_library on Humble

Description
This change was formerly needed (and reverted on main), but not in the Humble branch.
Checklist

[ ] Required by CI: Code is auto formatted using clang-format
[ ] Extend the tutorials / documentation reference
[ ] Document API changes relevant to the user in the MIGRATION.md notes
[ ] Create tests, which fail without this PR reference
[ ] Include a screenshot if changing a GUI
[ ] While waiting for someone to review your request, please help review another open pull request to support the maintainers

Seems like a PSM unit tests segfaulted here in both Humble jobs...?


Update AppContext.tsx

Purpose

Fixed issue related to authentication.  getProfilePhoto on line 106 was returning 404 in cases where users don't have an avatar. getProfilePhoto returning 404 was caught by the catch statement, meaning the remainder of the code in the try statement after line 106 was never executed. Also setAuthenticated was never being called in that same try catch statement, so I added that. Additionally, accessToken and sessionId were never set on initial signin, so I set added that too, and wrapped in a try catch statement just incase. Let me know if there are any changes I need to make.
Thanks,
Cameron Zuziak
Does this introduce a breaking change?

[ ] Yes
[X ] No

Pull Request Type
What kind of change does this Pull Request introduce?

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

How to Test

Get the code

git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install

Test the code

What to Check
Verify that the following are valid

...

Other Information

@microsoft-github-policy-service agree


Version 2.0 READ PLEASE

Hello, mates! 🏷
I'm very sorry to say, but if u're now working on PR, 99% chance, that it wouldn't be merged in our current release. To tell you the truth, we are going to migrate to 2.0 with lots of breaking changes and new design
Can't tell you more yet, but you can look at the draft version here: https://github.com/Yavanosta/yandex-dialogs-sdk/tree/v2-preview
We haven't come to consideration with @Yavanosta  in some points yet. Maybe we'll even freeze this repository and continue in another one (thanks to copyright©©© problems)
Stay tuned.

It's VERY early preview of new version and we still have a lot of things to discuss. If you have some view on design of library please write everything here and lets discuss.
Key points of v2.0 version:

Rewritten on Typescript from scratch with maximum strict compiler options, including strictNullChecks
Redesigned middlwears to match common syntax (context, next) => {}
Removed a lot of functionality, some of it will be ported back before switching to v2.0 version (if it will appear sometime), but some them (proxy for example) will not. You can implement proxy middleware if you want.
We've tried to make it as simple as possible. Core library contains nothing just one function for running middlewears and uploading images. Sessions, Stages etc are implemented as middlewears.
There is no dedicated way to create response now. Just return suitable to api spec json object from your command. But for most common cases we've created helper Reply which can generate most common replies like Reply.text('Hello!') or Reply.text({text: 'Hello', tts: 'Hell+o'})

Webhook server designed based on pure http package.
New concept as Stage, which adds relations between Scenes

We released it yesterday, yeah ✌


Yet another TODO list

Home

[x] contact modal from sends ok, but modal mask remains [Fixed that problem, but found another: form submits to wrong URL subsequent to a failed submission and form refresh]
[x] #239
[x] top 2 blurbs say far too little, imho

Search

[x] Uncaught DOMException: Failed to execute 'setItem' on 'Storage': Setting the value of 'last_search' exceeded the quota.

localStorage.clear() and it was ok

[x] ~results: container extends below window~ I think existing is the best compromise for variety of screen sizes: map has min-height of 400px, and results max-height of 40vh.
[x] "place portal" link sometimes delivers the single record detail page
[ ] place types and place categories filters are largely redundant

add count of categories post search?

[x] #238

Portal

[x] content extends well below 945px tall browser window

should be like the ds_browse page, e.g.

[x] #234

Dataset pub (ds_browse)

[ ] ~#233~

~e.g. https://dev.whgazetteer.org/datasets/62/places~

[x] some markers far too small (e.g. https://dev.whgazetteer.org/datasets/693/places)

orange against earth tones not best choice (they are red elsewhere)

[x] initial display of polygons invisible, nice highlight on hover (e.g. https://dev.whgazetteer.org/datasets/1155/places)
[x] problems:

ds 2, 12 (black, owtrad): TypeError: Cannot read properties of undefined (reading 'length'); both display on home page

[x] window width < 1200px hides add-to-collection if start/end
[x] #237
[ ] published datasets need images, better descriptions, essays, time slider choice <= Ali

Place collection pub (place_collection_browse)

[x] # 139 (Russian, East European, and Eurasian Studies)  fails to load: 500, "TypeError at /mapdata/collections/139/ Invalid type encountered in the arguments."

Dataset collection pub (ds_collection_browse)

[x] #'s 325, 172 ok; #'s 28, 2 not ok (2 is low priority)

NB whereas datasets theoretically must now be indexed, in practice they needn't be (e.g # 25 Ukraine, # 10 Central Asia)

this is enforced only in the ds_select option filtering to only 'indexed'

[ ] ~change of composition (add, remove dataset) requires new tileset, cache file~ Part of #221

Gallery

[x] #236
[x] need to filter out datasets with 'core' = True (e.g. TGN)
[ ] differentiate place and dataset collections?
[ ] some datasets created purely for collections - belong in gallery? (e.g. Frankopan*)

Tileset admin

[x] #232

installation.md

[x] #243

an old TODO, closing


nodejs support?

I'm not sure if this is a known issue, but it doesn't seem like jsdoc resolves any require() statements with node. For instance: https://gist.github.com/tmcw/bf5f616d1fcb08e69b98 - running jsdoc index.js on this code creates an empty documentation directory.

If I may restate your feature request: It sounds like you want JSDoc to automatically figure out that index.js should get the exact same documentation as b.js. Is that correct?
You may also want to look at the docs explaining how to document Node.js modules.

Yeah, I'm trying to decipher that... I've gotten as far as https://gist.github.com/193bc7ae0138bd0fa03d but it's still not quite exporting anything. The documentation for node.js modules is a little funky because all of the examples mention modules named things like color/mixer, which is not a valid name for something in npm.

So my question is more like: can jsdoc resolve calls to require(), or does it only parse files specifically given to it? If I have a modular system that's built on require, can jsdoc find & collect annotations to build documentation?

Yeah, I'm trying to decipher that... I've gotten as far as https://gist.github.com/193bc7ae0138bd0fa03d but it's still not quite exporting anything.

One issue is that you're using both a @module tag and an @exports tag. You would normally use one or the other, not both. In this case, you should remove the @exports tag.

The documentation for node.js modules is a little funky because all of the examples mention modules named things like color/mixer, which is not a valid name for something in npm.

Yes, but if you have an npm module named color that contains the file mixer/index.js, you would indeed be able to require('color/mixer');. Think of that example as the documentation for mixer/index.js. (There's probably a way to make this point more clearly in the docs. I'll file a new issue to address that.)

So my question is more like: can jsdoc resolve calls to require(), or does it only parse files specifically given to it?

The latter, but I don't think that prevents you from using JSDoc for your package. Let's work this out on the jsdoc-users mailing list so other JSDoc users can contribute to the discussion. Please send a message to the list, and I'll try to respond promptly.
I'll keep this issue open as a feature request for better static analysis of require() calls.


Ubuntu: Error creating GL context; Some of the required attributes are not supported

Which operating system does the issue occur on?
Ubuntu 17.10
If on linux, are you using X11 or Wayland?
Wayland?
It seems to build fine, but when I try to execute the binary, I get this error:
Error creating GL context; Some of the required attributes are not supported

#1083
env WAYLAND_DISPLAY= alacritty

Closing this as a duplicate of #1083.


Decode a length-delimited framing protocol.

Length-delimited frame decoder. (#474)
Motivation:
It would be useful to have a general ChannelInboundHandler that can decode a length-delimited framing protocol. This is a protocol that has messages prefixed with an encoding of their length.
Such a protocol would look like this:
<fixed width integer n in network byte order><n bytes>
The simplest version of this would be one that uses a fixed-width integer.
For now we won't bother with varint support.
Modifications:
Added a length-delimited frame decoder.
Result:
People can now decode length-delimited framing protocols.

I think this is now covered by apple/swift-nio-extras#17.


Experimental autogenerate docs with panvimdoc

Experimentally autogenerate docs using https://github.com/kdheepak/panvimdoc. It gets ~90% there while 10% are some weird cases it does not generate the right look.
Feel free to close if it's not exactly what you want 😅

@kdheepak there are some bugs that you might want to take a look? 😁

Thanks for making the PR and bringing this to my attention! I’ll take a look at this.

Ty, I'm not a big fan of automated documentation but it's interesting nonetheless.
Would you be able to squash this into a single commit so it's easier to review?

@mcchrish thanks for pinging me here! I made some changes that should make fix some of the formatting issues here.

@mcchrish can you regenerate the docs again? I made some more changes and I think it should fix most if not all the spacing issues.

I finally got to this part of my TODO list, I must say this is actually really nice and does a surprisingly good job converting the README, fantastic plugin @kdheepak!
Ty for bringing this to my attention @mcchrish, I had no idea this was even a thing and although I’d still have to write the dreaded wiki this can at least save me from the vimdoc chore.
One question though, how is thetags file generated, should it happen automatically once nvim detects a doc folder? It doesn’t seem to be generated for fzf-lua but other plugins do seem to have them auto generated by nvim, am I missing something?
I also tried to generate the tags file manually by running ctags fzf-lua.txt inside the doc folder but the tags file is generated with the top comments part only and no help tags.

I thought tags were generated by the plugin manager but I could be wrong about that. I was also meaning to look this up to figure out how to manually generate them.

Using :helptags ALL should re-generate all helptags for all doc directories in runtimepath.

Don't ask me why, I can't even answer this myself, I decided to play around with babelfish.nvim and ended up with a major rewrite of the converter code, I could have probably saved myself hours and hours of agony by just using panvimdoc but I wanted to experiment with treesitter lol
Although the code is a bit messy (TBH, at it's current state, babelfish to start with isn't very well written...) I'm quite happy with the result - let me know what you think and if there's room for improvement?

The output looks really good! I like how it handles the table with inline code in there too. I might borrow some stuff into panvimdoc too. Thanks for sharing the link to your source! I’ll go through it later.
You should definitely consider upstreaming your version. I’m sure @mjlbach would be interested in that too.
Should we close this PR then?

The output looks really good! I like how it handles the table with inline code in there too. I might borrow some stuff into panvimdoc too. Thanks for sharing the link to your source! I’ll go through it later.
You should definitely consider upstreaming your version. I’m sure @mjlbach would be interested in that too.

I just opened babelfish.nvim#7 for discussion, I look at this more of a WIP since I did this "quick and dirty" and I'm not happy with the interfaces/naming/code routes.

Should we close this PR then?

I guess we can close it, doesn't prevent is from continuing the discussion in a closed PR or maybe we can wait for the OP to close it?


Stop text wrapping in tab menu

resolves #235

@sumitarora Quick one?

@stevenkampen We also discussed adding ellipses for this


set default ttl field for cosmosdb to enable ttl support.

Signed-off-by: Mukundan Sundararajan<EMAIL_ADDRESS>Description
set default ttl field for cosmosdb to enable ttl support.
Based on https://learn.microsoft.com/en-us/azure/cosmos-db/nosql/time-to-live, without a non-null Default TTL, the TTL set per item will not be honored since TTL is disabled.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:

[ ] Code compiles correctly
[ ] Created/updated tests
[ ] Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

LGTM, although there's a small suggestion.
I have already made the change in the live infra. Will trigger a test now to confirm too.

LGTM, although there's a small suggestion.
I have already made the change in the live infra. Will trigger a test now to confirm too.

This components conformance test was rerun and it passed https://github.com/dapr/components-contrib/actions/runs/3621753015/jobs/6149562858


Creating an extensible task configuration.

This commit contains two changes.

changing the tasks.json to launch a shell command instead of dotnet so that any program can be invoked in a task.
only creating web config if Microsoft.AspNetCore.Server.Kestrel is detected.

Hi @rajkumar42, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@gregg-miskelly @chuckries @caslan

@DustinCampbell

@rajkumar42, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.
Thanks, DNFBOT;

i'm pulling out this PR to separate out the changes.


[AOT compatible] Resolve AOT Build Error in Peek.UI

Summary of the Pull Request
Resolved the AOT check error in Peek.UI by adding the partial keyword to the NeighboringItems and NeighboringItemsEnumerator classes.
Before:

After:

PR Checklist

[ ] Closes: #xxx
[ ] Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
[ ] Tests: Added/updated and all pass
[ ] Localization: All end user facing strings can be localized
[ ] Dev docs: Added/updated
[ ] New binaries: Added on the required places

[ ] JSON for signing for new binaries
[ ] WXS for installer for new binaries and localization folder
[ ] YML for CI pipeline for new test projects
[ ] YML for signed pipeline

[ ] Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

/azp run

/azp run

@crutkas @jaimecbernardo  - it looks like the packages and NOTICE.md aren't syncing up in CI for some reason.
But work when built and ran locally on my device

https://github.com/microsoft/PowerToys/pull/36136 is throwing same style error

@crutkas @jaimecbernardo - it looks like the packages and NOTICE.md aren't syncing up in CI for some reason.

@snickler  Looks like there's some auto reference thing added to the discovered packages?

@crutkas @snickler  @jaimecbernardo Thanks for assisting with the CI issue. I’ve identified the root cause: when a project includes the  property, it automatically references Microsoft.NET.ILLink.Tasks. When I tried adding this package to Directory.Packages.props, it resulted in a build error. To resolve this, I updated NOTICE.md by adding two lines. Do you think this is the right approach?

Hi @lei9444 , The idea of notice.md is listing the packages we are using from nuget. I think the proper fix here would be changing the verification script to remove the :Auto-referenced line from the generated lines before comparing with NOTICE.md .
Now I'm not sure if we should keep the Microsoft.NET.ILLink.Tasks (A) line in there, but in theory it's a package we end up using from nuget, even if it's automatically added, so I guess we keep it in NOTICE.md.

/azp run

Hi @jaimecbernardo  @snickler  @crutkas I have updated the script to remove the Auto_referenced line. This message describes what is (A) mean.

Please take a moment to review it when you have time.

Just triggered a Dart build to test, thank you 😄

Just triggered a Dart build to test, thank you 😄

Thank you! The first run failed, but I retriggered the pipeline, and everything passed.

Done

@lei9444 I've merged main since we've updated spellcheck in main. After CI passes, feel free to merge 😉  🎉


6T modified geometry for Coax Hexacopter

Errors in compiling when adding a new geometry.
@LorenzMeier

Rebased and applied, thanks!


docker ps hangs and does not log on docker 1.12.2

Description
We run many services with docker and kubernetes in a cluster. On upgrading to docker 1.12.2, we saw that docker ps on about 25% of the hosts was hanging. docker also stops logging when this is occurring, and we see error logs in the kubelet of the affected host. The goroutine stack traces I have attached below should have the most relevant info to diagnosing this issue.
Steps to reproduce the issue:

start a kube cluster on bare metal running kube 1.4.0 and docker 1.12.2
start up various services to get a workload
check the docker logs and/or run docker ps on the hosts (masters and nodes)

Describe the results you received:
docker ps hangs, the logs stop logging on about 25% of hosts
Additional information you deem important (e.g. issue happens only occasionally):

docker run is still functional when this is occurring (as are docker info, version, etc.)
the kubernetes kubelet logs with GenericPLEG: Unable to retrieve pods: operation timeout: context deadline exceeded
restarting the docker daemon resolves the issue, although we have seen it recur later on the same host
running strace -fF -p <pid> on the docker daemon pid also resolves the issue, and causes the logs to be flooded with error messages like Handler for GET /containers/json returned error: write unix /var/run/docker.sock->@: write: broken pipe. See the attached docker.log for the full output (~8800 lines). docker_log.txt
getting the goroutine stack trace shows there may be an issue with containers acquiring a semaphore, but it was hard for me to parse. I have attached three different goroutine stack dumps acquired by sending sigusr1.
goroutine1.txt
goroutine2.txt
goroutine3.txt

Output of docker version:
$ docker version
Client:
 Version:      1.12.2
 API version:  1.24
 Go version:   go1.6.3
 Git commit:   bb80604
 Built:        Tue Oct 11 18:15:17 2016
 OS/Arch:      linux/amd64

Server:
 Version:      1.12.2
 API version:  1.24
 Go version:   go1.6.3
 Git commit:   bb80604
 Built:        Tue Oct 11 18:15:17 2016
 OS/Arch:      linux/amd64

Output of docker info:
This is from a host where the issue is currently occuring:
$ docker info
Containers: 14
 Running: 12
 Paused: 0
 Stopped: 2
Images: 72
Server Version: 1.12.2
Storage Driver: aufs
 Root Dir: /var/lib/docker/aufs
 Backing Filesystem: extfs
 Dirs: 175
 Dirperm1 Supported: false
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins:
 Volume: local
 Network: overlay bridge host null
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Security Options:
Kernel Version: 3.13.0-55-generic
Operating System: Ubuntu precise (12.04.4 LTS)
OSType: linux
Architecture: x86_64
CPUs: 1
Total Memory: 1.955 GiB
Name: tuk5m1kmaster1
ID: AAWZ:IW2J:ELCJ:B35B:6YA3:JPBF:3IDG:REKJ:MNYT:NTF4:ANNH:JMKT
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): false
Registry: https://index.docker.io/v1/
WARNING: No swap limit support
Insecure Registries:
 <IP_ADDRESS>/8

Additional environment details (AWS, VirtualBox, physical, etc.):
bare metal
running Ubuntu 12.04 LTS with kernel 3.13.0-55-generic
kubernetes version info:
Server Version: version.Info{Major:"1", Minor:"4", GitVersion:"v1.4.0", GitCommit:"a16c0a7f71a6f93c7e0f222d961f4675cd97a46b", GitTreeState:"clean", BuildDate:"2016-09-26T18:10:32Z", GoVersion:"go1.6.3", Compiler:"gc", Platform:"linux/amd64"}```

Ping @mlaventure @tonistiigi @cpuguy83

This should be fixed by https://github.com/docker/docker/pull/27405

We are also seeing this on CoreOS v1185.5.0 (stable) with Docker 1.11.2. This is in a Kubernetes cluster, the messages from kubelet on the machines look like this:
E1208 16:37:03.554339   10554 kubelet.go:1085] Container garbage collection failed: operation timeout: context deadline exceeded
E1208 16:37:09.724304   10554 kubelet.go:2372] Failed cleaning pods: operation timeout: context deadline exceeded
E1208 16:37:13.274330   10554 generic.go:193] GenericPLEG: Unable to retrieve pods: operation timeout: context deadline exceeded
E1208 16:38:06.959099   10554 desired_state_of_world_populator.go:162] kubeContainerRuntime.findAndRemoveDeletedPods returned error operation timeout: context deadline exceeded.

As for @jdfless, using strace on the Docker daemon or restarting it temporarily solves the issue.
It looks like the fix will ship in 1.12.4, but are there any workarounds in the meantime? This is proving to be quite a pain on a Kubernetes cluster.

@louis-paul There's no telling if your 1.11 issue is the same as reported here.

@cpuguy83 Forgot to mention that the issue was the same with Docker 1.12.3 when we tried to upgrade. How can I check my stacktraces for this particular bug?

@louis-paul You can send SIGUSR1 to the dockerd process. This will spit out a stacktrace to dockerd's stderr.


[CMake] Do not list .gyb files as sources

Sources/Atomics/CMakeLists.txt currently lists .gyb files in its source lists, as well as the swift sources expanded from them. This doesn't seem to prevent the build from succeeding, but it seems undesirable.
Checklist

[X] I've read the Contribution Guidelines
[X] My contributions are licensed under the Swift license.
[X] I've followed the coding style of the rest of the project.
[ ] I've added tests covering all new code paths my change adds to the project (if appropriate).
[X] I've verified that my change does not break any existing tests.
[ ] I've updated the documentation if necessary.

@swift-ci test


Expose Typings for all Props

Purpose
Expose all props for the purposes of extending them in other projects.
Example Usage:
import { Dot, DotProps } from 'pure-react-carousel';
import React from 'react';

/**
 * Abstraction of Dot for purposes of setting the active
 * slide when the onClick event is triggered. Basically lets
 * us make the active slide highlighted
 */
interface ExtendedDotProps extends DotProps {
  setActiveSlide: (slide: number) => void;
}

const _ExtendedDot: React.SFC<ExtendedDotProps> = (props) => {
  const { setActiveSlide, ...rest } = props;
  const handleSlideChange = () => {
    return setActiveSlide(props.slide)
  }

  return <Dot {...rest} onClick={handleSlideChange} />
}

const ExtendedDot = React.memo(_ExtendedDot)

closes #102

@mrbinky3000 @tim-steele Any update on this? I would love to start leveraging this

@martinmckenna added to 1.18.0.


Adding Travis CI file

Setup a travis CI job with this file to find broken builds, go vet and golint errors.
Once you are happy with this, you can uncomment the last two lines to notify via IRC if build is broken
See: https://travis-ci.org/ryancox/go-collectd

Thanks @ryancox!

Great...  Are you going to setup an 'official' Travis page for the project? This is operating of my fork: https://travis-ci.org/ryancox/go-collectd
Would be nice to get a Build status badge on the README. LMK if there is anything I can do here...


Added Service Bus Topic sample.

Hi,
I would like to add the following sample to the list:
".NET Core Service Bus Topic consumer scaled by KEDA"
It's very similar to @tomkerkhove sample-dotnet-worker-servicebus-queue project but with the main difference that this is a .NET Core 3 Console app that consumes msgs from a Service Bus Topic instead of a .NET Core 3 ServiceWorker consuming msgs from a Service Bus Queue.

Thanks @felipecruz91


Scroll in iOS 12.2/12.4 cant's slide

I have a problem with scroll in iOS 12.2 & 12.4
Scroll works but doesnt slide. It looks static. On iOS 13 works fine:
Expected behaviour:
[
Problem:

i try this: https://github.com/ionic-team/cordova-plugin-ionic-webview/issues/326 and this https://github.com/ionic-team/cordova-plugin-ionic-webview/issues/326#issuecomment-483732402 without results:
I dont use ionic tags in my app. Only alerts and dropdowns. I try to put pointer-events: auto in my scroll container but it doesnt work.
This is my container css:
.flex-content {
    flex: 1;
    display: flex;
    flex-direction: column;
    overflow-y: scroll;
    overflow-x: hidden;
    height: 100%;
    transition-duration: 500ms;
    pointer-events: auto;
}

Any idea?

I have the same problem on iOS 12.2 & 12.4, any solution?
I tried some solutions like:
ion-app.platform-ios12 ion-content { pointer-events: auto; /*fix scroll lock on ios12.2*/ }
But not working.
Also the links <a href"XXX"> into my app have stopped working.
On IOS 13.x everything is OK.

Add this line to your scroll container in css:
-webkit-overflow-scrolling: touch;

Testeado y funcionando :)

Add this line to your config.xml file:
<preference name="ScrollEnabled" value="true" />


Parser accepts trailing comma's

The parser accepts a trailing comma after the last member of a complex object. According to the spec at json.org, this should not be allowed.  Is this on purpose?  Is this configurable?

It is on purpose. It is not configurable. You could create a strict version of JsonTextReader.

@JamesNK  How to create strict version of JsonTextReader to check the trailing comma? My code as below and it won't throw exception
            var jsonString = @"{ 'CPU': 'Intel',}";
            var reader = new JsonTextReader(new StringReader(jsonString));
            var t =  JToken.ReadFrom(reader);

Just wanted to add I use this library for my automation tests. Our automation OK'd the trailing commas, We've had a critical issue in production that other parsers fail duo to said trailing comma, that this library appears to ignore by design. Standards exist for a reason, and if you don't follow them, you hurt your ability to communicate with other libraries.
I urge you guys to allow to at least configure the strictness. I wish we could put someone to do it ourselves, but can't.


Inappropriate horizontal scrolling on mobile

Visit https://lbry.com/news/aug-dev#roadmap on mobile and it allows to scroll horizontally for no immediately discernible reason.

close close close


Pg-tap on GitHub actions

As a developer
I want to test database functionality on push to github
so that I can ensure data is being saved appropriately.
DoD:

[ ] pg-tap runs on GitHub actions
[ ] tests are passing
[ ] tests run on push to any branch

To test the database effectively we will need to bring in the testing harness pg-tap
DoD:
Test runner runs tests
At least 1 passing test

Relevant particularly for cases where we expect data to be immutable, or not able to be changed after submission. Can create tech debt card if there is clean-up post launch.

Find time to discuss what kind of test coverage we want.