Need spec and compile-time errors of incorrect JS interop usage

There are many invalid things that can be done with JS interop, but we don't have static errors for this.
Here are some examples of things that should be reported (or else, need to be specified and fixed in the DDC/dart2js):

[x] an anonymous JS class with positional arguments in the constructor. This is invalid, because it's supposed to create an object literal, and causes a DDC crash (see #32031)
[ ] use of generic functions/methods (currently unsupported in JS interop)
[ ] use of generic classes (what is intended behavior?)
[x] use of @JS on a class/library member without a @JS on the library
[ ] use of @JS on a class member instead of external
[ ] declaring Dart members on a @JS class (Dart code is silently ignored)
[ ] extends/mixin@JS classes from a Dart class (also unclear if implements is supported)
[ ] extends/mixin/implements Dart class from @JS class

Errors should be reported from Analyzer so people can see the problem sooner. (Once DDC migrates to Kernel, need a way of calling into Analyzer to produce these kinds of errors. Or we just have shared code between dart2js/DDC for validating JS interop usage.)

Also it would be lovely to have a specification. These would be useful:

where is @JS allowed to appear?
where is external allowed to appear?
inheritance implications of the various permutations of Dart and JS classes and member kinds
interaction with Dart generic classes
interaction with Dart generic methods/functions
meaning of Dart members on a JS class
semantics of anonymous JS types
how JS types fit into the Dart subtyping relation and Dart Type equality
how JS functions related to the Dart type system
static and dynamic call/get/set/index/operator/tearoff semantics
behavior of dart:core Object members (toString, hashCode, operator ==, noSuchMethod, runtimeType) for JS objects
interaction with the JS global namespace
interaction with JS modules

Another example is currently unsupported features, like use of generic methods.

Once DDC migrates to Kernel, need a way of calling into Analyzer to produce these kinds of errors. Or we just have shared code between dart2js/DDC for validating JS interop usage.

My preference is for the latter: I can imagine we'd have a js-interop checker implemented as a kernel-visitor that we can both share.

