User experience for Autoscaling of instance groups

I am working on adding the autoscaling feature to the Machine Group and I am trying to decide on the user experience.
In the Google Cloud Python API you can set the autoscaling only after the instance group is created, and when setting it you pass the number of maximum and minimum instances and the CPU utilisation target (a new instance will be launched when this target is hit).
So there are two ways to do it:

First one is to give the users full control over these parameters, but in that case the code sequence gets a little complicated since they will have to do it after they start the machine group and it will override the initial number of machines, the example code of how it will look like is shown below.

mg = inductiva.reaources.MachineGroup(num_machines=1)
mg.start()
mg.enable_autoscaling(max_machines=4, min_machines=1, cpu_utilization=0.4)

Or, we can take all the decision making into the backend, and simplify it for the user, but then most of these parameters will be set by us, and the maximum number of machines is the number set initially.

mg = inductiva.reaources.MachineGroup(num_machines=5, autoscaling=True)
mg.start()

Which one should I go for?

As is, the second option seems the most appropriate.
The purpose of autoscaling is to just provide the flexibility of starting machines on demand - when simulations arrive - and allow to save slightly on the costs. In my opinion, the option 2 provides this interface cleanly!
Obviously, we are not giving all the power to the user with the option 2. With the approach you present, there are few things that concern me on option 1, like the cpu_utilization, which isn't clear to us at the moment. For example, if we launch a machine with 2 physical cores and a ratio of 1vCPU per physical core, Google Cloud still counts usability as 50% usability for using the 2 physical cores - so the interface to the user is way more trickier at this moment in time.
Moreover, I don't really like the overriding of the initial num_machines, so I would say the max_machines=num_machines always. Thinking further, it seems that doing mg.start() for 4 machines would launch those machines first, and then the autoscaling would need to kill these machines to reduce to the min_machines number. This is cumbersome in terms of time and usage. To avoid it, you could pass these commands immediately on the start, together with the autoscaling argument.
So I would say, the first option should be something like:
mg = inductiva.resources.MachineGroup(num_machines=4)
mg.start(autoscaling=True, min_machines=1, cpu_utilization=0.5)

What do you guys think?

I'm not sure on this one, but if the parameters we want to expose are really significantly different, another option is to have a separate class name:
mg = inductiva.resources.ElasticMachineGroup(
                     min_machines=1,
                     max_machines=4,
                     cpu_utilization_threshold=0.5)
mg.start()

Something like that... or AutoscalingMachineGroup (which is a bit longer name...)

I'm not sure on this one, but if the parameters we want to expose are really significantly different, another option is to have a separate class name:
mg = inductiva.resources.ElasticMachineGroup(
                     min_machines=1,
                     max_machines=4,
                     cpu_utilization_threshold=0.5)
mg.start()

Something like that... or AutoscalingMachineGroup (which is a bit longer name...)

That's a good idea if we want to give more control to the user. However, as Ivan said at the moment CPU utilisation is not actually working because of the virtual and physical core differences. (Simulations use physical cores which is half of the total vCPU cores in the machine, and CPU utilisation only looks at the vCPU usage). So I don't think that this parameter is ready to be exposed, my bad for putting it out there in the first place. So as a first step maybe we can just implement a simpler solution with autoscaler=True and see the feedback we receive, and then decide if we should create a new class in the client side. Because no matter the client interface, the backend will be more or less the same. Wdyt?

I am not sure about what @begaiym-k said. Cores are cores, virtual or real. Then, there is threads. Some cores allow for two threads to run concurrently (e.g. AMD) while others don't (Intel). If I get it correctly -- not sure -- vCPU are actually equivalent to threads. That's way on Xeon CPUs we have a direct mapping from cores to vCPUs and on AMD we have a mapping from cores = 2 theads = 2 vCPUs.
Can anybody check this?
I think we should only think in terms of (v)CPUs, and maybe that is the parameter we should have exposed.
Having said that, I am not sure if that is a problem for autoscaling, right.
This seems good:
mg = inductiva.resources.ElasticMachineGroup(
                     min_machines=1,
                     max_machines=4,
                     cpu_utilization_threshold=0.5)
mg.start()

But what is missing here is the machine type.

I think there are a couple of orthogonal choices here:

whether we give control to the user about the autoscaling rule (e.g. expose an argument for that, or we just decide what's best, like we do for the default pool -- and the user does not pick that)
whether we create a separate class name for elastic machine groups, or we keep just one "MachineGroup" class with some additional arguments.

In my opinion it is fair enough to:

not expose the scaling criteria regarding cpu usage, and we pick a good one
if the class constructor of the two potential classes can be natural unified, I also don't oppose to having a single class. For example:

mg = inductiva.resources.MachineGroup(num_machines=5, autoscaling=True)
mg.start()

and document the autoscaling argument with something like "if set to True the size of the Machine Group will vary between 1 and num_machines depending on the computational load of machines. If set to False, it will have a fixed size = num_machines" ? Potentially documenting the actual details regarding CPU usage thresholds that we use from Google cloud...
The things I would definitely avoid are:

adding extra arguments to start method
adding a new method like enable_autoscaling

whether we create a separate class name for elastic machine groups, or we keep just one "MachineGroup" class with some additional arguments.

I prefer this. I think it makes the semantics cleaner. For example, why do we think the good starting point for an elastic group is 1 machine? It can be, for example, 5 and we are ok increasing it to 100. So, I really think there are two parameters to set the bounds and we can't assume that the min param is 1.

Ok, so I guess we need a new class. I think it's fair enough and we continue with the policy of making choices quite explicit.

I am not sure about what @begaiym-k said. Cores are cores, virtual or real. Then, there is threads. Some cores allow for two threads to run concurrently (e.g. AMD) while others don't (Intel). If I get it correctly -- not sure -- vCPU are actually equivalent to threads. That's way on Xeon CPUs we have a direct mapping from cores to vCPUs and on AMD we have a mapping from cores = 2 theads = 2 vCPUs.
Can anybody check this?
I think we should only think in terms of (v)CPUs, and maybe that is the parameter we should have exposed.
Having said that, I am not sure if that is a problem for autoscaling, right.
This seems good:

mg = inductiva.resources.ElasticMachineGroup(

                     min_machines=1,

                     max_machines=4,

                     cpu_utilization_threshold=0.5)

mg.start()

But what is missing here is the machine type.

I think this is a parallel issue, that we should address and clarify since it also interferes with the simulators. But we have been feeling the pain from the vCPU with the default machines, so @begaiym-k reason is still a fair point here.
@sarmento I will address you tomorrow with a complete clarification on the cores.
@hpenedones Thanks for the clarity! The simple approach seems the best to start and serve the purpose, and if users ask for more we give them more :)

whether we create a separate class name for elastic machine groups, or we keep just one "MachineGroup" class with some additional arguments.

I prefer this. I think it makes the semantics cleaner. For example, why do we think the good starting point for an elastic group is 1 machine? It can be, for example, 5 and we are ok increasing it to 100. So, I really think there are two parameters to set the bounds and we can't assume that the min param is 1.

But I can also pose the reverse question: isn't the good end point for an elastic group to be 1 machine? Achieve the minimal cost as simulations finish... I believe this was the main reason to have the autoscaling implemented.
I understand the point of giving total control, but that also brings more complexity to the interface, learning curve and user decision making.

I don't think it brings more complexity because the name of the class become unambiguous. I think complexity arises from having classes that can have multiple behaviors and thus require parameters to set a conditional behavior.
Having these multiple possible behaviors also makes wrting the class more complicated. In many cases you will have to have a "if is_elastic" condition inside some functions. Think for example about the cost estimate function. If it is not elastic it is easy. If it is elastic you are probably going to do some estimate or show different conditions. We certainly don't what to have class methods that have to check a number of parameters given by the user to then decide what to do. Having two separate classes makes not only the interface cleaner for the users but it also allows our own coding fo be simpler. They are both important aspects.

