Add Dropdown and Menu components

Added a separated Menu component once it can be used on other use cases as well.
Also fixed all the unit tests to support the theme context, and added the test run to the CI.

Hey @matthieuh, I tried to reuse the Radix Menu component for the InputTags component, as autocomplete, but I couldn't get the arrow navigation to work at all. Do you have any idea how we could handle that?

@paulocfjunior I also tried a bit on your PR but I didn't found a way. Btw Could you think of adding docs for all new components?

For some reason, the components MenuItemRadio, MenuItemRadioGroup, MenuItemSeparator and MenuItemLabel are not rendering on the Docs website (they work on the Storybook though), and the Menu's onDismiss callback is not working at all, neither on their documentation, that describes it as onClose but this prop actually doesn't exist on the component.
Trying to find a solution to this, but no luck so far.

@matthieuh I extracted the test changes to another pull request and the documentation issue with MenuItems is gone, now it doesn't work only on the unit tests.


Add ParkenDD by @kiliankoe

[X] Project URL: https://github.com/kiliankoe/ParkenDD
[X] One project per pull request
[X] Insert the project in alphabetical order
[X] Avoid iOS or open-source in description as it is assumed
[X] Use this commit title format if applicable: Add app-name by @github-username
[X] Use approved format for your entry (use 🔶 for Swift projects)

Thanks for the list 😊

Thank you for contributing to open-source-ios-apps :wink:


Styled Components or Material UI Styles

Sorry if I've missed this in the documentation but is there a reason why you are using material ui styles and styled components in the demo at https://codesandbox.io/s/material-ui-layout-7mn9xq3nnj
I'm trying to decide which to use and whether I need both in a new project.

You need both in order to use Layout v4 due to this issue https://github.com/mui-org/material-ui/issues/20521.
I tried to not introduce other css-in-jss apart from jss that material-ui is using but from the issue above jss is not enough, that's why styled-components or @emotion/styled is needed.

I recommend styled-components since Material-UI is adopting it in v5. Other questions is welcome.

Ok great thanks a lot.


VS Code: add remote context filters provider

Context

Closes subtasks from https://github.com/sourcegraph/cody/issues/3641

Add node RE2 package; the RFC specifies RE2
Fetch the enterprise rules with GraphQL
Provide a stub implementation for Sourcegraph endpoints which don't support this feature, with a liberal allow policy
Write a function which takes (repo, relative path) -> do/don't ignore

Builds on top of backend changes https://github.com/sourcegraph/sourcegraph/pull/61101
Mirrors backend logic from https://github.com/sourcegraph/sourcegraph/pull/61641

Test plan
CI and new unit tests

@valerybugakov @taras-yemets Do you folks have any context on why we need to pull in a third-party regex library (re2) for this?

Do you folks have any context on why we need to pull in a third-party regex library (re2) for this?

@philipp-spiess, as per RFC: "We support RE2 syntax (same as for the Search Query Syntax)."


help improve ui

wow that's nice, i think it will be good if we add hover animation on card
what do you think ? @yamitrvg12

@2pai yes, i like the idea, let me doing it for you :)


Switched license to apache to match all other repos

What is this PR About?
https://github.com/redhat-cop/bats-library/issues/6
cc: @redhat-cop/bats-library

LGTM

/lgtm


feat(templating): support popular transpilers

LESS and SASS extensions can now be loaded as <style> by <require from=style.less> in views.
Of course, content should previously have been properly compiled to CSS.
Fixed #277

@niieani can you do the same for HMR? I know the same logic is duplicated somewhere.

I can do the same for HMR, but could you export const styleResourceExtensions = ['.css', '.less', '.sass', '.scss'] here (or something along the lines)? This way, whenever we add or change the list, we'd only have to do it in one place. This is the place where it's got to be done for HMR.

@niieani I don't think it's a good idea.
Don't get me wrong: I agree the code is not pretty and hate that there is so much duplication between templ-res and HMR and that any change here must be reflected over there. It's clearly a bad smell.
But there are a handful of reasons why I don't fancy exporting this array, the main ones being:

hot-module-reload should not take a dependency on templating-resources. The latter is an optional Aurelia component, that is included when you do use.standardConfiguration() but is not mandatory. HMR should work without it, otherwise it creates a significant discrepency between a debug build and the production one.
Ideally I think we would want a clean configuration API with reasonable defaults, and introspection that can be use by HMR and others. Probably in aurelia-framework, something like: aurelia.use.stylesheets('.less'). In the meantime, exporting this array means some users might take a dependency on it, maybe cleverly modifying it before Aurelia starts. Once exposed it's a breaking change to remove :(
There is so much duplication here that exposing this array is a joke. hmr-css-resource.ts is a 100 lines copy (with 2 minor changes) of css-resource.js. Exporting this array is really the cherry at the top of the cake.

As I wrote in the issue, I see this as a dirty internal quick-fix to enable requiring styles for more users. I wouldn't change the layering of libs or the exposed surface before we have a better designed, comprehensive solution.
😕

OK, makes sense.

Thanks @jods4 @niieani @EisenbergEffect . Now my html only components are truely html only


Only bump peer dependents when the peer dependency is out of range

Part of the motivation behind this is I want to be able to minor version Emotion. I think doing this is a good thing because people can get back nearly the exact same behavior by changing from a caret to a tilde but people also have more control over when peer dependents are bumped.
This probably isn't something we should do soon since we should probably batch other breaking changes.

This probably isn't something we should do soon since we should probably batch other breaking changes.

Agree.
Would love @lukebatchelor 's opinion before we merge this change as it impacts atlaskit stuff.

Also, other problem - when a package has a peer dependency with an exact version and the peer dep is patch bumped, should the peer dependent be patch or major bumped?

Doesn't this PR lead to the issue where your peerDependency introduces some new functionality (minor change), it is still in range so you don't bump, then down the track you start consuming that functionality. At this point, your package.json is actually wrong right? Because you haven't bumped the minimum required version that would expose that feature to you, so its possible that a consumer has an older (but still satisfying) peerDep which breaks (I'll refer to this as the inconsistent package.json problem.
Or am I misunderstanding the change?
This does bring up a conversation that has been on our backlog for ages though, that I'll likely create a full issue for us to discuss Soon™.
The current bumping behaviour of automatically bumping the minimum versions of packages when they are released at the same time was added to prevent the inconsistent package.json problem. But it has an unintended side effect of creating "version fragmentation".
That is: it increases the total number of combinations of our packages that are not compatible with eachother, leading to package duplication on the consumer (yarn's upgrade behaviour is very no bueno here).
An example would be:

Button and modal are both version 1.1.1 and modal has a dep on button of version "^1.0.0"
A consumer currently has button 1.1.0 and modal 1.1.1
A feature is released to button (minor change) and consumed in modal at the same time (patch change)
button 1.2.0 and modal 1.1.2 are released, but modals dep on button is now "^1.2.0" (This is good! it prevents the bug we're talking about!)
The consumer updates their modal dep to latest (1.2.0)
The consumers own dep on button does not satisfy modals, so it now pulls in its own copy.

At that scale, its .seriously no big problem, everything working as expected. This falls down when you have hundreds of packages depending on eachother and you need to do a sweeping change on all of them at once.
Not only would we not have hit the inconsistent package.json problem, but we've now created a massive line in the sand where any consumer upgrading one package past that line, now need to upgrade every package above that line, otherwise they'll get massive duplication.
Theres a bit more nuance to this problem that we need to talk about but thought I'd mention it briefly (really Luke? Briefly?) to get you across it.

You're understanding this change correctly. The inconsistent package.json problem is certainly a concern with doing this but also, isn't it already potentially a problem because of our current behavour with non-peer dependencies since technically this case could happen:

<EMAIL_ADDRESS>depends on<EMAIL_ADDRESS>pkg-b is bumped to 1.1.0
pkg-a is bumped to 1.1.0 and starts depending on behavior added in<EMAIL_ADDRESS>but a package manager could technically keep<EMAIL_ADDRESS>installed

I feel like the trade off of "if you want Changesets to bump peer dependents on minors, use a tilde" is good because it allows both options, yes of course it increases the version fragmentation problem but idk, I feel like it's okay. ¯\_(ツ)_/¯

Also, other problem - when a package has a peer dependency with an exact version and the peer dep is patch bumped, should the peer dependent be patch or major bumped?

IMHO, in that case, it would be OK to just patch that peer dependent - given "pragmatically correct semver" policy 😉

pkg-a is bumped to 1.1.0 and starts depending on behavior added in<EMAIL_ADDRESS>but a package manager could technically keep<EMAIL_ADDRESS>installed

Not sure if I understand this. When<EMAIL_ADDRESS>could get installed without dragging with itself also<EMAIL_ADDRESS>A general note - aren't peer deps somewhat a special case in general? From things I use on a daily basis (or have used) the ones that were peers: React, Babel, TypeScript, Emotion, XState. Not really that many - usually we just deal with regular dependencies. So when one develops a monorepo with a thing used as peer dependency (such as Emotion) they should be aware of the extra risk when it comes to version management around this. Of course, this is a manual~ thing (mental overhead) and thus error-prone but I haven't yet seen many times people failing at this. However - with automated releasing, one's instinct around that might fail more often than when managing things manually.
Going back to how it works now - I don't even think this is an ultimate solution, it just maybe minimizes the risk a little bit, right? The only thing that it actually changes is that one might spot unmet peer dep warning after bumping peer-dependent package - but it's not in any way a solution that really makes things seamless (again - due to nature of peer deps). Most times in the wild when peer dep range changes it's actually a major bump for the peer-dependent package - but there is no way for changesets to automate this reliably. So maybe if it cannot be automated it just shouldn't affect (within minor ranger) other packages at all?

Is there any plan to release this change? It is a blocker for us to adopt changeset in firebase-js-sdk. I opened an issue yesterday, and @Andarist pointed me to this PR. Thanks @Andarist !
Currently changeset is guessing that the required minimal version of my peerDependencies changed or will change, and bump it for me, which is not necessarily true. What makes it unusable for us is that it triggers a major version bump on the dependents, while in firebase major versions are coordinated, we can't major release something just because of changes in peerDependency version, especially when they are still in range. I agree with @mitchellhamilton that if we want to avoid inconsistent package.json problem, we would just use tilde version or exact version instead.
I realized the current behavior could be desirable for some people, so perhaps we can add an option that allows peerDependencies to remain unchanged if it is in range. Something like updatePeerDepsWhenOutofRange: true? This way it is not a breaking change and we can release it sooner. @mitchellhamilton WDYT?

I would be much in favor of releasing this behavior under a flag - so people can actually use and test it, even if it comes as unsafe* or similar. I understand being hesitant to just change the current behavior within the current minor version so adding it as a new option (for now) would make a good compromise for both sides of the argument.

Yeah, :100:, this should exist as an unsafe option.
The way I think this should work is that there should be a onlyUpdatePeerDependentsWhenOutOfRange option in a ___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH option in the config.
Some thoughts on this:
When we add an option to this, it's us saying "There's probably a problem in here that needs to be solved but it hasn't been totally figured out yet, here's a solution to it that might be right but also might be wrong, we're not sure yet.".
This addresses the immediate problems that people have by letting people experiment with potential solutions but it also solves a much larger problem.
It doesn't restrict us in changing the implementation of Changesets but kind of more importantly, it doesn't restrict the mental model of what Changesets can do and become in the future because it doesn't force us to release solutions that are incomplete or not totally well thought out which means that instead of having to choose between two bad options in the future(1. Remove a feature which people thought was well-supported and therefore annoy a lot of people. 2. Keep a feature and annoy a lot of people/leave them with bad solutions). It's very important to note here that this does not mean we're saying "we don't care about this problem" instead it means, "we likely care about this problem but we won't restrict ourselves to this particular solution."
A practical thing: We'll likely do lots of majors to internal packages(as in the packages that aren't the cli package) when changing these options because otherwise we would have to use exact ranges everywhere but we won't do majors to the cli package when changing these options because that would create a lot of churn for consumers. Users of the experimental options should pin an exact version of @changesets/cli.

Thanks for the insight! That sounds reasonable to me. Do you have a sense of when you will be able to get to it (___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH )? It is the only blocker that prevents me from using changeset.

Probably not very soon. We'd be happy to accept a PR for it though. (assuming @Noviny has no objections to this which I doubt they will)

https://github.com/atlassian/changesets/pull/383 @mitchellhamilton Can you please take a look?

Closing this PR because having this behavior by default won't come from this PR


chore: add workflow to format using prettier

Which problem is this PR solving?

Refs: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2543
Refs: https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2545#issuecomment-2501209745

Short description of the changes

The first commit contains format command and a workflow. The CI fails as the TypeScript files are not formatted.
The second PR contains formatting for the 18 files listed in https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2543#issuecomment-2486997247. The CI succeeds.

cc @pichlermarc who'd enabled workflow for previous PR.

I'd like to propose an alternative so that we can get the examples/* being linted using mostly the same eslitn config as the rest of the repo uses (using the top-level "eslint.config.js").  I have a start at supporting this here:
https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2567


Main window displayed in a wrong place with window controls after changing display arrangement

Platform: macOS 10.12.6 (16G1114)
Kap Version: <IP_ADDRESS>1
This is probably related to #53
Steps to reproduce
This requires an external display to reproduce.

Start Kap.
Click on the Kap tray icon/use Kap in any way.
Leave Kap tray window visible.
Change arrangement of displays.
Click on the Kap tray

Current behaviour

Expected behaviour
Kap window stays as a tray window, attached to the tray icon.

Fixed in #494


Composition: $document Operation

I just pushed the current state of my $document branch:
https://github.com/jamesagnew/hapi-fhir/commit/49bdff1862a58a52b4204442eafe450b7acf8ab1
Currently only in DSTU3 it is implemented properly.
I figured out how to return a Document Bundle instead of a SearchSet but i'm now stuck on writing the tests/ getting the Test to run: https://github.com/jamesagnew/hapi-fhir/blob/49bdff1862a58a52b4204442eafe450b7acf8ab1/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/CompositionDocumentDstu3Test.java#L103
I always end up with a

ca.uhn.fhir.rest.server.exceptions.InternalErrorException: Exceeded maximum wait for connection: java.sql.SQLException: Cannot get a connection, pool error Timeout waiting for idle object

Does this fail consistently, or just some of the time?
One thing to note, the tests randomize the size of the DB connection pool (size will be 1-4 iirc) just to make sure we don't have any processes that require more than one connection to complete (that's a recipe for deadlocks in production).
Actually, I guess re-reading what you wrote, you mean it happens consistently.