It would be ideal for the analyzer to give errors/warnings before a compilation failure, if possible.
(It's not clear to me we have a good strategy yet for web/JS-specific analyzer checks, so maybe we need a thread/doc/discussion about that separate from js-interop checking in the compilation pipeline)

Yeah for JS interop, we can have shared checks between DDC/dart2js/Analyzer. That way they show up in IDE as well as compilers.
At the moment I don't really care where the checks live :) ... I'm mostly concerned that JS interop is completely unchecked and there's all kinds of undefined behavior people can, and do stumble into.
(And personally, when I have to touch DDC's implementation code for JS interop, I have no idea what's supposed to be legal or not, so it's really difficult to refactor any of the code that touches that.)

@vsmenon and I just ran into a client internally with another edge, non-`abstract classes, i.e.:
@JS()
library js_interop;

import 'package:js/js.dart';

@JS()
@anonymous
class Animal {
  external String get name;
  external set name(String name);
}

This should probably be a compile-time error, as it implicitly creates a default constructor.
The correct implementation would be:
@JS()
@anonymous
abstract class Animal {
  external String get name;
  external set name(String name);
}

Or, with an external factory constructor.

In DDC, everything above appears to work as if this was abstract
In Dart2JS above creates a Dart class with a JS-semantics getter/setter.

Nitpick: The second class also gets a default constructor, you just can use it to instantiate the abstract class. You can extend it, though, which you probably should also be disallowed.

additional checks that we have discussed recently:

[ ] instance method renames: we can warn that these are not supported. Today this means that instance method @JS declarations are unnecessary, so we can also warn about any use of them, but we can be a bit more lenient with benign annotations and only warn when there the rename is clearly the intent.
[ ] conflict between @JS and native types: to help users detect issues like #42200, we can pre-build a list of blacklisted names that cannot be bound via @JS and issue a warning on those.

I'm curious: what's blocking us from allowing instance method renames? I was talking a bit about this last week and figured it would be a good way to wrap overloading.

what's blocking us from allowing instance method renames?

The current interop design in dart2js does not allow it - though we can do it easily in ddc.
In dart2js all @JS() interop classes share effectively a single implementation. When the dart code has foo.bar() that ends up going through to a Dart method that forwards to the JS bar() method. If any JS interop needs to forward from the Dart name renamed to the JS named bar, then all JS interop would have the same rename.

If I recall correctly, ddc can only handle it when the invocation is typed. When invocations are dynamic (via a dcall) it will not apply the rename either.
I should note that even though the issue is more visible because of dart2js's implementation, there is an actual composition problem here. We allow users to define JS-interop classes without coordination. If two libraries expose the same JS class, they are both allowed and equally usable. Because dart2js dispatches based on the underlying value (in this case JS-instance), it needs to ensure that all JS-interop definitions for that JS class are consistent with each other. Because we want to avoid link-time errors, we enforce this modularly by limiting what the user can do. When we disallow renames (and method bodies among other things), we allow the declarations to be combined without conflict.

If I recall correctly, ddc can only handle it when the invocation is typed. When invocations are dynamic (via a dcall) it will not apply the rename either.

Correct.

Because dart2js dispatches based on the underlying value (in this case JS-instance), it needs to ensure that all JS-interop definitions for that JS class are consistent with each other.

Right, that's why we ideally would like a more statically predictable JS interop. If we can trust the calling side then we can do the rename there. That's what the DDC implementation for instance member renames looked like.

What is the status on this one?

This one issue is a long arc, and wont fit entirely into the September release, so we will be moving it to the October release soon (likely next week? We can move it now if you prefer).
Many checks have landed, and we'll continue to land them before and after the next milestone.

Adding on to what Siggi said, should we continue to move the milestone until everything we want here has been added? We'll likely continue to add things here and stretch this across multiple milestones.

Consider making this an Epic, with sub-issue inside it.  So each sub-issue can be place in different milestone.  Continue rolling this over to the next milestone defeat the purpose of the milestone.

I agree with making it an Epic. We'll need to break this down into bits that will make sense to include in milestones.

@srujzs @sigmundch - are we still using this as a tracking issue?  If so, it is P1 but no 2021 updates.  Should we drop to P2?

This was a bigger priority last year when we were adding a lot of static checks, but since many of those are now handled and many of the remaining checks aren't high priority, I'm okay with dropping to a P2.


Kills kernel daemons safely when closing notebook

For #11579

Codecov Report

Merging #11580 into master will increase coverage by 0.05%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##           master   #11580      +/-   ##
==========================================
+ Coverage   60.59%   60.64%   +0.05%
==========================================
  Files         618      618
  Lines       33644    33711      +67
  Branches     4733     4742       +9
==========================================
+ Hits        20386    20444      +58
- Misses      12279    12280       +1
- Partials      979      987       +8

Impacted Files
Coverage Δ

src/client/datascience/jupyter/jupyterNotebook.ts
4.21% <0.00%> (ø)

...client/datascience/kernel-launcher/kernelDaemon.ts
11.86% <0.00%> (-0.64%)
:arrow_down:

src/client/datascience/raw-kernel/rawKernel.ts
7.07% <33.33%> (+0.82%)
:arrow_up:

src/datascience-ui/react-common/arePathsSame.ts
75.00% <0.00%> (-12.50%)
:arrow_down:

src/client/common/utils/platform.ts
64.70% <0.00%> (-11.77%)
:arrow_down:

src/client/linters/pydocstyle.ts
86.66% <0.00%> (-2.23%)
:arrow_down:

src/client/datascience/debugLocationTracker.ts
76.56% <0.00%> (-1.57%)
:arrow_down:

src/client/common/process/proc.ts
14.49% <0.00%> (-0.73%)
:arrow_down:

...client/datascience/kernel-launcher/kernelFinder.ts
77.01% <0.00%> (+13.04%)
:arrow_up:

Continue to review full report at Codecov.

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


vocal accompaniment separation not working

So i downloaded https://huggingface.co/lj1995/VoiceConversionWebUI/resolve/main/RVC1006Nvidia.7z ran the web ui and when i select the file , specify the output and select the model and click convert the progress shows up and then throws all of this at me.
when i check the TEMP folder on the RVC folder theres nothing however it creates a copy of the original file at \AppData\Local\Temp but thats about it.
i changed nothing just downloaded, used 7z to extract and ran go-web.bat
any ideas on how i can get it to work?
10 Get Over It Demo Versionrw0zc47e.mp3.reformatted.wav->Traceback (most recent call last):
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\core\audio.py", line 155, in load
context = sf.SoundFile(path)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\soundfile.py", line 655, in init
self._file = self._open(file, mode_int, closefd)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\soundfile.py", line 1213, in _open
raise LibsndfileError(err, prefix="Error opening {0!r}: ".format(self.name))
soundfile.LibsndfileError: Error opening 'E:\RVC1006Nvidia\TEMP/10 Get Over It Demo Versionrw0zc47e.mp3.reformatted.wav': System error.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "E:\RVC1006Nvidia\infer\modules\uvr5\modules.py", line 74, in uvr
pre_fun.path_audio(
File "E:\RVC1006Nvidia\infer\modules\uvr5\vr.py", line 63, in path_audio
) = librosa.core.load(  # 理论上librosa读取可能对某些音频有bug，应该上ffmpeg读取，但是太麻烦了弃坑
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\util\decorators.py", line 104, in inner_f
return f(**kwargs)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\core\audio.py", line 174, in load
y, sr_native = __audioread_load(path, offset, duration, dtype)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\core\audio.py", line 198, in _audioread_load
with audioread.audio_open(path) as input_file:
File "E:\RVC1006Nvidia\runtime\lib\site-packages\audioread_init.py", line 111, in audio_open
return BackendClass(path)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\audioread\rawread.py", line 62, in init
self._fh = open(filename, 'rb')
FileNotFoundError: [Errno 2] No such file or directory: 'E:\RVC1006Nvidia\TEMP/10 Get Over It Demo Versionrw0zc47e.mp3.reformatted.wav'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\core\audio.py", line 155, in load
context = sf.SoundFile(path)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\soundfile.py", line 655, in init
self._file = self._open(file, mode_int, closefd)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\soundfile.py", line 1213, in _open
raise LibsndfileError(err, prefix="Error opening {0!r}: ".format(self.name))
soundfile.LibsndfileError: Error opening 'E:\RVC1006Nvidia\TEMP/10 Get Over It Demo Versionrw0zc47e.mp3.reformatted.wav': System error.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "E:\RVC1006Nvidia\infer\modules\uvr5\modules.py", line 82, in uvr
pre_fun.path_audio(
File "E:\RVC1006Nvidia\infer\modules\uvr5\vr.py", line 63, in path_audio
) = librosa.core.load(  # 理论上librosa读取可能对某些音频有bug，应该上ffmpeg读取，但是太麻烦了弃坑
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\util\decorators.py", line 104, in inner_f
return f(**kwargs)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\core\audio.py", line 174, in load
y, sr_native = __audioread_load(path, offset, duration, dtype)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\librosa\core\audio.py", line 198, in _audioread_load
with audioread.audio_open(path) as input_file:
File "E:\RVC1006Nvidia\runtime\lib\site-packages\audioread_init.py", line 111, in audio_open
return BackendClass(path)
File "E:\RVC1006Nvidia\runtime\lib\site-packages\audioread\rawread.py", line 62, in init
self._fh = open(filename, 'rb')
FileNotFoundError: [Errno 2] No such file or directory: 'E:\RVC1006Nvidia\TEMP/10 Get Over It Demo Versionrw0zc47e.mp3.reformatted.wav'

Make sure you installed ffmpeg and ffprobe in the root directory of rvc as mentioned in readme

Separation doesn't work for me on linux, either. Whereas voice training and conversion work really well.


integrating inside pytorch-vision

hello, very nice work; I was wondering if you considered integrating EfficientNet also inside pytorchvision package... in my opinion, it would be a great addition!

Also, there is and follow-up work on detection as EfficientDet https://twitter.com/quocleix/status/1197682880173920256?s=20

If you mean torchvision library than it is not an option unless the weights are reproduced with examples/imagenet/main.py script from scratch. No converted weights are allowed there.

hmmm and what about just adding the model/net/arch which can be trained individually on any standard or custom dataset?

torchvision.models is basically a set of pretrained most common classification networks (with segmentation and detection models added only recently) trained from scratch.
EfficientNet is:

Relatively new model, e.g. there is still no se-resnext's (ImageNet Top-1 dated 2017);
Hard to be reproduced with decent performance as every tensorflow-first architectures like Inception (there is none of it in the torchvision either, though it is dated as far as 2015).

If you can create more native implementation and achieve the paper scores with pytorch imagenet trainer script, godspeed to you.


Images served by next do not have cache-control set

I've noticed that Next does not set any cache-control headers when serving images generated by this plugin.  Given that many CDNs use these headers to determine whether or not to cache an image, it seems like this renders the point of the content hashes moot.

I solved this in my custom server on the catch all route by below solution.
Beware that we use custom routes for every page, and they are regeistered before this catch all, so we have the option to use other values for our pages.
Hope this helps!
    server.get('*', (req, res, done) => {
res.setHeader('Cache-Control', 'max-age=86400');
done();
}, (req, res) => handleRequest(req, res));

Just experienced this as well in NextJS + Vercel deployment. Basically defeats the whole point of this tool.

^^ don't copy/paste that snippet unless you absolutely understand what it does, and are confident that it won't inadvertently cache stale content or expose user-data in a CDN under your particular setup.

This is really an issue with Next.js as they don't have a way to set cache control for public assets not generated by them.

Something like what @Fritiof posted didn't work for me (next.js was always setting the max age to 0). This is what worked for me:
const nextApp = Next({ dev: !IS_PRODUCTION });
  const handleNextJsRequest = nextApp.getRequestHandler();

  await nextApp.prepare();

  const app = Express();

  // For optimized images set the cache headers to 1 year.
  app.get('/_next/static/images/*', (req, res, next) => {
    if (IS_PRODUCTION) {
      res.on('pipe', () => {
        res.setHeader('Cache-Control', 'public, max-age=31536000, immutable');
      });
    }

    next();
  });

  app.all('*', (req, res) => handleNextJsRequest(req, res));```

This issue can easily be fixed without overriding the built-in server and losing its benefits. In your next.config.js:
const withPlugins = require('next-compose-plugins');
const optimizedImages = require('next-optimized-images');

module.exports = withPlugins([
  [optimizedImages, {
    imagesFolder: 'chunks/images'
  }]
]);

The 'trick' here is that Next.js slaps a Cache-Control: public, max-age=31536000, immutable header on whatever is in the .next/static/chunks folder, which is fortunate (and appropriate). 🎉

@sincraianul Nice! Works for me

@cyrilwanner should chunks/images be the default image folder?

Thank you very much for this workaround @sincraianul!
Yes, I will make this the default folder with the next major version.


Add sport icons

Alternatives for football (/rugby-ball?) Maybe we should prefer to cover both with a single icon…?)

Nice work! I also had some of these, but I think most of yours are highly superior, with the exception of basketball, in which case I prefer mine (mostly because it's at a 45deg angle), and I also have bowling-ball and bowling:

Nice work! I also had some of these, but I think most of yours are either highly superior or pretty much exactly the same, with the exception of basketball, in which case I prefer mine (mostly because it's at a 45deg angle), and I also have bowling-ball and bowling

@karsa-mistmere Thanks, ha of course you did! Yeah cool with the basketball… was just trying to differentiate more between tennis-ball.
Nice, like the bowling ball idea!

Your football also has some guideline violations, but all in all I prefer your approach to mine

I have a bunch of iterations. Will post them soon…
Do you think soccer-ball would be a better name than football-soccer?  That ball clearly isn’t for kicking around with the foot 🤪)

Nice work on the new ones (I especially like volleyball).
I understand that yours is closer to real life, but maybe we need to bump bowling's ball in size for visuals:

Your football also has some guideline violations, but all in all I prefer your approach to mine

Maybe something like this as something inbetween?

 Will post them soon…

@karsa-mistmere See #1304 (comment)

I think I like the current one best. :)

maybe we need to bump bowling's ball in size for visuals:

@karsa-mistmere Done.

Should be soccer-ball and baseball-ball IMO.

So I think we should prioritize a bit.
Let's add one "bowling ball" and one "golf" sports icon.
And "shuttlecock", and "snooker" are too specific I think, we should keep them out.
As for the name of "soccer", doesn't use most parts of the world the word: "football"?

"shuttlecock", and "snooker" are too specific

Are those any less of a sport than the others? 🤷🏻♂

"soccer", doesn't use most parts of the world the word: "football"?

Yeah, myself included! Are you thinking we should go with football and football-american? cc: @karsa-mistmere @jguddas

american-football/soccer-ball, just due to being the most clear what it means, football means a lot of different sports depending on where in the world you are from.
And the line in the middle of the american-football makes gives weird pixel artifacts in 1x size, what about removing it?

the line in the middle of the american-football makes gives weird pixel artifacts in 1x size, what about removing it?

@jguddas What about this?

soccer-pitch looks a bit to heavy next to others. needs maybe a little tweak

soccer-ball looks like a life ring. needs a little tweak as well

@danielbayley I agree with @ygd. It need some tweaking. 🤔

from an agile point of view, PRs with multiple icons are slowing down shipping new icons.
eg. a PR with 10 icons, 9 are good but 1 is not so all icons are on hold and cannot be shipped. They must wait until the one which is not looking good is fixed.
I suggest PRs containing only one icon.
am I missing something, @ericfennis?

@ygd Yess you are right. A PR with a lot of icons is harder to review.
But sometimes multiple icons can have shared styling, for example same size of the ball or the same handle of the stick in this case. The benefit of having it in one PR is better to review the icon design "cohesion" of the same "theme".
It's important to have them matching.
So for me 1 icon per PR is not the "ideal" solution, also because every merge is triggering a release.
I think we need to find a balance, and be a bit stricter on splitting a PR if 80-90% of the icons are ready to merge.

This PR is merged to the lab branch. This PR does not match our PR template, which requires valid use cases. To move on we merged this PR to the lab branch, which will be moved to a separate repo in the future.

soccer-pitch looks a bit to heavy next to others. needs maybe a little tweak

soccer-ball looks like a life ring. needs a little tweak as well

I agree with @ygd. It need some tweaking.

@ericfennis I have some of these tweaks ready to go… but it’s a moving target! What is the procedure with this new lab branch? Do I just push everything to that branch now, for requested changes to all icons?

@ygd Yess you are right. A PR with a lot of icons is harder to review.
But sometimes multiple icons can have shared styling, for example same size of the ball or the same handle of the stick in this case. The benefit of having it in one PR is better to review the icon design "cohesion" of the same "theme".
It's important to have them matching.
So for me 1 icon per PR is not the "ideal" solution, also because every merge is triggering a release.
I think we need to find a balance, and be a bit stricter on splitting a PR if 80-90% of the icons are ready to merge.

Good point. Maybe grouping similar icons, like ball or bottle, in a PR makes more sense.
Do you think it is good to write this down in contribution guidelines?

@danielbayley You can create PR to make changes on the lab branch.
You can still make PRs to the main branch for new icons, but make sure you have clear use cases for the icons.

You can still make PRs to the main branch for new icons, but make sure you have clear use cases for the icons.

@ericfennis Does this imply that all icons already in this PR—and all others merged into lab—don’t have valid use cases?


fix: areachart missing tickSize prop

Motivation
I find that for the AreaChart, LineChart, LineErrorMarginChart, BubbleChart, ScatterChart the tickSize property passed to Axis component is horizontalAxis.tickLength.
This PR will update the tickSize property to have the corresponding value horizontalAxis.tickSize.
Fixes #51
Type of change

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

Checklist:

[x] I have done the work for both react and vue
[x] I have performed a self-review of my own code
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation (Storybook)
[ ] I have added unit tests that prove my fix is effective or that my feature works
[ ] New and existing unit tests pass locally with my changes

Hey @marrouchi, like requested i spread all the props passed to Axis components for React && Vue ✓


correctly handle orientation on android

Thanks for submitting a PR! Please read these instructions carefully:

[ x] Explain the motivation for making this change.
[x ] Provide a test plan demonstrating that the code is solid.
[x ] Match the code formatting of the rest of the codebase.
[x ] Target the main branch, NOT a "stable" branch.

Motivation (required)
the width / height returned by the library is swapped if the image is on portrait mode on some device (tested with Sony zx1 compact, and see also in some samsung device)
this because the library do not look on the image orientation when computing the w/h this is a problem also if the set the maxHight/Width since you are currently compare maxWidth with height and maxHeight with width
fix #1901 #1689
Test Plan (required)
run the example app, take the photo on portrait, check that the height > width
take a photo on landscape and check that width > height

I was trying to build but I have an error :
Utils.java:162: error: cannot find symbol
        if (needToSwapDimension(orientation)) {
            ^
  symbol:   method needToSwapDimension(String)

@Johan-dutoit I don't know who is actively working on this project, so I may have tagged the wrong person, but can you please merge this pull request?

We need this change immediately!

Guys I urgently need this pull request to be merged, still today in many devices if you take a vertical photo width and height are reversed. Could any of the project admins please respond? @Johan-dutoit

@vorjdux @marcshilling @yfuks @janicduplessis @rusfearuth @haitaoli @ravirajn22 @charpeni @janicduplessis @pedro

We need this change immediately!

I managed to solve the problem using the "react-native-image-size" library to understand the image orientation and fix the problem.. (Although it's a bad solution to use another library to fix the bug of a library)
this is how i solved it:
Imports:
import { launchCamera } from 'react-native-image-picker';
import ImageSize from 'react-native-image-size';

launchCamera:
 launchCamera(options, response => {
     if (response.didCancel) {
       console.log('Image selection cancelled');
     } else if (response.errorCode) {
       console.log(
         'Error selecting images:',
         response.errorMessage,
       );
     } else {
       const { uri, fileName, type, width, height } = response.assets[0];
       const { rotation } = await ImageSize.getSize(uri);
       const isVertical = rotation === 90 || rotation === 270;

       const normalizedWidth = isVertical ? height : width;
       const normalizedHeight = isVertical ? width : height;
     }
   });

@GiovanniVisentiniCasavo, thank you very much for the patch. Worked for me with the patch-package.

thanks @xaiamov
@Johan-dutoit do you have intention to merge it? if yes I can resolve the conflict, otherwise you can close this pr and explain way you don't accept this code.

thanks @xaiamov
@Johan-dutoit do you have intention to merge it? if yes I can resolve the conflict, otherwise you can close this pr and explain way you don't accept this code.

Hi,  if this genuinely solves an issue, I'll happily merge it.  If you can resolve conflicts, I'll run it on the test app and take it from there.

@Johan-dutoit rebase done!

@Johan-dutoit can you merge it?

:tada: This PR is included in version 7.0.2 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:

hi ,every one , thanks @GiovanniVisentiniCasavo for your work, but seems like still doesnot work on my Galaxy A10s, with any other my phones have not problems


Submit conformance test results and get certified

We have a documented way of running conformance tests. We should run them and send them to get certified.

This is done for v0.1.0: https://github.com/cncf/k8s-conformance/pull/959

@iaguis should we add a certified badge into the readme?

Sounds good! https://github.com/kinvolk/lokomotive/pull/418


How to increase community involvement and collaboration?

I'm going to try something different to grow community involvement with SQLPad.
Once someone has made a significant contribution (or maybe even just opened a PR for one) I'll send an invite to join as a collaborator, which gives commit access. Feel free to accept or not, there's no pressure.
With this shift I'm also making master a protected branch, with only approved PRs being allowed to merge. Right now this is just at 1, but if the number of active collaborators grows I may increase this to 2.
Feel free to review and approve PRs if it is something you feel comfortable doing! No need to wait on me :)
@murphyke @hessam61 @bruth @eladeyal-intel @dengc367 @yorek You'll all be getting invites shortly.
You join myself, @koszti, and @gonace.

Also what do you all think? Is this weird or too much? Any concerns?
I've also been contemplating moving sqlpad to a team/org or whatever so it isn't under my name. I think some additional GitHub features come with that too?

Also thanks to you all for helping with the project it is a lot more fun do be working with others ❤

Also what do you all think? Is this weird or too much? Any concerns?
I've also been contemplating moving sqlpad to a team/org or whatever so it isn't under my name. I think some additional GitHub features come with that too?

I don't know if there are any "benefits" in setting up a team or org. But surely make sense, give how broad sqlpad usage can be, to set up a dedicated org.

I think moving to an org is something I'd like to do. I've claimed the sqlpad org name... will probably wait until v5 is tagged as stable to move the repo.

cool, my company use the sqlpad, and I can help as much as possible. thanks for the excellent project.

@rickbergfalk at this time I will not be accepting collaborator status. I hope I will be able to contribute some more!

@rickbergfalk I appreciate the consideration! My invite expired, but if you changed your mind that is fine too. I can't commit to anything specifically, but will be a good steward where I can since my org is actively using this project.

@bruth I think it must be automatic. I didn't do anything at all... No pressure to do anything with the access either.
I'm mainly trying to plan for long term. At some point I won't have as much time to maintain things, and I don't want to end up being a bottleneck for contributions being merged in :)

I've moved sqlpad repo to sqlpad org! I might tinker with teams and things, so if any of you get notifications or re-invited to anything its just me setting things up!


Keepalive is not activated. GRPC channels do not respond within 15-20 minutes after switching the network

What version of gRPC and what language are you using?
Client: grpc-python-asyncio/1.51.1 grpc-c/29.0.0 (linux; chttp2)

grpcio = "1.51.1"
protobuf = "4.22.0"

What operating system (Linux, Windows,...) and version?
Client: Debian GNU/Linux 11 (bullseye). 5.4.110-1.el7.elrepo.x86_64
Server: Windows Server 2019
What runtime / compiler are you using (e.g. python version or version of GCC)
Client: Python 3.9.7
What did you do?
I have my gRPC client implementation in which I enable gRPC Keepalive
channel_options = [
    ('grpc.keepalive_time_ms', 5_000),
    ('grpc.keepalive_timeout_ms', 20_000),
    ('grpc.http2.max_pings_without_data', 0),
    ('grpc.keepalive_permit_without_calls', 1),
]

grpc.aio.secure_channel(
    target=f'{srv_conf.HOST}:{srv_conf.GRPC_PORT}',
    credentials=grpc.ssl_channel_credentials(
        settings.CERT.encode()
    ),
    options=channel_options,
),

What did you expect to see?

Fast connection recovery after the network switching
When I execute the command netstat -ctown. I want to see - keepalive timer status

netstat -c --timer

keepalive - when the keepalive timer is ON for the socket
on - when the retransmission timer is ON for the socket
off - none of the above is ON

What did you see instead?

After switching the network, GRPC channels do not respond within approximately 15-20 minutes.
Also when I exec to the Docker container and execute the command netstat -ctown. I don't see the expected status - keepalive. The actual status is - on / off (but not keepalive)

netstat -c --timer

keepalive - when the keepalive timer is ON for the socket
on - when the retransmission timer is ON for the socket
off - none of the above is ON

After setting verbose for logs

# TROUBLESHOOTING: https://github.com/grpc/grpc/blob/master/TROUBLESHOOTING.md
# * gRPC environment variables: https://github.com/grpc/grpc/blob/master/doc/environment_variables.md
GRPC_VERBOSITY: "DEBUG"
GRPC_TRACE: "http_keepalive
#GRPC_TRACE: "all,list_tracers,-timer,-timer_check"

I see such logs:
│ I0220 10:48:51.480487718 23 writing.cc:178]           CLIENT: Ping sent [ipv4:<server_host>:<server_port>]: 0/0
│ I0220 10:48:51.480602944 23 chttp2_transport.cc:2617] ipv4:<server_host>:<server_port>: Keepalive ping cancelled. Resetting timer.
│ I0220 10:48:51.541533370 23 writing.cc:178]           CLIENT: Ping sent [ipv4:<server_host>:<server_port>]: 0/0
│ I0220 10:48:51.541648382 23 chttp2_transport.cc:2617] ipv4:<server_host>:<server_port>: Keepalive ping cancelled. Resetting timer.
│ I0220 10:48:51.550680086 23 writing.cc:178]           CLIENT: Ping sent [ipv4:<server_host>:<server_port>]: 0/0
│ I0220 10:48:51.550783571 23 chttp2_transport.cc:2617] ipv4:<server_host>:<server_port>: Keepalive ping cancelled. Resetting timer.

