Allow type annotation on catch clause variable

TypeScript Version:  2.6.1
const rejected = Promise.reject(new Error());

async function tryCatch() {
  try {
    await rejected;
  } catch (err: Error) { // TS1196: Catch clause variable cannot have a type annotation
    // Typo, but `err` is `any`, so it results in runtime error
    console.log(err.mesage.length);
  }
}

function promiseCatch() {
  rejected.catch((err: Error) => { // OK
    // Compiler error; Yay!
    console.log(err.mesage.length);
  });
}

This was discussed in #8677 and #10000. It was closed as "fixed" in #9999, but as far as I can tell neither of the issues was actually resolved. In either event, I'd like to make a case for allowing type annotations in catch clauses.
Especially with the introduction of downlevel async functions, I'd suggest that disallowing catch clause type annotations leads to less safe code. In the example, the two methods of handling the promise are functionally equivalent, but one allows you to type the error, and the other doesn't. Without writing extra code for the try/catch version (if (err instanceof Error) { or const e: Error = err or something), you'll get a runtime error that you wouldn't get with the pure Promise version.
The primary rationale for not allowing this is that any object can be thrown, so it's not guaranteed to be correct. However, most of the benefit of TypeScript comes from making assertions about your and other people's code that can't be strictly guaranteed (especially when importing JavaScript). And unless one would argue that the Promise catch function also shouldn't allow a type annotation on the error parameter, this argument seems to make very little practical sense.
I believe one of the other arguments against is that it might be confusing, as it looks like the typed exception handling you might see in other languages (e.g., Java), and folks may think the catch will only catch errors of the annotated type. I don't personally believe that's a legitimate issue, but if it really is I'd propose at least allowing a catch (err as Error) { syntax or similar as a way of emphasizing that it's a type assertion.
If nothing else at all, it seems that there should be a way to trigger a warning (similar to an implicit any warning) when using an untyped err directly within a catch block.

I do like the idea of catch (err as Error) because, damn it, if you were going to cast it anyway, maybe we should make your life a little easier.

I agree, it is possible to safely annotate the error variable in many situations. Older discussion in https://github.com/Microsoft/TypeScript/issues/8677#issuecomment-220495646.
It also true that it's easy to misunderstand/abuse, but then so are type annotations in general, and people just do the cast in the first line of the catch block anyway.

@jaredru code like

function promiseCatch() {
  rejected.catch((err: Error) => { // OK
    // Compiler error; Yay!
    console.log(err.mesage.length);
  });
}

is dangerous and misleading. It only passes the type checker because the parameter of the catch method's callback is declared to be any. At best this is a disguised type assertion.
Promise.reject(NaN).catch((e: Error) => e.message.length);

I disagree. It is no more dangerous or misleading than const foo: Foo = JSON.parse(input);, which is a very reasonable pattern. It represents our expectations and assertions about the code we're dealing with.
The irony of your example is that it throws a TypeError, which gets at my underlying point. Aside from contrived examples, the expected currency for errors is Error. A simple search through the TypeScript repo itself shows that catch (e) virtually always assumes the e to be an Error. That doesn't make it "right", of course, but it highlights the realistic expectation of a large, real-world project that (presumably) reflects an understanding of the balance between correctness and pragmatism, particularly as it relates to TypeScript.

I disagree. It is no more dangerous or misleading than const foo: Foo = JSON.parse(input);, which is a very reasonable pattern. It represents our expectations and assertions about the code we're dealing with.

I don't think that is a good pattern either.
const foo = <Foo>JSON.parse(input);

Is much clearer as to intent.
As for the example, contrived as it may be, it doesn't matter what gets thrown (TypeError or not) because it is thrown from the catch causing an unexpected failure.

Sorry, consider it a typo. <Foo>JSON.parse(s) is a common pattern but is as unsafe as .catch((e: Error) => {. Both treat an any as another type with no guarantee of correctness beyond the developer's word.

it doesn't matter what gets thrown (TypeError or not) because it is thrown from the catch causing an unexpected failure.

Of course it matters. That the runtime throws an Error for all runtime errors strengthens the position that it's reasonable to expect the argument to catch to be an Error. This expectation is common in real world codebases--including, but certainly not limited to, TypeScript's.

My 2ct: If there is any party that should be able to type the exception it should be the called function, not the caller. I've seen codebases that throw strings everywhere or nothing at all (undefined). Just declaring that the type is Error doesn't make it more type safe, because you never know what other functions the function you are calling may call into and what things those nested functions throw - Errors are not strongly typed and not part of the interface contract. Maybe you know it know but in the next patch version of some transient dependency it could change. So the only correct way to mirror the runtime JS is to start with any and narrow it down manually
if (err instanceof MyError) ... else throw err

switch (err && err.code) {
  case 'ENOENT': ...
  case 'EPERM': ...
  default: throw err
}

const isExpectedHTTPError = (err: any): err is HTTPError => err && typeof err.status === 'number'

if (isExpectedHTTPError(err)) {
  res.status(err.status).set(err.headers || {}).send(err.body || '')
} else {
  res.status(500)
}

And if the check doesn't pass, rethrow the error. If you don't and your usage of the error doesn't directly produce some kind of TypeError then you might swallow unexpected programming errors and create a debugging nightmare.
Is this more code to write? Yes. But does that justify a new feature to make the unsafe version easier to write?

My argument is for pragmatism. One of the stated non-goals of TypeScript's design is to "Apply a sound or 'provably correct' type system. Instead, strike a balance between correctness and productivity." Yes, I could get something besides an Error, but in reality--in the projects I'm working in--I won't.
As in the TypeScript codebase itself (1 2 3 4 5), it is very often safe and reasonable to assume that the thrown value is not, in fact, undefined (or null, or a string, ...) and is with almost certainty an Error. I would simply like the compiler to help me out in this case without additional code or runtime overhead.

TypeScript language is already the best I've ever worked.
If you type the catch clause this will make much better.

I would certainly love a exceptionTypeUnknown flag that makes all exception types (and Promise rejections) type unknown instead of any. That would definitely make the error handling safer as opposed to allowing any annotation. Although this could also be solved by writing a TSLint rule that forces them to always be declared unknown, if we had this annotation.

need this

It's not productive for me to guess which properties are on the Error and have to look it up. Please implement.

@DanielRosenwasser

I do like the idea of catch (err as Error) because, damn it, if you were going to cast it anyway, maybe we should make your life a little easier while keeping it explicit.

I think it would be awesome !

I like @Andrew5569 's Idea, but want to have the ability to define it like I want:
try {
  // ...
} catch (err: unknown) {
  // ...
}

try {
  // ...
} catch (err: Error) {
  // ...
}

try {
  // ...
} catch (err: AWS.AWSError) {
  // ...
}

@Shinigami92 It will never be possible. TS is being transcoded to Javascript for execution. And Javascript does not implement the type.
What would be possible is
try {
  // ...
} catch (err: unknown | Error | AWS.AWSError) {
  if (err instanceof Error) {
    // ...
  } else if (err instanceof AWS.AWSError) {
    // ...
  } else {
    // ...
  }
}

@GabrielDelepine sorry you get me wrong. I only want to assume the type, not to define it.

Yeah, please just fix this :-)

The fact that this is not allowed is both inconsistent and inconvenient:
try { ... } catch(error: Error) { ... }

Especially considering that this is allowed:
promise.catch((error: Error) => ... )

You can't argue that allowing it would be dangerous, just because people might incorrectly assume the type is an error, when it can in fact be anything. If someone were to assumes that, they will just cast to Error anyway, which would be just as much of an error. And this could be said for any type used anywhere, so there's really no valid reason to treat errors as a special case.
Not to mention, that in the vast majority of cases, assuming that the error will in fact be an Error, is actually perfectly reasonable. Please be a bit more pragmatic about those things, especially when it is something that gets in our way on a daily basis.

While you are at it, please also add an optional reject type to promises:
interface Promise<TResolve, TReject = any>

Because yes, we can, and often do, guarantee that an async function will always reject with a certain type - and arguing against that is kinda ridiculous. We can just wrap everything in the function in a try block, and throw a custom exception if we end up in the catch.
For example, we have an ApiClient, which guarantees that if it rejects, it will always be with an ApiError. It's extremely annoying that we can't declare that fact in our types, but instead have to manually cast the errors everywhere we use it. That's neither practical, nor type safe.

These would not be breaking changes, they would provide a lot of value, and people have been asking for it for years - so please just implement it :-)

I would love it if it were implemented with a new Throws<TReason = Error> type and Promise<TResolve, TReason> for Promises.
Ability to define expected error types
function foo(): Throws<Error> { // should be inferred
    throw new Error()
}
try {
    foo()
} catch (error) { // <-- Error
    // noop
}

But you should be able to override it with anything
function bar(): Throws<number> { // should be inferred
    throw 42;
}
try {
    bar()
} catch (code) { // <-- number
    // noop
}

Multiple expressions within a try block?
try {
    foo();
    bar();
} catch (reason) { // <-- Error | number
    // noop
}

How about Promises?
async function baz(): Promise<never, TReason = Error> { // should be inferred
    Promise.reject(new Error())
}
async function qux() {
    try {
        await baz()
    } catch (err) { // <-- Error
        // noop
    }
}

Checklist
My suggestion meets these guidelines:

[x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
[x] This wouldn't change the runtime behavior of existing JavaScript code
[x] This could be implemented without emitting different JS based on the types of the expressions
[x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
[x] This feature would agree with the rest of TypeScript's Design Goals.

@jedmao
Why do you think your approach does not accept this?
function foo(): Throws<Error> { // should be inferred
    throw new Error()
}
try {
    const err: Throws<Error> = foo()
} catch (error) { // <-- Error
    // noop
}

I also think that Throws<TReason = Error> introduces a breaking change when it is inferred (and it should).
Throws<TReason = any> would not do this.

@Shinigami92 good point on the breaking change.
I don't think your example is any different than the never type.
function foo(): never {
    throw new Error();
}
try {
    const err: never = foo()
} catch (error) { // <-- any (existing implementation)
    // noop
}

+1 for having type-check flow through try/throw/catch code. This is driving me nuts currently for adopting async/await with try/catch where now I have no way of typing the error (with promises I could at least add a type).

At the very least, you should be allowed to use a type that extends the Error type, as is done with @types/mysql.
                try {
			// perform database fetch
		} catch (error: MysqlError) {
			if (error.code === 'ER_DUP_ENTRY') {
				return;
			} else {
				throw error;
			}
		}

The catch (err as Error) syntax seems related to (or is possibly a subset of) #10421

Is there any hope for this feature? (or something better)

I think a better solution would be to allow typed catch blocks to be transpiled to if (typecheck) else throw:
try {
  doSomethingA();
  doSomethingB();
  doSomethingC();
} catch (e: string) {
  console.error(e);
} catch (e: number) {
  console.error('example error message:', e);
} catch (e: Error) {
  console.error(e.message);
}

this could then be transformed into:
try {
  doSomethingA();
  doSomethingB();
  doSomethingC();
} catch (e) {
  if (typeof e === 'string') {
    console.error(e);
  } else if (typeof e === 'number') {
    console.error('example error message:', e);
  } else if (e instanceof Error) {
    console.error(e.message);
  } else {
    throw e;
  }
}

@zzzzBov
How about doing something similar except using instanceof?  That way, it could mimic aspects of other languages that folks may be used to.  If so, it would be nice if TS could at least warn if ordering of catch clauses caused one clause to become unreachable because a clause with a more general type is ahead of it.

@treynash

How about doing something similar except using instanceof?

I don't fully understand what you mean here. Are you saying it should not use instanceof at all or that it should only use instanceof?

it would be nice if TS could at least warn if ordering of catch clauses...

I definitely agree that a warning for unreachable code would be useful, however I'm not sure if that necessarily belongs in TS or whether that task should be delegated to a linter. I don't feel strongly one way or the other 🤷♂.

@zzzzBov
I should have been more precise.  What I meant was that instead of if (typeof e === 'Foo'), it could be something like if( e instanceof Foo).  That way, it should match anything that extends Foo as well as instances of Foo, which is what I meant by when I referenced the way some other languages work.

@treynash ah ok, Then I should point out that my example uses instanceof for Error. It can't use instanceof for String or Number types because primitive values are not instances.

@zzzzBov
Yep, indeed.  And I would imagine one would also want to be able to discern based on things like union and intersection types.

It's more verbose, but what about a syntax like c#'s exception filters: catch (identifier) when (expression) { statements; }, where identifier is in-scope in expression, and the same type guard semantics apply as in a regular if statement.
Maybe at that point you might as well just stick an if statement in your regular catch block, I suppose

Now that ESLint 7 is out with a new set of no-unsafe-* and the restrict-template-expressions rules, the untyped catch clauses make the lint output extremely noisy. The following short snippet contains a whopping 2 ESLint errors just because we are not easily able to assert that e is indeed an Error:
try {
    throw new Error("nope");
} catch (e) {
    console.log(`Had error: ${e.message}`);
    // ^-- Two lint errors here:
    // Invalid type "any" of template literal expression. eslint [@typescript-eslint/restrict-template-expressions]
    // Unsafe member access .message on an any value. eslint [@typescript-eslint/no-unsafe-member-access]
}

I'm fine with allowing any in template literals, but the no-unsafe-member-access rule is actually very helpful. Since ESLint also makes sure that no non-Errors are thrown, I do like @DanielRosenwasser's idea of casting explicitly (catch (e as Error)).

Up .message should be available when you're in catch clause

I can't believe this is still an open issue. Seems like a very obvious feature for TypeScript. Can't wait to see it!

Now that ESLint 7 is out with a new set of no-unsafe-* and the restrict-template-expressions rules, the untyped catch clauses make the lint output extremely noisy. The following short snippet contains a whopping 2 ESLint errors just because we are not easily able to assert that e is indeed an Error:
try {
    throw new Error("nope");
} catch (e) {
    console.log(`Had error: ${e.message}`);
    // ^-- Two lint errors here:
    // Invalid type "any" of template literal expression. eslint [@typescript-eslint/restrict-template-expressions]
    // Unsafe member access .message on an any value. eslint [@typescript-eslint/no-unsafe-member-access]
}

I'm fine with allowing any in template literals, but the no-unsafe-member-access rule is actually very helpful. Since ESLint also makes sure that no non-Errors are thrown, I do like @DanielRosenwasser's idea of casting explicitly (catch (e as Error)).

My ugly workaround for the new unsafe-* rules is just
catch (ex) {
	const error = ex as Error;
        logger.info('my error is meh', { error });
        logger.debug(error.message);

I prefer this over disabling the rule for the line/file. still meh.

This is more-or-less addressed in https://github.com/microsoft/TypeScript/pull/39015, which will land in 4.0.

@guidobouman I agree that #39015 is a small step towards this feature but in no way addresses it. That merge request is mostly does nothing to address #20024 IMO because it requires unknown or any. Even if that merge request did allow for types, it wouldn't solve this issue unless it caught only those types in the compiler.
Something like this is what we're looking for:
try {
  doSomethingA();
  doSomethingB();
  doSomethingC();
} catch (e) {
  if (typeof e === 'string') {
    console.error(e);
  } else if (typeof e === 'number') {
    console.error('example error message:', e);
  } else if (e instanceof Error) {
    console.error(e.message);
  } else {
    throw e;
  }
}

Right now it's a bit verbose. What if I want to catch only one exception and have it to continue to throw if it's not the one I wanted?
Currently I have to:
  try {
    something();
  } catch (err) {
    if (err instanceof SomeError) {
      doSomethingElse();
    } else if (err instanceof AnotherError) {
      doAnotherThing();
    } else if (err instanceof YetAnotherError) {
      yetAnotherThing();
    } else {
      throw err;
    } // Else //
  } // Try, Catch //

What I wouldn't mind seeing is:
  try {
    something();
  } catch (err: SomeError) {
    doSomethingElse();
  } catch (err: AnotherError) {
    doAnotherThing();
  } catch (err: YetAnotherError) {
    yetAnotherThing();
  } // Try, Catch //

Which would just output what was above. Would save me some typing and be pretty convenient like in Java and other languages.

Multiple catches are not a pattern that is known to JavaScript. Feels weird to change the structure of the language.

Multiple catches are not a pattern that is known to JavaScript. Feels weird to change the structure of the language.

Neither is private/public constructor fields, or class fields, etc etc.. Need I go on?

Neither is private/public constructor fields, or public/protected/private class fields, etc etc.. Need I go on?

Microsoft did those features in the earlier days of TypeScript, but not long after decided to more closely align with the ECMAScript specification, which they have done extremely well since. Their design goals are clear and they will not be introducing features like this that deviate from the specification.

The whole point of Typescript is to type check as a transpiler to JavaScript, which is why we're not using vanilla JavaScript.
If you feel a feature could benefit many others, but dismiss it because it "feels weird" but makes sense in other languages and is successful and many use those features, why do you raise your voice against it?
We use Typescript for the sugar that it provides. Decorators? Typed variables? Type checks? Generics? Function overloads? Don't forget the new variadic tuple types in typescript 4.
The point is, it's meant for a more Java or OOP programming that helps you have more typed and safe control over your code. It will always run JavaScript, that's a given (as I think they stated that as a mission goal).
But we shouldn't feel weird when using a language that transpiles into another language if there's differences.. there will always be differences, and that's good.. Typescript !== JavaScript.

but not long after decided to more closely align with the ECMAScript specification

Can you provide a source reference to this decision?

I don’t use TypeScript for the sugar it provides. The sugar is the very thing that is not in their design goals today. It’s more about confidence and type safety and all the type information disappears at compile time, as it should. Decorators are another one of the earlier features they implemented before ironing out the design goals, so it’s irrelevant to your argument.

I don’t think you’re going to get any traction requesting to change the design goals, BTW.

Can you provide a source reference to this decision? I can't seem to find it.

See Goals here: https://github.com/microsoft/TypeScript/wiki/TypeScript-Design-Goals

Can you provide a source reference to this decision? I can't seem to find it.

See Goals here: https://github.com/microsoft/TypeScript/wiki/TypeScript-Design-Goals

How would the multiple catch clause go against the mission goals exactly?

they will not be introducing features like this that deviate from the specification.

Are you speaking on behalf of Microsoft? Are you on the Microsoft team? Who's specification? JavaScript? Well introduce types and immediately it's against JavaScript specification.. what's your point?
In fact, I think this "aligns" with goal #1 very nicely.. "1. Statically identify constructs that are likely to be errors."

I’ve been following the project since the beginning and I’ve seen and even started enough of these types of conversations to know where Microsoft stands on these points. I didn’t like it at first, but now I 100% get it and respect their design goals.
There are other ways to statically identify constructs that are likely to be errors without adding new JS syntax that requires not just throwing away type information at compile time, but also modifying the AST.
I think the best way to think about it is if you throw away all the type information, the JS that is left should ideally be runnable, which might also require shims, but unable without modification to the AST.

Again, you seem to think this is against the design goals? Many changes in the language including types, change the AST (Abstract Syntax Tree). Have you ever worked on a compiler? I have (if anyone is curious I did a bit of work on the Free Pascal compiler).
I referenced what the JavaScript could would be here: https://github.com/microsoft/TypeScript/issues/20024#issuecomment-671116935
What the transpiler would emit. It's safe, simple, straight forward. Requires no shims, and does not somehow modify JavaScripts AST. :)

The first non-goal states the following:

use the behavior of JavaScript and the intentions of program authors as a guide for what makes the most sense in the language.

If this were Python, your request here would make perfect sense, as Python already supports it like so:
try:
  print(x)
except NameError:
  print("Variable x is not defined")
except:
  print("Something else went wrong")

But this is not Python.

Non-goal #5 states the following:

emit different code based on the results of the type system. Instead, encourage programming patterns that do not require run-time metadata.

You can see the problem pretty quickly if you try to transpile your proposal into JS. My best attempt would be the following:
  try {
    something();
  } catch (err) {
    if (err istanceof SomeError) {
      doSomethingElse();
    } else if (err instanceof AnotherError) {
      doAnotherThing();
    } else if (err instanceof YetAnotherError) {
      yetAnotherThing();
    }
  }

Notice how this now "requires runtime metadata"; furthermore, what should happen if SomeError were not a class, but an interface? What then? TS is not going to be doing run-time type checking. Remember that it's not just errors that can be caught, as you can throw anything.

Again, you seem to think this is against the design goals?

It's against the non-goals, as stated above. Also, you're looking for "sugar," which is not in the design goals.

Many changes in the language including types, change the AST (Abstract Syntax Tree).

Sure, but my point was "after the types have been removed."

Have you ever worked on a compiler?

I translated the entire PostCSS project into TypeScript, but I don't see how that's relevant.

What the transpiler would emit. It's safe, simple, straight forward. Requires no shims, and does not somehow modify JavaScripts AST. :)

It's only simple in your exact use case and only if all the types are classes or functions (instanceof-able). It also modifies the AST, for sure. I'm not sure why you think that it doesn't, as it has to translate catch clauses into separate if statements.

Notice how this now "requires runtime metadata"

It requires no runtime metadata, as you can see. :)
I asked if you worked on a compiler because you mentioned it modified the AST, which most real typescript language changes do? Arrow functions in fact translate to "function ()" and surrounds by an enclosure if I'm not mistaken? Either way, features like that drastically change the emitted AST.. Like most of the features when they target ES5.
Anyway, I'm done debating on this. If others would like it, they can voice.

Notice how this now "requires runtime metadata"

It requires no runtime metadata, as you can see. :)

It absolutely does. The instanceof checks are runtime checks. And there is no way to check these statements against and interface or type that is not a class or function.

I asked if you worked on a compiler because you mentioned it modified the AST, which most real typescript language changes do? Arrow functions in fact translate to "function ()" and surrounds by an enclosure if I'm not mistaken?

Only to support older targets, but you are mistaken if you are talking about compiling to a modern target. Aside from access modifiers and decorators, you should be able to compile ESNext TS into ESNext JS by simply removing the type nodes from the AST.

Have you ever worked on a compiler? I have (if anyone is curious I did a bit of work on the Free Pascal compiler).

Hey all, let's please be respectful in tone - this can come off as intimidating.
@cyraid Even if this is a syntactically-driven construct (i.e. only entities that resolve to both a value and a type are allowed in those catch clauses),

it is very confusing to reuse : EntityName for anything other than erasable type annotations that have no runtime behavior
it conflicts, or competes, with a potential future ECMAScript syntax.

On TypeScript, we intentionally aim not to add new constructs that have any sort of runtime behavior at all if they are not part of ECMAScript, and that stance has hardened over the years.

Sorry, my answer was a bit short. Sure, they are additions, but not structurally. Most additions can be removed without transpilation. I'm a fan of terser forms of expressing a goal. But not when it changes the mental flow of the language, and might possible collide with the development of EcmaScript itself. That's also reflected in their design goals, particularly:

Avoid adding expression-level syntax.

See https://github.com/microsoft/TypeScript/issues/18500#issuecomment-329834072 as a reference.

I wonder when will this issues be implemented?

For now, you can do this with type casting:
  /**
   * Add a new waitlist entry.
   *
   * @return  {Promise}
   */
  async addWaitlist(): Promise<void> {
    try {
      const response = await this.$waitlistRepository.AddWaitlist(
<EMAIL_ADDRESS>      )
      console.log(response.message)
    } catch (e) {
      const error = <ErrorAPIResponseInterface>e
      console.log('RESULT', error.message)
    }
  }

The error constant has intellisense-enabled for your API error custom response.

anything new on this ?

What is the status on this important feature?

I'm fine with allowing only a single catch so as not to change the behavior of javascript, but I think it's perfectly in alignment with TypeScript goals to allow for the specification of an error type.
Just like TypeScript doesn't enforce types during runtime for arguments, it would also not enforce types during runtime for exceptions.
I do hope however that emascript adds support for catching multiple types of exceptions, syntactically, like java at some point.

Addition to @kabaluyot 's suggestion, here is how I work around this
try {
  doSomething();
} catch (error) {
  const { status, message, errors, data } = error as MyError;
}

In addition, you guys should allow multiple catch clauses that catch different types of exceptions--just like every other programming language supports, like so:
  try {
      ...
  } catch (err: RdError<Server>) {
     // do stuff
  } catch (err: RdError<Client>) {
     // do other stuff
  } catch (err) {
     // catch all
  }

That would be awesome! Just sayin.
I'm sure we'll get there one of these days but who knows how long it will take to get there in JavaScript/TypeScript...

We already discussed this earlier
This will never be made from TS side
The typedefs are stripped away by using tsc and that would leaf only
try {
    ...
} catch (err) {
   // do stuff
} catch (err) {
   // do other stuff
} catch (err) {
   // catch all
}

How will you distinguish between the three different catch err? You see, it's impossible... 🤷
So please go to tc39 and propose something, but it's not TS related

This is now a real pain in typescript 4.4 with --strict, where exceptions are typed as unknown instead of any in the catch block.
I‘m not really happy with any solution I have seen, but will probably go with what @kabaluyot wrote.

catch (error: any) is not allowed by @typescript-eslint/no-explicit-any
catch (error: MyExceptionType) is forbidden by “TS1196: Catch clause variable type annotation must be ‘any’ or ‘unknown’ if specified.”

I would prefer to do the latter. Multiple catch blocks should be out-of-scope.

Still see no reason exceptions cannot be typed like most other languages.
catch (e) ->  catch (e: MyError) { ... } would make typechecking rejected promises using try/catch infinitely easier than adding another line to cast an untyped e like const error = <MyError>e; or const error = e as MyError.

Sorry to say, but you guys are wrong. Some years ago I was on the same track like you. And it does nothing to do with TypeScript, but with JavaScript and expectations. You can't just say catch (error: MyExceptionType) because, well, this wouldn't be true. It's like wrong typing. Wrong typing is worse than no typing at all, because you expect typings to be true. What should TypeScript do, if you expect MyExceptionType, but got IDontCareAboutDownstreamCodeExcpetion? Should TypeScript ignore it? How should TypeScript do that for types or interfaces? Does it make click? Therefore I think that any and unknown as default are best values, which TypeScript can offer atm. And yes, this is'nt convenient, but it reflects TyoeScript's type safety and limits.
@DanielRosenwasser has described the challenge short and precise in his comment.
I think it would be better to lock this discussion, because there are no new conceptual proposals, but +1 only. It can be unlock again, if there are relevant news from tc39.

Nothing yet?

Therefore I think that any and unknown as default are best values

@Bessonov I agreed with everything you said up to this point.  By the points you've given, then any shouldn't be allowed either!  any is even worse than MyExceptionType!
My real issue with it is its inconsistency.
If I were the TypeScript dictator, I would allow no type annotations in catch clauses.  However, since any is worse than any other annotation, if you allow any I think it makes sense to allow other types too

You could just cast to Error inside the catch:
try {
  // do something
} catch (error) {
  console.error(error as Error);
}

In TypeScript 4.4+, those errors are now unknown in the catch. Or, you just type check like they do in the example here: https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#use-unknown-catch-variables

@Shinigami92

How will you distinguish between the three different catch err? You see, it's impossible... 🤷
So please go to tc39 and propose something, but it's not TS related

It's possible. You can check error type inside catch and re-throw error if there was no intend to catch it. Just as suggested in other comments from @zzzzBov and from @cyraid.
So with this solution catch will work just as in most typed languages.
Actually it looks like a good solution. Or is there something wrong with it?

This is now a real pain in typescript 4.4 with --strict, where exceptions are typed as unknown instead of any in the catch block.
I‘m not really happy with any solution I have seen

@bodograumann As of TypeScript 4.4 you can set useUnknownInCatchVariables=false which restores the old any behavior. It's not ideal, but nobody was complaining about this in TypeScript 3.x.  (In fact the TypeScript compiler itself is currently building this way -- if you set useUnknownInCatchVariables=true in src/tsconfig-base.json, the build fails.)

A nice way to safely unwrap your unknown error is to use instanceof (as mentioned upthread)
catch (e) {
    if (e instanceof Error) {
       // type-safe, compiler can help you in this block
        console.log(e.message);
    }
    else {
        // optionally
        throw e;
    }
}

You're explicitly handling all possibilities, so it's type-safe.
I think catch variable as unknown is a good default as it encourages you to think about the values that variable can take.
It may make sense to have a tsconfig flag to allow catch (e: Error) to ease the pain for some users however.

Another (type safe) option is to allow to safely cast to something like:
} catch (e: MaybeError) {

where MaybeError is some custom type defined similar to:
type MaybeError = null | undefined | { message?: unknown; stack?: unknown; };

I.e. allow e: MaybeError if MaybeError is null, undefined, or is an object with all props being optional and of types unknown. In this case, it still allows to do lots of things (checking for props being present, checking for equality like e?.code === "some" etc.), but at the same time, does not compromise type safety.
This would work, because, when MaybeError is defined the above way, there is no JS thing which would not match it. Even null or a plain string or a number thrown will still match.


SALV: Download with initial layout

Hello,
in addition to ticket #745 one point to discuss.
If the initial layout is set with this statement lo_layout->set_initial_layout( 'ABCD' ) it is not handled in the method ZCL_EXCEL_CONVERTER_SALV_TABLE->LOAD_DATA
This leads to wrong output.
Here is a test report:
issue_report.txt
I changed this by adding ls_vari-variant   = lo_layout->get_initial_layout( ). in line 38 of the method.
What do you think about it?
thanks and regards
Matthias

I tested in 7.57, that sounds good. Would you mind proposing a pull request please?
Thank you!
Just a few comments in case we need it in the future:

"Line 38" corresponds to inserting your line right below line 78 in this code:
https://github.com/abap2xlsx/abap2xlsx/blob/9a424149fd4967fa2a7a90a8f1e594973c6a62db/src/zcl_excel_converter_salv_table.clas.abap#L72-L82
To reproduce:

Create test program below (few changes done to your example code)
Run it with ALV_ONLY option activated, to show the ALV only (it doesn't run abap2xlsx).
Save any layout to ABCD (e.g. change the order of columns)
Run again with ALV_ONLY option deactivated to generate the Excel file

It creates an empty Excel file:

Unhide the first columns, they are shown in wrong order:

After the fix you proposed, it shows correctly, columns are ordered according to the variant:

Test program:
REPORT.
PARAMETERS alv_only AS CHECKBOX DEFAULT 'X'.
CONSTANTS gc_save_file_name TYPE string VALUE 'issue_1119.xlsx'.
INCLUDE zdemo_excel_outputopt_incl.

START-OF-SELECTION.
  SELECT * FROM sbook UP TO 15 ROWS
  INTO TABLE @DATA(gt_sbook).
  TRY.
      cl_salv_table=>factory(
        IMPORTING
          r_salv_table = DATA(lo_salv)
        CHANGING
          t_table      = gt_sbook[] ).
      DATA(layout) = lo_salv->get_layout( ).
      layout->set_key( VALUE salv_s_layout_key( report = sy-repid ) ).
      layout->set_default( abap_true ).
      layout->set_initial_layout( 'ABCD' ).        "INITIAL LAYOUT
      layout->set_save_restriction( if_salv_c_layout=>restrict_none ).
      IF alv_only = abap_true.
        lo_salv->display( ).
      ELSE.
        cl_salv_bs_runtime_info=>clear_all( ).
        cl_salv_bs_runtime_info=>clear_all( ).
        cl_salv_bs_runtime_info=>set(
          EXPORTING
            display  = abap_false
            metadata = abap_true
            data     = abap_true ).
        lo_salv->display( ).
        DATA(excel)     = NEW zcl_excel( ).
        DATA(worksheet) = excel->get_active_worksheet( ).
        NEW zcl_excel_converter( )->set_option( VALUE #(
            filter           = abap_true
            subtot           = abap_true
            hidenc           = abap_true
            hidehd           = abap_false
            conv_exit_length = abap_true ) ).
        worksheet->bind_alv(
            io_alv   = lo_salv
            it_table = gt_sbook  ).
        lcl_output=>output( excel ).
      ENDIF.
    CATCH cx_root INTO DATA(lx).
      MESSAGE lx TYPE 'I' DISPLAY LIKE 'E'.
  ENDTRY.

Hi,
thanks for your answer. You get me puzzled, but I found the problem again.
Following steps are additionally done:

create layout ABCD and set default flag
create layout DCBA
run the report with set_initial_layout( 'DCBA' ).

The output will be with the layout ABCD.
If the variable ls_vari-variant is initial, the function module LT_VARIANT_LOAD loads the default variant ( done because of layout->set_default( abap_true ). ).
Could you cross check again.
thanks
Matthias

Sorry, I meant that I reproduced the bug in 7.57, and your fix sounds good ;-)
So, would you mind proposing a pull request please?


Fix ingress node validation

Fixes #174
The problem is that the conditional check that was gating ingress node validation evaluates to true when the ingress section is present in the file. Specifically this: p.Ingress.Nodes != nil

Good catch, how did the integration tests not catch this?

The plan files that do not have ingress defined in integration tests are missing that section entirely, instead of having it with expected count = 0.

Gotcha

LGTM

Merging as this is caused by the known issue with kube-dns.


cluster not found by elastic, while running jdbc script

Hi, i am trying to use the e-jdbc for its purpose, to move data from mysql to elastic. However it is not going too well. I have an error that will soon follow. I am running a homestead setup.
From other posts, I saw you instructed to change port to 9300. is this in the jdbc conf only or the elastic port it self as well? as my elastic connects and responds to 9200. (I changed to 9300 specifically and my elastic died out.)
jdbc settings:
`
#!/bin/sh
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
bin=${DIR}/../bin
lib=${DIR}/../lib
echo '
{
"type" : "jdbc",
"jdbc" : {
"url" : "jdbc:mysql://localhost:3306/hellopeterdb",
"statefile" : "statefile.json",
    "user" : "homstead",
    "password":"secret",
    "sql" : [{
            "statement": "select * from reviews "}
        ],
    "index" : "hpindex",
    "type" : "reviews",
    "metrics": {
        "enabled" : true
    },
    "elasticsearch" : {
         "cluster" : "elasticsearch",
         "host" : "localhost"
    }
}

}
' | java
-cp "${lib}/*"
-Dlog4j.configurationFile=${bin}/log4j2.xml
org.xbib.tools.Runner
org.xbib.tools.JDBCImporter
`
Error:
[17:38:57,065][ERROR][importer.jdbc            ][pool-3-thread-1] error while processing request: no cluster nodes available, check settings {autodiscover=false, client.transport.ignore_cluster_name=false, client.transport.nodes_sampler_interval=5s, client.transport.ping_timeout=5s, cluster.name=elasticsearch, flush_interval=5s, host.0=localhost, max_actions_per_request=10000, max_concurrent_requests=2, max_volume_per_request=10mb, name=importer, port=9200, sniff=false} org.elasticsearch.client.transport.NoNodeAvailableException: no cluster nodes available, check settings {autodiscover=false, client.transport.ignore_cluster_name=false, client.transport.nodes_sampler_interval=5s, client.transport.ping_timeout=5s, cluster.name=elasticsearch, flush_interval=5s, host.0=localhost, max_actions_per_request=10000, max_concurrent_requests=2, max_volume_per_request=10mb, name=importer, port=9200, sniff=false}         at org.xbib.elasticsearch.helper.client.BulkTransportClient.init(BulkTransportClient.java:164) ~[elasticsearch-helper-<IP_ADDRESS>.jar:?]         at org.xbib.elasticsearch.helper.client.ClientBuilder.toBulkTransportClient(ClientBuilder.java:113) ~[elasticsearch-helper-<IP_ADDRESS>.jar:?]         at org.xbib.elasticsearch.jdbc.strategy.standard.StandardSink.createClient(StandardSink.java:348) ~[elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.elasticsearch.jdbc.strategy.standard.StandardSink.beforeFetch(StandardSink.java:100) ~[elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.elasticsearch.jdbc.strategy.standard.StandardContext.beforeFetch(StandardContext.java:183) ~[elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.elasticsearch.jdbc.strategy.standard.StandardContext.execute(StandardContext.java:164) ~[elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.tools.JDBCImporter.process(JDBCImporter.java:199) ~[elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.tools.JDBCImporter.newRequest(JDBCImporter.java:185) [elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.tools.JDBCImporter.newRequest(JDBCImporter.java:51) [elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.pipeline.AbstractPipeline.call(AbstractPipeline.java:50) [elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at org.xbib.pipeline.AbstractPipeline.call(AbstractPipeline.java:16) [elasticsearch-jdbc-<IP_ADDRESS>.jar:?]         at java.util.concurrent.FutureTask.run(FutureTask.java:266) [?:1.8.0_101]         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [?:1.8.0_101]         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [?:1.8.0_101]         at java.lang.Thread.run(Thread.java:745) [?:1.8.0_101]
Current Elastic set up: please note my network host setting, local,site i am thinking this might be an issue?

======================== Elasticsearch Configuration =========================

NOTE: Elasticsearch comes with reasonable defaults for most settings.
Before you set out to tweak and tune the configuration, make sure you
understand what are you trying to accomplish and the consequences.

The primary way of configuring a node is via this file. This template lists
the most important settings you may want to configure for a production cluster                                                                                .

Please see the documentation for further information on configuration options:
<http://www.elastic.co/guide/en/elasticsearch/reference/current/setup-configur                                                                                ation.html>

---------------------------------- Cluster -----------------------------------

Use a descriptive name for your cluster:

cluster.name: my-application

------------------------------------ Node ------------------------------------

Use a descriptive name for the node:

node.name: node-1

Add custom attributes to the node:

node.rack: r1

----------------------------------- Paths ------------------------------------

Path to directory where to store the data (separate multiple locations by comm                                                                                a):

path.data: /path/to/data

Path to log files:

path.logs: /path/to/logs

----------------------------------- Memory -----------------------------------

Lock the memory on startup:

bootstrap.mlockall: true

Make sure that the ES_HEAP_SIZE environment variable is set to about half th                                                                                e memory
available on the system and that the owner of the process is allowed to use th                                                                                is limit.

Elasticsearch performs poorly when the system is swapping the memory.

---------------------------------- Network -----------------------------------

Set the bind address to a specific IP (IPv4 or IPv6):

network.host: [local,site]

Set a custom port for HTTP:

http.port: 9200

For more information, see the documentation at:
<http://www.elastic.co/guide/en/elasticsearch/reference/current/modules-networ                                                                                k.html>

--------------------------------- Discovery ----------------------------------

Pass an initial list of hosts to perform discovery when new node is started:
The default list of hosts is ["<IP_ADDRESS>", "[::1]"]

discovery.zen.ping.unicast.hosts: ["host1", "host2"]

Prevent the "split brain" by configuring the majority of nodes (total number o                                                                                f nodes / 2 + 1):

discovery.zen.minimum_master_nodes: 3

For more information, see the documentation at:
<http://www.elastic.co/guide/en/elasticsearch/reference/current/modules-discov                                                                                ery.html>

---------------------------------- Gateway -----------------------------------

Block initial recovery after a full cluster restart until N nodes are started:

gateway.recover_after_nodes: 3

For more information, see the documentation at:
<http://www.elastic.co/guide/en/elasticsearch/reference/current/modules-gatewa                                                                                y.html>

---------------------------------- Various -----------------------------------

Disable starting multiple nodes on a single system:

node.max_local_storage_nodes: 1

Require explicit names when deleting indices:

action.destructive_requires_name: true

@scripta55 Did you ever find a solution?

@MatthewHartz unfortunately not, facing the same issue?

Port 9200 is HTTP and does not work.
Port 9300 is Elasticsearch protocol port and is required for JDBC importer.


Use the selector atom-panel.modal instead of the overlay class.

In stylesheets/notebook.less: Use the selector atom-panel.modal instead of the overlay class.
Atom auto-generated deprecation warning.

Will be resolved in the upcoming release

Closing. Module abandoned.


Spring Cloud Stream Kafka: message consumption always on one thread no matter the number of CPU from machine

Hello Spring Cloud Stream team,
I would like to report a small issue regarding threading please, hope not to trouble.
I have a very simple Spring Cloud Stream Kafka + reactive functions project, as follow:
package com.example.micrometer;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.CommandLineRunner;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.builder.SpringApplicationBuilder;
import org.springframework.context.annotation.Bean;
import org.springframework.messaging.Message;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

import java.util.function.Consumer;

@SpringBootApplication
public class StreamReactiveConsumerApplication implements CommandLineRunner {

    private static final Logger log = LoggerFactory.getLogger(StreamReactiveConsumerApplication.class);

    public static void main(String... args) {
        new SpringApplicationBuilder(StreamReactiveConsumerApplication.class).run(args);
    }

    @Override
    public void run(String... args) {
    }

    @Bean
    Consumer<Flux<Message<String>>> consume() {
        return flux -> flux
                .flatMap(one -> myHandle(one) )
                .subscribe();
    }

    private Mono<String> myHandle(Message<String> one) {
        log.info("<==== look at this thread, it is always the same!" + "\u001B[32m" + one.getPayload() + "\u001B[0m");
        String payload = one.getPayload();
        String decryptedPayload = complexInMemoryDecryption(payload); //this is NON blocking, takes 1 second
        String complexMatrix = convertDecryptedPayloadToGiantMatrix(decryptedPayload);  //this is NON blocking, takes 1 second
        String newMatrix = matrixComputation(complexMatrix); //this is NON blocking, takes 1 second
        return myNonBlockingReactiveRepository.save(complexMatrix);
    }

}

pom:
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>org.example</groupId>
    <artifactId>streamreactiveconsumer</artifactId>
    <version>1.0-SNAPSHOT</version>

    <parent>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-parent</artifactId>
        <version>3.0.2</version>
        <relativePath/>
    </parent>

    <properties>
        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    </properties>

    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>org.springframework.cloud</groupId>
                <artifactId>spring-cloud-dependencies</artifactId>
                <version>2022.0.1</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>
        </dependencies>
    </dependencyManagement>

    <dependencies>
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-stream-binder-kafka</artifactId>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-maven-plugin</artifactId>
            </plugin>
        </plugins>
    </build>

</project>

I am consuming from a topic with 1 partition. The rate of the messages sent is one message per second.
The consumption and the processing of the message takes 3ish seconds second per message.
Important: please note the processing does not contain any blocking operation. It is a giant in memory decryption + giant matrix computation. It is BlockHound tested NON blocking.
Actual: When I consume the messages with project Spring Cloud Stream Kafka, the whole consumption + processing, message handling happens on one thread only. Everything happens on container-0-C-1
Everything will happen on container-0-C-1, tested with hardware with 2 CPUs, 4 CPUs, 8 CPUs
2023-02-06 10:42:59 8384 INFO  --- [KafkaConsumerDestination{consumerDestinationName='prod_audit_hdfs', partitions=3, dlqName='null'}.container-0-C-1] [stream-reactive-consumer,,] c.e.m.StreamReactiveConsumerApplication :
2023-02-06 10:42:59 8384 INFO  --- [KafkaConsumerDestination{consumerDestinationName='prod_audit_hdfs', partitions=3, dlqName='null'}.container-0-C-1] [stream-reactive-consumer,,] c.e.m.StreamReactiveConsumerApplication :
2023-02-06 10:42:59 8384 INFO  --- [KafkaConsumerDestination{consumerDestinationName='prod_audit_hdfs', partitions=3, dlqName='null'}.container-0-C-1] [stream-reactive-consumer,,] c.e.m.StreamReactiveConsumerApplication :

Expected:
We migrated from http webflux based to Kafka consumption based. The business logic did not change one bit.
On the Reactor Netty Spring webflux application, we could see processing happening from multiple thread, corresponding to the reactor cores. On a machine with many cores, this could keep up easily.
[or-http-epoll-1] [or-http-epoll-2] [or-http-epoll-3] [or-http-epoll-4]

The processing would just switch between any of those reactor-http-epoll-N.
I could see when reactor-http-epoll-1 is handling the complex in memory computation for the first message, reactor-http-epoll-3 would handle the computation for the second message, etc... The parallelism is clear.
I understand there are way to "scale" this application, but this a question in terms of Spring Cloud Stream Kafka with reactive functions itself.
I expect the messages would be handled in parallel. Some kind of container-0-C-1 for the first message, container-0-C-2 for the second message, etc...
How can I achieve that please?
What am I missing?
Thank you

Each partition of a topic can only be consumed on one thread so if the topic only has one partition, there will only be one consumer, regardless of the ...consumer.concurrency property. If there are multiple partitions, they will be distributed across multiple consumers, according to the concurrency.

Thank you for your answer @garyrussell,
I tried the same code on a topic with multiple partitions (2, 3, 8, 1000) and if the concurrency is set to 1, the app would still consume 2, 3, 8, 1000 on only one thread container-0-C-1.
Please let me know if my expectation is incorrect, as I am coming from Reactor Netty world, if the number of requests gets higher, while one thread is processing the http request, if the machine has multiple CPUs, i.e. multiple reactor cores, the framework will just let another thread, another reactor-http-epoll-N take care of the next request.
Here, in Spring Cloud Stream, even if I use reactive programming, due to the yet NON blocking but long processing of the message + that everything happens on one thread, I am quickly falling behind.
Is there a way to let the framework "switch thread" similar to Reactor Netty, when the processing of the message, even NON blocking, is lengthy please?
Thank you in advanced for your answer.

Your expectation is incorrect; the standard Kafka (message channel) binder is not reactive - it has one thread per consumer. The problem is that Kafka only maintains a pointer (last committed offset) into the partition. Individual records in the partition cannot be acknowledged.
The binder is not really designed for reactive workloads.
However, it does support multi-threading (at the application level), if you set the listener container property asyncAcks, it can support out of order commits, by deferring the commits for later offsets until the gaps are filled, but you have to manually acknowledge each record.
There is a newer Kafka binder, based on the reactor-kafka project, which can natively support reactive workloads.
https://docs.spring.io/spring-cloud-stream/docs/current/reference/html/spring-cloud-stream-binder-kafka.html#_reactive_kafka_binder
It is quite basic at the moment; the next patch release will allow much more control over the consumer configuration.
https://github.com/spring-cloud/spring-cloud-stream/pull/2637

Very clear answer, and I am getting excited about the reactor binder as we speak!
I gave it a go with this same example using Reactor Kafka only (no Spring Cloud Stream) and can observe the same issue.
Please let me gather more data and some test cases with both Reactor Kafka, and Spring Cloud Stream reactive Kafka binder and will update the issue

The KafkaConsumer is not thread safe; therefore, reactor-kafka has an event loop where all operations on the consumer are done on a single thread including emitting them to the sink. You need to add a publishOn() to the flux to go multi-threaded; but it, too, suffers from the out of order commit problems. To avoid that, you have to set the ReceiverOptions.maxDeferredCommits property to enable commit deferrals in the same manner that I described above. See the reactor-kafka docs.

As always, very clear, many thanks @garyrussell


Crashlytics add missing headers for builds

Needed for 1P builds

Hey Ryan and Paul, can I get a quick rubber stamp on this? Thank!

@ryanwilson that's right - for some reason the internal build system is more restrictive about having all the headers explicit. We were getting undefined symbols for eg. size_t


Update Spec to use the portable Deletion Vector serialization format

Description
The RoaringBitmap community has recently standardised on a "portable" 64-bit serialization format.
We should switch the Deletion Vector feature to use this format, as it will make adoption by 3rd parties easier, enabling them to use an off-the-shelf 64-bit implementation from the RoaringBitmap library of their choice, as long as it supports this format (which CRoaring already does and the next release of Java RoaringBitmap will do as well).
This PR makes the appropriate changes to the DV format description in the appendix of the Delta spec.
How was this patch tested?
N/A
Does this PR introduce any user-facing changes?
No.

This is fantastic, thanks for the update.


Mapping Path Truncates (from the left) on Browser Refresh

I was finally able to get some time to document an example and show you the 'truncation' issue we have been experiencing.
I was hoping that using XSD's for the multiple namespace source and target files would eliminate the issue. I find that it can be demonstrated using "standalone".  Essentially, the left most portion of the path is truncated, so mappings sort of 'derail' from the structure into the root and each refresh seems to make it more protracted. I know an answer is 'don't refresh the browser'!! but wanted to see if there is anything that can be done about this behavior.
Here is a video demonstration of the behavior and the files used.
https://user-images.githubusercontent.com/62360524/102819665-e4a8de80-43a1-11eb-81c1-8710a114a9de.mp4
Source.txt
Target.txt
Thanks!

Hey Marty - please give this another try - I believe I've fixed this recently:
wget https://repo1.maven.org/maven2/io/atlasmap/atlasmap-standalone/2.1.1/atlasmap-standalone-2.1.1.jar
http://localhost:8585/
Let me know or just close the issue if it works for you.
Thkx!

Hey Marty - please give this another try - I believe I've fixed this recently:
wget https://repo1.maven.org/maven2/io/atlasmap/atlasmap-standalone/2.1.1/atlasmap-standalone-2.1.1.jar
http://localhost:8585/
Let me know or just close the issue if it works for you.
Thkx!

That looks like it! Can you let me know when this will get into the master repository/branch?
I will keep you posted if the behavior returns

That looks like it! Can you let me know when this will get into the master repository/branch?
I will keep you posted if the behavior returns

Sure - it's on master now.  I'll close this and we can reopen if you run into the ssue again.

Sure - it's on master now.  I'll close this and we can reopen if you run into the ssue again.


Change python invocation

Use -B to prevent python from writing pyc files, this is wasted effort due to the container being
ephemeral
Set check-hash-based-pycs to never, this prevents python from scanning the entire file and
calculating its hash to check on pyc hits, instead forces it into the timestamp validation step
Use -OO to force runtime optimizations such as ignorance of assets, debug flags and docstrings

I'm sure these optimizations improve things, but how do we know that and how are we tracking regressions? Can you add some benchmarks to the test suite that demonstrate these are improvements?

I'm sure these optimizations improve things, but how do we know that and how are we tracking regressions? Can you add some benchmarks to the test suite that demonstrate these are improvements?

In this case this is part of the "Compile All" tests from the Python Interpreter Speed Tests spreadsheet. It should be noted that this is a split up PR out of that, with the follow up PR focused on generating compiled python bytecode using find, however first we need to generate base images with the findutils installed (so hence the split of the original PR). Going from no precompiled bytecode to compiled bytecode yields a 50% increase in interpreter boot time. The flags in this PR force the interpreter to act a certain way that is conducive to not stomping on the precompiled bytecode (although they should in general have other benefits as well in our case, but that has yet to be studied in isolation).
Sadly I don't have any great ideas around preventing regressions on this front, since general noise can be louder than the signal, and the study above does this 100 times to establish the signal. We could theoretically run the same test in our own test suites however it would take on the order of hours, and running on peoples dev machines might introduce even more noise making it very non-deterministic. Let me know if you have any ideas on how to prevent regressions here.

this won't help in prod as we override the entrypoint, the same change needs to be repeated elsewhere

this won't help in prod as we override the entrypoint, the same change needs to be repeated elsewhere

I'll make sure the change is repeated in the k8s, I do think they should be kept as similar as possible though to minimise drift between prod and dev.

Removed the optimize flags in line with the cluster discussion.


recognize scratch builds

Recognize the special status of scratch image. Adjust plugins to check for this to be able to succesfully utilize plugins even when basing on scratch. Changes included:

Do not attempt to inspect if using scratch image as this will always fail
Only perform ID check if we are not building on top of scratch image
Do not fail on inspection check when building on top of scratch image
Return empty inspect data when building on top of scratch image
Set empty parent_id in case the base image was scratch
Return empty component list for images based on scratch
Do not include base image in squash when building on top of scratch image

Orignally coded by<EMAIL_ADDRESS>but apparently abandoned.
Signed-off-by: Mark Langsdorf<EMAIL_ADDRESS>

looks good

This is a very nice contribution. Any chance to revisit it and resubmit?

This is a very nice contribution. Any chance to revisit it and resubmit?

Sure. We just haven't gotten the bandwidth to do that right now.


Widevine install hangs on setting up loop device

After the dialog window switches to 'Uncompressing image', nothing seems to happen anymore. This is inputstreamhelper v0.4.3 on Kodi 18.2 on Raspbian Stretch on an ARM-based RPi. I enabled debug logging and that shows the following.
2019-11-07 22:18:54.115 T:1543496416   DEBUG: [script.module.inputstreamhelper]: jsonrpc payload: {'params': {'setting': u'network.usehttpproxy'}, 'jsonrpc': u'2.0', 'method': u'Settings.GetSettingValue', 'id': 1}
2019-11-07 22:18:54.117 T:1543496416   DEBUG: [script.module.inputstreamhelper]: jsonrpc response: {"id":1,"jsonrpc":"2.0","result":{"value":false}}
2019-11-07 22:18:54.123 T:1543496416   DEBUG: CAddonSettings[script.module.inputstreamhelper]: loading setting definitions
2019-11-07 22:18:54.123 T:1543496416   DEBUG: CAddonSettings[script.module.inputstreamhelper]: trying to load setting definitions from old format...
2019-11-07 22:18:54.229 T:1543496416   DEBUG: [script.module.inputstreamhelper]: jsonrpc payload: {'params': {'addonid': u'inputstream.adaptive'}, 'jsonrpc': u'2.0', 'method': u'Addons.GetAddonDetails', 'id': 1}
2019-11-07 22:18:54.231 T:1543496416   DEBUG: [script.module.inputstreamhelper]: jsonrpc response: {"id":1,"jsonrpc":"2.0","result":{"addon":{"addonid":"inputstream.adaptive","type":"kodi.inputstream"}}}
2019-11-07 22:18:54.231 T:1543496416   DEBUG: [script.module.inputstreamhelper]: inputstream.adaptive is installed.
2019-11-07 22:18:54.232 T:1543496416   DEBUG: [script.module.inputstreamhelper]: jsonrpc payload: {'params': {'properties': [u'enabled'], 'addonid': u'inputstream.adaptive'}, 'jsonrpc': u'2.0', 'method': u'Addons.GetAddonDetails', 'id': 1}
2019-11-07 22:18:54.234 T:1543496416   DEBUG: [script.module.inputstreamhelper]: jsonrpc response: {"id":1,"jsonrpc":"2.0","result":{"addon":{"addonid":"inputstream.adaptive","enabled":true,"type":"kodi.inputstream"}}}
2019-11-07 22:18:54.235 T:1543496416   DEBUG: [script.module.inputstreamhelper]: inputstream.adaptive 2.3.17 is enabled.
2019-11-07 22:18:54.236 T:1543496416   DEBUG: [script.module.inputstreamhelper]: inputstream.adaptive 2.3.17 is installed and enabled.
2019-11-07 22:18:54.240 T:1543496416   DEBUG: CAddonSettings[inputstream.adaptive]: loading setting definitions
2019-11-07 22:18:54.248 T:1543496416   DEBUG: Previous line repeats 1 times.
2019-11-07 22:18:54.248 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Widevine is not installed.
2019-11-07 22:18:56.268 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Request URL: https://dl.google.com/dl/edgedl/chromeos/recovery/recovery.conf
2019-11-07 22:18:56.590 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Response code: 200
2019-11-07 22:18:56.717 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Find best ARM image to use from recovery.conf
2019-11-07 22:18:56.738 T:1543496416   DEBUG: [script.module.inputstreamhelper]: EXPRESSO<PHONE_NUMBER>) is smaller than BOB<PHONE_NUMBER>)
2019-11-07 22:18:56.756 T:1543496416   DEBUG: [script.module.inputstreamhelper]: BIG<PHONE_NUMBER>) is smaller than EXPRESSO<PHONE_NUMBER>)
2019-11-07 22:18:56.757 T:1543496416   DEBUG: [script.module.inputstreamhelper]: BLAZE (12371.75.1) is newer than BIG (12371.75.0)
2019-11-07 22:18:56.782 T:1543496416   DEBUG: [script.module.inputstreamhelper]: FIEVEL (12371.75.2) is newer than BLAZE (12371.75.1)
2019-11-07 22:18:58.685 T:1543496416   DEBUG: CAddonSettings[inputstream.adaptive]: loading setting definitions
2019-11-07 22:18:58.688 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Acquiring Widevine EULA from x86 files.
2019-11-07 22:18:58.689 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Request URL: https://dl.google.com/widevine-cdm/versions.txt
2019-11-07 22:18:58.909 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Response code: 200
2019-11-07 22:18:58.921 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Request URL: https://dl.google.com/widevine-cdm/4.10.1440.19-mac-x64.zip
2019-11-07 22:18:59.172 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Response code: 200
2019-11-07 22:18:59.173 T:1543496416   DEBUG: DialogProgress::Open called
2019-11-07 22:19:11.871 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Request URL: https://dl.google.com/dl/edgedl/chromeos/recovery/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin.zip
2019-11-07 22:19:12.107 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Response code: 200
2019-11-07 22:19:12.108 T:1543496416   DEBUG: DialogProgress::Open called
2019-11-07 22:19:20.677 T:1765450464   DEBUG: Thread JobWorker<PHONE_NUMBER> terminating (autodelete)
2019-11-07 22:19:20.678 T:1757057760   DEBUG: Thread JobWorker<PHONE_NUMBER> terminating (autodelete)
2019-11-07 22:19:20.678 T:1677746912   DEBUG: Thread JobWorker<PHONE_NUMBER> terminating (autodelete)
2019-11-07 22:19:20.733 T:1778488032   DEBUG: Thread JobWorker<PHONE_NUMBER> terminating (autodelete)
2019-11-07 22:29:07.614 T:1543496416   DEBUG: DialogProgress::Open called (already running)!
2019-11-07 22:35:20.778 T:1543496416   DEBUG: [script.module.inputstreamhelper]: [u'modinfo', u'loop'] cmd failed.
2019-11-07 22:35:20.779 T:1543496416   DEBUG: [script.module.inputstreamhelper]: [u'modinfo', u'loop'] cmd output:
                                            modinfo: ERROR: Module loop not found.
2019-11-07 22:35:20.780 T:1543496416   DEBUG: [script.module.inputstreamhelper]: loop is built in the kernel.
2019-11-07 22:35:20.877 T:1543496416   DEBUG: [script.module.inputstreamhelper]: [u'losetup', u'-f'] cmd executed successfully.
2019-11-07 22:35:20.877 T:1543496416   DEBUG: [script.module.inputstreamhelper]: [u'losetup', u'-f'] cmd output:
                                            /dev/loop0
2019-11-07 22:35:20.880 T:1543496416   DEBUG: [script.module.inputstreamhelper]: Found free loop device: /dev/loop0
2019-11-07 22:35:21.130 T:1543496416   DEBUG: [script.module.inputstreamhelper]: [u'fdisk', u'/home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin', u'-l'] cmd executed successfully.
2019-11-07 22:35:21.131 T:1543496416   DEBUG: [script.module.inputstreamhelper]: [u'fdisk', u'/home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin', u'-l'] cmd output:
                                            GPT PMBR size mismatch (8609948 != 4436015) will be corrected by w(rite).
                                            Disk /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin: 2.1 GiB,<PHONE_NUMBER> bytes, 4436016 sectors
                                            Units: sectors of 1 * 512 = 512 bytes
                                            Sector size (logical/physical): 512 bytes / 512 bytes
                                            I/O size (minimum/optimal): 512 bytes / 512 bytes
                                            Disklabel type: gpt
                                            Disk identifier: A3C949A5-3203-4746-9CCF-C7D83632A57C

                                            Device                                                                                Start     End Sectors  Size Type
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin1  4415488 4435967   20480   10M Microsoft basic data
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin2    20480   53247   32768   16M ChromeOS kernel
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin3<PHONE_NUMBER>487 4096000    2G ChromeOS root fs
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin4    53248   86015   32768   16M ChromeOS kernel
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin5   315392  319487    4096    2M ChromeOS root fs
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin6    16448   16448       1  512B ChromeOS kernel
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin7    16449   16449       1  512B ChromeOS root fs
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin8    86016  118783   32768   16M Microsoft basic data
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin9    16450   16450       1  512B ChromeOS reserved
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin10   16451   16451       1  512B ChromeOS reserved
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin11      64   16447   16384    8M unknown
                                            /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin12  249856  315391   65536   32M EFI System

                                            Partition table entries are not in disk order.
2019-11-07 22:46:52.845 T:1778488032   DEBUG: Thread JobWorker start, auto delete: true
2019-11-07 22:47:22.873 T:1778488032   DEBUG: Thread JobWorker<PHONE_NUMBER> terminating (autodelete)

I don't know if these messages about the GPT PMBR size mismatch and the partition table entries not being in disk order are safe to ignore.
It seems like _unzip, _check_loop, and _set_loop_dev  return True as they should.
_losetup, on the other hand, doesn't return but it spawns a process for losetup, as shown by ps:
root      3344  0.0  0.3   7516  2988 tty1     SN+  22:35   0:00 sudo losetup -o 163577856 /dev/loop0 /home/pi/.kodi/userdata/addon_data/script.module.inputstreamhelper/temp/chromeos_12371.75.2_veyron-fievel_recovery_stable-channel_fievel-mp.bin

So the process is waiting for something to complete, but that seems to last forever. mount doesn't show the file system, so I think that _mnt_loop_dev never gets called and execution hangs in _losetup.
The journal doesn't show any error. Any ideas what I could try?

I don't know if these messages about the GPT PMBR size mismatch and the partition table entries not being in disk order are safe to ignore.

Yes, these are not the cause of your problems.
Is it possible that sudo is waiting for a password? You may have to configure your system to allow for passwordless sudo to run losetup. There is no other way around this unfortunately.

I did add that to my sudoers file, but it turns out that there was another line later in the file that also applied to that user, which did not have NOPASSWD: set, overriding the first line. Sorry about that. After installing libnss3 everything works now.


Run tests in parallel

This is rebased and squashed version of #1012
I can't reproduce errors locally and on Travis. I've run the build a few dozens times and it passed.
I don't want to make the build flaky, but I have a feeling that's something were wrong with CI test environment.
If the build will nevertheless fail, this PR could remain opened, while we don't find the root cause.

Ok, for some reason the build fails only when I run it as a pull-request to the project and it doesn't when I run it from my fork on Travis.  Will investigate the stack trace...

Figured out changing logging levels needs to be protected, because LoggerContext#loggerContextListenerList is not thread-safe.

LGTM!


Changing SQL value varies frequency.

Here goes one of those weird as f**k bugs.
If you copy to VFO a frecuency from memory holding the M key (I only observed this behaviour if this is done as first step!), and then change SQL value, the VFO frequency changes. This happens, in both VFO.
As always, thanks for your effort in advance. Best regards.

I also encountered this problem
The problem is this

VFO mode frequency 438.500
MR mode, press M, copy frequency 438.600 to VFO
Modify SQL
The frequency has changed from 438.600 to 438.500

That sequence can't be done, you can't copy the freq into the VFO once you go into channel mode. You can only copy the channel freq into the VFO.

I can't reproduce this for some reason, don't suppose you could do a little video of it ?

I can't reproduce this for some reason, don't suppose you could do a little video of it ?

Sure thing!
https://github.com/OneOfEleven/uv-k5-firmware-custom/assets/17039268/0f6fb96e-1425-4d7b-bf57-d435d0ddbda8
Tbh, I don't know how I manage to find these bugs... lol

ok, I think it's putting the original VFO frequency back that was there before you did the channel-vfo copy by the looks of it.
Just need to fix it now.
Videos tell me what I need to know, words usually don't :)

ok, I think it's putting the original VFO frequency back that was there before you did the channel-vfo copy by the looks of it.

It's not exactly the previous frequency, but the previous frequency of THAT VFO band. In the case of the video, Band 6.

Try it now, I've probably broke it but hopefully not.
THE most confusing thing I've found with how the QS firmware is written is all the different channel/frequency/vfo variables that have to be changed and match up. It's a complete nightmare and has caused lots of problems for me. If the original author goes away then comes back years later, they too will see just how bad their coding was.

Try it now, I've probably broke it but hopefully not.
Yes! It looks like it is working well now! Thank you.

THE most confusing thing I've found with how the QS firmware is written is all the different channel/frequency/vfo variables that have to be changed and match up. It's a complete nightmare and has caused lots of problems for me. If the original author goes away then comes back years later, they too will see just how bad their coding was.
Perhaps the code was written by an AI? Think of it, lol

And sorry for the off-topic, let me ask you a question, out of pure curiosity (there is nothing wrong with it): why did you change the ALARM sound? Any technical reason, or simply because you liked the new sound more?

I only changed it from saw tooth sweep to triangle sweep to see how it sounded, I can change it back.

I can even get the radio to play musical notes, up to 2 tones at once, so a 2-tone cord

Oh no, don't worry. I don't think many people will be affected by this, so do as you like, of course. I just asked just for pure curiosity, nothing else.

I'm thinking the ET tune might be novel ;)


Recursion error when generating schema

Bug
For bugs/questions:

OS: ubuntu 18.04
Python version import sys; print(sys.version): 3.7.3
Pydantic version import pydantic; print(pydantic.VERSION): 0.25

Running the code below to generate schema causes a recursion error.
from __future__ import annotations
import typing
from pydantic import BaseModel, Schema

class Account(BaseModel):
  name: str
  subaccounts: typing.List[Account] = []

Account.update_forward_refs()
print(Account.schema_json(indent=2))
...

Snippet of recursion traceback
...
   flat_models |= get_flat_models_from_model(field.type_)
  File "/home/nav/.local/share/virtualenvs/project-oxBoBmI4/lib/python3.7/site-packages/pydantic/schema.py", line 355, in get_flat_models_from_model
    flat_models |= get_flat_models_from_fields(fields)
  File "/home/nav/.local/share/virtualenvs/project-oxBoBmI4/lib/python3.7/site-packages/pydantic/schema.py", line 394, in get_flat_models_from_fields
    flat_models |= get_flat_models_from_field(field)
  File "/home/nav/.local/share/virtualenvs/project-oxBoBmI4/lib/python3.7/site-packages/pydantic/schema.py", line 373, in get_flat_models_from_field
    elif lenient_issubclass(field.type_, main.BaseModel):
  File "/home/nav/.local/share/virtualenvs/project-oxBoBmI4/lib/python3.7/site-packages/pydantic/utils.py", line 232, in lenient_issubclass
    return isinstance(cls, type) and issubclass(cls, class_or_tuple)
  File "/home/nav/.local/share/virtualenvs/project-oxBoBmI4/lib/python3.7/abc.py", line 143, in __subclasscheck__
    return _abc_subclasscheck(cls, subclass)
RecursionError: maximum recursion depth exceeded in comparison

I'm not sure what the correct behaviour would be here?
Since the model is self referencing, the schema surely is infinitely recursive?
@tiangolo do you have an idea about what to do here? Maybe just a more constructive error.

Yes, it should be able to create the JSON Schema, assign an ID to it and then reference it with $ref. Here's the relevant section in the spec: http://json-schema.org/latest/json-schema-core.html#rfc.section.8.3
I'll try to fix it as soon as I get a chance.

Awesome, thank you. I think you're the best person to do it since you understand schema best.


Web: certain special characters in paths are not preserved when downloading

VSCode Version: 1.40.0-insider @ b7b21844e760e632f39bedf651da2bc93f54b0cc
OS Version: OS X Mojave 10.14

Steps to Reproduce:

Create a file named weird:file.txt in your web workspace
Download the file
Observe its downloaded name is weird_file.txt

Does this issue occur when all extensions are disabled?: Yes
Ref: https://github.com/microsoft/vscode/issues/83225

We may not be able to fix this.

I think this is not critical enough to invest. Bottom line, we create a DownloadURL transfer type (docs: https://www.html5rocks.com/en/tutorials/casestudies/box_dnd_download/) and that requires us to produce a string that is colon-separated.
I found no docs on how to escape this, if someone has an idea, feel free to chime in.

https://github.com/microsoft/vscode/blob/61f36a2346a0e2d621ea26f5a4b3b0db912198ce/src/vs/workbench/browser/dnd.ts#L334


Ability to connect a container to an existing network

Description
With #302 and and #303 it is possible for a container to disconnect from a network. Portainer seems to be lacking an option to have existing containers connect to a network.
I feel this should be added to both the container and network view, as both also support disconnecting a container.

Implemented via https://github.com/portainer/portainer/issues/848


FIFO missed first FFT output sample

Need to fix bug

FIXED: bram write enable going high one clock late. used XOR gate with write_disable and FFT output valid line as the valid line goes high at the correct time but remains high (continously outputting FFT result).
New:
First sample outputted for 2 clock cycles, could be BRAM latency. no problem as baud rate slower

no problem as baud rate slower

Just a hint of advice: don't rely on these kind of hacks.  It will bite you in future.  BRAM latency is particularly nasty, because you need to keep track of when you make it wait and when not.  Best to hook up your output "Ready" to the BRAM clock enable port (not sure if you do that -- I haven't checked the code).  Gating the BRAM clock with the output ready makes sure that everything runs nice and deterministically even when your output randomly makes it wait.

The IP seems to only have an enable port, no clken. Options are:

connect ready to port enable (for both read and write?) I see you say only the ready line is to be connected, not valid
AND ipClk and output ready and feed this to BRAM

When you run the wizard to get a BRAM block, there's a tick-box you need to tick to get access to the clock enable ports

Yes I could not find that...usually all IP has it but I think it may be linked with the port enable. will check datasheet

I'm on Vivado 2017, but I'm sure the newer ones will be similar...  Mine generates the clock-enables by default.  They call it ena and enb though, not clken like Quartus does (which I assume you mean when you say "port enable"?).  These are unrelated to the wea and web signals, which are "write enable" lines for the two ports.

With Avalon interfaces you can simply hook up the ready to the enable and call it a day.  With AXI you have to be a bit more careful, because the handshake runs the other way round.
You generally have an if-statement gating your state machine so that you can handle the waiting correctly.  Hook up the body of that if-statement to the BRAM enable.

I'm on Vivado 2017, but I'm sure the newer ones will be similar... Mine generates the clock-enables by default. They call it ena and enb though, not clken like Quartus does (which I assume you mean when you say "port enable"?). These are unrelated to the wea and web signals, which are "write enable" lines for the two ports.

Yes! I expected it to be aclken and bclken. Currently I just hold those high, will look into gating it. Seems low priority since the module is working correctly? Is it just for best practice? Cant determine from text message if you are empasizing or suggesting :grinning:

If it works, great.  Just don't do "AND ipClk and output ready and feed this to BRAM", that is bad.  Very bad.  If you do that then you're creating a new clock domain, with all the clock crossing related problems.


Error page

Display an error page
**Is your feature request related to a problem? **
Display an error site when the route doesn't match the pattern on the navbar menu.
Describe the solution you'd like
More attractive error site message than the default displayed by Netlify.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.

Solved

Solved on app


Insufficient information about decorated names.

Regardless of whether the format of decorated names is internal or not, the format should be described fully here.

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

ID: 39b37f92-1905-750c-3bea-4500081ab4d6
Version Independent ID: 8130ccd2-2db8-4ca8-1d65-4f1e5534c8b1
Content: Decorated names
Content Source: docs/build/reference/decorated-names.md
Service: visual-cpp
Sub-service: tools
GitHub Login: @TylerMSFT
Microsoft Alias: twhitney

This is something we will never get to.


Review Application

@sydnerecord @hlapp @sokole @ramirezmichelle Please review the BeetlePalooza application: [https://docs.google.com/forms/d/1LA0rvYkotA0JMh2_2xuaDdzruE6_EAFJNk9oydC8FCc/edit]

Almost done. Just need to have final approval by the organizing committee. I also need to update the link for the CFP once it is finalized by the committee.


Bump Mathlib

update lean-toolchain
bump lake-manifest
remove MatrixTU.lean
adapt everything to Matrix.IsTotallyUnimodular
make a version of Matrix.fromBlocks_TU for the definition from Mathlib

Ad last step:
https://github.com/leanprover-community/mathlib4/pull/19607