Thanks everyone for sharing different perspectives of this issue. To summon it, I think the right direction for now will be to have a separate class and make it explicit for the user, this is good in case we want to expose more parameters for the users. @sarmento brought a very fair point of estimating the prices of elastic machine groups, and I am not sure how exactly we will do it for now (probably with a new database, that's on its way, it will be easier).


Increase coverage on vm.sh in pkg/virt-handler

What this PR does / why we need it:
In an effort to counter the ever decreasing coverage we add some code path execution tests for pkg/virt-handler/vm.go.
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #
Special notes for your reviewer:
Release note:

NONE

/test pull-kubevirt-goveralls

/test pull-kubevirt-goveralls

/test pull-kubevirt-goveralls
/test pull-kubevirt-unit-test

/test pull-kubevirt-goveralls
/test pull-kubevirt-unit-test

/retest

/retest

/cc @enp0s3

/uncc @mhenriks @zcahana
/cc @iholder-redhat


B:18520 Update Nav Links To Return To Move Page (Integration)(MERGE AFTER 18654)

Agility ticket
MERGE AFTER 18654
18654 Agility ticket
Summary
Went through the milmove multimove workflows and updated the page navigations to route to customerRoutes.MOVE_HOME_PAGE (The Move) where needed. Let me know if I missed any workflows or navigations.
Verification Steps for the Author
These are to be checked by the author.

[ ] Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
[ ] Have the Agility acceptance criteria been met for this change?

Verification Steps for Reviewers
These are to be checked by a reviewer.

[ ] Has the branch been pulled in and checked out?
[ ] Have the BL acceptance criteria been met for this change?
[ ] Was the CircleCI build successful?
[ ] Has the code been reviewed from a standards and best practices point of view?

Setup to Run the Code

Instructions for starting storybook
Instructions for starting the MilMove application
Instructions for running tests

How to test

Access the milmove app as a service member
Go through the workflow of creating a user then a move. Go through the workflow of creating shipments.
Go through all workflows to catch where anywhere in the workflows navigations it should route back to the move.
Go through the edit workflows and backing out of the edit workflows.
Go through the workflows saving changes.

Frontend

[ ] There are no aXe warnings for UI.
[ ] This works in Supported Browsers and their phone views (Chrome, Firefox, Edge).
[ ] There are no new console errors in the browser devtools.
[ ] There are no new console errors in the test output.
[ ] If this PR adds a new component to Storybook, it ensures the component is fully responsive, OR if it is intentionally not, a wrapping div using the officeApp class or custom min-width styling is used to hide any states the would not be visible to the user.
[ ] This change meets the standards for Section 508 compliance.

Backend

[ ] Code follows the guidelines for Logging.
[ ] The requirements listed in Querying the Database Safely have been satisfied.

Database
Any new migrations/schema changes:

[ ] Follows our guidelines for Zero-Downtime Deploys.
[ ] Have been communicated to #g-database.
[ ] Secure migrations have been tested following the instructions in our docs.

Screenshots

After I submit the move as the customer, which page should I see next? Currently I am routed to the dashboard rather than the move page itself.

This is fixed. Missed the Agreement nav link.

Seeing the Oops, something went wrong page when I:

Go through customer flow
Whenever I can (either before or after submitting move) go to the Edit Profile page.
Click the Edit next to Contact Info

Click the save button at the bottom of the page. I didn't change any values before saving
Click Return to move, which tries to route me to http://milmovelocal:3000/move/:moveId. This is where I see the something went wrong page. Seemed related to orders.

CCing @danieljordan-caci to see if this popped up in any of the previous PRs on orders

It's an issue with how he's using the params. I added a comment.

Seeing the Oops, something went wrong page when I:

Go through customer flow
Whenever I can (either before or after submitting move) go to the Edit Profile page.
Click the Edit next to Contact Info

Click the save button at the bottom of the page. I didn't change any values before saving
Click Return to move, which tries to route me to http://milmovelocal:3000/move/:moveId. This is where I see the something went wrong page. Seemed related to orders.

CCing @danieljordan-caci to see if this popped up in any of the previous PRs on orders

This is fixed.

After I submit the move as the customer, which page should I see next? Currently I am routed to the dashboard rather than the move page itself.

This has been changed to send you back to the move.

Overall flowed nicely! I saw all the previous issues fixed, looking good.
I did see the Oops something went wrong page again when I:

submitted the move
Clicked Review your request
Clicked the edit on the Edit next the Profile on this page:

The page I see that on is: http://milmovelocal:3000/service-member/profile

Overall flowed nicely! I saw all the previous issues fixed, looking good.
I did see the Oops something went wrong page again when I:

submitted the move
Clicked Review your request
Clicked the edit on the Edit next to the Profile on this page:

The page I see that on is: http://milmovelocal:3000/service-member/profile

Do you see any errors in console?

Overall flowed nicely! I saw all the previous issues fixed, looking good.
I did see the Oops something went wrong page again when I:

submitted the move
Clicked Review your request
Clicked the edit on the Edit next to the Profile on this page:

The page I see that on is: http://milmovelocal:3000/service-member/profile

Do you see any errors in console?

This has been fixed. Needed to add moveId to state for when needing to navigate to the Profile page.

Actively fixing this for creating a move

When clicking the Edit button to edit the Okta profile, it takes me to the profile edit page and not the okta edit page.
https://github.com/transcom/mymove/assets/136510600/ac6c2cb3-e7ff-4646-9f95-66122d71480e

When clicking the Edit button to edit the Okta profile, it takes me to the profile edit page and not the okta edit page.
Screen.Recording.2024-03-14.at.10.26.10.AM.mov

Once this is fixed, I'll stamp ya

When clicking the Edit button to edit the Okta profile, it takes me to the profile edit page and not the okta edit page.
Screen.Recording.2024-03-14.at.10.26.10.AM.mov

Changed this back to  editURL={customerRoutes.EDIT_OKTA_PROFILE_PATH}. Over looked it routing to the Contact Info Path. @danieljordan-caci  Fixed.

This has been fixed. Needed to add moveId to state for when needing to navigate to the Profile page.

Looks good!
I did notice that from the
"Review your request" button
-> "Review your details" page
-> clicking that edit next to the profile
-> going to the Profile page, I can only "Return to Move" from the button at the top of the page
-> end back at the move page
Alternatively, If I click the  "Review your request" button
-> "Review your details" page
-> clicking that edit next to the Orders
-> scrolling to the bottom of the page, I click cancel
-> end back at the "Review your details page"
Just a difference in routes I noticed. Honestly could go either way on it. Anyone have thoughts?

This has been fixed. Needed to add moveId to state for when needing to navigate to the Profile page.

Looks good!
I did notice that from the "Review your request" button -> "Review your details" page -> clicking that edit next to the profile -> going to the Profile page, I can only "Return to Move" from the button at the top of the page -> end back at the move page
Alternatively, If I click the "Review your request" button -> "Review your details" page -> clicking that edit next to the Orders -> scrolling to the bottom of the page, I click cancel -> end back at the "Review your details page"
Just a difference in routes I noticed. Honestly could go either way on it. Anyone have thoughts?

I say that's working as intended. Don't mind it.

I say that's working as intended. Don't mind it.

Won't argue with that. Looks good @TevinAdams!


avoid duplicate get container

This change takes care of the double GET Container REST calls we have observed in tpc-ds.
Tested by running read 50, read 500, copy 50, wordcount, teragen, terasort, tpc-ds.  The read 500 workload in particular has close to 4000 objects, so we use it to stress the multiple pages logic.
Developer's Certificate of Origin 1.1
   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

Thanks.


Merge docs source back to master?

Merging the gh-pages-source branch back into master would fix #2 and pave the way for #1 to be fixed, too.
It would also put a small amount of sample content directly into the master repo, which I'm not sure is something we do or don't want. Perhaps we can then update the sample content to be more working documentation, then we don't need to maintain a separate branch with content for the gh-pages branch documentation.

@smerrill, I'm curious as to your thoughts.

I think that's a fine idea (especially making the sample presentation more working documentation.)
Until we start doing things that plain Jekyll can't handle, GitHub will also run Jekyll for us on each push to gh-pages, so we can also just kill the gh-pages-source branch for now, no matter what we do.


refine trigger condition

Type of Change
refine chatobot CI trigger condition

No need wait for CI result, just change CI trigger conditions


Resolution field<EMAIL_ADDRESS>incompatible with requested version<EMAIL_ADDRESS>

You might want to patch this warning too:
warning Resolution field<EMAIL_ADDRESS>is incompatible with requested version<EMAIL_ADDRESS>package.json has<EMAIL_ADDRESS>Should probably be<EMAIL_ADDRESS># Edit package.json to add the options 'rpm' and 'AppImage'
python3 <<EOF
query = '''
    "sharp/color/color-string": "1.7.4"
  },
'''
replacement = '''
    "sharp/color/color-string": "1.9.0"
  },
'''
data = None
with open("package.json",'r') as file:
    data = file.read()
    data = data.replace(query,replacement)
with open("package.json",'w') as file:
    file.write(data)
EOF

Then the warning should disappear.

Thank you for the fix !
But I think it's not my responsability to fix these little warnings, as they are not related to the RPM build process, but to Signal-Desktop project itself (you will have the same warning when building on Debian-based distrib too). Maybe you should open a pull request to fix the package.json directly on Signal-Desktop repo ?

Sorry, didn't mean to upset. You are right. 💙
I have still not been able to compile it for Fedora 35.
Maybe network problems or something that makes it bail out so I tried to search for clues what could be wrong in the log, and then I found that one. Of course it is not your responsibility to fix issues of their code.
Building 4th time here, let's see if it can complete the RPM... there are other errors like updating "node-abi" and such visible, so I guess they have some things to fix, but it doesn't seem to stop the build process.
I assume you managed to build it for Fedora 36 today?

Maybe you should open a pull request to fix the package.json directly on Signal-Desktop repo ?

#6021

I'm not upset at all :+1: sorry if my last message gave that impression^^

there are other errors like updating "node-abi"

Indeed, I always have seen this error, but since it's not related to fedora and it doesn't stop the build process, I never looked into it :angel:

I assume you managed to build it for Fedora 36 today?

Yeah I managed to build it for fedora 36 today (I just upgraded my client to 5.49.0 without any issue), and I just build it for fedora 35 too... Can you share the full build output please ?

Welcome to the world:
📦 signal-desktop-5.49.0.fc35.x86_64.rpm
I am curious where on my machine this was built. I searched for the data and it was not even seen :-) not in /tmp not in var/temp but maybe in the caches in $HOME directory... it is true magic how Docker does this.

Here is the log as attached file.
📄 log.txt
Hope that is readable.
.

Welcome to the world:
package signal-desktop-5.49.0.fc35.x86_64.rpm
I am curious where on my machine this was built. I searched for the data and it was not even seen :-) not in /tmp not in var/temp but maybe in the caches in $HOME directory... it is true magic how Docker does this.

Well, if it was built with my script (Signal-Desktop-Fedora), the file should be within the output folder, and you will be able to install it with make install
But I think the filename you gave, signal-desktop-5.49.0.fc35.x86_64.rpm, was not build with Signal-Desktop-Fedora, as the filename should be signal-desktop-5.49.0.x86_64.rpm with my script... where do you find it ? ^^

Here is the log as attached file.
page_facing_up log.txt
Hope that is readable.

It seems that the build was successull, try to run make install now :+1:

Here is the log as attached file.
page_facing_up log.txt
Hope that is readable.

Indeed, like you though, it comes from your connection having issues :
info There appears to be trouble with your network connection. Retrying...
info There appears to be trouble with your network connection. Retrying...
info There appears to be trouble with your network connection. Retrying...
info There appears to be trouble with your network connection. Retrying...
error An unexpected error occurred: "https://registry.yarnpkg.com/rxjs/-/rxjs-6.6.7.tgz: ESOCKETTIMEDOUT".
info If you think this is a bug, please open a bug report with the information provided in "/root/Signal-Desktop/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

This error should not happen
But it seems like the last time you runned make, the build was successfull, try to run make install now :+1:

But I think the filename you gave, signal-desktop-5.49.0.fc35.x86_64.rpm, was not build with Signal-Desktop-Fedora, as the filename should be signal-desktop-5.49.0.x86_64.rpm with my script... where do you find it ?

I renamed it manually so that I don't accidentally install it on Fedora 36 later. The fc35 is a standard identifier for version 35. fc36 is for Fedora 36 so I typed it in to be sure I know what it is.