Also, I wrote another client in Golang to the same server, with the following settings for the client.
Golang client go.mod file
module ...

go 1.19

require (
        ...
	google.golang.org/grpc v1.53.0
	google.golang.org/protobuf v1.28.1
)

require (
        ...
	github.com/golang/protobuf v1.5.2 // indirect
	google.golang.org/genproto v0.0.0-20230216225411-c8e22ba71e44 // indirect
        ...
)

Golang client code example:
import (
	"google.golang.org/grpc"
	"google.golang.org/grpc/credentials"
	"google.golang.org/grpc/credentials/insecure"
	"google.golang.org/grpc/keepalive"
	"google.golang.org/grpc/metadata"
	"google.golang.org/protobuf/proto"
)

var keepaliveParams = keepalive.ClientParameters{
	Time:                5 * time.Second,  // send pings every N seconds if there is no activity
	Timeout:             20 * time.Second, // wait seconds for ping ack before considering the connection dead
	PermitWithoutStream: true,             // send pings even without active streams
}

var dialOpts []grpc.DialOption
dialOpts = append(
  dialOpts,
  grpc.WithTransportCredentials(tlsCredentials),
  grpc.WithKeepaliveParams(keepaliveParams),
)

conn, err := grpc.DialContext(ctx, options.Address, dialOpts...)
if err != nil {
  return nil, fmt.Errorf("error connecting to GRPC server: %v", err)
}