Ok on more review, I'd be suspicious that this is a transaction boundary thing.
In FhirResourceDaoCompositionDstu3.java I think possibly you're loading the results outside of a transaction? What is the stack trace for that SQLException?


MarkdownServiceProvider not found

Hi,
We use chatter as our forum package. I was testing the new version on a clean laravel 5.4 installation and I get the following error. "Class 'GrahamCampbell\Markdown\MarkdownServiceProvider' not found". This is triggered by the ChatterController on line 27. I think you forget to add "graham-campbell/markdown" to the composer.json

Sorry I forget to read the readme correctly


demo gives Uncaught TypeError: ONECOLOR is not a function

Running index.html from zip file downloaded from:
http://bebraw.github.io/colorjoe
gives this error in console:
Uncaught TypeError: ONECOLOR is not a function
    at getColor (colorjoe.js:703)
    at setup (colorjoe.js:595)
    at Function.rgb (colorjoe.js:472)
    at main (demo.js:19)
    at demo.js:3

I used the following files from demo site:
colorjoe.js
onecolor.js
And it stopped errors I was getting when trying to use files from repo.

I wonder what's the right fix for this. Onecolor is a global dependency. I could maybe point to a CDN instead. Any ideas?

I think this should be fixed now.


Support for models deployed on Azure AI Studio

Description
Azure has an offering separate from the OpenAI Studio that allows you to deploy basically every other model outside of OpenAI's. I deployed a Llama-3.1-405B-Instruct endpoint (following this guide) and I'd love to use Mirascope for interacting with it. I imagine it'd be similar to the azure client wrapper, but not entirely sure.

Just took a look at the guide. It looks like it's not an OpenAI compatible endpoint but a separate Azure endpoint, which means the client wrapper won't work.
Instead, we would likely implement support specifically for the Azure SDK (like we do for all other providers).
I've put this into the v1.1 milestone :)

This is released in v1.2!


preferredVideoCodecs and preferredAudioCodecs config does not work after the player is loaded

Have you read the FAQ and checked for duplicate open issues?
Yes
What version of Shaka Player are you using?
4.3.5
Can you reproduce the issue with our latest release version?
Yes
Can you reproduce the issue with the latest code from main?
Yes
Are you using the demo app or your own custom app?
No
If custom app, can you reproduce the issue using our demo app?
N/A
What browser and OS are you using?
Chrome and MacOs
For embedded devices (smart TVs, etc.), what model and firmware version are you using?
N/A
What are the manifest and license server URIs?

What configuration are you using?  What is the output of player.getConfiguration()?

What did you do?

Load a player
Play some media
call player.configure with either the preferredVideoCodecs or preferredAudioCodecs keys

What did you expect to happen?
This sequence of events used to result in the codecs to switch to the selected ones in the preferred codecs array.
What actually happened?

When changing the config after the player has been configured and loaded prevents the preferred codecs to be switched. Note that we can still configure the preferred codecs in the initial configuration, but unfortunately not "at runtime".

This is the defined behavior because we do not reset the SourceBuffer to make a different codec selection.


Tweet any highlighted passage

From #2.

There is an external service for this
https://sumome.com/app/highlighter
I haven't tried it yet but it might be just the thing.


Integration no accessable after 1.0 update

After I updated to 1.0.0 and then 1.0.0b, the fan entity is greyed out and not accessible
home-assistant_2024-09-09T18-46-12.341Z.log
.

Try 1.0.2

That did it!
I'll test it out more when I get home today.
Thank you Jeff!


[Q] Is there a way to add an author to a post

Hi, is there any way to add an author to a post with this theme?

Same question for images in blog posts, I couldn't use astro's image

Same question for images in blog posts, I couldn't use astro's image

Upload all images into the public folder from where you can reference them. Below is a sample given
"featuredImage: '/assets/images/blog/consider-hybrid-work/featured.jpg'
For adding images other than the cover image, use an image tag . Eg:


bump reference for iam source module to v1.0.7

Updates IAM source module reference to use most recent version

Plan completed successfully


Add support to the Memcached authentication

As memcached docs said, memcached support authentication.
Looks like Heroku uses authenticated memcached for providing access to it.

Memcached authentication requires sasl, maybe we should hide it under a feature gate.

Implemented in https://github.com/apache/opendal/pull/4252


password generator in add account

this PR is related to issue #14

add password generator with -e or --gen flag when add account
add unit test for password generator

@rendyreivaldy25 thanks for implementing this feature! 🎉 I will find time starting next week to have a look!

You're welcome. Glad to help. I am looking forward to the next issue.


Feat/Split system-sidekiq deployments into 3 new deployments based on queues

Closes https://github.com/3scale/3scale-saas/issues/16
Right now we have a problem with system-sidekiq  in production SaaS, when billing runs everyday, low jobs are processed very slow and gets accumulated, making the redis memory usage to increase up to dangerous values. This is even worst with monthly billing on day 1.
The idea if this PR is to split the current single system-sidekiq deployment into 3 new deployment based on queues , where the most used queues have their own deployment:

system-sidekiq-billing: only billing jobs
system-sidekiq-low: only low jobs
system-sidekiq-default: the rest of jobs.

The most important and critical jobs can use all available threads.
The less important queues have a percentage priority, so will only be processed using the % priority if there are no important/critical pending jobs

In addition, the container queue args can be overriden via custom resource for every deployment, although theoretically all of them should use the default values per deployment.
How priority % works?

With the default value-q critical -q backend_sync -q events -q zync,40 -q priority,25 -q default,15 -q web_hooks,10 -q deletion,5:

critical, backend_sync, events will take all avaible pod threads if there are pending jobs from that queues, so taking all 15 threads per pod
For the rest of queues  where  a priority value is configured, once all pending jobs from previous queues are 0, it will be used 40+25+15+10+5=100, so 40/100 represents 40% of threads taking zync pending jobs for example. So on the example, to make it easier to understand, all values priority sums 100, so 40 means 40%, but we could have 5+10+35=50, so 5 means 10%, 10 means 20% and 35 means 50%

Alpha release v0.10.9-alpha.1 have not been generated, pending from @roivaz and @raelga feedback to proceed with the creation of the alpha release and deploy it on development environment
/kind feature
/priority important-soon
/assign

/retitle Feat/Split system-sidekiq deployments into 3 new deployments based on queues