As it's solved I will close, but feel free to open new issues if needed :+1:


[Feature]: Add ability to inherit metadata props from parent group

What feature would you like?
Allow metadata props to be inherited from parent token group:
{
  "brand": {
    "type": "color",
    "color": {
      "acid green": {
        "value": "#00ff66"
      },
      "hot pink": {
        "value": "#dd22cc"
      }
    }
  }
}

This means it's no longer necessary to add the type to every individual design token.
Use cases
Token type
It's a bit tedious to put the "type" prop on every token, especially for core/foundation tokens, where you may have many tokens of the same type grouped under the same token group.
This is currently also spec'd here https://design-tokens.github.io/community-group/format/#type-1
Options used in transforms, through token metadata
We're currently looking into adding a ts/size/rem transform, see https://github.com/tokens-studio/sd-transforms/pull/80
rem and em however, are dependant on some base font size. Since this changes based on context, I think it would be cool if tokens could contain this kind of context themselves.
For example, the metadata about spacing could be set on the button token group as a whole:
{
  "button": {
    "fontSize": {
      "value": "24",
      "type": "fontSize"
    },
    "padding": {
      "value": "24",
      "type": "spacing"
    },
    "$extensions": {
      "studio.tokens": {
        "spacing": {
          "unit": "em",
          "base": "{button.fontSize}"
        }
      }
    }
  }
}