And the problem is not reproduced more after switching the network. And also I see keepalive using the command - netstat -ctown:
root@:/# netstat -ctown | grep <my_grpc_port>
tcp        0      0 <ip>:56924      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (12.86/0/0)
tcp        0      0 <ip>:42904      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (13.85/0/0)
tcp        0      0 <ip>:45724      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (9.85/0/0)
tcp        0      0 <ip>:35064      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (8.86/0/0)
tcp        0      0 <ip>:39710      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (8.83/0/0)
tcp        0      0 <ip>:40580      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (3.75/0/0)
tcp        0      0 <ip>:42582      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (2.54/0/0)
tcp        0      0 <ip>:47130      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (6.82/0/0)
tcp        0      0 <ip>:57764      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (1.53/0/0)
tcp        0      0 <ip>:42150      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (11.85/0/0)
tcp        0      0 <ip>:32832      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (12.83/0/0)
tcp        0      0 <ip>:44566      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (4.89/0/0)
tcp        0      0 <ip>:52138      <ip2>:<my_grpc_port>      ESTABLISHED keepalive (7.52/0/0)

I don't think netstat -ctown is sound here. The keepalive referred to here is not TCP keepalive, but HTTP/2 ping frame-based keepalive.

After switching the network, GRPC channels do not respond within approximately 15-20 minutes.