@roivaz / @raelga ready to be reviewed

/approve


User activity sorted incorrectly with time

I have never faced this problem until now but an older user activity shows up within newer activities. Although, It is correct to say that the PR was reviewed 6 days ago and it was closed 19 hours ago.

I've tried restarting the app and other usual stuff but it still appears that way.

Apparently the API is returning the wrong value : https://api.github.com/users/ritiek/events
But the comment has an updated_at field that might fix the issue. I'll look into it.


Add support for verilator_debug

Depends on #776, #777, #778, and #779.
This adds the logic to handle verilator_debug option which inserts the requisite verilator comments.  It's hacky, but I think this whole flow is hacky, so I vote that we add this temporary support so downstream tools are working, then refactor this entire flow (debug metadata).
Unfortunately I forgot to run clang-format on previous commits, so this includes some style fixes from clang-format. Here are the actual important changes:

https://github.com/rdaly525/coreir/compare/patch-concat-order...patch-debug-definition?expand=1#diff-6c4fb4c3cc23c2c09bda419439646981R198-R202 (adds comment to ports)
https://github.com/rdaly525/coreir/compare/patch-concat-order...patch-debug-definition?expand=1#diff-6c4fb4c3cc23c2c09bda419439646981R180-R186 uses debug version of definition if available

Also changes some helper functions to be methods so that we don't have to pass around shared fields (verilator_debug), eventually I'll move all the functions to be methods because I think that can simplify some of the code (don't have to pass around references to shared data structures).

@rdaly525 if the style changes are too annoying to parse, let me know and I can extract out the semantic changes and submit the style changes in a separate PR.


powertoys run crashed after update to 0.82.0

Microsoft PowerToys version
0.82.0
Installation method
PowerToys auto-update
Running as admin
Yes
Area(s) with issue?
PowerToys Run
Steps to reproduce
click on the icon in taskbar / use the shortcut keys set
✔ Expected Behavior
powertoys run window appears
❌ Actual Behavior
Version: <IP_ADDRESS>
OS Version: Microsoft Windows NT 10.0.22000.0
IntPtr Length: 8
x64: True
Date: 7/3/2024 11:29:43 AM
Exception:
System.ArgumentException: Value does not fall within the expected range.
at Wox.Plugin.Common.Win32.NativeMethods.DwmSetWindowAttribute(IntPtr hwnd, Int32 attr, UInt32& attrValue, Int32 attrSize)
at PowerLauncher.ViewModel.MainViewModel.ToggleWox()
at PowerLauncher.ViewModel.MainViewModel.b__147_0()
at System.Windows.Threading.Dispatcher.Invoke(Action callback, DispatcherPriority priority, CancellationToken cancellationToken, TimeSpan timeout)
at System.Windows.Threading.Dispatcher.Invoke(Action callback)
at PowerLauncher.ViewModel.MainViewModel.OnHotkey()
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
Other Software
No response

Duplicate of #33612.

/dup #33504.
think we have a fix


Audioin

fixed the audioin enumeration issues
can now use the getSources function with a callback instead of using listSources. listSources is now deprecated.

nice, thanks @polyrhythmatic !

Do we still need to merge the two branches?
Please test it out and let me know what you think!


Any way to check iframe loaded successfully??

yes work
await page.waitFor(2000);
const frame = await page.frames().find(f => f.name() === 'ifrw');
const button = await frame.$('#selector');
button.click();

don´t work
// Was trying to select a iframe before it is actually loaded.
// Is possible check before, or wait for it to load ??
const frame = await page.frames().find(f => f.name() === 'ifrw');
const button = await frame.$('#selector');
button.click();

If you're using page.goto(), in that waits for the window load event to fire. So in theory, any frames on the page should also be loaded by that point in time.
If that's not working, you can use waitForSelector:
const frame = await page.frames().find(f => f.name() === 'ifrw');
await frame.waitForSelector('#selector');
const button = await frame.$('#selector');
await button.click();

Note: this will be easier with https://github.com/GoogleChrome/puppeteer/issues/1007, tracks waitForSelector returning Promise<ElementHandle> instead of Promise<undefined>.

I'm not using page.goto(), the frame is in a modal window within the same page. The way you indicated me has not worked for me

@fran0254 there are frame events that notify about frame structure changes:

frameattached
framenavigated
framedetached

You can wait until there's a desired frame attached with frameattached event.
Let me know if it doesn't work for you.

@aslushnikov  Could you explain how I can check these three events that you mention to me?
attached the frame
Frame {
  _client:
   Session {
     domain: null,
     _events:
      { 'Page.frameAttached': [Function],
        'Page.frameNavigated': [Function],
        'Page.frameDetached': [Function],
        'Runtime.executionContextCreated': [Function],
        'Network.requestWillBeSent': [Function: bound _onRequestWillBeSent],
        'Network.requestIntercepted': [Function: bound _onRequestIntercepted],
        'Network.responseReceived': [Function: bound _onResponseReceived],
        'Network.loadingFinished': [Function: bound _onLoadingFinished],
        'Network.loadingFailed': [Function: bound _onLoadingFailed],
        'Page.loadEventFired': [Function],
        'Runtime.consoleAPICalled': [Function],
        'Page.javascriptDialogOpening': [Function],
        'Runtime.exceptionThrown': [Function],
        'Security.certificateError': [Function],
        'Inspector.targetCrashed': [Function],
        'Performance.metrics': [Function] },
     _eventsCount: 16,
     _maxListeners: undefined,
     _lastId: 142,
     _callbacks: Map {},
     _connection:
      Connection {
        domain: null,
        _events: {},
        _eventsCount: 0,
        _maxListeners: undefined,
        _url: 'ws://<IP_ADDRESS>:45941/devtools/browser/75a70228-1c06-43fc-b1db-4bcb31ea555a',
        _lastId: 144,
        _callbacks: Map {},
        _delay: 0,
        _ws: [WebSocket],
        _sessions: [Map] },
     _targetId: '(4A708DD8E19462D630BAE640E87EFE61)',
     _sessionId: '(4A708DD8E19462D630BAE640E87EFE61):1' },
  _page:
   Page {
     domain: null,
     _events: { dialog: [AsyncFunction] },
     _eventsCount: 1,
     _maxListeners: undefined,
     _client:
      Session {
        domain: null,
        _events: [Object],
        _eventsCount: 16,
        _maxListeners: undefined,
        _lastId: 142,
        _callbacks: Map {},
        _connection: [Connection],
        _targetId: '(4A708DD8E19462D630BAE640E87EFE61)',
        _sessionId: '(4A708DD8E19462D630BAE640E87EFE61):1' },
     _keyboard: Keyboard { _client: [Session], _modifiers: 0, _pressedKeys: Set {} },
     _mouse:
      Mouse {
        _client: [Session],
        _keyboard: [Keyboard],
        _x: 32.5,
        _y: 652,
        _button: 'none' },
     _touchscreen: Touchscreen { _client: [Session], _keyboard: [Keyboard] },
     _frameManager:
      FrameManager {
        domain: null,
        _events: [Object],
        _eventsCount: 3,
        _maxListeners: undefined,
        _client: [Session],
        _page: [Circular],
        _frames: [Map],
        _contextIdToContext: [Map],
        _mainFrame: [Frame] },
     _networkManager:
      NetworkManager {
        domain: null,
        _events: [Object],
        _eventsCount: 4,
        _maxListeners: undefined,
        _client: [Session],
        _requestIdToRequest: [Map],
        _interceptionIdToRequest: Map {},
        _extraHTTPHeaders: {},
        _offline: false,
        _credentials: null,
        _attemptedAuthentications: Set {},
        _userRequestInterceptionEnabled: false,
        _protocolRequestInterceptionEnabled: false,
        _requestHashToRequestIds: [Multimap],
        _requestHashToInterceptions: [Multimap] },
     _emulationManager:
      EmulationManager {
        _client: [Session],
        _emulatingMobile: true,
        _injectedTouchScriptId: '1' },
     _tracing: Tracing { _client: [Session], _recording: false, _path: '' },
     _pageBindings: Map {},
     _ignoreHTTPSErrors: false,
     _screenshotTaskQueue: TaskQueue { _chain: [Promise] },
     _viewport:
      { width: 375,
        height: 667,
        deviceScaleFactor: 2,
        isMobile: true,
        hasTouch: true,
        isLandscape: false } },
  _parentFrame:
   Frame {
     _client:
      Session {
        domain: null,
        _events: [Object],
        _eventsCount: 16,
        _maxListeners: undefined,
        _lastId: 142,
        _callbacks: Map {},
        _connection: [Connection],
        _targetId: '(4A708DD8E19462D630BAE640E87EFE61)',
        _sessionId: '(4A708DD8E19462D630BAE640E87EFE61):1' },
     _page:
      Page {
        domain: null,
        _events: [Object],
        _eventsCount: 1,
        _maxListeners: undefined,
        _client: [Session],
        _keyboard: [Keyboard],
        _mouse: [Mouse],
        _touchscreen: [Touchscreen],
        _frameManager: [FrameManager],
        _networkManager: [NetworkManager],
        _emulationManager: [EmulationManager],
        _tracing: [Tracing],
        _pageBindings: Map {},
        _ignoreHTTPSErrors: false,
        _screenshotTaskQueue: [TaskQueue],
        _viewport: [Object] },
     _parentFrame: null,
     _url: 'https://www.milanuncios.com/mis-anuncios/?pagina=1201',
     _id: '(4A708DD8E19462D630BAE640E87EFE61)',
     _context:
      ExecutionContext {
        _client: [Session],
        _contextId: 16,
        _objectHandleFactory: [Function: bound createJSHandle] },
     _waitTasks: Set {},
     _childFrames: Set { [Frame], [Frame], [Frame], [Circular] },
     _name: undefined,
     _loadingFailed: false },
  _url: 'https://www.milanuncios.com/renovar/?id=245463427',
  _id: '(6E766493375121224990C9457A8F3AF7)',
  _context:
   ExecutionContext {
     _client:
      Session {
        domain: null,
        _events: [Object],
        _eventsCount: 16,
        _maxListeners: undefined,
        _lastId: 142,
        _callbacks: Map {},
        _connection: [Connection],
        _targetId: '(4A708DD8E19462D630BAE640E87EFE61)',
        _sessionId: '(4A708DD8E19462D630BAE640E87EFE61):1' },
     _contextId: 55,
     _objectHandleFactory: [Function: bound createJSHandle] },
  _waitTasks: Set {},
  _childFrames:
   Set {
     Frame {
     _client: [Session],
     _page: [Page],
     _parentFrame: [Circular],
     _url: 'https://vars.hotjar.com/rcj-99d43ead6bdf30da8ed5ffcb4f17100c.html',
     _id: '(A8BF5BD0B465B80948EF2963DD0BA529)',
     _context: [ExecutionContext],
     _waitTasks: Set {},
     _childFrames: Set {},
     _name: '_hjRemoteVarsFrame',
     _loadingFailed: false } },
  _name: 'ifrw',
  _loadingFailed: false }

@fran0254 sure.
So you had the following that works and want to get rid of timeout:
await page.waitFor(2000);  // <-- we want to get rid of this timeout!
const frame = page.frames().find(f => f.name() === 'ifrw'); // nit: no need to await
const button = await frame.$('#selector');
button.click();

For this, you can come up with a helper function waitForFrame:
function waitForFrame(page) {
  let fulfill;
  const promise = new Promise(x => fulfill = x);
  checkFrame();
  return promise;

  function checkFrame() {
    const frame = page.frames().find(f => f.name() === 'ifrw');
    if (frame)
      fulfill(frame);
    else
      page.once('frameattached', checkFrame);
  }
}

and use it later like this:
// 1. waiting for frame with name 'ifrw' to get attached
const frame = await waitForFrame(page);
// 2. waiting for the frame to contain the necessary selector
await frame.waitForSelector('#selector');
const button = await frame.$('#selector');
button.click();

Note the step (2): iframe will be attached first and then navigated to its url, so we want to make sure the button exists before clicking.

@aslushnikov The original posts mentioned knowing when a "frame is loaded". Is frameattached or framenavigated guaranteed to happen after window.onload? And where does fall in relation to the other waitUntil timings?

works perfect for me. Thank you

Actually, I do not think that this solves the issue at hand: It should be possible to "waitUntil" a frame is loaded without watching for events. This would be a great addition to the functionality of the frame class and in consequence also for page.

How to get the status of the iframe, as response.status using page.goto?

@jcppython you can track all requests using 'request' event, and then pick the suitable response:
const requestPerFrame = new Map();
const page = await browser.newPage();
page.on('request', request => {
  if (request.isNavigationRequest())
    requestPerFrame.set(request.frame(), request);
});
await page.goto('https://example.com');
const someFrame = page.frames()[1];
const frameNavigationResponse = requestPerFrame.get(someFrame).response();


Talk with Jessica about "import to waku" link

Pass through collection number to Waku (even after google authentication)

Email sent

Integration of Waku into JDA so that the user can access Waku directory from the collection page.

Passing through collection number to Waku

Could this be accomplished with a little bookmarklet that would be pressed on a collection page and open up the waku site?

access Waku directory from the collection page.

What type of access are you thinking here?
Another idea might be to throw wacu into an iframe on JDA but that'll most likely not be very pretty. @jessyurko, are there any specific reasons you built wacu separately?

Could this be accomplished with a little bookmarklet that would be pressed on a collection page and open up the waku site?
Yes definitely. One issue I can think of might be confusion between this and the "add to jda" bookmarklet, but worth thinking about pros/cons compared to a direct link from a collection page.

What type of access are you thinking here?
I suspect the ability to create new waku projects from that collection; see existing public wakus made by other users from that collection; see any of your own wakus created from that collection.

are there any specific reasons you built wacu separately?
To have more control over development environment; to have a bit more freedom in thinking about interoperability with other platforms (namely Curarium); efficiency (versus integrating with existing zeega backend); because I feel more comfortable with javascript :)

