Random segfaults on MacOS

I tried to understand where this comes from without success, maybe you can help me :)
MacOS 10.13.4
libvips 8.6.2 (installed using brew)
ruby-vips 2.0.12
ruby 2.5.1p57 (2018-03-29 revision 63029)
I am using ruby-vips to resize images using the ImageProcessing gem and Shrine. I previously used the mini_magick backend without problems, but since I switched to ruby-vips I am getting random segfaults when running my test suite on my Mac. I have not seen those crashs outside of the test suite (dev or production), or on CI (Linux containers on CircleCI).
I see those crashs about 70% of the time, and I have not been able to find out if there is something specific. Relaunching the tests a few times without changing anything makes them pass.
I suspect that this is caused by the fact that when running tests the resize is done in the main process and not in a worker like in development/production. My tests are using a piloted web browser (headless Chrome, using Capybara) and about 40 images are resized during them, all in the same ruby process.
The segfault never happens at the start of the test suite, only near the end, hence my suspicion it is related to the number of vips-resized images that occured.
Here is the stack trace I got using lldb
* thread #134, name = 'thread_pool.rb*', stop reason = EXC_BAD_ACCESS (code=1, address=0x7000044724a0)
  * frame #0: 0x000000010008de23 ruby`rb_gc_mark_machine_stack + 99
    frame #1: 0x00000001001e2b59 ruby`rb_execution_context_mark + 137
    frame #2: 0x000000010005bb5b ruby`cont_mark + 27
    frame #3: 0x0000000100097961 ruby`gc_start + 2785
    frame #4: 0x00000001000946cb ruby`gc_start_internal + 635
    frame #5: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #6: 0x00000001001cfcc1 ruby`vm_exec_core + 12785
    frame #7: 0x00000001001e17ee ruby`vm_exec + 142
    frame #8: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #9: 0x00000001001de0f2 ruby`yield_under + 898
    frame #10: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #11: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #12: 0x00000001001e17ee ruby`vm_exec + 142
    frame #13: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #14: 0x00000001001dcbca ruby`rb_yield_values2 + 74
    frame #15: 0x000000010006afce ruby`inject_i + 110
    frame #16: 0x00000001001ef132 ruby`invoke_block_from_c_bh + 802
    frame #17: 0x00000001001dc92e ruby`rb_yield + 158
    frame #18: 0x0000000100003ae9 ruby`rb_ary_each + 57
    frame #19: 0x00000001001ee192 ruby`vm_call0_body + 546
    frame #20: 0x00000001001dc17d ruby`rb_call + 157
    frame #21: 0x00000001001dceee ruby`rb_iterate0 + 206
    frame #22: 0x00000001001dcffc ruby`rb_block_call + 76
    frame #23: 0x0000000100068535 ruby`enum_inject + 613
    frame #24: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #25: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #26: 0x00000001001e17ee ruby`vm_exec + 142
    frame #27: 0x00000001001e0261 ruby`vm_invoke_proc + 369
    frame #28: 0x00000001001e857f ruby`vm_call_opt_call + 159
    frame #29: 0x00000001001cfcc1 ruby`vm_exec_core + 12785
    frame #30: 0x00000001001e17ee ruby`vm_exec + 142
    frame #31: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #32: 0x00000001001dc92e ruby`rb_yield + 158
    frame #33: 0x0000000100003ae9 ruby`rb_ary_each + 57
    frame #34: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #35: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #36: 0x00000001001e17ee ruby`vm_exec + 142
    frame #37: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #38: 0x00000001001de0f2 ruby`yield_under + 898
    frame #39: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #40: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #41: 0x00000001001e17ee ruby`vm_exec + 142
    frame #42: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #43: 0x00000001001de0f2 ruby`yield_under + 898
    frame #44: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #45: 0x00000001001e80db ruby`vm_call_opt_send + 651
    frame #46: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #47: 0x00000001001e17ee ruby`vm_exec + 142
    frame #48: 0x00000001001e0261 ruby`vm_invoke_proc + 369
    frame #49: 0x00000001001e857f ruby`vm_call_opt_call + 159
    frame #50: 0x00000001001cfcc1 ruby`vm_exec_core + 12785
    frame #51: 0x00000001001e17ee ruby`vm_exec + 142
    frame #52: 0x00000001001e0261 ruby`vm_invoke_proc + 369
    frame #53: 0x00000001001e857f ruby`vm_call_opt_call + 159
    frame #54: 0x00000001001cfcc1 ruby`vm_exec_core + 12785
    frame #55: 0x00000001001e17ee ruby`vm_exec + 142
    frame #56: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #57: 0x00000001001dc92e ruby`rb_yield + 158
    frame #58: 0x0000000100003ae9 ruby`rb_ary_each + 57
    frame #59: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #60: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #61: 0x00000001001e17ee ruby`vm_exec + 142
    frame #62: 0x00000001001e0261 ruby`vm_invoke_proc + 369
    frame #63: 0x00000001001e857f ruby`vm_call_opt_call + 159
    frame #64: 0x00000001001cfcc1 ruby`vm_exec_core + 12785
    frame #65: 0x00000001001e17ee ruby`vm_exec + 142
    frame #66: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #67: 0x00000001001de0f2 ruby`yield_under + 898
    frame #68: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #69: 0x00000001001e80db ruby`vm_call_opt_send + 651
    frame #70: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #71: 0x00000001001e17ee ruby`vm_exec + 142
    frame #72: 0x00000001001e0261 ruby`vm_invoke_proc + 369
    frame #73: 0x00000001001e857f ruby`vm_call_opt_call + 159
    frame #74: 0x00000001001cfcc1 ruby`vm_exec_core + 12785
    frame #75: 0x00000001001e17ee ruby`vm_exec + 142
    frame #76: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #77: 0x00000001001dc92e ruby`rb_yield + 158
    frame #78: 0x0000000100003ae9 ruby`rb_ary_each + 57
    frame #79: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #80: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #81: 0x00000001001e17ee ruby`vm_exec + 142
    frame #82: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #83: 0x00000001001effde ruby`catch_i + 78
    frame #84: 0x00000001001de3ad ruby`vm_catch_protect + 173
    frame #85: 0x00000001001deb82 ruby`rb_f_catch + 66
    frame #86: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #87: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #88: 0x00000001001e17ee ruby`vm_exec + 142
    frame #89: 0x00000001001eef8e ruby`invoke_block_from_c_bh + 382
    frame #90: 0x00000001001dc92e ruby`rb_yield + 158
    frame #91: 0x0000000100003ae9 ruby`rb_ary_each + 57
    frame #92: 0x00000001001e70d4 ruby`vm_call_cfunc + 292
    frame #93: 0x00000001001cf257 ruby`vm_exec_core + 10119
    frame #94: 0x00000001001e17ee ruby`vm_exec + 142
    frame #95: 0x00000001001e0261 ruby`vm_invoke_proc + 369
    frame #96: 0x00000001001e857f ruby`vm_call_opt_call + 159
    frame #97: 0x00000001001cfcc1 ruby`vm_exec_core + 12785
    frame #98: 0x00000001001e17ee ruby`vm_exec + 142
    frame #99: 0x00000001001e0261 ruby`vm_invoke_proc + 369
    frame #100: 0x00000001001a56c2 ruby`thread_start_func_2 + 1762
    frame #101: 0x00000001001a4fb8 ruby`thread_start_func_1 + 184
    frame #102: 0x00007fff51e93661 libsystem_pthread.dylib`_pthread_body + 340
    frame #103: 0x00007fff51e9350d libsystem_pthread.dylib`_pthread_start + 377
    frame #104: 0x00007fff51e92bf9 libsystem_pthread.dylib`thread_start + 13

At some point I got a message after crashing saying that this may be in bug in the Ruby interpreter, but I did not save the full message and I am no longer able to get it again.
Any ideas on how to find out what is happening and how to investigate this further?

Hello @renchap, oh dear, that sounds very bad.
I guess there's some problem with the GC. Could you make an example test that fails? I can try on my mac tonight.

I already spent a few hours trying to have a simple repro case before opening this issue, but I havent been able to recreate the segfault outside of my test suite…
I extracted the image processing code from my app and called it 1000 times on my test files, without reproducing.
I know it is related to ruby-vips as if I switch back to the ImageProcessing's MiniMagick driver it no longers occurs.
I added some additional logging around the image-processing code and I saw that it sometimes occurs outside of it, in tests that do not involve any calls to ruby-vips. Could it be possible that ruby-vips mess up with MRI's memory and the crash only occurs when the next GC is triggered?
I will try to see if I can repro with Ruby 2.4. I can also try with Valgrind, but I am not sure it will be very helpful if the crash is in the GC.

I had a thought: could it be to do with the way you are running your test suite? Are you using rspec?
On some systems I've seen problems where the test driver loads libvips, but then fork()s for each test. This causes a range of horrible problems with threading -- locks that should be locked become unlocked, etc. etc.

I am using the default Rails System testing feature, powered by Minitest.
I just ran my test suite 10 times with Ruby 2.4.4 and I have not been able to reproduce the bug. This is starting to look like an MRI bug. Do you see anything else that I could try, or should I open a bug with MRI?

I think minitest will fork by default. I use rspec and it's fine (for me, anyway). Is there an option for minitest to stop it forking?