This, however, seems worrying. I don't see any issue with the channel args you pasted above, so I'm going to take some time to see if I can reproduce.

Hi, @gnossen. Thanks for your help. :-)
I saw in the gRPC core implementations where the gRPC settings parameter are override  TCP_USER_TIMEOUT (https://man7.org/linux/man-pages/man7/tcp.7.html) to avoid issues where the TCP packets are not ACKed in a reasonable time. That should be solved all the edge conditions regarding channels getting blocked indefinitely (and keepalive too).
In this doc - https://man7.org/linux/man-pages/man7/tcp.7.html (socket options TCP_USER_TIMEOUT) I understand that is socket options (not part of the Linux kernel).

Increasing user timeouts allows a TCP connection to
survive extended periods without end-to-end connectivity.
Decreasing user timeouts allows applications to "fail
fast", if so desired.  Otherwise, failure may take up to
20 minutes with the current system defaults in a normal
WAN environment.

My waits also take 15 to 20 minutes. And they are treated either by waiting or by an instant restart of the client or server. Therefore, I decided to write the client in another language (Golnag) and thereby exclude the server part
Perhaps this is the reason?

@gnossen, We see network switching between regions on the network metrics. Also, I created on Python service which just sends ping-pong GRPC requests (between client and servers) and stores each request (latency and error rate) to the Prometheus metrics. And I see losses of approximately 15-20 minutes after switching networks.
I think there is definitely a problem here. When I do the same thing in Golang with Keepalive, then everything is fine. I see network switching on network metrics, and a small number of errors in the service in Golang. And very fast connection recovery.

Is it possible that this is related to this old issue caused by bad TCP keepalive settings? In particular, note this comment:

The client doesn't actually hang indefinitely as originally reported, but it will roughly take 15-20 minutes for it to time out.

The solution that was implemented tied the TCP-timeout to the grpc-keepalive settings: gRFC A18. I'm wondering if possibly the Python gRPC library isn't correctly configuring timeout options; I've observed separately that keepalive_time_ms seems to be ignored.


Conflicting 'basket_vouchers' models in application 'basket'

Hi1. I try fork-the-oscar-app basket but  it show error
http://prntscr.com/sxphwj
My code
http://prntscr.com/sxplqr
I tried searching but found no solution
https://django-oscar.readthedocs.io/en/2.0.0/topics/customisation.html#fork-the-oscar-app
Same error:
<EMAIL_ADDRESS>https://stackoverflow.com/questions/43006599/can-i-use-the-existing-oscar-oscar-api-object-model-to-implement-a-single-item-o

Hi there. We use Github only for managing bugs and feature requests, and not for support. We recommend that you ask for help on Stack Overflow using the django-oscar tag. You will need to provide more information than you have here - in particular the steps you took to fork the app, and the contents of your settings file, as the issue is very likely to be a misconfigured INSTALLED_APPS setting. Thanks.

Thank you.
I moved "from oscar.apps.basket.models import * " last line  . It worked
http://prntscr.com/sxx08n


added goreleaser

Codecov Report

Merging #28 (135d545) into main (02135d8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #28   +/-   ##
=======================================
  Coverage   79.53%   79.53%
=======================================
  Files          10       10
  Lines         767      767
=======================================
  Hits          610      610
  Misses        108      108
  Partials       49       49

Continue to review full report at Codecov.

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


Reported video time is incorrect and video ending never occurs

Have you read the FAQ and checked for duplicate open issues?
yes
If the problem is related to FairPlay, have you read the tutorial?

What version of Shaka Player are you using?

Can you reproduce the issue with our latest release version?
yes
Can you reproduce the issue with the latest code from main?
yes
Are you using the demo app or your own custom app?
both
If custom app, can you reproduce the issue using our demo app?
yes
What browser and OS are you using?
Only happens in any Safari (macOs or iOs)
For embedded devices (smart TVs, etc.), what model and firmware version are you using?
iPhone 11, iPhone 14, ...
What are the manifest and license server URIs?
http://tenmasters.b-cdn.net/BUGSHAKASAFARI/master.m3u8
This is NOT the complete video (only the first 4 .ts files are there), as it is copyrighted by the company, but it is enough to show and reproduce the bug.
What configuration are you using?  What is the output of player.getConfiguration()?
Does not matter
What did you do?
Load the video in any shaka player running under Safari.
What did you expect to happen?
Video playing with the correct timing.
What actually happened?
The video plays and shows the correct duration, but after a second or so, the duration shown is incorrect. Instead of 16:20 it changes to 23:16, which is incorrect. The video element has a duration of 1396 (matching the 23:16) which is incorrect. The timeline, though, seems correct, as going to the end of the scroll it approaches the time of 16 min.
Also, the video never finishes, as it just stops a few seconds before the end, and keeps buffering.
Screenshot showing the timeline and not matching time attached

.
EDIT - FORGOT TO ADD INFO:
The bug only happens using Shaka. Opening the m3u8 directly by the browser works perfectly. Using another web browser, no issues. Shaka + Safari = BUG happens.

The UI is using the duration of the video element to show the time duration, I have doubts that for vod we should not show seekRange.end - seekRange.start. @theodab @littlespex, what do you think?

Shaka does the same for all stream types, the difference is that in src= it returns this which is the strange thing:
document.getElementById('video').duration
1396.08
shakaDemoMain.player_.seekRange()
{start: 0, end: 980.92}

I'm going to create a PR to use seekRange in the UI.


ListItem and ListItemExpand: Ripple is quite fast

Describe your feature request
This can be seen on the sidebar in the docs, https://material-bread.org/getting-started/react-native
It should be slowed to match material design docs

Any updates on this?


Allow a "break" directive

It's awkward to write a profile like: "change temperature the first time X happens". What if actions in the repeat block can have an optional break directive, which will exit any top-level repeat if the action executes?
type: repeat
while: ....
actions:
  - type: update
    if: ...
    break: True # maybe an expression??
    options:
        ...

This would require us to keep track of future actions, so we can cancel them...

Solved with a when type


SimpleHostRoutingFilter messes up empty query parameters

This pull request will fix the issue described in #824. The handling of query parameters is also cleaned up and done only once per request.

Thanks. Did you fill out the contributors agreement (link in readme)?

Hi @dsyer, i have filled out the contributor agreement this morning.


Do task have the attribute of DICTIONARY

Dear tech-srl,
maybe hear is a mistake.

Hi @UnBuen ,
Thank you for reporting this!
The task should indeed be NEIGHBORS_MATCH.
This flag can also be omitted because this task is the default.
I will fix this in the README.
Best,
Uri

Dear Uri,
Sorry to bother you again~
when I run command python run-gcn-2-8.py,
There maybe is a problem with the number of training examples and test examples.

Have you encountered this problem, or do you have any solutions?
Best, Unbuen

Hi @UnBuen ,
This is not a bother at all.
With depth=2, the number of leaves is 4, so combinatorially there is only a small number of possible different examples.
depth=2 is really a toy example.

Hi @UnBuen ,
This is not a bother at all.
With depth=2, the number of leaves is 4, so combinatorially there is only a small number of possible different examples.
depth=2 is really a toy example.

Hi, Uri,
Oh, I understand now, thanks a lot

Hi @urialon ,
I ran into another problem when I ran the tf-gnn-samples/run_qm9_benchs_fa.py and tf-gnn-samples/run_varmisuse_benchs_fa.py
My development(cent os 7 and python3.6)

Problem description

If you have encountered this problem, please help me or give me some suggestions, thanks.
I saw other people also have this problem on github and some blog, but many people do not have a correct solution.
If you have time, please give me some suggestions, it's okay if you are busy, I can also consult my seniors and mentors.
Wish you success in your studies and work.
Best,
UnBuen.

Hi UnBuen,
Sorry for the delayed response!
I did encounter this in the past. The run_qm9_benchs_fa.py script is originally from Microsoft's tf-gnn-samples repo.
It is simply a loop that runs train.py 5 times. The problem is that if train.py fails, it hides the errors.
One way to see what was the error is to check the directory that you provided to run_qm9_benchs_fa.py as the first argument. For example, if you ran:
python run_qm9_benchs_fa.py myLogsDir/

then check myLogsDir/ to see if you can identify the error.
If you can't find the error, try running train.py using Microsoft's instructions here: https://github.com/tech-srl/bottleneck/tree/main/tf-gnn-samples#running.
Let me know how it goes.
Best,
Uri

Hi UnBuen,
Sorry for the delayed response!
I did encounter this in the past. The run_qm9_benchs_fa.py script is originally from Microsoft's tf-gnn-samples repo.
It is simply a loop that runs train.py 5 times. The problem is that if train.py fails, it hides the errors.
One way to see what was the error is to check the directory that you provided to run_qm9_benchs_fa.py as the first argument. For example, if you ran:
python run_qm9_benchs_fa.py myLogsDir/

then check myLogsDir/ to see if you can identify the error.
If you can't find the error, try running train.py using Microsoft's instructions here: https://github.com/tech-srl/bottleneck/tree/main/tf-gnn-samples#running.
Let me know how it goes.
Best,
Uri

Hi Uri,
Thank you for your valuable comments. I have solved my problem. The reason for the previous failure is due to the dataset and data processing part. I have found a solution from https://github.com/microsoft/tf-gnn-samples,. Thank you again for your patience

Great, glad to hear!
Let me know if you have any further questions.


Add support for unfollowing a project

This endpoint isn't documented, but it's useful for managing followers.

Looks great, thanks @eculver !


fix: since we don't support optionals return empty array

This fixes and issue with the online_boutique sample app crashing on a null array

What was the crash?

https://github.com/TBD54566975/ftl-flutter-online-boutique/blob/main/lib/api/cart.dart#L114
Since we don't yet support optionals and this was null