Ok I added a little (admittedly ugly but hopefully style conforming) dialogue window to help tell new users a little bit about waku. Jessica, what do you think that text should read?

I like this text! Maybe capitalize "Create Waku"? Also, let's think about icons -- I'll look through font awesome / noun project and see if I can find anything.

@meeshmon, Jess said this little text blurb should be fine to use, here is what google translate tells me, can you audit?
Wakuにこのコレクションを使用して、メディアリッチな物語を構築します。Wakuがキュレーションとプレゼンテーションのためのストーリーテリングプラットフォームです、それは試してみます！

Waku使うことによって、このコレクションをマルチメディアな物語へと構築することができます。Wakuはキュレートされた震災記録から個人の物語を紡ぎ、発表、シェアできるプラットフォームです。ぜひお試しください!

Ok great this is all ready to go. Jessica, let me know when you find an icon you like!

Any update on this issue?

Cool, I merged this and it's live. We can replace the icon and style later on.


[FLINK-10933][kubernetes] Implement KubernetesSessionClusterEntrypoint.

What is the purpose of the change
This PR is part of FLINK-9953(Active Kubernetes integration). Entrypoint for a Kubernetes session cluster is added.
Brief change log

Add KubernetesSessionClusterEntrypoint

Verifying this change
Just a entrypoint for Kubernetes and most functionality is in ClusterEntrypoint. Will test it manually.
Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
The serializers: (no)
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? (no)
If yes, how is the feature documented? (not applicable)

Additional two non-blocking suggestions:

It is always good to add some unit tests for verifying the new classes.
If we remove the changes in KubernetesClusterDescriptor.java, then this PR might be merged beforehand. It is up to which PR would be firstly merged.

@zhijiangW Thanks for your review. You are right. It is good to add some unit tests. However, the new classes are too simple -:)
Compared with Yarn implementation, it is same.
I will remove the changes of KubernetesClusterDescriptor so that this PR could be merged quickly.

The change needs to be based on #9984 to compile I think.

@zhuzhurk Yes, your are right. This is PR depends on #9984. I will rebase and trigger the travis again.


delete-planks has to be filtered

I'm using an openshift  multi user env on which each developer has his own infrastructure application project and we decided to use it to deploy plank when we are debugging.
Example : project called "infra22" on which we have multiple application (pod) running but by executing
 squashctl utils delete-planks --squash-namespace infra22

the result is
delete-planks --squash-namespace infra22
Found 4 plank pods in namespace infra22
Deleted plank pod kiali-7df9d879b4-dqvvg.
Deleted plank pod plank2mbjl.
Deleted plank pod plankc7kmt.
Deleted plank pod plankpjxxr.

the delete is removing all PODS !!!  in my case the kiali pod is deleted
is it possible just to remove pod with a name starting with "plank%" to avoid to restart all pods running in this workspace ?
I'm using squashctl :
$ squashctl --version
squashctl version 0.5.7, created 2019-04-03.16:43:21

Thanks for pointing the out @dwojciec
I will add a filter
I had assumed that only squash-related pods would exist in the chosen squash namespace but there is no need for that constraint


Update django.po

fixed a typo.

Thanks for the report, however, translations are handled at Transifex, not through GitHub.


fixes #1380 Refactor social links styling

Pull Request Checklist