which would result in a value of 1em for the button.padding token, after running a transform on this. Meaning that if the fontSize changes (e.g. if it's a variable), the button padding will change with it due to using em unit. And this would go for any button property that's using some kind of dimension-type value, since this metadata exists on the button group level.

If the padding was set to 1em already in the Figma Plugin, this spacing metadata wouldn't really do anything with the padding token, it would just stick to that 1em as the current unit is already the desired unit.

Challenges
There's no easy way to know what kinds of metadata should be inherited, and for some metadata, the inheritance could be problematic.
I think for now, it's only safe to do it for our own metadata under "studio.tokens" namespace, and "type" as this property is spec'd to be inherited in the W3C Design Token spec draft, I don't really see a risk with that one.
Implementation
I'm thinking we could perhaps do this in StyleDictionary parse step, take the dictionary, recursively loop over everything and if we encounter a type of tokens-studio metadata, we can use something like deepmerge to merge it into the child token groups and tokens if "type" or tokens-studio meta already exists in children.
We should definitely test if this isn't too expensive of an action to be doing for all tokens..
@six7 would love your feedback on this.
Would you be available to contribute this feature?

[X] Yes, I would like to contribute this

We already support setting type at the group level inside the plugin itself, so I'd love to see this in the transforms as well!
As for the metadata: definitely an interesting one! The base font size could be another one that you might want to set. We don't allow inheritance in the plugin, but users can basically define a 'root font size' which they could change eg from 16 to 20px or so (meaning 1rem has different sizes).

Now that we support W3C spec I don't really think this still necessary. I haven't seen or heard anyone mention needing this.


fix tests after registration migration & rename meta-reference -> basic / llm_as_judge provider

What does this PR do?

fix broken tests after registration migration
move meta-reference scoring into basic / llm_as_judge provider

Feature/Issue validation/testing/test plan
pytest -v -s -m meta_reference_scoring_together_inference scoring/test_scoring.py
pytest -v -s -m braintrust_scoring_together_inference scoring/test_scoring.py
pytest -v -s -m llm_as_judge_scoring_together_inference scoring/test_scoring.py

Before submitting

[x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
[x] Ran pre-commit to handle lint / formatting issues.
[x] Read the contributor guideline,
Pull Request section?
[x] Updated relevant documentation.
[x] Wrote necessary unit or integration tests.

are fn_defs still needed?

Yes, since we want to pre-register the scoring functions, we are keeping them in code.


Updated CFBinaryPropertyList.php

Changes description
fix: String offset access syntax with curly braces is deprecated in php >= 7.3
Checklist
Please check if your PR fulfills the following specifications:

[ ] Tests for the changes have been added
[ ] Docs have been added/updated

References

Closes #N/A

Is there any way we can move this along? I'm currently getting the same error in a project that uses PHP 7.4

Obsoleted by #63


Decide approach for sub-domain consent

What
Confirm that we can go with the sub-domain approach to gathering consent for cookies across design-system.service.gov.uk and frontend.design-system.service.gov.uk
Why
The privacy team raised some concerns in an email thread about taking this approach. We think we'll be able to do this, but we need to make it clear in the cookie banner and cookie page, but we should double check.
Who needs to know about this
Performance Analyst; Product Manager; Developers
Done when

[ ] Chatted with privacy team about sub-domain consent
[ ] We know what we need in place (from a legal point-of-view) to add sub-domain tracking, e.g: what content we need on the site to explain it to users
[ ] Relevant cards updated

Sent Joe Levey an email on 26 April to find out next steps - copied in Vanita and Trang for their reference. Awaiting reply.

We got a response back from Joe
"I've had a look at the proposal and I have a few initial thought:
On the banner you should allude to the essential cookies as a matter of best practice. i.e. we use essential cookies on this site for blah. We also use analytics cookies help us understand how our website is being used.
In the cookie policy, I would take the time to explain that we're talking about more than 1 site. Maybe start with a sentence like 'this cookie policy applies to the collection of sites that make up the GOV.UK Design System' including Site A and Site B. And be clear that by consenting to cookies on one domain, it applies to the other domain.
What data will you be getting out of GA? Will it be clientID level data or aggregate data? For example, this user (clientID) did ABC and then came back 1 week later and did DEF vs 1000 individual users did ABC. If the former, I think it is likely to constitute personal data because the ClientID enables you to distinguish individual users from other users. But I have raised a question with our legal support to confirm / deny this. In the meantime, you can only really call it anonymous if you are doing the latter. "
@vanitabarrett @SaraC19 @calvin-lau-sig7 It might be worth getting the content right while we have Joe's attention so we have it ready for when we have GA setup for both Design System website and Frontend Docs. The priority for us should be get GA on the Design System website, Frontend Docs can wait. If this is too much of a distraction, we can park this for now. What do you think?

@trang-erskine Got a catch-up next week with Sara, Calvin and myself to talk about the content. I'm going to move this card back to upcoming sprints for now, as I think our main focus will be getting the content for the main design system site, but I'll update this card if we do discuss the changes we'll need to make for sub-domain consent.

Looks like GOV.UK Pay is using the same cookie to register consent both on their main site and their technical documentation. It could be worth getting in touch with them about policies and wording (though both sites share the same navigation menu, if not the same layout, which may come into play).


Add coloured and bold output

Coloured 'SUCCESS!', 'ERROR!' and 'WARNING!' output, and some more.
Rebase of https://github.com/wimpysworld/quickemu/pull/36/commits/dcb93f70a2e83b9b13deb629183ac4f6e63f55ec.

Things have diverged considerably. Closing this.


Git scrape Product and Group information for all agencies

We would like to understand changes over time to the Littlepay Product and Group details for each agency we work with. This has come up recently as we learned about a change to a Product ID after that Product was duplicated to create a new Product -- we didn't expect this to create a TWO new Products 🤷
So we'd like to capture the output of our littlepay groups and littlepay products CLI commands, in CSV files stored in this repository, to be able to assess those changes over time more easily.
Acceptance Criteria

[ ] For each agency in production (MST, SacRT, SBMTD as of opening this issue)
[ ] On some schedule (at least once per day)
[ ] Run littlepay groups and littlepay products
[ ] Store the data in a corresponding CSV file in this repository: data/[agency slug]_[groups|products].csv
[ ] With column headers matching the fields in the corresponding GroupResponse and ProductResponse objects
[ ] Also store the output of littlepay groups products in a CSV file in this repository: data/[agency slug]_linked_groups_products.csv
[ ] With column headers group_id, product_id
[ ] A new commit is made each time this process runs, for each agency, when any data file is changed

Additional context
Sort of related to the work in https://github.com/cal-itp/benefits/pull/1889, can be implemented in workflow in a similar fashion.
More about the git scraping technique.

cc @indexing @o-ram, here's the ticket write up for collecting Littlepay Group and Product info over time in this littlepay repository.
Question for you both: how frequently and when should this process run?

Going with once per day in the evening (roughly 10-11pm Pacific) for now -- this will capture any changes made throughout the day, and we'll note the timestamp of when we capture the changes to hopefully make later analysis a little easier.

@thekaveman The time and frequency sound good to me; seems like a good match for the infrequnecy with which products change.

@thekaveman also sounds good to me!


Stale UDP connections from NGINX to CoreDNS on fargate with kube-proxy replacement

Is there an existing issue for this?

[x] I have searched the existing issues

Version
equal or higher than v1.16.0 and lower than v1.17.0
What happened?
https://github.com/cilium/cilium/issues/28820
https://github.com/cilium/cilium/issues/28821
These issues describe the termination of UDP connections. I do not see this happening. I'm running CoreDNS in Fargate. When I restart the CoreDNS NGINX, it keeps being connected to the old pods that no longer exist.
I suspect there is still something off when using Fargate for these pods.
The hostNetworkOnly setting workaround still works.
How can we reproduce the issue?

Install EKS cluster with Cilium
Enable kube-proxy replacement
Run CoreDNS in fargate pod
Start NGINX
Do a request that uses DNS from the nginx process, like using auth check (see below)
Make sure netstat shows an established UDP connection to 53
Restart CoreDNS pods
Try to resolve anything from NGINX process

Cilium Version
cilium-cli: v0.16.19 compiled with go1.23.1 on linux/amd64
cilium image (default): v1.16.2
cilium image (stable): v1.16.3
cilium image (running): 1.16.3
Kernel Version
Linux ingress-nginx-controller-controller-6f5c7c9679-7pqt9 6.1.112 #1 SMP Thu Oct 24 18:31:38 UTC 2024 aarch64 Linux
Kubernetes Version
Client Version: v1.30.3
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.30.6-eks-7f9249a
Regression
No response
Sysdump
cilium-sysdump-20241105-124721.zip
Relevant log output
No response
Anything else?
Tested with this ingress:
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/auth-url: https://google.com
  name: test
spec:
  ingressClassName: nginx
  rules:
  - host: test.test.com
    http:
      paths:
      - backend:
          service:
            name: test
            port:
              name: https
        path: /
        pathType: Prefix
---
apiVersion: v1
kind: Service
metadata:
  annotations:
  name: test
  namespace: default
spec:
  externalName: google.com
  ports:
  - name: https
    port: 443
    protocol: TCP
    targetPort: 443
  sessionAffinity: None
  type: ExternalName

Command:
~$ curl -k https://test.test.com --resolve "test.test.com:443:<IP_ADDRESS>"

<title>500 Internal Server Error</title>

500 Internal Server Error

nginx

-> Delete all coredns pods
~$ curl -k https://test.test.com --resolve "test.test.com:443:<IP_ADDRESS>"

New pods
kube-system coredns-f6678c8d8-4rvvw ● 1/1 Running 0 <IP_ADDRESS> fargate-ip-10-5-43-91.eu-central-1.compute.internal 97s
kube-system coredns-f6678c8d8-qdl5x ● 1/1 Running 0 <IP_ADDRESS> fargate-ip-10-5-21-19.eu-central-1.compute.internal 97s

Old IP's still ESTABLISHED:
ingress-nginx-controller-controller-654964c689-hvdcn:/etc/nginx$ netstat -napel | grep :53
udp        0      0 <IP_ADDRESS>:52251       <IP_ADDRESS>:53          ESTABLISHED -
udp        0      0 <IP_ADDRESS>:47983       <IP_ADDRESS>:53           ESTABLISHED -

NGINX log
2024/11/05 11:46:29 [error] 26#26: *1510 google.com could not be resolved (110: Operation timed out), client: <IP_ADDRESS>, server: test.test.com, request: "GET / HTTP/2.0", subrequest: "/_external-auth-Lw-Pre
2024/11/05 11:46:29 [error] 26#26: *1510 auth request unexpected status: 502 while sending to client, client: <IP_ADDRESS>, server: test.test.com, request: "GET / HTTP/2.0", host: "test.test.com"
<IP_ADDRESS> - - [05/Nov/2024:11:46:29 +0000] "GET / HTTP/2.0" 500 170 "-" "curl/7.81.0" 29 30.001 [default-test-https] [] - - - - 03ee00187735a1dce3308336932ae1df

Cilium Users Document

[x] Are you a user of Cilium? Please add yourself to the Users doc

Code of Conduct

[x] I agree to follow this project's Code of Conduct

Aha. I did not set that. I'll test this after next week as I'm unable to do it right now. Thanks!
I'm confused by the configuration options. The helm config enables the termination option by default when socketLB is enabled:
https://github.com/cilium/cilium/blob/aa0a7d7c781a6f9ecdac52c62b367a3f40cc2815/install/kubernetes/cilium/templates/cilium-configmap.yaml#L741C1-L743C11
And Cilium seems to be enabling socketLB based on the kubeProxyReplacement switch
I think here: https://github.com/cilium/cilium/blob/aa0a7d7c781a6f9ecdac52c62b367a3f40cc2815/daemon/cmd/kube_proxy_replacement.go#L58
Although I had read that kubeProxyReplacement relies on socketLB, I figured it was on because of the cilium-dbg status telling me it was enabled and, therefore, omitted the setting in the helm values file as if it was the default setting. This caused the setting not to enable, although it seems intended to react to the enablement of the kubeProxyReplacement setting.
Should we add this to the kube_proxy_replacement.go override or to the helm config?

I've changed the setup to use:
    socketLB:
      enabled: true
      terminatePodConnections: true

and the debuginfo shows:
bpf-lb-sock-terminate-pod-connections:true

But it still keeps the connection open:
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -
> both CoreDNS pods are killed and restarted with a different IP address
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -
udp        0      0 <IP_ADDRESS>:34430       <IP_ADDRESS>:53            ESTABLISHED -

We are seeing the exact same behaviour running Cilium v1.16.4, Kubernetes v1.30.6, with Loki & Mimir NGINX and CoreDNS. We were hoping an upgrade and the new terminatePodConnections would help, but it doesn't appear to have any effect unfortunately.

Related https://github.com/cilium/cilium/issues/27300

Hi @fe-ax, thank you for reporting the issue! Could I get a sysdump when you set  socketLB.enabled: true and terminatePodConnections: true (but it doesn't work as expected).
Regarding the helm chart, currently, terminatePodConnections is not enabled unless socketLB or terminatePodConnections is explicitly enabled, and it doesn't seem to work in the auto-enabling case.
https://github.com/cilium/cilium/blob/v1.16/install/kubernetes/cilium/templates/cilium-agent/daemonset.yaml#L318-L324
https://github.com/cilium/cilium/blob/v1.16/install/kubernetes/cilium/templates/cilium-configmap.yaml#L719-L723

I'm sorry my response is so late. It takes me quite some time to strip a cluster down and send a sysdump publicly here.
cilium-sysdump-20241217-204231.zip

Looks like bpf-lb-sock-hostns-only is true which means the service translation for the packets from the pod ns is handled by the per packet LB, not the socketLB. bpf-lb-sock-terminate-pod-connections is not the case with this setting.
  bpf-lb-sock: "true"
  bpf-lb-sock-hostns-only: "true"
  bpf-lb-sock-terminate-pod-connections: "true"

I'm sorry. I did it in a rush and forgot to use the proper rollout. I've done it more carefully now.

cilium-sysdump-20241220-161706.zip


fix(ComboButton): Update combo button PR

Contributes to #
Started clean PR for incorporate new changes
What did you change?
How did you test and verify your work?

i noticed there's no issue tied to this. just out of curiosity, what's the reason behind these changes?

bump


Xcode 15 XCResult File generate empty report

Run test with Xcode 15
xchtmlreport <.xcrestlt> This command generate empty screen for the tests

Warning: Can't find invocation record for : /Al-Mosafer_Dev.xcresult


Way to figure out if this integration is working/no working in automations

Is there a way to ge the state of this integration (working/error/re-auth needed etc) in an automation (as condition) so that I can use backup automations if Alexa Media Player is not working?
I need this because some of my automations require user input to turn off/on lights. If AMP fails due to re-auth, then my automations do not send actionable notifications and I need to run some other automation to toggle lights.

There are events when a reauth is required. You can use that. The wiki has automations that use that.

Perfect, got it: https://github.com/custom-components/alexa_media_player/wiki/Examples%3A-Automation#notification-when-amazon-login-required. Thanks.


Scheduled downtime

⚠ Please verify that this feature request has NOT been suggested before.

[X] I checked and didn't find similar feature request

🏷 Feature Request Type
New Monitor
🔖 Feature description
I have servers that have scheduled downtime for maintenance and it would help to set the scheduled downtime so it doesn't notify me for nothing.
✔ Solution
A server/service/device shuts down every night for 1 hour and I receive every night a notification that the device is down, and then back up. I suggest we set the expected uptime hours/minutes for every service (should me very detailed, like hours, days of the week, months, etc.)
❓ Alternatives
No response
📝 Additional Context
No response

Duplicate of #900 (#233) for scheduled downtime for notifications, and #191 for periodic scheduled downtime


Webpack cannot compile remark-lint

I know this library is for node.js. But, I need this for browser to show warnings.
Although I've already checked the code, I'm still confused about where should I fix.

Fix webpack configuration
Fix remark-lint
Fix npm-prefix or load-plugin

This is the error message from webpack.
WARNING in ./~/load-plugin/index.js
36:9 Critical dependency: the request of a dependency is an expression

ERROR in ./~/rc/index.js
Module parse failed: /Users/sarah-seo/Code/Inpad/node_modules/rc/index.js Unexpected character '#' (1:0)
You may need an appropriate loader to handle this file type.
| #! /usr/bin/env node
| var cc   = require('./lib/utils')
| var join = require('path').join
 @ ./~/npm-prefix/index.js 3:9-22
 @ ./~/load-plugin/index.js
 @ ./~/remark-lint/lib/index.js
 @ ./~/remark-lint/index.js
 @ ./src/lib/markdown.js

This seems to stem from rc, which does have a browserify field. Is it possible to support those in webpack?

@Sarah-Seo Ping!

Sorry, I forgot to answer.
Yes, It seems to be possible. 👍 I'll try it.
https://github.com/webpack/docs/wiki/configuration#resolvepackagemains

WARNING in ./~/load-plugin/index.js
36:9 Critical dependency: the request of a dependency is an expression

It is compiled anyway. But I got the critical warning.
I think loading all plugins should be done while compiling.
I'm afraid it would requires some drastic changes

loading all plugins should be done while compiling

To my knowledge, that’s what happens. But I’m guessing that as the code is still included, webpack warns.

I'm afraid it would requires some drastic changes

I think a fix could be to expose the same interface in load-plugin when in the browser, but exposing functions that throw an error instead of trying to require something?

Could you try if the browser branch of load-plugin works?

I don't know how to replace the branch. So, I just download it and replace all scripts.

Yeah that should work to.
What I mostly do is git clone the project, and checkout the branch. Then, I use npm link from the project, and npm link load-plugin from the project I want it into.

Thanks for your comment! Compiling didn't return any critical messages.
I'll inform you again after I actually implement this feature to my app. 👍

Alright, I pushed 2.1.0 of load-plugin, which includes this fix. Thanks for helping Sarah!


Feature request: Add rapport item tooltip

Is it possible to add a tooltip for the rapport item like the tooltip for the merchant location?

Probably a duplicate of #53 if that's what you mean


Redshift, canonical event schema and documentation issue

Hello,
We are currently planning to use the canonical event model as described in the documentation.
However, there is this line that is a bit confusing:

The field joined on is the distribution and sort key for both tables, so queries are as fast as if the data were in a single table

Which conflicts a bit with the schema proposed here which gives a SORTKEY on the collector_tstamp (and which does make sense) and not the event_id.
So is a SORTKEY better on the collector_tstamp or on the event_id (in the atomic.events table)?

Thanks, updated the documentation, closing!


🤖 Build static html export

This creates a new command myst build --html which exports the site as a static HTML page that can be hosted on GitHub pages or similar.
Requires:

https://github.com/executablebooks/myst-theme/pull/68

There is a bit more work that needs to be done to actually set up for using this for real, but getting close!

Thanks a lot, this is exactly what we were looking for after hearing the Myst talk at JupyterCon! I just tried this on Gitlab, by moving _build/html/ to public/. No error is raised, but I get a 404 error. Does _build/html/ contain the whole website?

It is likely that the project is in a nested folder and there is no index page, you can confirm by seeing what is inside of _build/html (likely a folder, that then has an index.html?!). To fix that, you can remove the site.projects list entirely in your myst.yml.
site:
  title: 'Pixels and Their Neighbours: Finite Volume'
-  projects:
-    - slug: pixels-and-their-neighbours
-      path: .
  actions:
    - title: Download PDF
      url: ./cockett-heagy-oldenburg-2016.pdf
      static: true
  logo_text: Pixels and their neighbours

This will need the most up to date site templates (as of yesterday), which can be pulled/updated with a myst clean --templates and then myst start.
I think that up next we should change the default to being 1 site == 1 project, where as currently the default is that 1 site >= 1 project, which requires some sort of folder nesting and redirects, which are not preserved in the static build.
Let me know if that works?!

Thanks for the help. So with @corentinmorice1 we created a repo https://gitlab.dsi.universite-paris-saclay.fr/corentin.morice/myst-test from the myst website tutorial, added a index.md file and removed the site.projects section from the myst.yml file. Now we are able to get a static website from gitlab pages, but the website is badly shaped
https://corentin.morice.gitlab.dsi.universite-paris-saclay.fr/myst-test/
I have the same building the html on my laptop, so it does not come from gitlab pages process. Something seems missing but what ? Any idea ?

I think that you need to set the BASE_URL environment variable to /myst-test.
export BASE_URL="/myst-test"
That will need to be updated for any repository, Github actions adding this works:
env:
  BASE_URL: /${{ github.event.repository.name }}

If you know something similar in gitlab, let me know and I will add to the docs I am writing now!

Thank you for the new doc about deployment. I tried the npx serve _build/html command and the web site was nice on my laptop. But I still fail to have a static website through giltab pages, despite your advice. I was able to set BASE_URL but it still fails:
$ echo $CI_PAGES_URL
https://corentin.morice.gitlab.dsi.universite-paris-saclay.fr/myst-test
$ echo $BASE_URL
/my-test

I haven't found an equivalent to the variable you are mentionning in github actions. Also, I don't really understand this sentence in the docs: "The build and site assets are in the /build folder, which would point outside of the current repository to a repository called 'build', which probably doesn't exist!" Maybe if you rephrase it to me I can guess how to fix gitlab pages.


Aerwhale overworld fix attempt

Just an attempt to add a limit to the aerwhale flight y, check line 90 of the aerwhale code.

Since the targeting AI for aerwhales is screwed up I don't believe that this code change actually makes a difference, from my testing, they find targets but don't move, and it appears to be the same in the current codebase

Also in the future keep other changes separated, the leaf texture is included in this PR

Rejected. This has been fixed in the 1.17 branch. You additionally lied in discord over this being done in 1.17.
As a reminder, active development has swapped to 1.17 until it's initial release, so please base further prs off that branch


BatchServerInventoryView is created twice

Brokers retain two copies of BatchServerInventoryView. One through BatchServerInventoryViewProvider and one through FilteredBatchServerInventoryViewProvider. This causes excessive use of heap when there are a large quantity of segments and servers.

This is a 0.9.1 regression from https://github.com/druid-io/druid/pull/2848

Here's an example of the Eclipse MAT analysis of a heap dump from one of the brokers.

Why does the broker need both? Shouldn't it only have a FilteredServerInventoryView?

@himanshug  / @nishantmonu51 ^^^^?

It looks like the one place it asks for injection of a ServerInventoryView instead of FilteredServerInventoryView is DruidBroker.java. Switching that to a FilteredServerInventoryView should work too and seems like a cleaner solution.

@gianm I'd like to get https://github.com/druid-io/druid/pull/3238 anyways so the space that replicants of a segment take up are minimized

agree with @gianm DruidBroker needs to be fixed to use FilteredServerInventoryView
will submit a fix shorty.


no_spaces_inside_parenthesis needed 2nd pass

Bug report
when applying this config: https://github.com/DataTables/Editor-PHP/blob/f8d4471daab6ea82f2ffeace3887caba00697ada/.php-cs-fixer.dist.php
over: https://github.com/DataTables/Editor-PHP/tree/f8d4471daab6ea82f2ffeace3887caba00697ada
the fixer needed to be run 2x to stabilize
   1) datatables-editor-php/Editor/SearchPaneOptions.php (no_spaces_inside_parenthesis)
      ---------- begin diff ----------
--- /__w/datatables-editor-php/datatables-editor-php/Editor/SearchPaneOptions.php
+++ /__w/datatables-editor-php/datatables-editor-php/Editor/SearchPaneOptions.php
@@ -412,7 +412,7 @@

         for ($i = 0, $ien = count($rows); $i < $ien; ++$i) {
             $set = false;
-            for ($j = 0; $j < count($res); ++$j ) {
+            for ($j = 0; $j < count($res); ++$j) {
                 if ($res[$j]['value'] == $rows[$i]['value']) {
                     $out[] = [
                         'label' => $formatter($rows[$i]['label']),

      ----------- end diff -----------

Thanks for reporting! I've made a proposal for a fix for this.


Legacy polyfills addition, Array.isArray, Array.prototype.some etc.

I noticed that the legacy ES5+ polyfills are not complete. Would it be too much to ask for these to be added or to be completely removed from ractive source?
I understand that Ractive core may not actually be using all of them, however, IMO, I think you should either should include them all or none, in the latter case, legacy support could be achieved by including something like the es5-shim as a hard requirement for legacy support.
I think that's a better idea than having to load ractive-legacy along with another es5-shim to complete it, but still partially redundant.

I do like the idea of not having a legacy version, and letting people bring their own shims. Unfortunately es5-shim doesn't include addEventListener and getComputedStyle, which Ractive needs, so it's not all that practical.
On the flip side, adding in all the missing pieces that es5-shim includes would bulk things out a fair bit - it's a pretty big file, compared to the 200-odd lines for the bits we currently include.
Not quite sure where I land on this. Would welcome other opinions.

It would be extra work, but you could split the polyfills out into their own library, a la ractive-es5-shim. That way the shims aren't in the main library, and if you need the shims you can easily import a shim library that includes addEventListener and getComputedStyle.

@Rich-Harris, I would

drop the legacy code
move the addEventListener  and getComputedStyle to the main ractive src (meaning not the legacy), I am assuming these 2 won't execute on good browsers. It is true that the LOC number will increase a bit for everyone, but for a good cause. Also, it doesn't look like your getComputedStyle function is getting added to the window anyways, according to this block, so that's another reason why you can't count on an external lib to shim it for you.
ship 1 "type" of ractive from now on, no more ractive-legacy
require everyone who wants ie8 support to include the es5-shim

The legacy polyfill is moving to its own file, so you will be able to shim/sham before it or wherever you like. See #2801


The 'complete', 'partial', etc filters are broken.

They all return 0 apis

I just removed all that :)


use nbsp in "data set"

Before:

After:

I would prefer Dataset which is okay according to https://en.wiktionary.org/wiki/dataset, but other sources say it's less academic


Support for waymo open dataset

Can you please give updates on the support for waymo open dataset. I am stuck on a project that requires the same. It would be really kind of you if it can be done soon.

Sorry I didn't check your recently closed issue


Why are we manually attaching?

The following snippet of code is what I use on my plugin lsp-rooter to attach automatically without having to attach it on the lspconfig config.
local on_attach = function(client, bufnr)
  -- Attach logic goes here
end

local _start_client = vim.lsp.start_client
vim.lsp.start_client = function(lsp_config)
  if lsp_config.on_attach == nil then
    lsp_config.on_attach = on_attach
  else
    local _on_attach = lsp_config.on_attach
    lsp_config.on_attach = function(client, bufnr)
      on_attach(client, bufnr)
      _on_attach(client, bufnr)
    end
  end
  return _start_client(lsp_config)
end

Would be great to stop the manual thing, and use the solution provided instead.

Thanks for the suggestion.
I updated the codes and added setup(cfg) function using start_client.  I will test it a few days before I update the document.

Closing this. As document updated. You can use setup() to initialize the plugin.


Add ClosestMatch.id

This field keeps IDs unique across multiple invocations of
ClosestMatch::Add().

Not quite right.


Windows install

Hi
does anyone have a windows installation guide?

Sucks this project has died, would have been of great use right about now..

It works pretty well on latest PMS version on Linux 😉

It works pretty well on latest PMS version on Linux 😉

I managed to make it function on windows.. more or less.
But my plan was to use another machine for hardware encoding, and apparently this can't do that. Soo that's out the window

This project don't support HW encoding 🥺


Basic documentation

Add content to the Documentation page on the frontend application. Topics should include, but not be limited to:

Hosting/ running
Design and architecture? i.e what is monitor? What is a job? When is a job late? How often do we check and notify for late jobs?
API docs (if not already covered in #14
code snippets/ examples (use tabs here - and build the docs as per how we want the integrations to look!)

Python (decorator)
Bash (using curl and jq)
Rust (reqwest)
NodeJs (TypeScript and/ or JavaScript?)
cron-mon-cli

For API docs there's https://github.com/koumoul-dev/vue-openapi which might be helpful, but I'm not sure how easily it could be made to fit the rest of the app's aesthetic. https://nuxt.com/modules/nuxt-openapi-docs-module might be better but it uses Nuxt, however it does say it's using reusable Vue components so we could just use those.
May also e worth a look at https://github.com/scalar/scalar


System reboots in the middle of map creation causes trajectories pre- and post-reboot to have no established connection

Hi there!
We have been using Cartographer for our project and encountered an issue due to the limited memory of our device. I apologize beforehand if a similar issue has already been asked, but since my search efforts have not turned up anything, I decided to explicitly ask for the case of our system.
While creating a map for a large indoor area, due to the limited nature of our system's memory, a reboot occurs while in the middle of map creation. Due to this, the previously known nodes and trajectories have been flushed from the work queue and it starts back to node 0.
My question is
Is there any way to establish a connection to the trajectories prior to the reboot in order to avoid the distortion of the created map?
As of writing, after reboot, Cartographer treats it as a different location and, therefore, the image created goes out of bounds of what it the actual dimensions of the room. We would like Cartographer, post-reboot, to still recognize trajectories created pre-reboot to avoid a decline in the created map image as much as possible. So if there is a anything we can utilize in Cartographer's existing codes, it would be much appreciated!
Thank you.

If you don't have to map in real time, you can save rosbag(s) and run cartographer later. Or you can save trajectories on some interval as pbstream files and continue mapping from that file.


Wrong Alertmanager version for v1 API removal in the logs?

What did you do?
I looked at Releases page. I looked at the source code https://github.com/prometheus/alertmanager/blob/cb9724db47dc1c1e4b8352baf62d21e9a6919d36/api/v1_deprecation_router.go#L55-L58
What did you expect to see?
I expected the log notice to say 0.27.0, as it's the first release that drops /api/v1.
What did you see instead? Under which circumstances?
I see 0.28.0 in the log message. It it a typo or my misunderstanding? :-)

Looks like a mistake! Thanks 👍


db import doesn't fix triggers

Hello everybody, I was starting a magento 2 project and importing a previous database with some triggers.
Warden db import fixes the triggers identifiers (changing the username) but it seems it doesn't change the host, leaving "localhost" instead of the db container name, resulting in errors during the importing phase.
Does anybody have the same problem?

I haven't seen this be an issue TBH, the regex used to do the replace when piping through warden db import is here:
https://github.com/davidalger/warden/blob/develop/commands/db.cmd#L37-L40
Particularly this bit:
LC_ALL=C sed -E 's/DEFINER[ ]*=[ ]*`[^`]+`@`[^`]+`/DEFINER=CURRENT_USER/g'

You could do something similar to strip your db further:
cat dump.sql | LC_ALL=C sed -E '...' | warden db import

Going ahead and closing this. If you can provide reproducible test case where the output of mysqldump fails due to a bad DEFINER statement, feel free to re-open with more details.

Going ahead and closing this. If you can provide reproducible test case where the output of mysqldump fails due to a bad DEFINER statement, feel free to re-open with more details.


feature confirmation

When I tried to download an anime, it says the episode is already downloaded and skipping download but it still download it anyway and overwriting my current dir. Is it a bug or not? if not may I ask to make it skip the already downloaded file completely?
thank you before.

can you give the full output of the command with the -ll DEBUG flag added to it?

Closing because inactivity, if you still have this issue open it again.


[iOS][URL Support] Telephony urls not recognized as such on iOS

Platform
What platform is your issue or question related to? (Delete other platforms).

iOS

Author or host
Host rendering in-house authored cards.
Version of SDK
iOS SDK v1.2.3
Details
When a telephony link is embedded in text using markdown syntax, it is not being treated as a link on iOS. Instead, the entire markdown construct is printed out as plain text.
Example:
[(555) 555-5555](tel:5555555555)

On the web and Android, this is converted to a link with the text "(555) 555-5555" that launches call functionality when tapped. On iOS, the entire construct is simply displayed as text:

@nesalang thoughts on this so far? Can you also update the bug with your findings? thanks!


Misleading Unused Variable Warnings if there's a syntax error in the file

Bicep version
Bicep CLI version 0.4.613 (d826ce8411)
Describe the bug
If there is a syntax error in the file making it unparseable, the unused variable warning can fire even if the variable is actually used.  In the below example, the accountName variable most certainly is used, but I presume the usage isn't detected as a knock on from the parser error (lack of closing brace on the resource).
To Reproduce
Build the following bicep snippet
var accountName = 'SomeName'
resource x 'Microsoft.Storage/storageAccounts@2021-04-01' = {
  name: accountName

Likely related/same issue as #3569


Value of the info.x-trapi.test_data_location property for the improving-agent ARA entry in the Translator SmartAPI Registry

The current value (as of August 4th, 2022 morning) of the info.x-trapi.test_data_location property is set for the improving-agent ARA entry in the Translator SmartAPI Registry to a non-existing github target. Please set the URL to a single JSON file resource, preferrably at the end of a https://raw.githubusercontent.com/ endpoint (if in Github. Actually, it can be anywhere on the internet as long as it is a REST accessible JSON file resource).

@RichardBruskiewich Thanks for alerting me.
Can you point me to the specification for the test data? I thought it was still under development, but if it's ready, I can put something together.


"follow instructions in this blog post" contains no link to a blog post

In the "How do I tune performance of my managed instance" section (https://docs.microsoft.com/en-us/azure/sql-database/sql-database-managed-instance-faq#how-do-i-tune-performance-of-my-managed-instance), it describes some instructions in a blog post, but there is no link to such blog post.

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

ID: aa11b569-0d56-63b7-aac5-4b2dd9a859e2
Version Independent ID: 87d7d50e-7fae-c36e-43af-ec18dbef9b48
Content: SQL Database managed instance FAQ
Content Source: articles/sql-database/sql-database-managed-instance-faq.md
Service: sql-database
Sub-service: managed-instance
GitHub Login: @jovanpop-msft
Microsoft Alias: jovanpop

@mikgam  Thanks for the feedback! I have assigned the issue to the content author to review further and update the document as appropriate.

@mikgam - thanks for raising this issue. I just update the article with the missing link and it should appear live soon.
Here's the link to the blog post: https://medium.com/azure-sqldb-managed-instance/impact-of-log-file-size-on-general-purpose-managed-instance-performance-21ad170c823e

#please-close


[SPARK-39836][SQL] Simplify V2ExpressionBuilder by extract common method.

What changes were proposed in this pull request?
Currently, V2ExpressionBuilder have a lot of similar code, we can extract them as one common method.
Why are the changes needed?
Simplify V2ExpressionBuilder by extract common method.
Does this PR introduce any user-facing change?
'No'.
Just update inner implementation.
How was this patch tested?
N/A

ping @huaxingao cc @cloud-fan

The failed tests are unrelated to DS v2, thanks, merging to master!

@cloud-fan @huaxingao Thank you!


Dropdown not working on live link while searching for repository

On searching org, it is showing the results. The issue is the dropdown not getting displayed 👎

Tags are working when manual link is changed 👍

changing  requests to the CDN from http to https:

Solved with commithttps://github.com/sagaragarwal94/github-search/commit/fc45b7af7e65e033651d6ed7805e365dd6a0c97e.


[Android][Cards cut off at a max height]

Platform

Android

Author or host
Both author and host
Virtual Assistant Client
Version of SDK
v1.2.1
Details
Discovered at microsoft/botframework-solutions#2356
Cards appear to be cut off at an unknown maximum height when using the Android SDK.
Initial investigations showed this as resolving by fixing https://github.com/microsoft/AdaptiveCards/blob/master/source/android/adaptivecards/src/main/java/io/adaptivecards/renderer/action/ActionSetRenderer.java#L60 so that the second MATCH_PARENT to WRAP_CONTENT.
Expected

Actual

@chon219 can provide more details if necessary

@golddove doesn't one of your PRs fix this issue?

This should be fixed in the latest builds; please re-open if this is not the case and we'll re-engage. Thanks!


SPIKE: Puppet Agent Container + PDK Gems

[ ] Use puppet-agent base container
[ ] Install the PDK runtime gems on top of this
[ ] Ruby commands can be run within this container (called externally)
[ ] Document pros / cons for the PoC for evaluation from team after

Experiment Notes 2021-09-13
Went to pursue a minimal experimental container which is built from the public puppet/puppet-agent container, using this dockerfile:
FROM puppet/puppet-agent
RUN gem install pdk
RUN apt-get update && apt-get install build-essential libssl-dev libyaml-dev zlib1g-dev libffi-dev -y
ENTRYPOINT ["/opt/puppetlabs/puppet/bin/pdk"]
CMD ["validate"]

If I navigate to any puppet module on my system, from inside that module folder I can run:
docker run -it  -v ${PWD}:/module -w /module pdkdocker

and it will display the expected validation behavior, using the appropriate ruby/puppet, downloading any missing dependencies, and running all the validators.
The image size (entirely unoptimized) is 479MB - large, but manageable.
It is also possible to install any/all native compiled gems, including pry.
I also experimented with calling gem install puppet-strings, which then allowed me to call puppet strings as expected.
Initial thoughts:

This could work really well for ubuntu agents; it gets us away from having to build the ruby runtime ourselves
We will have to figure out mac containers (if we want to) and Windows containers for ourselves - luckily I think the latter should be very straightforward and we might even be able to get the Puppet Agent repo to take ownership of a base image if we contribute it
We will be pinned to only supporting whatever releases of Puppet Agent get containerized which is good because that's all we care about
We could ship multiple versions of these containers: with/without dev tools, with/without everything pre-installed
The main thing to note for us is we'll always want to mount the folder somewhere reliable and set it as the working directory, which I think we can abstract in our go code easily enough.
This will require that the end user have Docker and access to either the public docker repo or their own mirror
This can be consumed neatly inside of a GitHub Action
This iteration did a very simplistic install of the PDK in order to get current functionality, but we may want to think about an alternate method (or maybe a new metagem that includes whatever runtime deps we care about?)
The Puppet Agent containers have a specific version tag, including 6.y.z / 7.y.z and a latest tag which is an alias of the newest 7.y.z but not a 6x or 7x tag - we may need to do some work around this on our end and may just want to build our own tags pinned to whatever those versions are that can be found on the registry.

Summary Thoughts
I think this is a fairly strong contender for our solution, assuming a docker-based solution is going to be functional for us at all - it's xplat, low lift (we maintain an additional set of dockerfiles and rebuild when upstream gets built) and manage via our own minimal interface eventually but initially we can just document how to use the dockerfiles directly. These dockerfiles could also be used as VSCode dev containers without too much issue, I think.

Experiment Notes 2021-09-15
Went to see how much work it would be to build our own Windows container that installs the Puppet agent and PDK gems; eventually, I think the windows container's base (ie just adding Puppet agent) could be moved upstream to puppetlabs/puppet-agent which would be a boon for everyone.
I got to an 80% effective solution:
FROM mcr.microsoft.com/windows:20H2
SHELL ["powershell", "-Command", "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'SilentlyContinue';"]

ADD https://downloads.puppetlabs.com/windows/puppet/puppet-agent-x64-latest.msi C:\\puppet.msi
RUN mkdir C:\Puppet ; msiexec /qn /norestart /i C:\puppet.msi INSTALLDIR=C:\Puppet
ADD https://github.com/msys2/msys2-installer/releases/download/nightly-x86_64/msys2-base-x86_64-latest.sfx.exe C:\\msys2.exe
RUN "C:\\msys2.exe -y -oC:\\"

RUN Remove-Item C:\msys2.exe ; \
  function msys() { C:\msys64\usr\bin\bash.exe @('-lc') + @Args; } \
  msys ' '; \
  msys 'pacman --noconfirm -Syuu'; \
  msys 'pacman --noconfirm -Syuu'; \
  msys 'pacman --noconfirm -Scc'; \
  msys 'pacman --noconfirm -S autoconf autogen automake-wrapper diffutils file gawk grep libtool m4 make patch sed texinfo texinfo-tex wget'; \
  msys 'pacman --noconfirm -S mingw-w64-x86_64-binutils mingw-w64-x86_64-crt-git mingw-w64-x86_64-gcc mingw-w64-x86_64-gcc-libs mingw-w64-x86_64-headers-git mingw-w64-x86_64-libmangle-git mingw-w64-x86_64-libwinpthread-git mingw-w64-x86_64-make mingw-w64-x86_64-tools-git mingw-w64-x86_64-winpthreads-git pkgconf mingw-w64-x86_64-pkgconf'
  # The line above installs x64 build tools, the line below x32
  # msys 'pacman --noconfirm -S mingw-w64-i686-binutils mingw-w64-i686-crt-git mingw-w64-i686-gcc mingw-w64-i686-gcc-libs mingw-w64-i686-headers-git mingw-w64-i686-libmangle-git mingw-w64-i686-libwinpthread-git mingw-w64-i686-make mingw-w64-i686-tools-git mingw-w64-i686-winpthreads-git pkg-config mingw-w64-pkg-config'

# Make sure Puppet's vendored ruby/gem are available on the path
RUN setx path "'%PATH%;C:\\Puppet\\puppet\\bin'"

ENTRYPOINT [ "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" ]
# RUN gem install pdk
# RUN apt-get update && apt-get install build-essential libssl-dev libyaml-dev zlib1g-dev libffi-dev -y
# ENTRYPOINT ["/opt/puppetlabs/puppet/bin/pdk"]
# CMD ["validate"]

Where this gets us:

Puppet Agent is installed/available
Msys2 and the build tool packages are all installed
We can build nokogiri but not http-parser for some reason; this matters because http-parser is a dep for honeycomb-beeline which means this will blow up when bundle installing for any IAC module
We can't use the naive gem install pdk because the vendored facter from Puppet Agent will conflict with the gem install facter
The container is giant, 17.2GB - could definitely be slimmed down with windows server core but there's some constraints around which images my machine can build/run bc of OS versions and the way these containers work
The container is slow, a noticeable longer-than-a-second pause at spin up and spin down

Summary Thoughts
This route is promising for the Windows container but there's definitely still dragons to fight. If we can get the native gem compilation stuff to work I think we can definitely slim the image down and address everything else.

Decided on this as the way forward, everything noted here is addressable.


Recovery

What version of Ajv are you using? Does the issue happen if you use the latest version?
Ajv options object

JSON Schema

Sample data

Your code

Validation result, data AFTER validation, error messages

What results did you expect?
Are you going to resolve the issue?

What version of Ajv are you using? Does the issue happen if you use the latest version?
1.1
Ajv options object
JSON Schema1.92.34
Sample data
BCH, LTC, Dash (https://github.com/LocalCryptos/LocalCryptosWalletBackupExplorer/commit/cf54a981efa122b4e1af18f61df4d27852c66528)
Release 1.1.0 (https://github.com/LocalCryptos/LocalCryptosWalletBackupExplorer/commit/eb00f0b5d442937bb1961b09e1c81916962a4a28)
Merge branch 'master' of github.com:LocalCryptos/LocalCryptosWalletBackupExplorer (https://github.com/LocalCryptos/LocalCryptosWalletBackupExplorer/commit/be007627d74b343840f602d6f409739380b312e3)
Fix compatibility with old LocalEthereum wallets. (https://github.com/LocalCryptos/LocalCryptosWalletBackupExplorer/commit/13a3ea7c9af9f0b45c0dd292aa77a9ece70d7562)
Delete _config.yml (https://github.com/LocalCryptos/LocalCryptosWalletBackupExplorer/commit/cbae974b24002b093cb289a2e619901ae51450e0)
Set theme jekyll-theme-cayman (https://github.com/LocalCryptos/LocalCryptosWalletBackupExplorer/commit/9345215c25b00ce09de212bfb2fca2796ed3b66a)
Link to release tab 👉 (https://github.com/LocalCryptos/LocalCryptosWalletBackupExplorer/commit/233dfd075978a33b567ae8eb999ed22edc05998c)
Your code
Validation result, data AFTER validation, messages

What results did you expect?
Are you going to resolve the issue?


UDF Style Updates

Description
• placeholder text on UDFs now displays as helper text
•  updating multiselect to use react-select
• updating oneof-select to be a regular select if options > 2, otherwise default to radio display.
• adding password prop to make helper text conditional
Type of Change

Non breaking change ('update', 'change')

Note to Reviewers
Please check that multiselect and oneof select components are still good. Currently there aren't any one click apps that have a multiselect UDF, but some of the security stackscripts under community do- you can view the updated component there.

Thanks @martinmckenna , I updated per your feedback.

I think the UDF multi select is still malfunctioning. After console logging the output of the UDF state, I get something like this

Notice how the object nested under the allow key is a duplicate of the parent object.
The manyof UDFs need to be send back as a comma separated list. So when you select an option from the dropdown, the full UDF payload should look something like
{
  LOGRO_FREQ: "monthly"
  LOGRO_ROTA: "12"
  /** This is the manyof UDF - notice how it's a comma-seperated list */
  allow: "NTP Service: UDP 123,Mail Submission: TCP 587,SSL IMAP Server: TCP 993"
  icmplevel: "Ignore all ICMP"
  loglevel: "Nothing"
  sshd_group: "sshusers"
  sshd_passwordauth: "Yes"
  sshd_permitroot: "No"
  sshd_port: "22"
  sshd_protocol: "2"
  sshrange: "0/0"
  sudo_usergroup: "wheel"
}

Let me know if you still need help / further explanation about the old code and what exactly it was doing

@alioso @martinmckenna Thanks for reviewing!
Alban- I updated the error to have spacingTop.  I need to get clarification on where the error is coming from, doesn't seem to be defined in manager. The workaround for testing is to select another plan (ex. Nanode) and that should get you through a successful creation.
Marty- Thanks for providing screenshots, I updated so now allow will come back as a comma separated string.


[Feature Request]: Integrating Email sending backend for contact us.

Is there an existing issue for this?

[X] I have searched the existing issues

Feature Description
The integration of an email-sending backend for the "Contact Us" section would allow users to submit their inquiries, feedback, or support requests through a web form. Upon submission, an email would be sent to the designated recipient (e.g., the support team or a specific email address), ensuring prompt communication and response.
Use Case

User Interaction: A user fills out the "Contact Us" form, providing their name, email address, subject, and message.
Form Submission: Upon clicking the "Send" button, the form data is sent to the backend.
Email Sending: The backend processes the request and sends an email to the specified recipient, notifying them of the new inquiry.
User Feedback: An acknowledgment message is displayed to the user confirming that their message has been sent successfully.

Benefits

Enhanced User Experience:

Convenience: Users can easily submit their inquiries or feedback without having to navigate away from the website, ensuring a seamless experience.
Quick Acknowledgment: Automated email responses can reassure users that their messages have been received and are being addressed.

Efficient Communication:

Direct Interaction: Establishes a direct line of communication between users and the support team, facilitating quicker resolutions to issues or questions.
Prioritization of Inquiries: The support team can categorize and prioritize incoming emails, allowing for more effective management of user requests.

Add ScreenShots
No response
Priority
High
Record

[X] I have read the Contributing Guidelines
[X] I'm a GSSOC-EXT contributor
[X] I'm a HACKTOBERFEST contributor
[X] I have starred the repository

@AbhijitMotekar99  It has already been done.


fix(ci): remove unused "type: ignore" comment to unblock precommit check in CI

fix(ci): remove unused "type: ignore" comment to unblock precommit check in CI
SUMMARY

Fix blocking CI in feature branches, for example:

Link: https://github.com/apache/superset/actions/runs/10200377636/job/28219694742
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

At least pre-commit checks CI  step should pass green
ADDITIONAL INFORMATION

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

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

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

The pre-commit checks CI step is green now. PR is ready for review! :D
https://github.com/apache/superset/actions/runs/10200704000/job/28220850318?pr=29830

Thanks!


Hulu not working - Posible solution

Hi,
Since hulu has blocked almost every VPS. Is it posible to hire any SmartDNS provider that works, and use that service on my own server to redirect all traffic through that SmartDNS?
This will allow me to use 2 different IP address at the same time, to use one SmartDNS provider with the same account. I understand that clearly is not what they want, but I was just wondering it from a technical point of view.
Clearly they do something like that with their servers because I don't think that they need to change their smart dns server ip every time they get blocked. I think the have on server that internally redirects all traffic to other non blocked ip, so when those get blocked they just go for new ones and customers don't need to change the DNS every time.
Is that the way it works? Can it be replicated with this this schema?
My Device -> SmartDNS (DigitalOcean)  -> SmartDNS(Any SmartDNS provider that is working with Hulu) ?
Best Regards,

Hi there, yes you could define a DNS zone so it returns any IP address. So just define a separate zone for hulu.com and have it return the IP of your other provider.
-- ab1

On 26 Jan 2016, at 14:53, tomcontr<EMAIL_ADDRESS>wrote:
Hi,
Since hulu has blocked almost every VPS Is it posible to hire any SmartDNS provider that works, and use that service on my own server to redirect all traffic through that SmartDNS?
This will allow me to use 2 different IP address at the same time, to use one SmartDNS provider with the same account I understand that clearly is not what they want, but I was just wondering it from a technical point of view
Clearly the do something like that with their servers because I don't think that they need to change their smart dns server ip every time they get blocked I think the have on server that internally redirects all traffic to other non blocked ip, so when those get blocked they just go for new ones and customers don't need to change the DNS every time
Is that the way it works? Can it be replicated with this this schema?
My Device -> SmartDNS (DigitalOcean) -> SmartDNS(Any SmartDNS provider that is working with Hulu) ?
Best Regards,
—
Reply to this email directly or view it on GitHub.


Search by slug

If you search dotenv the brand does not appears. It would be great if brands can be searched by slug also.

Done in https://github.com/mondeja/simple-icons-website-rs/commit/578c8b965f4d7149924fd41ea22d82ad370e3042


feat(workflow): add workflow for publishing json-sql to hub

This adds a new workflow for publishing json-sql smartmodule to the hub.
It uses the reusable publish workflow for smartmodules defined at here.
A similar workflow can be found here.

bors r+


api: add field to allow clients to ignore unknown HTTP filters

Signed-off-by: Mark D. Roth<EMAIL_ADDRESS>Commit Message: api: add field to allow clients to ignore unknown HTTP filters
Additional Description: This allows control planes to tell clients that a given HTTP filter is okay to ignore if it is not supported.  This is useful in environments where the clients are not centrally controlled and therefore cannot be upgraded at will, where it is desirable to start using a new filter for clients that do support the new filter without breaking older clients.  Unlike a client-capability-based approach, where the client tells the server which filters it supports, this avoids cache pollution and resource-name-based complexity on the server.  And because the control plane can set this on a per-filter basis, this approach does not impose risk that clients will silently fail to apply filters that provide mandatory functionality (e.g., authz policies).
Risk Level: Low
Testing: N/A (actual functionality will be implemented in a subsequent PR)
Docs Changes: Included in PR
Release Notes: N/A
Platform Specific Features: N/A
CC @htuch @ejona86 @dfawley @lidizheng @dapengzhang0

LGTM. @lizan can you provide a non-Google API review here, want to sanity check we're not over-fitting to Google needs?


FFTW3 linker error

I run .\vcpkg.exe install fftw3:x64-windows successfully. I added #include <fftw3.h> to my code and tried to compile. I got the errors
error LNK2001: unresolved external symbol __imp_fftw_plan_dft_r2c_1d
error LNK2001: unresolved external symbol __imp_fftw_execute

in release mode, and in debug mode i got:
error LNK2019: unresolved external symbol __imp_fftw_execute referenced in function main
error LNK2019: unresolved external symbol __imp_fftw_plan_dft_r2c_1d referenced in function main

I already run .\vcpkg.exe integrate install successfully too, before this install. Did I do something wrong? In folder vcpkg\packages\fftw3_x64-windows I didn't find any .dll file.
I am using MSVS Community 2019 version 16.1.2 and vcpkg version 2018.11.23-nohash.

I managed to use it by installing it manually.

download the windows dlls from (FFTW)[http://www.fftw.org/install/windows.html]
run the lib.exe on the files as described on the page
copied the files near my project and added that folder as an Additional Library Directory for the Linker
added libfftw3-3.lib;libfftw3f-3.lib;libfftw3l-3.lib as Additional Dependencies

Important: If you want to compile FFTW as a DLL, you should add a line #define FFTW_DLL to fftw3.h and ifftw.h before compiling. This will add the requisite __declspec decorations to function declarations etc.

https://github.com/microsoft/vcpkg/blob/a6960ed0b168a06414fce1e4063772134f319f3d/ports/fftw3/portfile.cmake#L102-L108
SOURCE_PATH must be replaced by CURRENT_PACKAGES_DIR
as a workaround add FFTW_DLL to your preprocessor definitions. For CMake users we could patch fftw3 CMakeLists where add_definition(-DFFTW_DLL)  could be replaced with a target_compile_defintions(... PUBLIC FFTW_DLL) call which than adds the defintion to the generated targets. Too late in my timezone for doing that now ;)

Dear @Neumann-A
Thank you for your fast reply.

I am sorry but I am not sure what your answer is referring to exactly. After the above mentioned handmade installation I still needed to provide the dll-s, but this wasn't my problem. My problem was that although I think I followed the procedure of the fftw installation via vcpkg, I still couldn't compile my code due to a linker error (and not runtime error). Was your answer referring to this issue?
My fftw.h contains #define FFTW_DLL at line 50 (the 3rd non commented line). However, ifftw.h cannot be found.
A took a look on vcpkg/ports/fftw3/portfile.cmake but I couldn't find the lines mentioned. Was  "SOURCE_PATH must be replaced by CURRENT_PACKAGES_DIR" an instruction for me?
I added FFTW_DLL to the preprocessor definitions. After that I got the same errors and a new warning: C:\local\vcpkg\installed\x64-windows\include\fftw3.h(50,1): warning C4005:  'FFTW_DLL': macro redefinition.

I am sorry but I am not sure what your answer is referring to exactly.

Interesting what github shows and what can be checked out with git are too different things...
the DLLs should be located at vcpkg\packages\fftw3_x64-windows\bin or vcpkg\installed\x64-windows\bin
error LNK2001: unresolved external symbol __imp_fftw_plan_dft_r2c_1d
error LNK2001: unresolved external symbol __imp_fftw_execute

I cannot find those symbols in fftw3. Do you have other macro redefinition warnings in your build? I also checked how fftw3 generates its name and this does not look like one of those generated names. They should all start with fftw<prec>_ .
as seen here:
#define FFTW_CONCAT(prefix, name) prefix ## name
#define FFTW_MANGLE_DOUBLE(name) FFTW_CONCAT(fftw_, name)

#define FFTW_DEFINE_API(X, R, C)                                        \
FFTW_EXTERN void                                                        \
FFTW_CDECL X(execute)(const X(plan) p);

#if defined(FFTW_DLL) && (defined(_WIN32) || defined(__WIN32__))
   /* annoying Windows syntax for shared-library declarations */
#  if defined(COMPILING_FFTW) /* defined in api.h when compiling FFTW */
#    define FFTW_EXTERN extern __declspec(dllexport)
#  else /* user is calling FFTW; import symbol */
#    define FFTW_EXTERN extern __declspec(dllimport)
#  endif
#else
#  define FFTW_EXTERN extern
#endif

#if defined(_WIN32) || defined(__WIN32__)
#  define FFTW_CDECL __cdecl
#else
#  define FFTW_CDECL
#endif

FFTW_DEFINE_API(FFTW_MANGLE_DOUBLE, double, fftw_complex)

Maybe try again with a clean VCPKG folder?

Thank you @Neumann-A again for the fast reply.

No fftw dlls were found vcpkg\packages\fftw3_x64-windows\bin or vcpkg\installed\x64-windows\bin. Especially, the first folder doesn't even exist.
Only those 2 errors were present, no other warnings were shown.
What do you mean by "a clean vcpkg folder"? I created a new folder, pulled vcpkg there, installed with .\bootstrap-vcpkg.bat and asked .\vcpkg install fftw3:x64-windows but I was told that

The following packages are already installed:
    fftw3[core]:x64-windows
Starting package 1/1: fftw3:x64-windows
Package fftw3:x64-windows is already installed

And of course, point 3. didn't solve my problem.
After these I tried the following as well without any success. I restarted VS and created a new solution and project that requires boost and fftw (both installed with vcpkg). But I have got the same linker errors again, only for fftw.
One more thing to mention: if I add /VERBOSE to the linker, it tells that
1>    Searching C:\local\vcpkg\installed\x64-windows\lib\fftw3.lib:
1>    Searching C:\local\vcpkg\installed\x64-windows\lib\fftw3f.lib:
1>    Searching C:\local\vcpkg\installed\x64-windows\lib\fftw3l.lib:

and they are indeed there, but nothing else in regards to fftw (beside the errors). Shouldn't it tell that it found something in those libraries? Those libs in vcpkg's folder are considerably larger (9MB) than those I produced from the downloaded file from FFTW's webpage (200KB, but with those come the dlls at around 2MB). NB: the self-produced libs are named like libfftw3-3.lib are added to as an additional library to the linker in the project properties.

No fftw dlls were found vcpkg\packages\fftw3_x64-windows\bin or vcpkg\installed\x64-windows\bin. Especially, the first folder doesn't even exist.

that is strange

What do you mean by "a clean vcpkg folder"? I created a new folder, pulled vcpkg there, installed with .\bootstrap-vcpkg.bat and asked .\vcpkg install fftw3:x64-windows but I was told that

that is even stranger. Are you sure the folder was empty?

After these I tried the following as well without any success. I restarted VS and created a new solution and project that requires boost and fftw (both installed with vcpkg). But I have got the same linker errors again, only for fftw.

has nothing to do with VS so anything you do there will do nothing to solve the problem.
You VCPKG folder/installation is somehow corrupted.

Shouldn't it tell that it found something in those libraries? Those libs in vcpkg's folder are considerably larger (9MB) than those I produced from the downloaded file from FFTW's webpage

That tells me you are using an old VCPKG version where FFTW was building a static library using x64-windows. (which was changed 3 weeks ago or so)
My best guess here is delete anything vcpkg related and remove the vs integration first. Then restart from scratch with git clone https://github.com/microsoft/vcpkg.git

Dear @Neumann-A
Thank you for your efforts, we succeed. I removed the vcpkg folder and recreated, then pulled vcpkg and installed again all the packages. It works now.
Was the vcpkg faulty, or was the package information too old? How could I refresh the required units next time without removing the whole folder and reinstalling all the packages?
And a wish: is there a way to ask the compiler to merge the fftw3.dll file with my executable in a single file so I don't need to copy both of them to the place where I'd like to run them?

Was the vcpkg faulty, or was the package information too old?

Thanks for posting this issue. This issue has been fixed. So I am closing this issue for now.