Unless it changed recently, minitest uses threads, not forks. There is a plugin to use forks (https://github.com/jeremyevans/minitest-parallel_fork) but I am not using it.

It looks a lot like https://bugs.ruby-lang.org/issues/14714

Oh, perhaps.
ruby-vips has had quite a bit of stress-testing, so I would be very alarmed if there was a bug ;(

I posted on the Ruby bug and will see how it goes. I will update this issue if I get more useful info :)

This is what you get when using too fresh versions of Ruby ..P

I've had a similar segfault and managed to solve it by setting cache_set_max_mem explicitly:
Vips.cache_set_max_mem(50 * 1024 * 1024)  # 50mb

Hope this helps.

Interesting, I will try it with my test suite and see if this prevents the segfault.

I disabled the cache after requiring vips and I am still getting the segfault each time I launch my tests.

That's great news @renchap, thank you for the update.

And here is the patch that fixed it: https://bugs.ruby-lang.org/issues/15362
It has not yet been backported to 2.5, but seems planned. I am closing this issue as there is nothing to be done in ruby-vips.


[bugfix] previous value and has previous value of computed and refactor logic

Refactor the core of the library in order to fix issues with previousValue and hasPreviousValue of Computed and simplify the logic.

@manuel-plavsic
This is the only critical change I am not sure about
https://github.com/nank1ro/solidart/pull/45/files#diff-ff1336af3256cbb826fb12484c57514aeec782f3a3fc8c6a0f8d4a77a5572fe0R210
If T is something nullable I may skip the value.

Yep unfortunately I just confirmed this bug with a new test:
    test('nullable derived signal', () async {
      final count = createSignal<int?>(0);
      final doubleCount = createComputed(() {
        if (count() == null) return null;
        return count()! * 2;
      });

      await pumpEventQueue();
      expect(doubleCount.value, 0);

      count.set(1);
      await pumpEventQueue();
      expect(doubleCount.value, 2);

      count.set(null);
      await pumpEventQueue();
      expect(doubleCount.value, null); // <-- FAILS HERE, IT'S 2 AND NOT NULL
    });

Yes, it is because null is supposed to be a possible value in case T is nullable. I think that using T? is more pragmatic than using the wrapper class I suggested in a previous PR, however it makes these things a bit less intuitive.
I would change
if (changed && newValue != null) {
  _setValue(newValue);
}

to:
if (changed) {
   _setValue(newValue as T);
}

I haven't run the tests yet, but I think it should work.

Actually, I am not so sure my suggestion works with non-nullable T.

I actually fixed with
    if (changed && newValue is T) {
      _setValue(newValue);
    }

This is safe, the new test passes! thank you so much for your suggestions 💙


Implement platform specific symbol mangling suppression using \x01

Many platforms have some symbol mangling even for "unmangled" C symbols. For example adding a leading _ or trailing @argcount. LLVM supports suppressing this by prefixing the symbol name with \x01. cg_clif will need to implement this too to support building with -Cpanic=unwind on Windows: https://github.com/rust-lang/rust/blob/2d8651a92761421b0437ffb44ba5670bea5ee1df/library/panic_unwind/src/seh.rs#L213

Removed from the rustup component milestone as that will use a panic_unwind compiled using cg_llvm.

Perhaps the lang team ought to consider whether Rust wants to expose LLVM's \x01 escape code or provide some other means of escaping link names?
Even if the decision is to stick with (and, I guess, document) the \x01 escape, it'd be perfectly reasonable to detect that escape in rustc and lower in some other way to cranelift.

Documenting it or providing another mean of escaping link names are both fine with me. It doesn't help with implementing this in cg_clif though. cranelift_module and cranelift_object currently don't expose any way to implement it. Once they do, an implementation in cg_clif should be trivial no matter how it is implemented.


Fix listing processDefinition the schedulerReleaseState will never be null

Purpose of the pull request
Due to we upgrade the mybatis-plus, now the schedulerReleaseStatue in processDefinition will not be null, even if the scheduler is not exist.
Brief change log

Add ProcessDefinitionDao
Refactor the listingProcessDefinition method.

Verify this pull request

This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:

(or)
If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

Codecov Report

Merging #11888 (19df25e) into dev (3664d85) will decrease coverage by 0.07%.
The diff coverage is 36.50%.

@@             Coverage Diff              @@
##                dev   #11888      +/-   ##
============================================
- Coverage     38.55%   38.48%   -0.08%
+ Complexity     4037     4028       -9
============================================
  Files           993      994       +1
  Lines         36698    36695       -3
  Branches       4273     4269       -4
============================================
- Hits          14149    14121      -28
- Misses        20922    20948      +26
+ Partials       1627     1626       -1

Impacted Files
Coverage Δ

...olphinscheduler/api/controller/BaseController.java
53.52% <ø> (ø)

...scheduler/api/service/impl/ProjectServiceImpl.java
59.64% <0.00%> (-1.89%)
:arrow_down:

.../dao/repository/impl/ProcessDefinitionDaoImpl.java
10.00% <10.00%> (ø)

...heduler/api/service/impl/SchedulerServiceImpl.java
8.66% <20.00%> (+0.45%)
:arrow_up:

...api/service/impl/ProcessDefinitionServiceImpl.java
32.03% <76.47%> (-0.28%)
:arrow_down:

...er/api/controller/ProcessDefinitionController.java
49.50% <100.00%> (+0.50%)
:arrow_up:

...eduler/server/worker/task/WorkerHeartBeatTask.java
68.08% <0.00%> (-8.52%)
:arrow_down:

...er/master/dispatch/host/assign/RandomSelector.java
77.77% <0.00%> (-5.56%)
:arrow_down:

...erver/master/processor/queue/TaskEventService.java
75.00% <0.00%> (-5.36%)
:arrow_down:

...org/apache/dolphinscheduler/remote/utils/Host.java
42.55% <0.00%> (-2.13%)
:arrow_down:

... and 18 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


task #2827 Update Salesforce libraries to latest version

Update Salesforce libraries to 58.0.0 version

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[x] Run mvn clean install apache-rat:check to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
[x] If you have a group of commits related to the same change, please squash your commits into one and force push your branch using git rebase -i.
[x] Mention the appropriate issue in your description (for example: addresses #123), if applicable.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

[x] I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004
[x] In any other case, please file an Apache Individual Contributor License Agreement.

let's use this moment to set up https://developer.salesforce.com/ so we can add tests for this too.

Great idea!

Fantastic, thank you for keeping the Salesforce connectors updated.


docs/articles/wiki: example output for DataStructures has wrong capitalization for page

Mistake in caps "P".
What did you do?
Learn example on page https://golang.org/doc/articles/wiki/
Execute code:
'''
func main() {
p1 := &Page{Title: "TestPage", Body: []byte("This is a sample Page.")}
p1.save()
p2, _ := loadPage("TestPage")
fmt.Println(string(p2.Body))
}
'''
What did you expect to see?
This is a sample Page.
What did you see instead?
On web page:
'''
$ go build wiki.go
$ ./wiki
This is a sample page.
'''

@Konstantin8105 thank you for reporting this. Would you like to submit a CL fixing this as per https://golang.org/doc/contribute.html (just in case you need the reference)? If not, no worries, either way great catch.

Yes I will fix that.


AMBARI-25010. Cover move_namenode_config_initializer

What changes were proposed in this pull request?
Cover move_namenode_config_initializer
Please review Ambari Contributing Guide before opening a pull request.

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/4704/
Test PASSed.


504 errors for Ministry Client since Friday

Describe the bug
A ministry client is asking if we changed Firewall settings last week because they are now getting 504 errors.
Search the CRM for "aest" for details on the systems.  There is also a post in #aws-how-to over in Rocket Chat.
Additional context

The ministry is reporting this is affecting their production systems, medium/high severity

Definition of done

Help diagnose and fix the immediate problem for AEST
Create tickets for more resilient approach changes that may need to happen afterwards

Notes:
We believe(not 100% sure) this might be caused by an issue in the alibip lambda code. When someone deletes the alb or for some reason, the ALB DNS Name does not exist anymore the lambda fails at that point and the target groups do not get updated anymore and get in an unhealthy state. which results in 504's

Notes:
We believe(not 100% sure) this might be caused by an issue in the alibip lambda code. When someone deletes the alb or for some reason, the ALB DNS Name does not exist anymore the lambda fails at that point and the target groups do not get updated anymore and get in an unhealthy state. which results in 504's

Prabhu says we're 100% sure (we will break and repro in ECF Assembly)

Closed as investigation is done.  Making other tickets


Create CollectMutation test

Closes #46
reopened from #62
@aalda please describe the problem you'd spotted whenever you have some spare time.
Thanks in advance

Codecov Report

Merging #63 into master will increase coverage by 0.53%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #63      +/-   ##
==========================================
+ Coverage   70.91%   71.45%   +0.53%
==========================================
  Files          39       39
  Lines        2046     2046
==========================================
+ Hits         1451     1462      +11
+ Misses        467      456      -11
  Partials      128      128

Impacted Files
Coverage Δ

balloon/visitor/collect.go
100% <0%> (+100%)
: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 b2e1de6...2f90186. Read the comment docs.


Update before in geojson layer

This resolves #127 for geojson layers also, I hope.

Hey @oliverbienert Thank you for the PR, I will have a look as soon as I have some free time 👍

Published with version<EMAIL_ADDRESS>


Error Handling!!!

According to your last answer error codes are integers, and we should create a function to handle errors... you gave me a link to the documentation (which I have to say, it's very poor), in your documentation, there is NOTHING mentioning that errors are integers.

Here is my code
async authenticateUser() {
      await faceio.authenticate({
        'locale': 'es' // Default user locale
      }).then((userData: any) => {
        console.log(userData)
        console.log('Success, user identified')
        console.log('Linked facial Id: ' + userData.facialId)
        console.log('Payload: ' + JSON.stringify(userData.payload))
      }).catch((errCode: any) => {
        console.error(errCode)
        this.handleError(errCode)
        // handle authentication failure. Visit:
        // https://faceio.net/integration-guide#error-codes
        // for the list of all possible error codes
      }).finally(() => {
        faceio.restartSession()
      })
    },

I have my own handle error function like this:
export const handleError = (errCode: any) => {
  console.log('handleError: ', errCode)
  const errorMessages: { [key: number]: string } = {
    1: 'Access to the Camera stream was denied by the end user', // 'fioErrCode.PERMISSION_REFUSED'
    2: 'No faces were detected during the enroll or authentication process', // 'fioErrCode.NO_FACES_DETECTED'
    3: "Unrecognized face on this application's Facial Index", // 'fioErrCode.UNRECOGNIZED_FACE'
    4: 'Two or more faces were detected during the scan process', // 'fioErrCode.MANY_FACES'
    5: 'User enrolled previously (facial features already recorded). Cannot enroll again!', // 'fioErrCode.FACE_DUPLICATION'
    6: 'Presentation (Spoof) Attack (PAD) detected during the scan process', // 'fioErrCode.PAD_ATTACK'
    7: 'Calculated Facial Vectors of the user being enrolled do not match', // 'fioErrCode.FACE_MISMATCH'
    8: 'Wrong PIN code supplied by the user being authenticated', // 'fioErrCode.WRONG_PIN_CODE'
    9: 'Server side error', // 'fioErrCode.PROCESSING_ERR'
    10: 'Your application is not allowed to perform the requested operation (eg. Invalid ID, Blocked, Paused, etc.). Refer to the FACEIO Console for additional information', // 'fioErrCode.UNAUTHORIZED'
    11: 'Terms & Conditions set out by FACEIO/host application rejected by the end user', // 'fioErrCode.TERMS_NOT_ACCEPTED'
    12: 'The FACEIO Widget could not be (or is being) injected onto the client DOM', // 'fioErrCode.UI_NOT_READY'
    13: 'Client session expired. The first promise was already fulfilled but the host application failed to act accordingly', // 'fioErrCode.SESSION_EXPIRED'
    14: 'Ongoing operation timed out (eg, Camera access permission, ToS accept delay, Face not yet detected, Server Reply, etc.)', // 'fioErrCode.TIMEOUT'
    15: 'Widget instantiation requests exceeded for freemium applications. Does not apply for upgraded applications', // 'fioErrCode.TOO_MANY_REQUESTS'
    16: 'Origin or Referer HTTP request header is empty or missing', // 'fioErrCode.EMPTY_ORIGIN'
    17: 'Domain origin is forbidden from instantiating fio.js', // 'fioErrCode.FORBIDDDEN_ORIGIN'
    18: 'Country ISO-3166-1 Code is forbidden from instantiating fio.js', // 'fioErrCode.FORBIDDDEN_COUNTRY'
    19: 'Another authentication or enrollment session is in progress', // 'fioErrCode.SESSION_IN_PROGRESS'
    20: 'Error while establishing network connection with the target FACEIO processing node', // 'fioErrCode.NETWORK_IO'
  };

  alert(errorMessages[errCode] || 'Unknown error occurred');
};

As you can see I had to assume that the integer errorCodes are ordered the way it's listed in your documentation, since there is NOTHING indicating how this should be handled, for example in your documentation you state this in the boilerplate example:
switch (errCode) {
			case fioErrCode.PERMISSION_REFUSED:
				console.log("Access to the Camera stream was denied by the end user");
				break;
			case fioErrCode.NO_FACES_DETECTED:
				console.log("No faces were detected during the enroll or authentication process");
				break;

				//... the rest of the code

but fioErrCode is never defined in your boilerplate... also if it's true that the integers are ordered the way they are listed in your documentation (which I have to assume), I'm getting error codes like 21, which points to nothing!....
Please explain this... be consise, fix your documentation, and provide a reproducible link to see how should this work if possible!.

Hello,
fioErrCode is the first object defined on fio.js when you include the library via the FACEIO CDN. Are your integrating the library via CDN or using the NPM package?

Hello,
fioErrCode is the first object defined on fio.js when you include the library via the FACEIO CDN. Are your integrating the library via CDN or using the NPM package?

I’m using the NPM package.

Hello,
fioErrCode is the first object defined on fio.js when you include the library via the FACEIO CDN. Are your integrating the library via CDN or using the NPM package?

So... how would it be implemented with the NPM package? I did check all the Vue and React articles mentioned in your site, and this is not explained at all.

So, I had to check the CDN, unminify the code... and get this simple object that should be in the documentation by default:
const fioErrCode = {
PERMISSION_REFUSED: 1,
NO_FACES_DETECTED: 2,
UNRECOGNIZED_FACE: 3,
MANY_FACES: 4,
PAD_ATTACK: 5,
FACE_MISMATCH: 6,
NETWORK_IO: 7,
WRONG_PIN_CODE: 8,
PROCESSING_ERR: 9,
UNAUTHORIZED: 10,
TERMS_NOT_ACCEPTED: 11,
UI_NOT_READY: 12,
SESSION_EXPIRED: 13,
TIMEOUT: 14,
TOO_MANY_REQUESTS: 15,
EMPTY_ORIGIN: 16,
FORBIDDDEN_ORIGIN: 17,
FORBIDDDEN_COUNTRY: 18,
UNIQUE_PIN_REQUIRED: 19,
SESSION_IN_PROGRESS: 20,
FACE_DUPLICATION: 21,
MINORS_NOT_ALLOWED: 22,
},
For anyone needing this...

We are introducing the fetchAllErrorCodes() method for the NPM Package for much better error logging experience. This update is scheduled to be released in 72 hours on the official package manager.

We are introducing the fetchAllErrorCodes() method for the NPM Package for much better error logging experience. This update is scheduled to be released in 72 hours on the official package manager.

I'm getting a new error when enrolling an user:
TypeError: Assignment to constant variable.
    at faceIO._fillParameters (fio.js:1:20508)
    at fio.js:1:22052
    at new Promise (<anonymous>)
    at faceIO.enroll (fio.js:1:21829)
    at faceio.enroll (index.js:42:34)
    at Proxy.enrollNewUser (faceio-store.ts?t=1686248276642:49:20)
    at Proxy.<anonymous> (pinia.mjs:1366:30)
    at store.<computed> (pinia.mjs:916:40)
    at onSubmit (NewEmployee.vue:138:13)
    at callWithErrorHandling (runtime-core.esm-bundler.js:158:18)

This error isn't even handled with any errCode.

Our official NPM package is now upgraded to fio.js V2.1 which implements Age Verification & Minors Access Prevention.
Your error handler now should look like the following with the introduction of the  faceio.fetchAllErrorCodes() method:
function handleError(errCode){
   const fioErrCode = faceio.fetchAllErrorCodes();
    switch (errCode) {
    case fioErrCode.PERMISSION_REFUSED:
        console.log("Access to the Camera stream was denied by the end user");
    break;
    // etc...
    // Refer to the boilerplate at: https://gist.github.com/symisc/34203d2811a39f2a871373abc6dd1ce9
    // for the list of all possible error codes.
}

Please update to the latest NPM package release to get access to the latest features.

Our official NPM package is now upgraded to fio.js V2.1 which implements Age Verification & Minors Access Prevention.
Your error handler now should look like the following with the introduction of the faceio.fetchAllErrorCodes() method:
function handleError(errCode){
   const fioErrCode = faceio.fetchAllErrorCodes();
    switch (errCode) {
    case fioErrCode.PERMISSION_REFUSED:
        console.log("Access to the Camera stream was denied by the end user");
    break;
    // etc...
    // Refer to the boilerplate at: https://gist.github.com/symisc/34203d2811a39f2a871373abc6dd1ce9
    // for the list of all possible error codes.
}

Please update to the latest NPM package release to get access to the latest features.

I don't see a v2.1 version in your NPM package...
The last one was actually 2 hours ago, version 1.0.9

Our official NPM package is now upgraded to fio.js V2.1 which implements Age Verification & Minors Access Prevention.
Your error handler now should look like the following with the introduction of the faceio.fetchAllErrorCodes() method:
function handleError(errCode){
   const fioErrCode = faceio.fetchAllErrorCodes();
    switch (errCode) {
    case fioErrCode.PERMISSION_REFUSED:
        console.log("Access to the Camera stream was denied by the end user");
    break;
    // etc...
    // Refer to the boilerplate at: https://gist.github.com/symisc/34203d2811a39f2a871373abc6dd1ce9
    // for the list of all possible error codes.
}

Please update to the latest NPM package release to get access to the latest features.

I don't see a v2.1 version in your NPM package...
The last one was actually 2 hours ago, version 1.0.9

Looking directly in the script, I can see version says 2.1
**

**
Probably you did set the NPM package version wrong... but anyway, I'm still getting the same error message:

The NPM package version is 1.0.9. fio.js, the core library is 2.1.
Symisc Systems, SUARL
-----Original Message-----
From: Josue @.>
To: symisc/faceio-npm @.>
Cc: PixLab | Symisc Systems @.>, Comment @.>
Sent: Fri, 09 Jun 2023 5:33
Subject: Re: [symisc/faceio-npm] Error Handling!!! (Issue #7)

Our official [NPM @.***/fiojs) is now upgraded to fio.js V2.1 which implements Age Verification & Minors Access Prevention.
Your error handler now should look like the following with the introduction of the faceio.fetchAllErrorCodes() method:
function handleError(errCode){
   const fioErrCode = faceio.fetchAllErrorCodes();
    switch (errCode) {
    case fioErrCode.PERMISSION_REFUSED:
        console.log("Access to the Camera stream was denied by the end user");
    break;
    // etc...
    // Refer to the boilerplate at: https://gist.github.com/symisc/34203d2811a39f2a871373abc6dd1ce9
    // for the list of all possible error codes.
}

Please update to the latest NPM package release to get access to the latest features.

I don't see a v2.1 version in your NPM package...
The last one was actually 2 hours ago, version 1.0.9

Looking directly in the script, I can see version says 2.1
**

**
Probably you did set the NPM package version wrong... but anyway, I'm still getting the same error message:

--
Reply to this email directly or view it on GitHub:
https://github.com/symisc/faceio-npm/issues/7#issuecomment-1583961848
You are receiving this because you commented.
Message ID: @.***>

You have to clear your browser cache inorder to load the new version of fio.js.
Symisc Systems, SUARL
-----Original Message-----
From: Josue @.>
To: symisc/faceio-npm @.>
Cc: PixLab | Symisc Systems @.>, Comment @.>
Sent: Fri, 09 Jun 2023 5:33
Subject: Re: [symisc/faceio-npm] Error Handling!!! (Issue #7)

Our official [NPM @.***/fiojs) is now upgraded to fio.js V2.1 which implements Age Verification & Minors Access Prevention.
Your error handler now should look like the following with the introduction of the faceio.fetchAllErrorCodes() method:
function handleError(errCode){
   const fioErrCode = faceio.fetchAllErrorCodes();
    switch (errCode) {
    case fioErrCode.PERMISSION_REFUSED:
        console.log("Access to the Camera stream was denied by the end user");
    break;
    // etc...
    // Refer to the boilerplate at: https://gist.github.com/symisc/34203d2811a39f2a871373abc6dd1ce9
    // for the list of all possible error codes.
}

Please update to the latest NPM package release to get access to the latest features.

I don't see a v2.1 version in your NPM package...
The last one was actually 2 hours ago, version 1.0.9

Looking directly in the script, I can see version says 2.1
**

**
Probably you did set the NPM package version wrong... but anyway, I'm still getting the same error message:

--
Reply to this email directly or view it on GitHub:
https://github.com/symisc/faceio-npm/issues/7#issuecomment-1583961848
You are receiving this because you commented.
Message ID: @.***>


Changes (#5)

Merge with  arif98741

Thanks for your interest to join in this repo

Ya dear I know. I will also do this in future also


Add LyricsFreak provider

Site: https://www.lyricsfreak.com/
Example: https://www.lyricsfreak.com/p/parkway+drive/idols+and+anchors_20657144.html

We could get lyrics url by make a GET request by artist name like https://www.lyricsfreak.com/o/one+direction/:

And find lyric's URL by song name in response:

18 Lyrics

It will looks like this:

Hi Can I work on this issue?

Hi @ajay201402 !
Sure! Take a look at the implementation of providers as examples here:
https://github.com/skuill/LyricsScraperNET/tree/main/LyricsScraperNET/Providers
Feel free to create a pull request with your changes, and I’ll review the code.
If you have any questions, you can write here or reach out on Telegram: https://t.me/skuill

Hi @skuill
I started working on it. Can you please assign it to me.
Thanks!

@skuill Created a pull request. Please let me know your inputs


Minio Downloads don't work

{"code":404,"message":"path /api/v1/buckets/cortex/objects/download was not found"}

We need to fix this for GA if it's still an issue.


Add more estates to the trial

Adds North East, Yorkshire, Humberside, Kent and Sussex estates, except
Wealstun, Full Sutton and Askham Grange.
Needs to be deployed by 30th of September.

@alan looks good except please remove Lewes (they haven't nominated admins yet)


fix: adjust initial state of report modal

SUMMARY
This fixes a bug where the initial state and state reset wasn't passing through the correct report format
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
n/a
TESTING INSTRUCTIONS
Create a chart without changing anything, and it should pass in the correct initial state.
ADDITIONAL INFORMATION

[ ] Has associated issue:
[ ] Required feature flags:
[ ] Changes UI
[ ] Includes DB Migration (follow approval process in SIP-59)

[ ] Migration is atomic, supports rollback & is backwards-compatible
[ ] Confirm DB migration upgrade and downgrade tested
[ ] Runtime estimates and downtime expectations provided

[ ] Introduces new feature or API
[ ] Removes existing feature or API

/testenv up

/testenv up

/testenv up

/testenv up FEATURE_ALERTS_REPORTS=true

/testenv up FEATURE_ALERT_REPORTS=true


Display "feature last accessed at" in flipper-ui

Hi there,
We're happy users of the open source version of Flipper and Flipper UI here at Vimeo.
We've made a bunch of features (~100ish) and it's sometimes challenging to manage all of them.
The problem: we suspect that there are several features in our features list which are not actually referenced anywhere in our code anymore. We have around 100 features. It would be nice if the UI could help us identify these so we can delete them and pare down our list.
The solution we're imagining: the table in flipper-ui could have a new "last accessed at" column displaying when was the last time Flipper[:foo].enabled? was last called. That would help us identify which features are not being checked anymore.
When an engineer introduces a new feature flag in the code, it would be neat if it automatically showed up in flipper-ui the first time it's checked. That would save us the step of manually adding it. That feels kind of related to this "last accessed at" idea, but perhaps that's a stretch 🙂 .
We'd be happy to try putting together a PR if this sounds like something you'd be interested in. WDYT?
Note: we're currently using 0.17.2, so I apologize if you've made changes in newer versions and we should just upgrade 😅 .

Hey! Really glad you dropped an issue here. First, its awesome that Vimeo uses flipper. I didn't know that and that pumps me up.

We've made a bunch of features (~100ish) and it's sometimes challenging to manage all of them.

Wow! I love it. I'd also love to chat. It's rare that I get to hear from power users like yourself. If you would be willing to chat, you can snag a time here. I'm out for a couple weeks with a weird schedule but you should be able to get a time after that.

The problem: we suspect that there are several features in our features list which are not actually referenced anywhere in our code anymore. We have around 100 features. It would be nice if the UI could help us identify these so we can delete them and pare down our list.

Legit problem. At GitHub we used instrumentation to solve this. We instrumented every time enabled? was called to statsd (and later datadog). This gave us a general idea of true/false results and total number of invocations (which we could use to find dead features).
You can do the same using the automatic instrumentation flipper comes with (statsd example).
If you don't want all that and just want the last_accessed_at, you could do this:

Add a last_accessed_at column to flipper features table (assuming you are using flipper active record adapter).
Add instrumentation like this to your app (I just added this file as example).
Change the instrumentation above to update last_accessed_at instead of an in-memory hash. I'd also recommend not updating that more than once a day or something. If you have memcached you could do an add for features/#{feature_name}/last_accessed_at with a 24 hour expiration around the SQL update. Or however you ensure things only happen on some interval at Vimeo.

If you have any questions or any of that wasn't helpful, let me know. Closing to keep things tidy here but I'm up for more discussion.

@jnunemaker thanks for the thorough reply. Looking forward to chatting in a few weeks.
I see now that we can define our own instrumenter and do whatever we want there, which is cool. It would be fun and useful to surface whatever metadata we record in flipper-ui, because that's where we're looking all the time. But if we need to surface it somewhere else, no worries, we can make do.
I'm curious if you had any additional thoughts on the bonus idea I mentioned. This one: When an engineer introduces a new feature flag in the code, it would be neat if it automatically showed up in flipper-ui the first time it's checked. That would save us the step of manually adding it.

UI last accessed

It would be fun and useful to surface whatever metadata we record in flipper-ui, because that's where we're looking all the time.

Ah yes. Forgot about that. One quick hack would be to set the description_source for the UI to include the last accessed at.
Flipper::UI.configure do |config|
  config.descriptions_source = ->(keys) {
    # select features from database for keys
    # create hash of { String key => "#{description}. Last accessed: #{last_accessed_at}" }
  }
end

Because the description source is loaded before each request (source), this should work.
If that isn't good enough, we could also do one of these options:

go more specific and add last_accessed_at similar to description.
go more generic and make description easier to use for extra bits of info.

automatically add new features

When an engineer introduces a new feature flag in the code, it would be neat if it automatically showed up in flipper-ui the first time it's checked. That would save us the step of manually adding it.

enabled? is the hot path. So I've intentionally avoided loading any extra data from this method or doing any extra work.
Thinking on it, this could be done with instrumentation. I'd even be happy to create/merge a PR that accomplished this for people to use (let me know if you want me to put an example together).
I'm pretty sure I don't want to do this by default though.
Each enabled call would need to:

read the set of known features
check if the feature being checked is a member of that set
then do a write if not in set.

Even if this was all cached in memory that would be a lot of extra work for every check.
I've seen pages on GitHub.com that did thousands of feature checks. Adding this extra work would have been prohibitive.
All that said, I think we could make an instrumentation thing that would maintain the set in memory though and do this check for those that want it. Wouldn't be an issue for most. I just don't think it should be the default. We might even be able to just make it a config option. config.auto_add = true or something.
Let me know if you have any thoughts either way on any of the above. I'm enjoying the questions! Fun to think through what's possible currently and what should be.


ROX-24117: Implement NodeIndex ACS dataflow

Description
This is a follow-up to #12227, which picks up the generated Node IndexReport and sends it to Central.
This PR stops at the Central pipeline.
The next step is to reach out to Sensor and enrich storage.node with scan results based on the received IndexReport (follow up in ROX-25797).
User-facing documentation

[x] ~CHANGELOG is updated OR~ update is not needed
[x] ~documentation PR is created and is linked above OR~ is not needed

Testing and quality

[x] ~the change is production ready: the change is GA or otherwise~ the functionality is gated by a feature flag
[ ] CI results are inspected

Automated testing

[ ] added unit tests
[ ] added e2e tests
[ ] added regression tests
[ ] added compatibility tests
[ ] modified existing tests

How I validated my change
Deployed to an OpenShift cluster and verified that the message received node index report for node %s with %d packages from %d content sets showed up in Central logs.

/test all

/retest

/retest

/retest


saveImageTypeError: this.parseZip is not a function

I'm having a problem with the parseZip function in file channel_image.coffee. It seems that this function is not yet coded.
Some PSD file has zipped images inside and psd.js failed to transform them.
Do you know how I can do?

Any news about this issue ? I have the same problem when I parse psd on 16 or 32 bits, the compression value is 3 and parseZip is undefined in this case. @Bouret


Remove source image of image title and add background

Fixes #223 by @makkoncept
Description
Remove source-image icon of image-title and add background black. Add source in link title.
Other information
Test in Firefox and Chromium.
Checklist:

[x] My pull request has a descriptive title (not a vague title like Update index.md).
[ ] My pull request targets the master branch of the repository.
[x] My commit messages follow best practices.
[ ] I added tests for the changes I made (if applicable).
[ ] I added or updated documentation (if applicable).
[x] I tried running the project locally and verified that there are no
visible errors.

Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

@rubenlorenzo Please post a GIF(or some screenshots) demonstrating the change.

@makkoncept, Here you have the demonstration in gif.

Hi @makkoncept now, the bookmark section is correct. You can see in the gif.

You're welcome @makkoncept, I am happy about it.


Adding debug: true to Home Assistant 2024.5.0b0 generates Exception in _state_message_received errors

Version of the custom_component
5.1.0

Configuration

Logger: homeassistant.util.logging
Source: util/logging.py:98
First occurred: 6:45:40 AM (10 occurrences)
Last logged: 6:45:41 AM

Exception in _state_message_received when handling msg on 'frigate/camera/recordings/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _motion_message_received when handling msg on 'frigate/camera_2/motion/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 247, in _motion_message_received self._attr_motion_detection_enabled = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _state_message_received when handling msg on 'frigate/camera_2/recordings/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _motion_message_received when handling msg on 'frigate/g_camera/motion/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 247, in _motion_message_received self._attr_motion_detection_enabled = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _state_message_received when handling msg on 'frigate/g_camera/recordings/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?

Describe the bug
Changing line 247 and 241 from decode to encode fixes the error but I have no idea if this is the right path
Debug log

Add your logs here.

The behavior of this seems to be that the switch for recordings (and motion?) provided by the integration always says "off" even if the recording state is actually "on" inside Frigate.

The behavior of this seems to be that the switch for recordings (and motion?) provided by the integration always says "off" even if the recording state is actually "on" inside Frigate. The switch still works if you toggle it once for on and twice for off - but its state can no longer be relied upon.

Just updated to Home Assistant 2024.5.0 and I now have this problem too. It affects the Recordings and Motion switches for all my cameras in the integration, regardless of the state inside Frigate. Switches can be toggled to on but after 30s turn off again. The Detection and Snapshots switches work as they should.

I have the same issue as well. only with the recordings and motion switches/booleans in HA. I tried rolling back HA to 2024.4.0 but the problem persists with the opposite issue, the booleans will report and stay on regardless of what the status is in the frigate UI.

YMMV on this but I rolled back to 2024.4.4 without any issues. Everything is back to working as expected.

YMMV on this but I rolled back to 2024.4.4 without any issues. Everything is back to working as expected.

I can confirm this does allow the switches/booleans to operate as intended.
Is this a HA issue or a frigate integration issue?

Sorry, my English is not very good.
This change in the "camera.py" file fixes the problem.
241:        try:
242:            self._attr_is_recording = msg.payload.decode("utf-8") == "ON"
243:        except AttributeError:
244:            self._attr_is_recording = msg.payload == "ON"
255:        finally:
246:            self.async_write_ha_state()
...
251:        try:
252:            self._attr_motion_detection_enabled = msg.payload.decode("utf-8") == "ON"
253:        except AttributeError:
254:            self._attr_motion_detection_enabled = msg.payload == "ON"
255:        finally:
256:            self.async_write_ha_state()

I think my file is different than yours.

I think my file is different than yours.

Sorry, it was a mistake!

@WilldabeastHA, what file is mentioned in the error message?  The error message i got when turning on debug: true provided everything i needed to fix the file locally.  I am also running the frigate integration with a manual install under config/custom_componenets though.
also, it is not turning on debug mode for the integration.  2024.5 added an additional debug option at the core level
homeassistant:
  debug: true

my instance is working as expected with the changes mentioned in my error message

I was not getting any error messages, just that the Recordings and Motion switches show off. I can see MQTT messages and see set ON or set OFF transissions and can see the State ON or State OFF replies. I thought I would try your suggestion so in camera.py, I changed decode to encode in lines 241 & 247. It didn't appear to make any difference but I now got an error message logged:-
Logger: homeassistant.util.logging
Source: util/logging.py:98
First occurred: 13:34:53 (14 occurrences)
Last logged: 13:35:43

Exception in _state_message_received when handling msg on 'frigate/camera_name_1/recordings/state': 'b'ON'' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.encode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'bytes' object has no attribute 'encode'. Did you mean: 'decode'?
Exception in _motion_message_received when handling msg on 'frigate/camera_name_1/motion/state': 'b'ON'' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 247, in _motion_message_received self._attr_motion_detection_enabled = msg.payload.encode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'bytes' object has no attribute 'encode'. Did you mean: 'decode'?
Exception in _state_message_received when handling msg on 'frigate/camera_name_2/recordings/state': 'b'ON'' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.encode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'bytes' object has no attribute 'encode'. Did you mean: 'decode'?
Exception in _motion_message_received when handling msg on 'frigate/camera_name_2/motion/state': 'b'ON'' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 247, in _motion_message_received self._attr_motion_detection_enabled = msg.payload.encode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'bytes' object has no attribute 'encode'. Did you mean: 'decode'?

AttributeError: 'bytes' object has no attribute 'encode'. Did you mean: 'decode'?   !!!!!!!
I have changed it back to decode and the error message has cleared.

@pdobrien3 what changes did you make?

this is my working code:
    @callback  # type: ignore[misc]
    def _state_message_received(self, msg: ReceiveMessage) -> None:
        """Handle a new received MQTT state message."""
        self._attr_is_recording = msg.payload.encode("utf-8") == "ON"
        self.async_write_ha_state()

    @callback  # type: ignore[misc]
    def _motion_message_received(self, msg: ReceiveMessage) -> None:
        """Handle a new received MQTT extra message."""
        self._attr_motion_detection_enabled = msg.payload.encode("utf-8") == "ON"
        self.async_write_ha_state()

I would assume you need to be on 2024.5

Yes, I am on 2024.5 and I tried your code (encode instead of decode). It did not fix the problem, the Recording and Motion switches remain OFF but it did create logged errors ........... AttributeError: 'bytes' object has no attribute 'encode'. Did you mean: 'decode'?
Your set up must be different to mine in some way. Thinking about it, this relates to received messages which should already be encoded in UTF-8 so surely, it should be decode and not encode ?

I am in no way smart when it comes to coding.  I enabled debug: true in beta, restarted, and got the error message.  i made changes according to the error message and the error message went away.  i am not even sure my motion/recording switches were broken to begin with,  I do not use HACS, i install all my custom components manually.  This is likely where our installs differ but i do not know.  hopefully someone responsible for the integration will chime in soon

@pdobrien3 I am definitely in the novice category when it comes to coding. I installed my set up via HACS instead of manually but I am not sure why that would make a difference.

It definitely shouldn't be encode. It's not clear to me if this issue is always happening or only when debug is enabled

I can revert my changes and turn off debug and see what happens.

Things are working as expected for me with the latest home assistant version, not sure why only some are seeing issues

so checking again before reverting.
changing line 247 and 241 from decode to encode with debug: true set, i was able to change all the switches (motion, detect, snapshot, and recording) on and off from both home assistant and frigate and see a correct reaction on the opposite platform without any errors in home assistant
changing line 247 and 241 back to decode and hashing out debug: true, I got the error at startup as well as when switches are changed:
Logger: homeassistant.util.logging
Source: util/logging.py:98
First occurred: 8:17:50 AM (16 occurrences)
Last logged: 8:21:11 AM

Exception in _state_message_received when handling msg on 'frigate/one_camera/recordings/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _state_message_received when handling msg on 'frigate/two_camera/recordings/state': 'OFF' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _motion_message_received when handling msg on 'frigate/two_camera/motion/state': 'OFF' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 247, in _motion_message_received self._attr_motion_detection_enabled = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _motion_message_received when handling msg on 'frigate/two_camera/motion/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 247, in _motion_message_received self._attr_motion_detection_enabled = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?
Exception in _state_message_received when handling msg on 'frigate/two_camera/recordings/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?

I was able to change all the switches (motion, detect, snapshot, and recording) on and off from both home assistant and frigate and see a correct reaction on the opposite platform but motion and recording generated errors.

I am on HA Core (linux amd64) on version 2024.5.1 with Frigate V5.1.0 and Frigate Server 0.13.2 and I also have the same error message (please see below), whatever debug mode is activated or not.
If I create new additional MQTT switch for controlling the recordings and snapshots, everything is fine and event the switches created by the frigate add-on works as well (even if from times to times the error log happens again) but at least all switches (new manually added ones and the ones from frigate addons work).
If I remove my manually added MQTT switches then the ones from frigate won't work anymore.
and I also have the following error message : Exception in _state_message_received when handling msg on 'frigate/cam_garage/recordings/state': 'ON' Traceback (most recent call last): File "/config/custom_components/frigate/camera.py", line 241, in _state_message_received self._attr_is_recording = msg.payload.decode("utf-8") == "ON" ^^^^^^^^^^^^^^^^^^ AttributeError: 'str' object has no attribute 'decode'. Did you mean: 'encode'?

btw I also get now the following error msg : Error fetching information from http://xxxxx:5000/api/stats: Cannot connect to host xxxxx:5000 ssl:default [Connect call failed ('xxxx', 5000)]
I did not have at all the issue with the previous HA version. It seems it tries to connect using SSL on a non SSL port.

I am on HA Core (generic x86-64) on version 2024.5.1, Supervisor 2024.04.4, OS 12.2 with Frigate Integration V5.1.0 and Frigate Server 0.13.2. I don't get any error messages unless I change lines 241 and 247 in camera.py to encode instead of decode. I have returned them to decode which is what they originally were.
I used developer tools > services to publish MQTT messages with topic frigate/camera_name/recordings/set with payload of ON or OFF and Frigate responds. I can do the same with detect, motion and snapshots. Frigate always responds.
I can listen to MQTT topic frigate/camera_name/recordings/state and always get a message with payload either ON or OFF immediately after calling the service. This also works for detect, motion and snapshots.
So I took it one stage further and set up a test automation that is triggered by frigate/camera_name/recordings/state with payload of ON (and a second trigger with payload of OFF). The action is to turn on or off an input_boolean for testing.
It works for detect and snapshots but does NOT work for motion or recordings. The payload in the motion and recordings state does not trigger the Automation even though the MQTT listener shows it as ON or OFF, exactly the same as in detect and snapshots. If it can't trigger the Automation, may be the problem lies somewhere else and not within the Frigate Integration ??

Things are working as expected for me with the latest home assistant version, not sure why only some are seeing issues

You will need to be more specific as 2024.5.2 is released now.

I suspect there might be multiple unrelated reports mixed here. I cannot get any exceptions thrown no matter what I do, with or without debug: true.
However, I can definitely see the switch state behavior problem with recordings and motion only. In this image, I turn all 4 entities on and 2 of them immediately reset:

Versions:

Integration/Frigate: 5.1.0/0.13.2-6476f8a
HA: 2024.5.1

MQTT contents of a recordings topic:
$ mosquitto_sub -h localhost -t 'frigate/kitchen/recordings/state'
ON <-- Initial value

OFF <- I toggle off in Frigate UI
ON <- I toggle on in Frigate UI

ON <- I toggle on in HA UI
<- Nothing new on MQTT topic, yet HA switches it off

This sounds like the same issue I have since I upgraded to 2024.5.0, for reference only reported here.

Some progress: The recordings state data is being received as b'ON' not ON, yet the comparison to determine the state in the integration is just against ON. In Python b'ON' != ON. If we decode the string first to ascii, the recordings switch starts to work correctly. There's a similar issue in reverse in camera.py which explains the exceptions shown at the start of this issue.
I think motion might be a subtly different problem / race condition, still looking...

I suspect there might be multiple unrelated reports mixed here. I cannot get any exceptions thrown no matter what I do, with or without debug: true.
However, I can definitely see the switch state behavior problem with recordings and motion only. In this image, I turn all 4 entities on and 2 of them immediately reset:

Versions:

Integration/Frigate: 5.1.0/0.13.2-6476f8a
HA: 2024.5.1

Same here with same Versions of Frigate and HAOS

I suspect there might be multiple unrelated reports mixed here. I cannot get any exceptions thrown no matter what I do, with or without debug: true.
However, I can definitely see the switch state behavior problem with recordings and motion only. In this image, I turn all 4 entities on and 2 of them immediately reset:

Versions:

Integration/Frigate: 5.1.0/0.13.2-6476f8a
HA: 2024.5.1

Same here with the same Versions of Frigate and HAOS

The motion problem is different. Taking HA entirely out of the picture, looking just at the MQTT messages. Here is recordings, top console is publishing a message, bottom is subscribing. Every published message sets the state correctly, and it's echo'd back in the state topic:

Here is motion topic, only ON is republished back ...

@NickM-27 Is there something fundamentally different about how these channels are intended to behave on the backend?

Checking the backend code, I think this is probably intentional:
    def _on_motion_command(self, camera_name: str, payload: str) -> None:
        """Callback for motion topic."""
        detect_settings = self.config.cameras[camera_name].detect
        motion_settings = self.config.cameras[camera_name].motion

        if payload == "ON":
            if not motion_settings.enabled:
                logger.info(f"Turning on motion for {camera_name}")
                motion_settings.enabled = True
        elif payload == "OFF":
            if detect_settings.enabled:
                logger.error(
                    "Turning off motion is not allowed when detection is enabled."
                )
                return   <--------------------------------- HERE

            if motion_settings.enabled:
                logger.info(f"Turning off motion for {camera_name}")
                motion_settings.enabled = False

        self.config_updater.publish(f"config/motion/{camera_name}", motion_settings)
        self.publish(f"{camera_name}/motion/state", payload, retain=True)

i.e. Frigate will not allow motion to be disabled, when detection is turned on. If I turn detection off, it behaves correctly. So I think this is all WAI, and the user simply needs to accept that they cannot turn off motion but keep detection on.
As such, the draft PR probably resolves this issue as much as it will be resolved.

The fix PR is merged. I'll do a v5.2.0 release tomorrow.

Checking the backend code, I think this is probably intentional:
    def _on_motion_command(self, camera_name: str, payload: str) -> None:
        """Callback for motion topic."""
        detect_settings = self.config.cameras[camera_name].detect
        motion_settings = self.config.cameras[camera_name].motion

        if payload == "ON":
            if not motion_settings.enabled:
                logger.info(f"Turning on motion for {camera_name}")
                motion_settings.enabled = True
        elif payload == "OFF":
            if detect_settings.enabled:
                logger.error(
                    "Turning off motion is not allowed when detection is enabled."
                )
                return   <--------------------------------- HERE

            if motion_settings.enabled:
                logger.info(f"Turning off motion for {camera_name}")
                motion_settings.enabled = False

        self.config_updater.publish(f"config/motion/{camera_name}", motion_settings)
        self.publish(f"{camera_name}/motion/state", payload, retain=True)

i.e. Frigate will not allow motion to be disabled, when detection is turned on. If I turn detection off, it behaves correctly. So I think this is all WAI, and the user simply needs to accept that they cannot turn off motion but keep detection on.
As such, the draft PR probably resolves this issue as much as it will be resolved.

Isn't this backwards? The motion switch is toggling off even when on. This "prevention" you're describing seems like it would only force the switch to stay on?

i.e. Frigate will not allow motion to be disabled, when detection is turned on. If I turn detection off, it behaves correctly. So I think this is all WAI, and the user simply needs to accept that they cannot turn off motion but keep detection on.

This has always been the case and it should stay that way imho.

Isn't this backwards? The motion switch is erroneously toggling off even when on. This "prevention" you're describing seems like it would only force the switch to stay on?

That's right. It cannot be turned off, when detection is turned on. You can try to turn it off in HA, but it will reset to on as the backend is rejecting the request.

New release: https://github.com/blakeblackshear/frigate-hass-integration/releases/tag/v5.2.0

Right but the issue at hand is that the switch provided by the integration is toggling itself off

Prior to the "fix", the backend would report 'ON' but the integration would interpret it as 'OFF'. That causes the HA state to appear to be out of sync. Here's how it looks now:

It's a little hard to see without the mouse cursor, but:

When detect is manually turned on, you'll notice motion turns on too.
When motion is manually turned off, but detect is on, motion will reset itself to on.
When detect is manually turned off, then motion can be turned off.

New release: https://github.com/blakeblackshear/frigate-hass-integration/releases/tag/v5.2.0

Just installed v5.2.0 and I can confirm that all the switches are now working correctly.
Many thanks for fixing this, much appreciated.

yes, thank you.  looks like it is working for me also

Thanks for the confirmation!

Confirming 5.2.0 working as expected, and many thanks for the quick fix. (thumbs-up + party emoticons)

Right but the issue at hand is that the switch provided by the integration is toggling itself off

Prior to the "fix", the backend would report 'ON' but the integration would interpret it as 'OFF'. That causes the HA state to appear to be out of sync. Here's how it looks now:

It's a little hard to see without the mouse cursor, but:

When detect is manually turned on, you'll notice motion turns on too.
When motion is manually turned off, but detect is on, motion will reset itself to on.
When detect is manually turned off, then motion can be turned off.

I have the same issue with the "Motion" boolean.
I can confirm the other booleans (Detect, Recordings, Snapshot) work correct after the v5.2 update .
Thank you for your support and attention to this. Much appreciated!


minVal configuration script_metric

How single script works in opensearch ?
I have this single script in elastic and planning to use in opensearch but its not working. also, if i use in metric_script, not sure the exacts configuration in script_metric.
In Elastic:
"response_time_min": {
"min": {
"script": "doc['metric_type.keyword'].value == 'Average Response Time (ms)' ? doc['minVal'].value/1000 : 0"
}
},

@macohen - Do you know why this might not be working?

@srout60 can you let us know what version of Elastic and what version of OpenSearch you're using?


MSSQL Primary Key Related

The primary key column is defined in the struct as:

Col_nUserNo       int     gorm:"column:nEMID;primary_key"

And the following is my save function:
func Save(firstname string, lastname string, username string, password string, email string, ip string, gender int, token string, time string) (err error) {
    user := Account{
        Col_sFirstName: firstname,
        Col_sLastName: lastname,
        Col_sUserID: username,
        Col_sUserPW: password,
        Col_sUserName: username,
        Col_sEmail: email,
        Col_sUserIP: ip,
        Col_sGender: gender,
        Col_sLastToken: token,
        Col_sLastSeen: time,
    }
    db.NewRecord(user)
    if query := db.Create(&user); query.Error != nil {
        return query.Error
    }
    return nil
}

The Column nEMID is defined as the following in MSSQL table:
[nEMID] [int] IDENTITY(1,1) PRIMARY KEY NOT NULL,

And I get the following error:
mssql: Explicit value must be specified for identity column in table 'tAccounts' either when IDENTITY_INSERT is set to ON or when a replication user is inserting into a NOT FOR REPLICATION identity column.

My observation has been that  when the above 'save' function runs, this query is executed first when it is not needed since I'm not supplying the primary key explicitly:
SET IDENTITY_INSERT tAccounts ON

Kindly look into it.

I have a patch for this if you'd like to check it out- https://github.com/euforia/gorm/commit/b65a7a29ad9219e1be4fb23c16b9e2b0af5d83a1#diff-fb0ef55974bb35e6e1da71205071fd31R15
Basically adding this to the conditional - && !scope.PrimaryKeyZero()

As a workaround I have added below statement in my init function
gorm.DefaultCallback.Create().Remove("mssql:set_identity_insert")

@dipbhi thank you, helps me a lot

@dipbhi What does your command/workaround do?  Mind describing the effect?

It removes the set identity params from the insert sql statement.  Its on by default in gorm

@euforia @dipbhi Calling gorm.DefaultCallback.Create().Remove("mssql:set_identity_insert") in my init() function only works for the first insert.  Subsequent inserts fail.  Do I need to call that Remove function on every insert?

I think it's each time after you call Open. I don't have any issue with
multiple inserts afterwards.
On Mon, Nov 21, 2016 at 12:23 PM, ryanwalls<EMAIL_ADDRESS>wrote:

@euforia https://github.com/euforia @dipbhi https://github.com/dipbhi
Calling gorm.DefaultCallback.Create().Remove("mssql:set_identity_insert")
in my init() function only works for the first insert. Subsequent inserts
fail. Do I need to call that Remove function on every insert?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jinzhu/gorm/issues/941#issuecomment-261858072, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALu0yHQpyap5vZh3hPxegiziJGMMPWIcks5rAT_tgaJpZM4H9G0V
.

We should fixed all issues for mssql, if you still have the same issue, please create a new issue.
Thank you.


[SPARK-20401][DOC]In the spark official configuration document, the 'spark.driver.supervise' configuration parameter specification and default values are necessary.

What changes were proposed in this pull request?
Use the REST interface submits the spark job.
e.g.
curl -X  POST http://<IP_ADDRESS>:6066/v1/submissions/create --header "Content-Type:application/json;charset=UTF-8" --data'{
"action": "CreateSubmissionRequest",
"appArgs": [
"myAppArgument"
],
"appResource": "/home/mr/gxl/test.jar",
"clientSparkVersion": "2.2.0",
"environmentVariables": {
"SPARK_ENV_LOADED": "1"
},
"mainClass": "cn.zte.HdfsTest",
"sparkProperties": {
"spark.jars": "/home/mr/gxl/test.jar",
"spark.driver.supervise": "true",
"spark.app.name": "HdfsTest",
"spark.eventLog.enabled": "false",
"spark.submit.deployMode": "cluster",
"spark.master": "spark://<IP_ADDRESS>:6066"
}
}'
I hope that make sure that the driver is automatically restarted if it fails with non-zero exit code.
But I can not find the 'spark.driver.supervise' configuration parameter specification and default values from the spark official document.
How was this patch tested?
manual tests
Please review http://spark.apache.org/contributing.html before opening a pull request.

Can one of the admins verify this patch?

Not a big deal. Waht do you think about wrapping the command you ran with ``` ... ``` in the PR description?
curl -X POST http://<IP_ADDRESS>:6066/v1/submissions/create --header "Content-Type:application/json;charset=UTF-8" --data '{
  ...
}

I think this makes the PR description more readable.

@HyukjinKwon
But I wrote, no use ...
My computer is window7 system.
The browser is Chrome.

I will probably test it by my self latter. Thanks for taking my comment into account.

Test build #3666 has started for PR 17696 at commit 3e74d73.

Please cut a picture, I see how you show on your computer?

Yup, I will let you know when I made it. I think this does not block this PR BTW :).

Yup, I will let you know when I make it. I think this does not block this PR BTW :).

Test build #3666 has finished for PR 17696 at commit 3e74d73.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

@guoxiaolongzte, I just tested the backquotes on Windows 7 with Chrome as below:

It is a backquote ` not a single-quote '. Could you try again? I see you made many PRs and I just would like them to be more readable.

@HyukjinKwon
I will try to improve my ability to translate, sorry.

Test build #3671 has started for PR 17696 at commit 16655db.

Test build #3671 has finished for PR 17696 at commit 16655db.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged to master/2.2


Workspace - Welcome message doesn't have the new format

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number:  1.4.63-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers<EMAIL_ADDRESS>Issue reported by: Applause - Internal Team
Issue found when executing PR https://github.com/Expensify/App/pull/38023
Action Performed:

Go to workspace settings
Click on 'Members'
Select a contact & click 'Next' button

Expected Result:
Welcome message should have the new format
Actual Result:
Welcome message have the old format
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos
Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/35f784cb-47a8-4b88-a9b6-e201a8317b11
View all open jobs on GitHub

@mallenexpensify  FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

We think that this bug might be related to #wave-collect - Release 1

Good 👀 @rayane-djouah , thanks for tagging and linking.  Awaiting feedback from @Gonals

Hey @lanitochka17 I'm a bit unsure of what you were expecting in that box? Do you mind outlining a bit more information on what you think the issue is. I don't think we changes the contents of the welcomeNote, we just changed the notification/email that we send once it's done.

Pinged @Gonals in NewDot for 👀 on this and the PR

@Gonals gonna assign you to help catch your 👀
Added Needs Reproduction cuz I don't want to add to a project til I know what's up.

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

Looks like the expected result concerns to email sent to inbox. Email templates are decided at backend so this should be internal I guess.

Waiting for internal pickup. We have site issues and deploy freeze, I'll revisit next week.

Posted in #wave-collect and cross posted in #engineering  to try to get a volunteer
https://expensify.slack.com/archives/C036QM0SLJK/p1715735812026129
https://expensify.slack.com/archives/C03SSAQ7P/p1715735821061659

@Gonals back in a few days

Waiting for @Gonals return

I'm back!

@mallenexpensify, @trjExpensify, if I'm understanding this correctly, we basically want the same message to be sent for existing accounts and for new accounts, right?

if I'm understanding this correctly, we basically want the same message to be sent for existing accounts and for new accounts, right?

Yep, that's correct. We want to use the same message for existing and new accounts invited to workspaces in NewDot. 👍

Not overdue. In fact, this should be fixed. Closing


Line 6 is disturbing

Remove the line that says:
#todo: sparse polynomials?

Seriously, @aytekinar and I were thinking to try to implement support for sparse coefficients. In our forks we will make a branch and incrementally try to implement this.

What's disturbing about it. You seems to suggest that it is a legit TODO item so it should be kept there until it is implemented.


Implement sangita in Telugu

New language - Telugu

[ ] Find Data corpus Telugu language. Add links in the comments to this Issue

[ ] Implement tokenizer for Telugu

[ ] Create a lemmentizer for Telugu

[ ] Create a POS Tagger for Telugu.

I'd like to work on this

Corpus for telugu language can be found in this link.TDIL

Kindly look for the original source of this dataset.
TDIL tends to take open source licenses and make them closed source. So look for the original dataset for the resource. Also look for a few more datasets if possible.


api: Add UDPA resource names to SotW API, RouteAction, and ClusterStats

Signed-off-by: Mark D. Roth<EMAIL_ADDRESS>Commit Message: api: Add UDPA resource names to SotW API, RouteAction, and ClusterStats
Additional Description: Adds the new UdpaResourceName and UdpaResourceLocator fields to various additional places where it's needed in the API.
Risk Level: Low
Testing: N/A
Docs Changes: Included in PR
Release Notes: N/A
Use of UdpaResourceLocator in the SotW API will require using the client feature to tell the server to wrap the returned resources in the Resource message, as discussed in #13201.
The changes in RouteAction and ClusterStats are necessary to allow us to refer to a cluster that has a new-style name.
CC @htuch

I'm still a bit confused philosophically about naming here. We have various names across the APIs that are used to point at other objects (URL) and also name the object itself (URI). If I'm following your logic correctly, you're saying that a route lookup could resolve to a URL which could potentially match multiple URIs (e.g. if we have flexible context parameter matchers in the URL).

First, a terminology nit: When referring to a name, I think we should use the term URN, not URI.  URNs and URLs are both forms of URIs..
But back to your question, I think my reference to collections was actually misplaced here; I think that's actually orthogonal to the question of flexible matching.  I think that even if/when we add more flexible matching rules for context params, it will still need to be clear from a given URL whether it is referring to an individual resource or a collection of resources.  (If it refers to a collection resource type or the id ends in a /*, then it points to a collection; otherwise, it points to a single resource.)
I think the issue here is just about whether a URL with a particular set of context params can match a URN with a slightly different set of context params.  For example, imagine if we allowed a URL to contain a regexp for a context param value instead of an exact-match string.  If it's not requesting a collection, then the URL must still match exactly one URN, but the syntax of the URL and the syntax of the URN may not be exactly the same.  That's why I think all cases of referring to a resource (as opposed to providing a resource) should be URLs, not URNs.

Sticking with the route resolving to cluster example. If we have a URL with flexible context parameter matching, it might actually plausibly resolving to > 1 distinct cluster URN. How do we disambiguate?

I think that's something we'd have to define as part of any proposal to add flexible matching.  I don't think we need to fully define that now, but one approach we could take is to define some rules that dicate how "close" a given URN is to matching the URL, and we could say that the closest match wins.
Regardless of how the matching works, though, my main point here is that the reason we added URLs to the design in the first place was to have a different type for referring to resources vs. when naming a resource, so that we'd have flexibility to figure out this kind of thing.  The cases you're referring to are exactly the intended purpose of URLs.  If you are arguing against using them here, then I think you're actually arguing against having URLs in the design at all.  I'm really hoping we don't need to reopen that debate at this late stage. :)

I don't think I'm arguing against using them in the design, I'm just trying to grok what the concrete implications are, given we already have these referential relationships in Envoy today where there is a 1:1 match. We're moving from a model of very easy to reason about opaque strings with exact match to match/resolution of URL/URN when references cross resources. We're basically removing a proxy-local shortcut for referencing resources and going from 1:1 to 1:N. This makes the data model more complicated for users, but is fine for machines IMHO.
I'm OK with this, but it's a major design change in xDS. We should verify that other @envoyproxy/api-shepherds are good with this, given the impact.

I'm certainly happy to hear input from the other API shepherds, but I have to say that I really don't understand the concern with this particular PR.
I don't believe that this PR is actually introducing any 1:N mapping, because UdpaResourceLocator does not actually imply or allow anything like that; it is purely 1:1.  I understand the concern that it would be bad if we accidentally introduced 1:N mapping as part of some future design for flexible matching (and I do agree that we should do not do that!), but I think that's a concern to discuss if/when we have such a proposal, not something related to this PR.
I also don't think this PR is any sort of "major design change in xDS".  Note that you already previously added UdpaResourceLocator in a number of places in #11810, so if this is some sort of problem, the problem already exists.  All that's happening in this PR is to add it to places that weren't handled in the earlier PR.
I think our positioning here is exactly the same as it was when we agreed on the new naming scheme design: We're using different types for URLs and URNs, because URLs already support a couple of things that URNs don't (glob matches and directives), and because we wanted to allow for the possibility of adding other things in the future (such as flexible matching for context params).  Keeping this possibility open doesn't mean that we're commiting to do it; it just means that we have options as we move forward.

I think @htuch thinks this ResourceLocator can potentially point to multiple targets, which is very different than what we have today. @htuch is that right? If that is right, is it up to the impl to fail to load if that is the case in a location where that doesn't make sense.

Yes, this is what it boils down to. I'm alright with either defining the resolutions as most specific or failing if there are multiple matchers, I just want to make sure we're comfortable with this as a conscious decision. To Mark's point on this already being a feature of the new naming scheme, the existing anchor points for URLs are locations where we were explicitly performing a config source resolution and lookup, whereas the points in this PR are "internal" (e.g. a cluster action for a route), they assumed the existence of the resource effectively. Given how things are done with gRPC (where there are no internal references) and a move towards viewing the API as a true DAG with edges being URLs, I think these "internal" references are really a vestigial artifact and we should think of any reference to resources in terms of URL/URN.
In summary, I'm good with URL, wanted to make sure everyone is on the same page.

There is some discussion about a flat string vs. a structured impl, right? Is the summary there that we want the structured impl in the API even though we might store a flat string internally?

I think there is a strong argument for structured in DiscoveryRequest/Response, since it will facilitate efficient representation of things like context parameters. Mark and I chatted about this and a scheme like HPACK dynamic table might eventually make sense when context parameters become a major contribution to config size, so having some structure makes sense. The argument could also be made that structure then makes sense everywhere in the API.

Thanks for the discussion.  If I'm understanding correctly, the concern here is about the fact that Envoy deals with clusters as "internal" references and not real resource fetches, so the semantics could be different.  And since this is a different code-path, the semantics of that lookup could accidentally provide 1:N semantics instead of the intended 1:1 semantics.  Is that right?
I think the way I would model this would be to treat these as real references to resources that just happen to be pre-fetched by Envoy.  So the same matching rules (which do require 1:1 match) apply as they would to any other resource; it's just that Envoy is applying those matching rules against its pre-fetched list of resources instead of using the xDS client code (which may or may not already have the resource cached).
To say that another way, the fact that Envoy is pre-fetching the cluster resources can be considered an implementation detail; the matching rules should work exactly as if it was not implemented that way.  I think that property will be necessary in the long run  anyway, since I think we'll need to support on-demand CDS at some point.
Also, responding to something @mattklein123 asked:

I'm concerned it's going to be very painful to keep track of all cases of this. Even though it would also be painful, should we deprecate existing string clusters and replace with only ResourceLocator or a message that wraps a oneof between a flat string and a ResourecLocator?

I do think that it would help to have a more concrete plan for how we are going to migrate from old-style to new-style names.  For example, is it Envoy's intent to move completely to new-style names, deprecating the old-style names, or will it always accept both?
Also, for whatever interval the old- and new-style names co-exist, how exactly does that work?  I can think of two possible approaches:

We define a bi-directional conversion between old- and new-style names (e.g., representing the old name as the "id" part of the new-style name, percent-encoding characters like ?, and using an empty authority).  Internally, we store everything as a new-style name.  When a client talks to a given server, it will decide which form to send based on whether the server supports new-style names (which will have to be configured somewhere, maybe in the ConfigSource).  Servers that are handling this migration will expect to serve the same resource for a given name, regardless of whether the name is expressed as an old-style or new-style name.

We say that old- and new-style names basically exist in different namespaces, and there is no conversion between them.  This means that whenever we store a resource name internally, we need to store an explicit indication of whether it's an old-style or new-style name, so that we know how to encode it on the wire.  (Internally, we can continue to store names as simple strings if we want.  For example, we could basically automatically prefix all old-style names with "xds:", and new-style names will start with "udpa:", so we can tell which are which.  Note that the "xds:" prefix would not be visible to the API; it would just be an internal storage thing so that the code can tell how to handle it.)

I think option 2 makes more sense, because it does not require separtely configuring whether a given server supports new-style names; we can simply ask for whatever name we are told to ask for, whether it's old- or new-style, and the right thing will happen.  (The right thing will not happen if someone configures us to ask for a new-style name from a server that does not understand new-style names, but that's true regardless of how we do this.)
Anyway, a lot of this is not directly related to this PR, but I do think it would be useful to identify a more concrete migration strategy in order to answer some of these questions.

I will defer to @htuch on the details of internal implementation in Envoy, but I do think we are getting to the crux of the issue. My concrete preference is:

Make sure we are not going to cause issues in terms of Envoy implementation (I will defer to @htuch)
I think we should deprecate string clusters and replace them with a new message which has a oneof which contains either a string or resource locator (your (2) above)
Double check that all things in this PR should actually be replaced. Concretely, I don't think Node should change to a resource locator. The cluster string in Node is just a string, it's not a CDS resource.

I think we should deprecate string clusters and replace them with a new message which has a oneof which contains either a string or resource locator (your (2) above)

If we are actually going with (2), then I think that affects all resource types, not just clusters.  So does that imply that we want to add a message that contains a oneof for any resource name, and use that everywhere?  That would presumably also require changing all of the places where the new field was added in #11810.
Also, I think it may be worth first answering the question about whether we intend to eventually deprecate the old-style names or whether we intend both styles to continue to work indefinitely.  If we do intend to eventually deprecate the old-style names, then creating a new message containing a oneof and migrating everything to that seems like it will leave us in a bit of an ugly state, since we'd eventually be left with the new field in a sub-message in a oneof for no real benefit.

Concretely, I don't think Node should change to a resource locator. The cluster string in Node is just a string, it's not a CDS resource.

What is it actually used for?  If it's a cluster name and we move to a world where all clusters use new-style names, does it do any good at all for this to just be a string?
I'm happy to make that change; I just want to make sure that there are no implications we're missing here.

/cc stevenzzzz

After some discussion @stevenzzzz, I'm inclined to add to the proposal above the following:

Allow resources in the data model to have empty names.
If a client receives a resource with an empty name, we use the resource name from the transport when it was delivered on the client-side to populate the name (at least conceptually). In things like config dump, we will include the full xdstp:// name. Internally, in the proxy, we may use a more efficient representation.

This avoid repeating the xdstp:// string twice for every resource, allows more efficient wire optimization and simplifies some control plane proxy interactions.

From our offline discussion, it sounds like you mean that we would omit the name field that exists inside the data model protos (e.g., Listener.name, RouteConfiguration.name, Cluster.name, and the confusingly-named ClusterLoadAssignment.cluster_name), since the resource name would be sent in the Resource wrapper proto instead.
This sounds fine to me.  I actually think we should ultimately try to move away from the name fields inside of the individual data model protos -- they should ideally be empty whenever the Resource wrapper proto is used.

This sounds fine to me. I actually think we should ultimately try to move away from the name fields inside of the individual data model protos -- they should ideally be empty whenever the Resource wrapper proto is used.

I think this is my question also after reading this. If we can set the name to empty and everything works, why do we have the name at all. Is that a historical artifact that we can get rid of?

I think for config dump reasons etc. there was a push earlier in the API history towards having all resources being "self-describing". This meant that we had name fields. I think it's awkward and would best be replaced by a wrapper envelope like Resource. I think we'd want to be more prescriptive on using alt_stats_name or the moral equivalent.

I think for config dump reasons etc. there was a push earlier in the API history towards having all resources being "self-describing". This meant that we had name fields. I think it's awkward and would best be replaced by a wrapper envelope like Resource. I think we'd want to be more prescriptive on using alt_stats_name or the moral equivalent.

Yeah agreed. Either way we should carefully document all of this so it's clear.


SVG not being reflected while using in amp-list tag

When used outside of amp-list tag, it works fine. Code screenshot attached for reference.

This is related to https://github.com/ampproject/amphtml/issues/8214. As this is not related to amp-by-example, I am closing this bug.