[X] I have added screenshots and videos to show before and after the working of my code.
[X] I have ensured that the screen size is set to 100% while making the video.
[X] I have synced the latest fork with my local repository and resolved any conflicts.
[X] I have mentioned the issue number which I created before making this PR .(format to mention issue number is : fixes #(issue number) )
[X] I understand that if any the above conditions are not met , my PR will not be MERGED .

https://screenrec.com/share/esxnWDy87d

issue number ?

@harjasae2001 on going through your code , I understand that you have not synced your fork and made a PR , I will be closing this PR , kindly refork , resync the repo and then make the changes , also , you have already ticked the checkbox which says that your fork is synced , make sure to follow the instructions rather than just ticking the checkbox

Sure, i will update my fork and then create a new PR from my end. Sorry for inconvienence.


Putting single charms/workloads on the mesh

Enhancement Proposal
We need to add support to the service mesh lib to be able to send back needed labels to put a charm on the mesh and connect to a waypoint.
labels needed to achieve this are
"istio.io/use-waypoint": {waypoint-name}, "istio.io/dataplane-mode": "ambient"

This is complete.


Updated validations.py python script

Updated validations.py python script.
Fixed the behavior of validate_user function in validations.py.
hola worls ncschxn jkagvj
Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

[x] Tests pass
[x] Appropriate changes to README are included in PR

cwc


Challenge workflows - Stage 1

This adds support for the new EmailVerificationChallenge and
PasswordResetChallenge workflows.
The goal here is to support both the old and the new workflows in place,
so as to ensure a smooth upgrade.

See: https://github.com/aptible/auth.aptible.com/pull/255

Same as https://github.com/aptible/auth.aptible.com/pull/255: this might not be 100% ready just yet, but please discuss there regarding the general architecture.

@krallin - Dashboard-private is now in production and should be our main repo for now.  Up to you if it's worth merging this here and opening a new merge PR in dashboard-private, or just closing these and re-opening them in dashboard-private.

See: https://github.com/aptible/dashboard-private/pull/20


Adopt resources into release with correct instance and managed-by labels

Alternative implementation to https://github.com/helm/helm/pull/7627, https://github.com/helm/helm/pull/7625, and https://github.com/helm/helm/pull/7575.
Help with validation would be appreciated, but in theory this closes https://github.com/helm/helm/issues/6850, closes https://github.com/helm/helm/issues/4824, closes https://github.com/helm/helm/issues/2947, and closes https://github.com/helm/helm/issues/7418. This implementation would also help make https://github.com/helm/helm/issues/2730 very approachable.
How it Works
~Option A: Standard Labels~ https://github.com/helm/helm/pull/7649/commits/a29365b3c663f1c182ea78461825ae28b8573915
Helm will assume that it has the right to "adopt" any existing Kubernetes resource containing the following labels:

app.kubernetes.io/managed-by: Helm
app.kubernetes.io/instance: <RELEASE_NAME>

The major downside is that in order to get this functionality, setting these labels is required. Despite this having been a documented best practice since well before Helm 3, that might be hard to accomplish for chart authors who have historically been using non-standard values for these labels. A workaround for this would be to enable the same behavior via custom annotations.
Option B: New Helm Annotations https://github.com/helm/helm/pull/7649/commits/271cb2268bfd94012639f4866e8d52189e12a97b
Helm will assume that it has the right to "adopt" any existing Kubernetes resource containing the following annotations:

meta.helm.sh/release-name: <RELEASE_NAME>
meta.helm.sh/release-namespace: <RELEASE_NAMESPACE>

For consistency and to make things easier for chart authors, we will automatically inject or override existing values for these annotations. I think this ends up being the simplest solution, and probably something we can safely expose without a feature flag.
To Do

Validate that 3-way diffs work correctly. See PR comment below.
Decide if this should be opt-in/behind an experimental feature flag
Error out if annotations list a different release as owner of resource (fix for https://github.com/helm/helm/issues/7699 / #2388)

This will be enabled in a followup PR, but is implemented in https://github.com/helm/helm/pull/7649/commits/8d1de39fe3234c8925dac6d3efca6aba687ebf87

~Add tests~

Release Note
Helm will no longer error when attempting to create a resource that already exists in the target cluster if the existing resource has the correct meta.helm.sh/release-name and meta.helm.sh/release-namespace annotations, and matches the label selector app.kubernetes.io/managed-by=Helm. This facilitates zero-downtime migrations to Helm 3 for managing existing deployments, and allows Helm to "adopt" existing resources that it previously created.
In order to allow an existing resource to be adopted by Helm, add release metadata and the managed-by label:
KIND=deployment
NAME=my-app-staging
RELEASE=staging
NAMESPACE=default
kubectl annotate $KIND $NAME meta.helm.sh/release-name=$RELEASE
kubectl annotate $KIND $NAME meta.helm.sh/release-namespace=$NAMESPACE
kubectl label $KIND $NAME app.kubernetes.io/managed-by=Helm

See also discussion on labels in https://github.com/helm/helm/issues/6365#issuecomment-547151315

thanks @jlegrone. I'm going to give this a spin today. Looks like there are some significant behavioural changes under the hood to helm install with the change to call KubeClient.Update, so I want to make sure we're not introducing any regressions here.

is this still a work-in-progress, or can we remove that label?

is this still a work-in-progress, or can we remove that label?

@bacongobbler yes, see TODOs in the description. I think the main outstanding issue is to validate 3-way diff behavior, which is why I've kept the wip label for now.

A basic test looks promising!
I created two pods: foo and bar. One had the annotations, the other didn't.
Here's what I used for testing: testchart.zip
To create the pods, I used:
><> unzip testchart
><> helm template foo testchart | kubectl create -f -
><> helm template bar testchart --set addAnnotations=true | kubectl create -f -

Then I tested with the following:
><> helm install car testchart
NAME: car
LAST DEPLOYED: Thu Mar  5 10:44:28 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
><> helm install foo testchart
Error: rendered manifests contain a resource that already exists. Unable to continue with install: existing resource conflict: namespace: default, name: foo, existing_kind: /v1, Kind=Pod, new_kind: /v1, Kind=Pod
><> helm install bar testchart
NAME: bar
LAST DEPLOYED: Thu Mar  5 10:44:34 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None

It looks like the existing_kind and new_kind error messages when there's a resource conflict aren't populated correctly. That looks unrelated to this PR as I can see this with helm 3.1.1 as well. I'll file a new ticket for that.

3-way merge seems to check out.
I modified the template a little to add junk labels to the manifest when requested: testchart.zip
$ helm template foo . --set addAnnotations=true | kubectl create -f -
$ helm template bar . --set addAnnotations=true,addLabels=true | kubectl create -f -
$ helm install foo . --set addLabels=true
$ helm install bar .

Both foo and bar had the labels and annotations attached as expected.
Another test was to attempt to take ownership over an object residing in the cluster during an upgrade:
$ helm template foo . --set addAnnotations=true | kubectl create -f -
$ helm template bar . --set addAnnotations=true | kubectl create -f -
$ helm install foo .
$ kubectl edit pod bar # change meta.helm.sh/release-name to "foo"

I added bar to the template, intending to take ownership of the object in the next upgrade:
apiVersion: v1
kind: Pod
metadata:
  name: {{ .Release.Name }}
  {{ if .Values.addLabels -}}
  labels:
    meta.helm.sh/release-name: "{{ .Release.Name }}"
    meta.helm.sh/release-namespace: "{{ .Release.Namespace }}"
  {{- end }}
  {{ if .Values.addAnnotations -}}
  annotations:
    meta.helm.sh/release-name: "{{ .Release.Name }}"
    meta.helm.sh/release-namespace: "{{ .Release.Namespace }}"
  {{- end }}
spec:
  containers:
  - name: {{ .Release.Name }}
    image: busybox
    command: ['sh', '-c', 'echo Hello Kubernetes! && sleep 3600']
---
apiVersion: v1
kind: Pod
metadata:
  name: bar
  {{ if .Values.addLabels -}}
  labels:
    meta.helm.sh/release-name: "{{ .Release.Name }}"
    meta.helm.sh/release-namespace: "{{ .Release.Namespace }}"
  {{- end }}
  {{ if .Values.addAnnotations -}}
  annotations:
    meta.helm.sh/release-name: "{{ .Release.Name }}"
    meta.helm.sh/release-namespace: "{{ .Release.Namespace }}"
  {{- end }}
spec:
  containers:
  - name: bar
    image: busybox
    command: ['sh', '-c', 'echo Hello Kubernetes! && sleep 3600']

$ helm upgrade foo .

Finally, doing that same workflow, but I did not add any ownership annotations to the resource.
$ helm template foo . --set addAnnotations=true | kubectl create -f -
$ helm template bar . --set addAnnotations=true | kubectl create -f -
$ helm install foo .
$ # edit the template as described above to take ownership of "bar"
$ helm upgrade foo .
Error: UPGRADE FAILED: rendered manifests contain a resource that already exists. Unable to continue with update: existing resource conflict: namespace: default, name: bar, existing_kind: /v1, Kind=Pod, new_kind: /v1, Kind=Pod

Looks like everything checks out here!

@bacongobbler I'd be happy to add a release note. Should I just add a comment to this PR or would that be a PR against the community repo?
I'm removing the WIP label; test cases have been added and error messages should now be much more descriptive.
My last question is whether we should go with labels instead of annotations. It could be very useful to have consistent labels for querying resources, both for end users and for implementing things like #6364. At the same time I'm not a fan of having duplicate values for app.kubernetes.io/instance and meta.helm.sh/release labels everywhere. Perhaps this is moot if we think it's feasible to support both these new annotations and standard k8s labels in the future.

@jlegrone thank you for this amazing contribution! A much needed feature.
Regarding the labels/annotations - after adding these if needed and having helm adopt the resources can the labels/annotations be removed in case I do not want my resources to include them?

after adding these if needed and having helm adopt the resources can the labels/annotations be removed in case I do not want my resources to include them?

The labels and annotations used to track ownership will be added to all resources managed by your chart each time you do a deployment; this behavior is required in order to tackle #7699. So removing ownership metadata would usually not make sense because it will just be added back on the next upgrade.

I decide that I no longer want that release to be managed by Helm, so I delete the Helm release
Sometime later, someone comes along and creates a new release named foo. because the release is gone for that, Helm allows this

Yes, this a potential problem. I tried to hedge against it by requiring the app.kubernetes.io/managed-by label to be set to Helm in addition to requiring release annotations to match.
In practice I don't know of any deployment tools that actually set this label, but if this becomes a more standard practice in the future that will improve the safety of Helm's resource adoption model.  It might not be a bad idea for us to bring this up in sig-apps and open issues against Spinnaker, kapp, etc. to get this done.

That makes sense. So I'll re-iterate that I am good to go on this one.

In which version this feature will be available? I don't see them in release notes of helm 3

In which version this feature will be available? I don't see them in release notes of helm 3

3.2.0, which is not yet released. @bacongobbler should we consider a release candidate for this change to get some early feedback?

@jlegrone I think this would be a great idea. I already tested on master and looks good so far. However do we agree that we have to annotate tag the release before being able to adopt it? Maybe this should be added to the changelog.

However do we agree that we have to annotate tag the release before being able to adopt it? Maybe this should be added to the changelog.

@zifeo does the release note in this PR look adequate to you? Applying these annotations will also be implemented as a Helm subcommand (possibly as early as the 3.2.0 release). See #2730

@jlegrone sounds good to me!

RE: cutting a 3.2.0 release candidate, there are a few tickets in the milestone yet to be resolved before we go ahead with a release candidate: https://github.com/helm/helm/milestone/83
For now, users can use the canary release to test this out in a development environment.
Hope this helps

I'd be happy to add a release note. Should I just add a comment to this PR or would that be a PR against the community repo?

@jlegrone I would suggest opening a PR in the docs repo, adding a section to https://github.com/helm/helm-www/blob/master/content/en/docs/topics/advanced.md

Does this also fix #7697?

Does this PR also fix #7697?

Yes this should fix that issue.

After digging through numerous issues concerning existing resource conflict. I seem to have reached the holy grail. However it seems undocumented so far? Is there documentation anywhere on how to use this new feature?

I did not found any documentation either but error messages where trying to install with existing resources are pretty clear with helm 3.2.0 :)
And thanks for this, it helps a lot !

Should this be cleaning up resources that were "adopted" but are not part of the new chart that's being installed? Lets say:

there's a ConfigMap that is part of the pre-Helm version of the application
the appropriate labels and annotations are applied to that ConfigMap
the helm chart that would "adopt" the other resources for the given application is installed

Shouldn't the update/install take care of removing that ConfigMap?

Feel free to open a PR/feature request for that functionality, @tomcruise81. I'm not sure that was in scope for this PR, but I could see a use for that functionality.

It doesn't seem to work with hook resources (Helm v3.2.3) such as pre-install

Sadly it's not working for me.  Added the 2 annotations outlined in the solution (the label was already there) and it's still throwing the upgrade error.

@thelo-gaultier That's a different problem to this, see #7219 and https://github.com/hickeyma/helm-mapkubeapis. The helm-mapkubeapis plugin specifically supports rbac.authorization.k8s.io migration (https://github.com/hickeyma/helm-mapkubeapis/issues/8). I'm pretty sure adding the 'adopt' metadata was the wrong fix, as something deployed by Helm 3.2.0 would already have the metadata.
That said, I suspect this working for your use-case was accidental, and possibly a bug.
My guess of the flow is below, assuming that I have your use-case correct:
Installed already is a chart with rbac.authorization.k8s.io/v1beta1 ClusterRole ("old version"), and you're upgrading it with a chart that has been updated to rbac.authorization.k8s.io/v1 ClusterRole ("new version") with the same name.
Without the annotations: "old version" is deleted, "new version" is created. Because getting the "new version" returns something (the old version in the new format), the upgrade is rejected.
With the annotations: The 'new version' is scheduled to be replaced with itself, as we pretend it always existed in the chart. However, that should have left the "old version" in the delete list, and since updates happen before deletes, the net output should have been updating the resource (by the new name) and then deleting the resource (by the old name).
I don't know why that didn't go badly wrong for you. I don't see anything in this PR that would protect that, unless I'm misunderstanding something.

Are the docs updated to reflect this possibility?
Can not find the new annotations in the docs.

Since this PR is merged (and hence "done"), I opened https://github.com/helm/helm-www/issues/755 so that at least we're tracking that the documentation is missing.

It doesn't seem to work with hook resources (Helm v3.2.3) such as pre-install

I'm observing the same behavior on 3.2.4

For ClusterRole which is not a namespaced resource from kubernetes, if helm assumes the right to adopt with annotations containing namespace, there is a problem when the chart has to be re-deployed in the same cluster in a different namespace. For eg, when the chart is deployed for the first time in namespace1, ClusterRole is created. In the second attempt, when it is tried to install the chart on a different namespace namespace2,  helm will try to adopt the existing ClusterRole and there is a error thrown.

Error: rendered manifests contain a resource that already exists. Unable to continue with install: ClusterRole "XXXX" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-namespace" must equal "namespace2": current value is "namespace1"

In that case, you will have to manually edit the resource's metadata to indicate that you want it to be adopted. This is the same as if you'd created the ClusterRole by some other mechanism than Helm.
The only time Helm can safely adopt a resource without manual intervention if it it is orphaned and then adopted by the same release in the same namespace, since clearly if it can be installed, then there isn't already a release by that name in that namespace. That is why it will only adopt resources that are already marked as belonging to the installation being attempted.
There's no way for Helm to know that it's safe to adopt that resource otherwise, as it cannot guarantee that it can accurately see the status of the release that the metadata points to.
#2730 was a proposal to add tooling to edit the metadata for you, but it hasn't moved in a while.

Okay so I understand that I will have to manually update meta.helm.sh/release-namespace value to namespace2. What would be the effect of this for the resources in namespace1?
This issue is seen because helm adds the annotations while creating ClusterRole. And while deploying it checks for the annotations. Annotations will not be added if ClusterRole is created directly using kubernetes.

Wait...
I had read 're-deployed' as "uninstalled and reinstalled", I guess perhaps you meant "installed a second copy"?
Just to be clear, you should only change those annotations if the ClusterRole has been orphaned, left behind by another release of the chart after uninstallation. If you still have the release in namespace1, then don't adopt that ClusterRole into the release in namespace2, or your release in namespace1 will be in an invalid state, and you're creating the same or worse problems for yourself (or someone else) later.
If this is your situation, then it's a problem in the chart itself: it should not be creating cluster-scoped resources that will conflict with other installations of the same chart in different namespaces. Some charts that do this will have an option to disable installing the ClusterRole (or other cluster-wide resources) so that the second parallel installation onwards will not conflict with the first, but things will break if you uninstall the first install before the others.
A better solution is for cluster-wide resources from a chart to contain the name of the namespace they were installed into, so they won't conflict. Or put the resources into a different chart that you install precisely once. The latter case is a bit like the situation with CRDs, which are cluster-wide resources that you can't rename.

Sorry for the confusion on "re-deployed" statement. Yes I tried to install a second copy.
I accept your point of having namespace in the ClusterRole name. But from kubernetes point of view ClusterRole is not namespaced resource. Helm is introducing the annotations and restricting the resource to be re-used in a different namespace.

Helm is not 'restricting the resource to be re-used'. Helm is restricting 'the resource from being owned by more than one installation'. Helm does not support having a resource owned by two different chart deployments, whether they're in the same or different namespace, or are the same or different chart or not. It's the chart-creator's responsibility to ensure that their chart only produces unique resources, including unique non-namespace resources.
The one option the chart creator does have to 'share' resources is a hook, which will create the resource, but never delete it. It's not really owned by the chart, it's more like a resource-creation script that is run at install-time. As I mentioned, CRDs are a special-case example of non-namespaced resources, with their own hook-like behaviour.
This is now a different topic from the 'adoption' mechanism for this MR. as the adoption mechanism is for taking ownership of unowned resources (that maybe used to be owned by a chart, but that's not the main use-case, I believe).
I'd suggest opening a new GitHub issue if you want to explore your situation and needs further.

Seems like this change is backward incompatible with older Helm installations.
We have a situation that this change fails our cluster upgrade process,

Old cluster k8s 1.16 had releases installed with helm 3.0.2
Cluster is updated with new image which has k8s 1.18, helm 3.4.1 and updated charts
Now helm-3.4.1 upgrade <existing release> <updated chart> fails with,
Error: UPGRADE FAILED: rendered manifests contain a resource that already exists. Unable to continue with update: ClusterRoleBinding "<rolebinding name>" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "<release name>"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "<namespace>"

To work around this error, seems we are left with 2 choices,

For each k8s resource deployed from previous Helm charts, run the k annotate and k label commands to update them.  This would require us to gather all the resources for each chart and update manually.
helm-3.4.1 upgrade<existing release> <old charts> BEFORE helm-3.4.1 upgrade <existing release> <updated charts> to let helm to backfill needed annotations and label.  This requires us to carry old charts when upgrading new charts.

Neither approach above seem ideal.  Is there a better alternative?

Does this happen for all resources or just some resources? The error suggests that the "updated charts" include resources that weren't previously part of the original chart, and those resources already existed outside Helm on your cluster.
Perhaps there's a different bug? A resource that was part of the old install should not be "imported into the current release", the annotations are not how Helm knows if something is part of the current release, that data is held in the release Secret. The annotations are only relevant when trying to adopt non-Helm resources into a Helm release.
Do you have a public chart where your problem manifests, so it can be reproduced?
If helm-3.4.1 upgrade <existing release> <old charts> and then helm-3.4.1 upgrade <existing release> <updated charts> fixes the problem, the issue might be related to silent resource upgrades between k8s versions, where the chart has also been upgraded. Although {{ClusterRoleBinding}} shouldn't be one of those.

I'm also having a problem switching from 3.0.2. I am trying to move a release to a new namespace, so I intentionally orphaned some resources then tried to adopt them with a new deployment

I created a helm deployment with 3.0.2 in the default namespace
I used kubectl to delete the helm release secrets in the default namespace
I manually changed meta.helm.sh/release-name and meta.helm.sh/release-namespace to the new, correct namespace and name (via kubectl edit ...)
The label app.kubernetes.io/managed-by=Helm was already present and still is
I ran helm upgrade --install --namespace new-namespace new-release-name and got this error: (still using 3.0.2)

Error: rendered manifests contain a resource that already exists. Unable to continue with install: existing resource conflict: kind: Service, namespace: myNamespace, name: myService

I upgraded to helm 3.4.1 and re-ran step #5 and got the exact same error.
Also worth noting, the addLabels and addAnnotations options do not seem to work for me with helm template. If I run helm template .... --set addAnnotations=true,addLabels=true I do not the label or either of the annotations on any of the objects

In our case, there was an API version upgrade (certmanager.k8s.io/v1alpha1 to cert-manager.io/v1 from k8s 1.16 to k8s 1.18 ) in a sub chart of the rel that required us to run helm mapkubeapis beforehand.
Not sure if the discontinued API version could be what contributed to the "Helm trying to adopt non-Helm resources but failed" behavior?
To workaround this, we opted to patch all resources from previous release with the annotations and label before upgrade.

The cert-manager documentation seems to strongly disrecommend upgrading in-place from their v1alpha1 API under certmanager.k8s.io/v1alpha1 to their new api under cert-manager.io/v1, and I suspect the helm-mapkubeapis plugin was the wrong thing to use with that transition.
I don't think helm-mapkubeapis changes the state of the resources on the cluster, just the data Helm holds, so unless you also did the manual uninstall/reinstall of your cert-manager resources per that above link, that should be unrelated.
Built-in kubernetes resources have magic behind them that exposes even removed API resources at their replacement API versions, which is what helm-mapkubeapis fixes the Helm-side view of.
For CRDs, when the old API CRD is removed, all the related data is deleted too.
What I think happened is that you have run helm-mapkubeapis, which upgraded the Helm metadata for ClusterRoleBinding from rbac.authorization.k8s.io/v1alpha1 to rbac.authorization.k8s.io/v1 because you're on 1.18, but your chart (or some subchart) still uses the old API Version rbac.authorization.k8s.io/v1alpha1.
This means that the chart now tries to create rbac.authorization.k8s.io/v1alpha1 ClusterRoleBinding as new, because the metadata says it doesn't exist, but the resource does exist.
So my suspicion is that you shouldn't have used helm-mapkubeapis, or perhaps have used it wth a map file that doesn't include the ClusterRoleBinding.
This is one of the reasons it's a plugin, and not a core part of Helm, because the correct changes to make depend on the changes made in the chart that is going to be applied. and helm-mapkubeapis doesn't know that, so you have to create the appropriate map yourself.
helm-mapkubeapis isn't something to just be run because your cluster is upgraded. Things work fine in that case, it's for running when your charts have changed their API versions and you're trying to upgrade across the gap.
Making the appropriate map is hard. I'd suggest using something like the helm-diff plugin to compare your proposed upgraded chart with the current metadata, and identify any proposed delete/adds that are supposed to be updates as potential map entries.

@TBBle Thanks for the feedback in https://github.com/helm/helm/pull/7649#issuecomment-733622185.

helm-mapkubeapis isn't something to just be run because your cluster is upgraded. Things work fine in that case, it's for running when your charts have changed their API versions and you're trying to upgrade across the gap.

This is a good point. Maybe the plugin should only update once an API is removed and not when deprecated?

I think the plugin is advertised to fix the wrong problem, and that it's because the problem has been mis-described in the Helm docs, and points people at the plugin as part of the "what if APIs have been removed by k8s version upgrades?" docs. So I think the question is more complex than 'removed' vs 'deprecated'.

The problem is described as an issue of removal of deprecated APIs from kubernetes, but that's not the issue, that's the underlying trigger for chart changes that cause the problem.
The problem that helm-mapkubeapis solves is that if a chart has the APIVersion of a multi-versioned resource changed between last installed version and version being upgraded to, without changing the name. This can happen even if the old API is still present on the cluster, i.e. without a kubernetes update; and doesn't just affect built-in resources, but also affects multi-versioned CRDs with a Conversion WebHook, e.g. cert-manager, as far as I know.
Specifically, the trigger is that if a chart is installed with a resource on one version of an API, e.g. certmanager.k8s.io/v1alpha1/Certficate, and then an upgrade of the chart has changed the API version, e.g. certmanager.k8s.io/v1/Certificate, then Helm (correctly) sees that as a delete old GVK resource and create new GVK resource, as it has no data (currently) that there's a relationship between them. It then fails because when it talks to the API server, the same resource is visible at both versions, and so the create (first) fails. Had the delete been done first, it would have succeeded, albeit sub-optimally, as any local changes to the resource would be lost, where a strategic merge would have succeeded.
helm-mapkubeapis turns this delete+create into a strategic merge, but this should only be done for resources which have changed API version when comparing the installed release and to-be-installed manifest. Even applying it blindly to all known-deprecated resources fails if the chart author has, for example, fixed their old Deployment resources, but overlooked an Ingress object was removed in a later k8s release and so wasn't fixed at the time.
This can also happen with non-deprecated resources, e.g., a chart with a autoscaling/v1/HorizontalPodAutoscaler releases a new version where that's been changed into an autoscaling/v2beta2/HorizontalPodAutoscaler to take advantage of non-CPU-based scaling. autoscaling/v1/HorizontalPodAutoscaler is not deprecated, but it's the same resource by either GVK, so the same problem exists.
Which effectively means the mapping file for helm-mapkubeapis depends on the diff between the two chart versions, and lining up the GVK changes correctly. Which is why I mentioned the helm diff plugin, because it can show you the changes between on-cluster chart release, and the proposed upgrade, and any unexpected create+delete can be added to the appropriate map file.
This is pretty advanced behaviour for the target Helm user though, which I assume is why helm-mapkubeapis bundles a map file for the k8s APIs known to be deprecated/removed.
I feel like that might be a mistake, as it can lead to situations like this, where blindly running the mapping after a k8s upgrade will make it impossible to upgrade if the chart has not yet itself changed the api versions in its source.

Now I've written that, it's possible the adoption logic has made this worse, because now when trying create+delete, the create by the new API Version will succeed if it was installed with recent-enough Helm (as the necessary metadata is added by default), and then it will delete the resource by its old API Version, leaving the resource missing.
That should be trivial to put a test-case together for, as it doesn't require changing anything on the cluster, merely creating with a deprecated API Version, and the upgrading to a more-recent APIVersion.
That said, this is way off topic for the MR. If I find the time to put together a test-case for the above situations and the problem pans out like I'm guessing, I'll open a new issue or two for it.

Thanks for the details feedback @TBBle. I would like to try and improve this if we can. I have some feedback below.
It might be worth opening an issue against the plugin to handle this?

I think the plugin is advertised to fix the wrong problem, and that it's because the problem has been mis-described in the Helm docs, and points people at the plugin as part of the "what if APIs have been removed by k8s version upgrades?" docs. So I think the question is more complex than 'removed' vs 'deprecated'.

The docs mention the plugin as follows:
"The manifest is a property of the Helm release object which is stored in the data field of a Secret (default) or ConfigMap in the cluster. The data field contains a gzipped object which is base 64 encoded (there is an additional base 64 encoding for a Secret). There is a Secret/ConfigMap per release version/revision in the namespace of the release.
You can use the Helm mapkubeapis plugin to perform the update of a release to supported APIs. Check out the readme for more details."
Can you describe what is misleading in this?

The problem is described as an issue of removal of deprecated APIs from kubernetes, but that's not the issue, that's the underlying trigger for chart changes that cause the problem.

This is the concrete issue that breaks Helm when trying to do an upgrade of a release which contains API versions (from previous deployment) which are now removed from the Kubernetes version.

helm-mapkubeapis turns this delete+create into a strategic merge, but this should only be done for resources which have changed API version when comparing the installed release and to-be-installed manifest. Even applying it blindly to all known-deprecated resources fails if the chart author has, for example, fixed their old Deployment resources, but overlooked an Ingress object was removed in a later k8s release and so wasn't fixed at the time.

This can also happen with non-deprecated resources, e.g., a chart with a autoscaling/v1/HorizontalPodAutoscaler releases a new version where that's been changed into an autoscaling/v2beta2/HorizontalPodAutoscaler to take advantage of non-CPU-based scaling. autoscaling/v1/HorizontalPodAutoscaler is not deprecated, but it's the same resource by either GVK, so the same problem exists.

Which effectively means the mapping file for helm-mapkubeapis depends on the diff between the two chart versions, and lining up the GVK changes correctly. Which is why I mentioned the helm diff plugin, because it can show you the changes between on-cluster chart release, and the proposed upgrade, and any unexpected create+delete can be added to the appropriate map file.

The plugin works in a very simple non opinionated way. It will just update a release metadata with the mappings defined in the mapping file. This needs to be done for a release which contains API versions which are removed in a Kubernetes version. Otherwise, you will not be able to upgrade the release anymore. An example of this is if you had a deployment object in Kubernetes 1.15 with API: apiVersion: apps/v1beta1. If you then upgrade the cluster to 1.16, this API version is now removed. In the cluster upgrade, Kubernetes will upgrade the deployment object to supported version: apiVersion: v1 but the version in the Helm release metadata stays in the unsupported version. There will be an error then when trying to do a Helm upgrade. The plugin updates this release metadata.
From what you describe, I think the issue with the plugin is that the update of the API versions in the metadata should only be once that version is removed and not while still available but in a deprecated state.

I think I'm still not being clear.

In the cluster upgrade, Kubernetes will upgrade the deployment object to supported version: apiVersion: v1

That's not what actually happens, as far as I understand. The Kubernetes API server was already offering the same object at both apiVersion v1beta1 and apiVersion v1 before the cluster upgrade. In fact, per https://github.com/kubernetes/kubernetes/issues/52185, even after the upgrade the old apiversion is what is stored until the resource is next written; auto storage migration is not yet implemented as the migrator is not production-ready. It's just that the old API version is no longer visible to API callers.
(This is also one of the core issues identified in the CRD discussions for Helm, as the same thing applies there.)
After the cluster upgrade, the apiVersion v1beta1 is not longer available, so the same Helm chart version cannot be used, so the user grabs an updated version, which contains the same resource but with apiVersion v1, and we have the delete+create issue.
However, that exact same chart upgrade could be done on a cluster that still supports both APIs, e.g. the new one may be beta like HorizontalPodAutoscaler, and the same problem would happen.
All the API removal on cluster upgrade does is force the user to get a new chart version, because they can't reinstall the existing chart version anymore.
And as seen here, that is leading users (or at least one...) to see it as "After upgrading the cluster, run helm-mapkuebapis on your charts", which is the wrong result.
Or at least, that's my reading of the problem and the docs. As mentioned, I haven't produced a simple repro case yet, but I hope to soon, as I suspect it will also reveal a new data-loss case from the new adoption support.

How to import multi-namespaced and non-namespaced resources into Helm?
I have a helm chart that creates various namespace, secret, configmap, clusterrole, clusterrolebinding.
I have stupidly applied it as in various k8s clusters.
cat <<EOF | kubectl apply --context $context -f -
$(helm template rbac-chart -f $values)
EOF

Now, I want to import these resources inside the helm but I am not sure what value to keep for meta.helm.sh/release-namespace=$NAMESPACE as some of the resources are

namespace
secret per namespace
role per namespace
Non-namespaced objects too[which doesn't belong to any namespace]

clusterrole
clusterrolebinding

@jlegrone

meta.helm.sh/release-namespace should be the namespace the chart is deployed into, i.e the place Helm will put its own Secrets to track release status. This is separate from the namespace of the resources created by the chart. If you pass --namespace to helm install etc, that's the release namespace. Otherwise it's the namespace that's active when you run helm.
A resource without a meta.namespace will be created in the namespace of the release, so for single-namespace charts, it's a good practice to not use meta.namespace in the resources, so the chart is portable. You can also reference the releases's namespace with {{ .Release.Namespace }} in your templates.
In your case, your resources have explicit namespaces rather than using {{ .Release.Namespace}}, so you can install the chart into whatever namespace you like.
If your chart does reference {{ .Release.Namespace}}, then you'll need to work out what value helm template used for that (it could be a dummy value?) and use that as the release namespace.

The labels/annotations only function during a helm install or helm upgrade, because they simply change the behaviour of "Thing you are trying to create already exists" from "fail" to "treat it as an upgrade" during helm install or helm upgrade.
If you have an existing chart deployment, and you have secrets that you want to become Helm-managed, then that implies you're trying to upgrade from a version or configuration of the chart that did not contain those secrets, to a version or configuration that does contain those secrets. That's what this feature is for.
I don't use helmfile, so I don't know what it's doing underneath, but if it's just driving helm upgrade, then it should "just work".
If it's trying to do more, particularly inspecting existing deployed resources, then it will have to implement a similar adoption logic to recognise that if it tries to create a new resource, and finds that it already exists and has the right labels/annotations, it should should that object as if it was part of the previous release with the current content, and the existing resource will be replaced with the one generated by the chart (since a 3-way merge between X and Y applying to X will always produce Y).

Thanks for the reply. I have actually narrowed this down to what seems to be a bug in helm itself. Another user mentioned it here. I don't know how this feature can be usable with this particular issue.
https://github.com/helm/helm/issues/6561#issuecomment-687382740
Does anyone know if this is already written as a bug or would be accepted as one.
Basically if we have a very tiny chart with a tiny object like a service with eg
labels:
keya: valuea
And we install and then annotate that object to be part of another helm release via the appropriate annotations and labels.
If we then try to upgrade the new release, the helm upgrade of course will not fail due to the annotations. If during that upgrade the object has slightly changed to
labels:
keyb: valueb
The resulting object in the cluster after the install, instead of having only keyb: valueb only which is the intention, it has both labels.
labels:
keya: valuea
keyb: valueb
Its like that 3 way merge logic is not behaving very well in this scenario. No yaml file in the test has both labels. The pre-upgrade yaml file has keya, the target / upgraded yaml file has keyb, but the result of the upgrade is both keys. Do you think this is worth writing a bug on?

The comments you linked to sound like a bug (or at least are surprising to me) if the outcome of the helm upgrade with adoption is the file as it was instead of either as it is in the upgraded manifest or some merge of the two. But that's not what you're seeing, so I think you're seeing something different.
Your case sounds like a reasonable 3-way merge behaviour, if I understand your example correctly... You have an object with
labels:
 keya: valuea

and you have annotated it for adoption into a release, and applied an upgrade to that Helm release where the object was created during the upgrade as
labels:
  keyb: valueb

then it's reasonable (although I didn't realise it worked this way) that a key-by-key merge will see the "created" step as "add labels.keyb: valueb", and instead of creating a new resource with that value, it's applied that change to the existing, ending up with both labels. That implies that I had misunderstood the 3-way merge done here, and in fact the adoption does a merge between 0 and Y applying to X, which would produce X+Y.
That said, I'm really unsure I understood your example, perhaps some command-lines would make that clearer, and also allow reprodution.
And besides all that, since this is a merged PR: Yes, a new Issue is probably a better place to raise this, so it's more-visible.

Thanks. I have created it as an issue here. https://github.com/helm/helm/issues/9958
For me, having contents of both an old and a new manifest, in the resulting manifest is definitely not good and not expected. Its leading to objects failing to upgrade as they contain details from both their old manifest AND their new manifest, not just the new one.

@bacongobbler @jlegrone
This works only for Helm not to complain about existence of a resources, but if you try to update that resource with helm - it doesn't work. Resource is not getting updated.

Hi Guys,
Does helm adopt resources created by a helm hook, for example, by a helm pre-upgrade hook? We are getting below error during the upgrade:
upgrade.go:434: [debug] warning: Upgrade "abc" failed: no Service with the name "abc-headless" found Error: UPGRADE FAILED: no Service with the name "abc-headless" found helm.go:84: [debug] no Service with the name "abc-headless" found helm.sh/helm/v3/pkg/kube.(*Client).Update.func1         helm.sh/helm/v3/pkg/kube/client.go:257 helm.sh/helm/v3/pkg/kube.ResourceList.Visit         helm.sh/helm/v3/pkg/kube/resource.go:32 helm.sh/helm/v3/pkg/kube.(*Client).Update         helm.sh/helm/v3/pkg/kube/client.go:230 helm.sh/helm/v3/pkg/action.(*Upgrade).releasingUpgrade         helm.sh/helm/v3/pkg/action/upgrade.go:376 runtime.goexit         runtime/asm_amd64.s:1571 UPGRADE FAILED main.newUpgradeCmd.func2         helm.sh/helm/v3/cmd/helm/upgrade.go:201 github.com/spf13/cobra.(*Command).execute         github.com/spf13/cobra@v1.5.0/command.go:872 github.com/spf13/cobra.(*Command).ExecuteC         github.com/spf13/cobra@v1.5.0/command.go:990 github.com/spf13/cobra.(*Command).Execute         github.com/spf13/cobra@v1.5.0/command.go:918 main.main         helm.sh/helm/v3/cmd/helm/helm.go:83 runtime.main         runtime/proc.go:250 runtime.goexit         runtime/asm_amd64.s:1571
What we are doing is:
We are creating a K8S service in the pre-upgrade hook and adding relavant labels and annotations so that Helm would adopt this service. But this is not working as expected. However, if we create the same resource before the upgrade, Helm adopts the resource and works as expected. Furthermore, if we run helm upgrade the second time it also passes! Hence we would like to know if Helm supports adopting of resources creating during the upgrade by a hook?

I'd suggest opening that as a new ticket, with more details, e.g., the chart you're trying to deploy or a minimal replication of the problem. That way the issue/question is likely to get more attention from people who know whether it's a bug, or if it's working-as-intended, e.g., what you're trying to do is not supported.

Does it work other way around?
I mean if I remove those 2 annotations and the label will the resource be unpinned from the chart?


[UI] Cannot understand the error message(Forbidden) in the new replication rule dialogue.

what does "Forbidden" mean?

Looks like a http 403, could you check the developer tool from browser?

@steven-zou  attached is the 403 response.

maybe, we can have an error message wrapper. The output should be the readable message for the end user.

We have message wrapper, but unfortunately sometimes there are no meaningful messages returned by backend service.
We have setup a rule for handling error messages:
If there is an error returned,
check the existing of error.message, if yes display it, otherwise
check the existing of error._body, if yes display it, otherwise,
show the general error message like 'Some errors have occurred when performing backend service calling: ' + error.

@wknet123
Please take a loot at this issue. We can have a discussion based on my above proposal.

Maybe to avoid user seeing unreadable information on UI.
We only apply this approach:
Some errors have occurred when performing backend service calling: ' + error.

General error message handling problem. Will refactor the error handling mechanism.

@wy65701436 @reasonerjt @lweitzman @tonycgil @akshayl @wknet123
Propose of error handling:

Map the error to the following unified messages based on the status code
400: (If this error is returned in UI, must be a bug) We're unable to perform your operation_(action?)_ because of bad request.
401: Your session is invalid or has expired. You need to sign in to continue your operation
403: You are not allowed to perform the operation because of lacking required privileges.
404: We're sorry what you're requesting is not existing in our system.
409: We're unable to perform your operation because what you're submitting is already existing.
500: We're unable to perform your operation because server internal errors have occurred.
ELSE: Unknown errors have occurred. Please try again later

For some specific scenario, such as email/ldap verification, we can provide concrete error message.
Failed to verify mail configuration with error:
What's your opinions?

are they stored in the configuration file? If so, it has the expand-ability and easy to do the globalization.
403: Insufficient access rights to perform the operation.
404: We're sorry the requested url was not found on this server.

Right, they're stored in message files which support globalization.
"404: We're sorry the requested url was not found on this server." : Not only page, sometimes including data entries/items not found.

Modify the message as shown below. use the term "action" instead of "operation"
when adding text to the json file, let me know so i can review it.
don't think we need the text in the "[ ]" because its obvious in most cases:
thanks,
400: (If this error is returned in UI, must be a bug) We're unable to perform your operation(action?) because of bad request.
redo: We are unable to perform your action because of a bad request.
401: Your session is invalid or has expired. You need to sign in to continue your operation
redo:  Your session is invalid or has expired. You need to sign in to continue your action.
403: You are not allowed to perform the operation because of lacking required privileges. [Please confirm you have the right role or membership]
redo: You do not have the proper privileges to perform the action.
404: We're sorry what you're requesting is not existing in our system. [Please confirm everything is valid]
redo: Your request can not be completed because the object does not exist.
409: (At present we have done pre-check to avoid this situation in mail,username and project) We're unable to perform your operation because what you're submitting have conflicting items with existing ones. [You can adjust your data to avoid conflict]
redo: We are unable to perform your action because your submission has conflicts.
500: We're unable to perform your operation because server internal errors have occurred. [Please try again later]
redo: We are unable to perform your action because internal server errors have occurred.
ELSE: Unknown errors have occurred. Please try again later.

I'll implement a new error handler service based on the above agreement!
Change owner to me!

close it as fixed.


ISPN-6005 Avoid OOME when sending client events

When adding a client listener, if state is to be provided, do it
asynchronously to avoid locking up Netty event loop thread. This is
necessary because if the event loop thread is hanging, events cannot be
written and instead they get queued up.
To avoid overloading clients, apply backpressure in server by checking
if the channel is writable before writing each event. For efficiency
reasons, events are batched now.

All, we need to get moving with this one...

@galderz There are some test failures

@gustavonalle Looking closer some might be related, I'll get those fixed ASAP

@gustavonalle Tests should be fixed

@galderz Do we want ClientEventsOOMTest to run during suite? I'd disable it and make a note.

@anistor It does run, but with slower amount of data and sleep times. I think it's good to have such test run but we can also have a manual test that adds more data and has the original sleep times you had. WDYT?

I'd move this to "stress" category and have it run with lots of large entries and big sleep time.

@anistor Hmmm, the problem with stress tests is that they're manual. I'd rather have two, one lightweight, and then extend it for stress options. Why would you be against that?

I'm not against that. Pls do it.

Good, I'll update the PR...

Is the test in this PR able to reproduce the issue before the fix? If not, IMO it's redundant, and I'd prefer a stress test that can be run and verify that everything is fine.

That's what I thought too, but if Galder would like to have a 'lighter' version of this test in the normal suite to be extra safe that's fine too. I'm thinking of entries just a bit larger than netty's buffers and quite few so as not to increase the duration of the suite.

Is the test in this PR able to reproduce the issue before the fix?

Yes.

@galderz  Apparently there are failures in the hotrod-server suite for me, let's see what CI tells

There are failures in hotrod-server

Looking into it

All concerns have been addressed and tests should be fixed now

Actually, still some issues pending...

Looking good now

Actually, one more thing! The OOME stress test is running...

Ok, talking to Dan on IRC, we reached final decision: we're removing the stress test version since the functional test already shows the problem. Instead, I''ll ammend the functional test to optionally take sys props that can be used to easily alter the number of entries and client sleep time.

Should be finally be ready :)

@galderz Is it possible to avoid the wrapping into a java.util.concurrent.CompletionException? I'm referring to the same issue @vjuranek seen in RemoteListenerWithDslFilterTest.

@anistor I'll see what it can done... but remember that this is urgent ;)

@anistor done

thanks! I still see // Regular expression adjusted to support multi-line messages

@anistor done

I hit some more (compared to TC) test failures in HR client module, but all of them are random and don't seem to be related to the change (I was able to find most of them in TC already failing in the past).

@vjuranek Looks fine to me, I've run a few times and no failures

Integrated, thanks @galderz !

Great, thanks all for the help sorting this out :)


The LM model expected is at word level or at token level?

I wanted to confirm whether the LM model is expected to be at word level or token level? Usually KenLM model is trained at word level and in our case, we are using tokenizer (n=1000), and should I need to train it at token level or word level?

Hi,
The LM used for rescoring should have the same encoding as the Conformer model.
We used the NVIDIA NeMo toolkit to train a token level 6-gram for our models:
https://docs.nvidia.com/deeplearning/nemo/user-guide/docs/en/main/asr/asr_language_modeling.html
The trick is to tokenize the training corpus using the corresponding bpe tokenizer and then to replace each token by a special character to create a new corpus. This new corpus can be used to train a bpe n-gram:
https://github.com/NVIDIA/NeMo/blob/stable/scripts/asr_language_modeling/ngram_lm/train_kenlm.py
I added the missing 6-gram to the shared folders if you would like to recover the paper results.
You should be able to access it here.
Best,
Maxime


make install-mongodb doesn't complete on macos

Owing to the image used in the chart bitnami/mongodb:12.1.31 not supporting arm64 (none of the https://hub.docker.com/r/bitnami/mongodb ones do) this chart never installs - and the only way to progress with the install on MacOS is to scale the deployment to 0 during install.
We need to either:

find a replacement drop-in image that works in macos
replace the chart
make the chart skip on macos

We should do this for mariadb, postgres, and mongodb as being optional in a local-stack but required for tests?

make the chart skip on macos