[account-web-vault] Refactor how database endpoints are specified

Key requirements:

We need to pivot the architecture so that an end user, by default, is responsible for setting the service endpoint(s) to use for a given application context.
We also need to ensure an application running on testnet that talks to user's Vault ensures the Vault is running in testnet mode as well.
We need to make connecting an application to the Vault much simpler, which is currently complicated by the need to specify database endpoints.

Changes required:

[x] Applications are no longer required to specify endpoints when connecting an account to the Verida network via the Vault (this will still be required when using the account-node package). Task
[ ] The QR code must specify the network being connected to (testnet, mainnet), so the Vault can construct an appropriate response with correct service endpoints for the requested network. Task
[ ] The Vault must be updated to understand different networks (login screen, settings). Task
[ ] The Vault must be updated to allow users to specify their own default endpoint or host their own. Task
[ ] The developer interface for connecting a user to an application must be simplified to it's minimal requirements. Task

Suggested interface:
const options = {}
const context = new Context(`CONTEXT_NAME`, 'testnet', options)

I think this is mostly right, except how does an application developer supply a default storage node if they choose? If I'm using a Primacy app I think I'd expect to use their node unless I explicitly decide otherwise.
I think maybe something like this is what we want (this really should be a state table):
Case 1

dev specifies mainnet/testnet
user specified nothing
Storage is in Verida node on mainnet/testnet

Case 2

dev specified mainnet/testnet
user specified their own preference
Storage is on the user chosen preference

Case 3

dev specified their own storage
user specified nothing
Storage is on dev specified node

Case 4

dev specified their own storage
user specified their own preference
Storage is on the user chosen preference

That makes sense. The key here is the user has choice, which will need to be presented to them in the Vault screen when they sign into an application context for the first time.
The user will always have a default storage node available (via the Vault). As such, I think there's just two options:

Developer doesn't specify a storage node. The storage node will default to the user's Vault default.
Developer does specify a storage node. The user will be asked which storage node to use; the developer supplied storage node or their own. This could even have a default setting where the user always selects one or the other

What if the developer specified testnet and the user chooses their own node?
The dev then moves into production, specifying mainnet, and the user remains on their own node. The context name is the same (I assume?)
The user might move back to the testnet version. Would this then corrupt (overwrite/combine) data?

What if the developer specified testnet and the user chooses their own node?

The Vault will now have a distinction between testnet and mainnet nodes.
As the QR code will be updated to specify the network, the Vault can use this to ensure it only links a node for the user that is running on the correct network.

But when the user specifies their own storage do they need to understand the difference between mainnet and testnet?

does the context name and environment need to be combined somehow? So if you have on personal storage node, the same context name and are switching between mainnet and testnet they appear different environments?

But when the user specifies their own storage do they need to understand the difference between mainnet and testnet?

Probably not.
The majority of users will be on mainnet when they first install the app and will remain in that mode unless they specifically go into their settings and switch to testnet.
As such, adding a storage node will add that node to the currently selected network in the app.
I'd expect developers to know what they're doing and be following instructions to switch to testnet before adding a testnet storage node.

does the context name and environment need to be combined somehow? So if you have on personal storage node, the same context name and are switching between mainnet and testnet they appear different environments?

I'm not sure what you mean here.

Looking for @pranavburnwal feedback here.

Closing this old issue. Will raise new ones if we decide to change further.


Fix File/Class Names

Change Log

Change main java file/class name from SkyblockCore to SkyBlockCore and all references
Change command file/class name from SkyblockCoreCommand to SkyBlockCoreCommand and all references

Should we change the config file name before this gets used in a real project?
Only developers will suffer from the change, definitely worth it before we have to start considering backwards-compatibility for users.

Decided this was a topic for another PR, let's merge this.


rectify function and variable referred in pkg/archive/changes_linux.go

rectify function and variable referred in pkg/archive/changes_linux.go.

yeah, that is more better, has been restored.


uop_given_valid in real_strides

simplified idx allows deriving more strides

no process replay diff now. 2 more TODO cases in TestRealStrides after this


netbsd adding couple of handy helpers from lb=ibutil

r? @Amanieu
(rust-highfive has picked a reviewer for you, use r? to override)

@bors r+

:pushpin: Commit b9d89c726ef2adfaf1097c6a2668c0fc9f3068ef has been approved by Amanieu

:hourglass: Testing commit b9d89c726ef2adfaf1097c6a2668c0fc9f3068ef with merge 122f95d2f6ac257535f6858331ba33c69fda5999...

:sunny: Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: Amanieu
Pushing 122f95d2f6ac257535f6858331ba33c69fda5999 to master...


Download Secure Files false positive on downloading error string.

Note
Issues in this repo are for tracking bugs, feature requests and questions for the tasks in this repo
For a list:
https://github.com/Microsoft/azure-pipelines-tasks/tree/master/Tasks
If you have an issue or request for the Azure Pipelines service, use developer community instead:
https://developercommunity.visualstudio.com/spaces/21/index.html )
Required Information
Entering this information will route you directly to the right team and expedite traction.
Question, Bug, or Feature?
Type: Bug
Enter Task Name: DownloadSecureFile@1
list here (V# not needed):    1.198.0
https://github.com/Microsoft/azure-pipelines-tasks/tree/master/Tasks

Environment

Server - Azure Pipelines or TFS on-premises?

Azure pipeline
- If using TFS on-premises, provide the version:

- If using Azure Pipelines, provide the account name, team project name, build definition name/build number:

Agent - Hosted or Private:

If using Hosted agent, provide agent queue name:

If using private agent, provide the OS of the machine running the agent and the agent version:

Issue Description
[Include task name(s), screenshots and any other relevant details]
File downloaded is an error string , instead of expected file. Task is marked as successful
When downloading a file , sporadically a failure will occur (one in maybe 30 runs) , upon tracking it down the file has the text: '{"$id":"1","innerException":null,"message":"TF15004: The download request signature has expired.","typeName":"Microsoft.TeamFoundation.Framework.Server.DownloadTicketValidationException, Microsoft.TeamFoundation.Framework.Server","typeKey":"DownloadTicketValidationException","errorCode":0,"eventId":4008}'
Inside, instead of the normal expected text.
This seems to be a duplicate of https://github.com/microsoft/azure-pipelines-tasks/issues/15479, however we are already using DownloadSecureFile@1.
https://github.com/microsoft/azure-pipelines-tasks/blob/master/Tasks/DownloadSecureFileV1/predownloadsecurefile.ts
The task itself expects the file to be present, and if so, is deemed a success. I think that the securefile-common (https://github.com/microsoft/azure-pipelines-tasks/blob/master/common-npm-packages/securefiles-common/securefiles-common.ts), On line 50, where it writes to a pipe is responsible for this false-positive as it is writing the error output from the web-api call to the file, in the case that the azure api fails and is thereby fulfilling the task check that the file exists, where it should have failed.
Task logs
[Enable debug logging and please provide the zip file containing all the logs for a speedy resolution]
Troubleshooting
Checkout how to troubleshoot failures and collect debug logs: https://docs.microsoft.com/en-us/vsts/build-release/actions/troubleshooting
Error logs
[Insert error from the logs here for a quick overview]

It is also worth noting that sometimes the task will fail with the error ##[error]Error: connect ETIMEDOUT <IP_ADDRESS>:443, which is expected. I think the difference is that the ETIMEDOUT is caught in the azure api and the promise is rejected , while the '{"$id":"1","innerException":null,"message":"TF15004: The download request signature has expired.","typeName":"Microsoft.TeamFoundation.Framework.Server.DownloadTicketValidationException, Microsoft.TeamFoundation.Framework.Server","typeKey":"DownloadTicketValidationException","errorCode":0,"eventId":4008}' is the direct promise return.


Bug/fix some other problems

Changes:

Fix issue with scorecard body content styles in storybook
Use ChannelValueProps in place of deprecated props in scorecard stories
Set publish date in changelog and update version

I'm not going to keep re-running this through circleci. Everything passed in the previous commit and all I did was update the release date in the CHANGELOG.md for the last commit. We'll be battling this all the way to master so I'm ignoring it at this level as it keeps failing due to memory.


Fatal Error on Mac OS High Sierra

Hello,
After upgrading to High Sierra mmfa is no longer working.
fatal error: MSpanList_Insert

runtime stack:
<empty>

I've uninstalled and re-installed via homebrew and by hand through github.

@AaronEast @jperez-dx Sorry for the delay. I've released 0.3.1 which should address the issue. Give it a whirl and let me know if it works for you.

@thbishop worked for me! 👍

Closing. If you hit any issues, feel free to report back. Thanks.

@thbishop I'm finally getting back to this, sorry for the delay.  It's working now, thanks.


[Bug]: passing paths with ~ doesn't work

What went wrong?
When running nvim ~, it doesn't recognize it as a special character and tries to create a file names ~. This should resolve to home if not escaped.

This is still not fixed. Running nvim ~ opens a file called tilda.


Document patching cooldown period guidelines as discussed 2022-10-20

With:
@BillyONeal @ras0219-msft @ValeriaConde @JavierMatosD @vicroms @markle11m @danshaw

This topic came up as a result of https://github.com/microsoft/vcpkg/pull/27260
See also: https://github.com/microsoft/vcpkg/pull/27379

What about build system/script fixes/changes and about CMakelists.txt vcpkg owns (could also be considered patching)?

This topic came up as a result of https://github.com/microsoft/vcpkg/pull/27260

What about build system/script fixes/changes

Exactly.
In the past there seemed to be a more relaxed policy with regard to patches to the build system in order to control the build of tests, examples, docs and tools, in contrast to features that modify the actual source code. In fact, I felt encouragement to move tools out of default features. Aka "vcpkg is a library manager".
Raising the requirements for interaction with upstream will raise the barrier for actively contributing. Given the current quality of ports and of reviews, a stricter policy may do more harm than good. In particular with platorms which are not first tier neither in vcpkg nor upstream.

In the past there seemed to be a more relaxed policy with regard to patches to the build system in order to control the build of tests, examples, docs and tools

Especially tests and examples can drastically increase build time or introduce extra dependencies. Therefore, I think it is ok to patch them out. In the case of docs, it depends on things like the size of docs or to wich location the files are installed. Given that the maintainer guide doesn't say "don't build tools by default" or tools might be needed to use the port, there is less argument to patch them out.
In general, I think that it depends: We have to balance usability [size, build time, install paths (e.g. for docs)] and maintainability of a port. In cases where the patch is very small, I think it is ok to patch out docs, examples or tests. Overall, this shouldn't be cosmetic changes.

Raising the requirements for interaction with upstream will raise the barrier for actively contributing.

We could at least partially make it easier to handle patches that don't apply anymore after a port update by providing a guide on this topic.
Is there any evidence that ports with at least one patch receive less PRs and is there a correlation between the size of patches and the number of PRs?

Is there any evidence that ports with at least one patch receive less PRs and is there a correlation between the size of patches and the number of PRs?

There is evidenve of ports being in barely useable state (at least for some "supported" triplets) over extended periods of time despite having patches. This is an indication of too few people either using the port or contributing to it.
There is evidence that users switched to other package manager due to unhandled port issues.
There is no reason to look at the size of patches here. Of course, the current patch in #27260 is bug because it is as-upstreamed. For vcpkg, a minimal patch would avoid the formatting changes, and just add if/ endif etc.

@Neumann-A

What about build system/script fixes/changes and about CMakelists.txt vcpkg owns (could also be considered patching)?

We tend to do that only for very simple resulting build systems, and we should be notifying upstream at least 30 days in advance before doing so in the future.

@dg0yt:

In the past there seemed to be a more relaxed policy with regard to patches to the build system in order to control the build of tests, examples, docs and tools, in contrast to features that modify the actual source code.

Yes, because it is less likely that upstream would disagree with such changes. The intent is that "including but not limited to" and the associated reasons we try to avoid patching would carry the burden here. The problem is that it's difficult to codify exactly what kinds of build system changes are acceptable. Patching out tests/examples or warnings? Clearly OK. Patching in huge build system changes to support another architecture or similar? Clearly a problem.

Of course, the current patch in https://github.com/microsoft/vcpkg/pull/27260 is bug because it is as-upstreamed.

#27260 certainly straddles the line on this policy. I argued that it is over the line of needing upstream approval attempted because it adds an entirely new configuration of this thing that users may expect upstream to be supporting. It isn't "upstreamed" while upstream's maintainers have yet to look at it.

Raising the requirements for interaction with upstream will raise the barrier for actively contributing.

The intent is not to raise the requirements for interaction with upstream; the intent is to clarify what our principles are for when upstream interaction is required, so that it doesn't change according to which particular vcpkg maintainer is reviewing the PR. We have always requested that upstream be kept in the loop for substantial changes, this is just codifying that we do that.

There is evidenve of ports being in barely useable state (at least for some "supported" triplets) over extended periods of time despite having patches. This is an indication of too few people either using the port or contributing to it.

Being an official triplet means that we test for it and have an opportunity to fix it if it breaks, not an obligation on the part of any library that wants to be indexed that it must support that triplet.

There is evidence that users switched to other package manager due to unhandled port issues.

I agree that that's unfortunate but the alternative where we do things upstream disagrees with, and thus claim to be them but are not them, is worse. The Debian OpenSSL key generation bug was caused by similar well-meaning-maintainer-patches.

There is no reason to look at the size of patches here.

The size of the patch can matter due to the license-entanglement and maintainability parts. However, it's easy to write short patches that upstream would disagree with, so we intentionally included nothing here about patch size. A 1 line patch is not presumed good. A 1000 line patch is not presumed bad (if the patch is easy to maintain across versions and the patched thing is MIT licensed).

In particular with platorms which are not first tier neither in vcpkg nor upstream.

If it is going to create headaches for upstream that they may not want, they deserve the ability to say no. They are, ultimately, the owners of the installed content.

@Thomas1664

Especially tests and examples can drastically increase build time or introduce extra dependencies.

Added
* Disabling irrelevant-in-vcpkg components of the build such as tests or examples.

to the list of presumed-minimal. Adding features that strip out dependencies still should be worked out with upstream.

Given that the maintainer guide doesn't say "don't build tools by default" or tools might be needed to use the port, there is less argument to patch them out.

Unfortunately what "tools" means in this context is difficult to codify into guidelines. However, most libraries-that-come-with-tools also come with an upstream that cares about what happens with those tools, and if they care, we should be making an attempt to keep them in the loop.

We could at least partially make it easier to handle patches that don't apply anymore after a port update by providing a guide on this topic.

I wish I had good advice on how to handle that in general; in general I've been replaying all such changes manually :(

Is there any evidence that ports with at least one patch receive less PRs and is there a correlation between the size of patches and the number of PRs?

I don't know :(

https://github.com/microsoft/vcpkg/pull/27260 certainly straddles the line on this policy.

How about not adding spaces so that the patch is smaller? (even though the patch wouldn't be accepted upstream in that form )

If it is going to create headaches for upstream that they may not want, they deserve the ability to say no. They are, ultimately, the owners of the installed content.

I disagree. The used license determines usage. They can say that they don't support x but it doesn't stop user to use it on x without support (of course problems then have to be solved by yourself; and x is probably often windows ;) ).

in general I've been replaying all such changes manually :(

I also do that manually. Don't see a way to fix patches automatically.

Is there any evidence that ports with at least one patch receive less PRs and is there a correlation between the size of patches and the number of PRs?

In the past the glib chain (atk,cairo,glib, etc.) up to GTK was very outdated since it was all vcpkg vendored CMakeLists.txt. With the switch to the upstream buildsystem, it at least appears to me, that the chain is kept more in sync. Another example is #27444 where I thought at least 10 times of updating it but having the vendored CMakeLists.txt really stop me doing it.

Close this PR and please track it with https://github.com/microsoft/vcpkg-docs/pull/35.


Replace window with element.ownerDocument.defaultView

There  are many instance where to get an element's window, the window variable is simply used.  However, this should be changed to using element.ownerDocument.defaultView, because an element might not be in the top level window

Done


add functionalities to initialize a new project from a project.json file

Add the capability that the tool can read in a project file. The file must contain a project key for the new project. The file may contain multiple issues which will be created in the neew project. The file may contain links for the issues to be connected (Task - Sub-Tasks)

A project is not created by the file. The project is created manually by the Jira admin, and the initial issues are created by pyJiraCli


Support IPXE Boot Option

What would you like to be added: Add support for IPXEBootConfig to enhance boot options for infrastructure.
Why is this needed: Better flexibility and pluggability in managing boot processes.

cc @defo89 @MalteJ @stefanhipfel


Fixing inconsistencies in HBase meta

There were several exceptions "org.apache.hadoop.hbase.exceptions.FailedSanityCheckException: Requested row out of range for doMiniBatchMutation on HRegion"
Recommended action for this to repair the hbase meta using
sudo hbase hbck -fixMeta  "table name"
But when I run this command, it fails with permission error
WARN  [main] util.HBaseFsck: Got AccessDeniedException when preCheckPermission
org.apache.hadoop.hbase.security.AccessDeniedException: Permission denied: action=WRITE
Please rerun hbck as hdfs user hbase
Please advice, how to run these commands under "hbase" user

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

ID: 6cf5cf11-5255-58cd-02a6-2549db81a219
Version Independent ID: 62838324-6415-c63b-b935-7c394c7d8f11
Content: Troubleshoot HBase by using Azure HDInsight
Content Source: articles/hdinsight/hbase/apache-troubleshoot-hbase.md
Service: hdinsight
GitHub Login: @nitinver
Microsoft Alias: nitinver

@dhananjaypatkar  Thanks for the feedback. Your feedback has been shared with the content owner. We will get back to you shortly.


Templating, dart_model and augmentation language version

In this PR I use {{uri#name}} in macro-generated augmentation "code" to signal types that need to be resolved, as suggested by Remi here.
This does make the macro code look a lot nicer and I guess we'll want something along these lines if it's workable.
Concerns :)

Performance. I'm guessing it's trivially fast enough, but it's worth checking.
Representation on the wire, versioning.

By "representation on the wire" I mean: are templates parsed on the client then sent as structured data to the host, or are they sent as strings to the host and parsed there? Then, crucially: what happens if the template format changes?
Now that I think about this, I actually think it's the same problem as language versioning. It can be the case that an augmentation is valid Dart 3 code but not valid Dart 4 code. So, sending code as raw strings is already a problem: what we should be doing is tagging the augmentation with a language version. So the macro can say "this is Dart 3.3 code" and the host, if it needs Dart 4 code, knows it must either convert 3.3->4 or fail.
And if we're doing that we can at the same time specify a template language. So the macro can say "this is Dart 3.3 code before v1 template expansion". And then the host knows how to deal with that.

I don't think we want a templating thing like this baked into the macro support - it can happily live on the client side in its own library (as Remi did). We can probably support resolving many of these in a single query so it will also be fast enough.
Longer term, this type of template should go away entirely, and be replaced by user created identifiers + tagged strings.


fix for sensitive logging

no longer logs password as requested in issue https://github.com/aio-libs/aioredis/issues/399

Can you please add add a test capturing and checking this log message.
Logs can be captured like this https://github.com/aio-libs/aioredis/blob/master/tests/pool_test.py#L469-L479
and password setup can look like this https://github.com/aio-libs/aioredis/blob/master/tests/connection_test.py#L266-L291

Oops - right, I should provide the tests.  I’ll come back to it.  I hit a snag trying to figure out how I’d test if the password was included in the log.  The examples you showed me (as well as the other tests I searched through) only seem to pass the password via the kwargs and not the address string (only host and port are being passed there it seems).

On Sep 11, 2018, at 3:31 AM, Alexey Popravka<EMAIL_ADDRESS>wrote:
Can you please add add a test capturing and checking this log message.
Logs can be captured like this https://github.com/aio-libs/aioredis/blob/master/tests/pool_test.py#L469-L479 https://github.com/aio-libs/aioredis/blob/master/tests/pool_test.py#L469-L479
and password setup can look like this https://github.com/aio-libs/aioredis/blob/master/tests/connection_test.py#L266-L291 https://github.com/aio-libs/aioredis/blob/master/tests/connection_test.py#L266-L291
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/aio-libs/aioredis/pull/459#issuecomment-420192158, or mute the thread https://github.com/notifications/unsubscribe-auth/AFpHSt32CB32NPr4qa5kzqdpUD3W8jblks5uZ3TJgaJpZM4Wi0oi.

Are you talking about this — redis://:passwd@host:1234/?


close CVTheque

We choosed to open the cv-theque at the end of the event

I hope you tested this with a date like 21/02/2018 to see if it works :) !


apply-changes fails with "maximum execution time exceeded error"

Hi @chiarnglin ,
Hope you are safe and doing well!
We face an issue with sncicd-apply-changes action where it fails within a few seconds after the action starts to run. It occurs intermittently and we could not relate it to any platform changes. I see the following message in the instance logs:
Cancelling transaction #20151 /api/sn_cicd/sc/apply_changes (maximum execution time exceeded): Thread API_INT-thread-3 (null, 44D65168DB5870102CE3172505961970), after 314838ms
Are there any timeout values set for the actions? Would it be possible to get a list of exceptions, their causes and resolutions for all the ServiceNow GitHub Actions which we can refer when we come across similar issues?

Thanks
Ramya

Hi Rayma,
Hope you and your family are safe as well, we're lucky to have enough vaccines here in the US.
This issue is new to me. What it sounds like is there's two levels of timeouts.

The instance has a timeout for API calls and responses. I searched up some posts on Google, eventually got to https://docs.servicenow.com/bundle/quebec-platform-administration/page/administer/platform-performance/concept/c_DefaultQuotaRules.html. It doesn't look like the roughly 5 minutes in your example though, but that log message sounds like the instance was the one that cancelled the transaction for the API call the GitHub Action was using.
There's a sleep function with a default timeout of 3000ms defined, but I think it's only used to rate limit the polling of the responses, and I don't see a cap in the number of retries. https://github.com/ServiceNow/sncicd-apply-changes/blob/master/src/App.ts#L17. My guess is this is on the instance side. Something is going on, on the instance, and it's basically not completing the Apply Remote Changes operation in a timely manner. Is it because there's too many apps trying to Apply Remote Changes at once hitting a thread limit?

Thanks,
Chiarng

Hi @chiarnglin,
Thank you for the quick response. We just have two applications using the pipeline currently and both are less critical apps. They don't go through a lot of changes, so the deployments are less frequent. And like you said, the DefaultQuotaRule settings are not set to a value so low that the workflow fails in less than 30s from the trigger time. Please let me know if there is anything else that I could check on the instance.
Thanks
Ramya

@ramyaramdasan You have a good point, I would like to find out if there's a rate or thread count limit too on the performance side. I've asked our engineering team to help take a look, but to start with - is this on one of the DPP instances or one of the Shell instances? If it's the latter, we might need you to go help open a CS with support to start the conversation (so we can have HIHOP access into your instance via that support case).

Hi @chiarnglin,
We encountered this issue on the Shell Instance. I just wanted to get clarity on whether it was an issue with GitHub or instance. Now that you said it was most likely an instance related issue, I could create a HI request and have the support team investigate it. Thanks for the suggestion.
You take care!

Ramya


Unable to trigger sphere_point_contains and bounds_point_contains

Description
sphere_point_contains and bounds_point_contains don't seem to work. See lines 110 and 113 of the following demo : https://github.com/mvvvv/StereoKit-rust/blob/master/examples/demos/math1.rs
Platform / Environment
Ubuntu 23.10 / Gnome Wayland
Quest 2 / v60
Logs or exception details
I also made a short video about what I'm doing : https://youtu.be/cWEeM5TaojY

About the video and the intersection with a mesh there is something counter-intuitive since I have to indicate Cull::Back (line 137) for it to work correctly. Wouldn't Cull::Front make more sense?


Footer not centre aligned

Describe the bug
Footer content is not centre aligned
Screenshots

Are you working on this issue? (Yes/No)
No

Can I work on this?

Footer can be remade by using CSS Grids rather than HTML tables and it fixes the alignment.

I am working on it

Already started @rishipurwar1 :)

I already resolved the issue.

Oh, great...
Keep contributing...

Thanks!
I've made changes using Gitpod.

Can you tell me how to create a Pull request?
I followed this article: [Pull Request using Gitpod]((https://github.com/CircuitVerse/CircuitVerse/wiki/Pull-Requests-using-Gitpod) but it is giving the following permission error after I execute step 7:

@kulkarniatharv can you fix the footer before we push the new UI to production, it is breaking responsive of pages.

@kulkarniatharv can you fix the footer before we push the new UI to production, it is breaking responsive of pages.

What part is not responsive in the footer? The changes I made or some other part? Because they seem to reposition and resize according to change in display size.
The change I made includes only the logos section. I didn't edit code which was handling other stuff.

I can't see your pull request. Can you link it here ?

I can't link the pull request to the issue. I guess I don't have the authorisation.
But you can check the PR here:

Link to the PR is here
Idk how is it not linked to this issue even when I followed this


No Cache Invalidation After Verify

Hitting the /verify route verifies that account but does not invalidate the unverified account object in the cache. If a user verifies their account quickly enough, the account data comes back as unverified even though they have already verified.

Hi @blyork , can you provide some reproduction steps to create the stale account problem, and where you are seeing the stale account data?  In particular, I want to confirm that you are using the /verify route that is served by this library, and not the default one on api.stormpath.com?  Using the default one will result in a stale cache, but if you're going through this library you shouldn't have a problem. Per your stormpath/stormpath-sdk-node#481, that method is already avoiding the cache with the {nocache:true} option, so the _evict call shouldn't be required.
The steps I just did were:

Enable email verification for the directory of my Stormpath application and set the Link Base URL of the template to http://localhost:3000/verify

Run the Express-Stormpath Example Project
 and configure it to use redis.

Register for an account at http://localhost:3000/register

Use redis-cli to confirm that there is a new account entry, and that the status of the account is UNVERIFIED

Click on the email link, landing on http://localhost:3000/verify and then redirected to http://localhost:3000/login?status=verified

Use redis-cli to confirm that there is a new account entry, and that the status of the account is ENABLED

Login with the new account and navigate to http://localhost:3000/me and see my status as ENABLED

Can you let me know how this differs from your workflow?  Thanks!

@robertjd
We are using the verify route provided by express-stormpath.
Are you sure nocache used? I did a search on the whole code base including dependencies and that single line is the only place that references nocache.
Environment:
We have one redis master and two redis slaves all being watched by three redis sentinels. I connected them to the stormpath SDK using the method recommended in stormpath/stormpath-sdk-node#467.
We do not currently use the register route since our account workflow is based on invitation rather than signup. Instead we call createAccount off of the desired directory.
We have a web server and a CRUD server. Both are connected to the same Redis cache environment. The createAccount is called from the CRUD server. Verification is of course done on the web server. CRUD authentication and authorization is done by passing the user's token via a Bearer Authorization header and calling authenticateApiRequest off the application object.
Problem:
In the current release, we can log in but none of the CRUD data loads due to the fact that the Redis cache contains an unverified user.
After this pull request, everything works perfectly.

Thanks for the info!  I'm going to try some more reproduction with the situation you describe.  Regarding the nocache option, it isn't an explicitly enumerated option, but any call to getResource, with query options, will bypass the cache, which you can see here

@blyork can you tell me how the user gets the token that they use for bearer authentication?  Do they login after they've landed on the /verify route?  Thanks!

Also: for login, are you using the /login route, or the /oauth/token route with the password grant flow?

BTW, your fix for the node sdk is totally correct, we do need to invalidate that resource and I'll get that merged after adding some tests.  I just want to understand the rest of your use case for posterity :)

We pull the tokens from the cookies and then add them to the header for our CRUD requests.
After hitting /verify, I am able to log into the web server using /login but not authenticate on CRUD due to the stale cache.
Thanks. Do you know when it might make an official release so that we can switch back to using NPM as our source for express-stormpath?

I've merged the changes into the node SDK, we should get that release out tomorrow.  We have some other express-stormpath issues we are working through, and we plan to make a release of express-stormpath this week.  Thanks for helping us find this issue!  I will leave this open until the release is made.


Cosine becomes non-deterministic in unit test

Description
I've run into a bizarre bug. I have a unit test which basically just checks that c == cos(x), where c = cos(x). Inlining it as cos(x) == cos(x) does not make it fail.

Steps to reproduce
Put this code into main.cpp and compile with g++ -fPIC -O1 main.cpp.
#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <doctest/doctest.h>
#include <random>

TEST_CASE("gcc-cos-sin-fpic-O1-bug")
{
    std::mt19937 g(0);
    std::normal_distribution<double> d(0.0, 1.0);
    for (int i = 0; i < 1000; ++i)
    {
        const double a = d(g);
        const double s = sin(a);
        const double c = cos(a);
        INFO(i);
        REQUIRE(s == sin(a));
        REQUIRE(c == cos(a));
        REQUIRE(a == a);
    }
}

Running ./a.out I get the following output
[doctest] doctest version is "2.4.8"
[doctest] run with "--help" for options
===============================================================================
main.cpp:5:
TEST CASE:  gcc-cos-sin-fpic-O1-bug

main.cpp:16: FATAL ERROR: REQUIRE( c == cos(a) ) is NOT correct!
  values: REQUIRE( -0.1912064051 == -0.1912064051 )
  logged: 833

===============================================================================
[doctest] test cases:    1 |    0 passed | 1 failed | 0 skipped
[doctest] assertions: 2501 | 2500 passed | 1 failed |
[doctest] Status: FAILURE!

Note that the test does not fail if you omit any of the flags.
Extra information

doctest version: 2.4.8~ds-1 (distributed with Ubuntu)
Operating System: Ubuntu 22.04.2 LTS
Compiler+version: g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0 (distributed with Ubuntu)

Edit: You can actually omit -fPIC compile flag, compiling with optimization -O1 suffice to reproduce the bug.

What would happen if you weren't using doctest at all but simple if statements?

This runs fine (compiled with g++ -O1)
#include <cassert>
#include <random>
#include <stdexcept>

#define REQUIRE(statement) if (!(statement)) throw std::runtime_error(#statement)

int main()
{
    std::mt19937 g(0);
    std::normal_distribution<double> d(0.0, 1.0);
    for (int i = 0; i < 100000000; ++i)
    {
        const double a = d(g);
        const double s = sin(a);
        const double c = cos(a);
        REQUIRE(s == sin(a));
        REQUIRE(c == cos(a));
        REQUIRE(a == a);
    }
}

Further down the rabbit hole...
Compiled without optimizations and full debug info: g++ -Wall -Wextra -Werror -Wpedantic -Og -g3
#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <doctest/doctest.h>

TEST_CASE("gcc-cos-bug")
{
    const double a = 1.2475784324341769;
    const double c = cos(a);
    REQUIRE(c == cos(a));
}

Seems you've found something worth investigating!
However, I won't be able to help with this issue anytime soon (if ever - #554 )

I've seen so, cool to see you still hanging around ;)
Anyways, this does not seem to be a doctest issue at all, more likely a GCC bug (or there is some intricate part of the standard that is being overlooked somehow...)
The following program fails
#include <cmath>
#include <stdexcept>

#define REQUIRE(statement) if (!(statement)) throw std::runtime_error(#statement)

int main()
{
    const double a = 1.2475784324341769;
    const double c = cos(a);
    [&] {
        REQUIRE(a == a);
        REQUIRE(c == cos(a));
    }();
}

Something about the lambda going on there.

Filed a bug to GCC: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116331
Long story short, this issue is very intricate and is most likely related to something called "excessive precision", which can make any floating point math non-deterministic on x86 family of processors. For the interested reader, read up on the infamous bug 323 in GCC.
I believe the reduced test case fails due to some weirdness in constant folding done by the compiler. However, the original bug in this thread can impossibly be due to that (unless GCC do some weird constant folding of random number generators... I hope that is not the case!). I'll keep posting updates as I make progress on the issue, I don't expect anyone to comment on it, just for documentation purposes.

This is actually not related to excess precision! The reduced test case with the hard coded literal made GCC optimize some calculations at compile time outside the lambda, but the calls inside are compiled into library calls. Hence there are 2 different cos at play here!
Now, the original bug is obviously not related to constant folding. Digged into the assembly a bit and discovered that GCC optimize the paired cos and sin into a non-standard sincos. So the issue is that the vales outside REQUIRE are computed using sincos, but the sin and cos in the REQUIRE macro will be evaluated in separate lexical contexts which does not allow for this optimization to happen. I am now pursuing this path, starting with a bug report to the package maintainers of glibc on Ubuntu: https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/2076653 .
It's interesting how exactly the same double value could trigger two completely different bugs in GCC/glibc! And I start to realize that == is probably broken for floats (I discovered -Wfloat-equal in GCC, I will probably start using this in all my code from now on...).
Reading up again on the reference for handling floats in test cases in doctest: https://github.com/doctest/doctest/blob/master/doc/markdown/assertions.md#floating-point-comparisons , I use doctest::Approx a lot, but I never use it where I really expect == to hold. Is there a way to compare floating point values with an error specified in ULP instead of relative error using epsilon? Ideally I would like to assert something like this
REQUIRE(cos(a) == Approx(c).ulp(1))


Adding methods to python client.

Added several functions to the python client.

ensure asset  - if an asset doesn't exist, create it
get/create/delete/update/ensure assettype
ping - ping the collins api
soft update - update an existing asset key, changing only if required
optimized imports, corrected some comment spacing

Don't have CLA filled out, will work on that today.

This is awesome @ytjohn!  Do you want to add yourself to the author list? https://github.com/tumblr/collins/blob/master/support/python/collins_client/setup.py#L8

Will do (re: author's list)

This LGTM (i cant approve? @Primer42 harsh...)

@Primer42 Ping? Can this get merged? Looks straight forward to me.


[SIP-7] Ensuring Superset accurately represents the underlying data warehouse

[SIP] Proposal for ensuring Superset accurately represents the underlying data warehouse
Motivation
Over time there can be a discrepancy between the actual metadata of a Druid datasource or SQLA table and the corresponding Superset representation. Columns or metrics can be added, updated, or deleted, and thus overtime creep grows between the systems.  The proposal is to try to add functionality to better ensure consistency between Superset and the underlying data warehouses which should help boost user confidence.
Proposed Change
Currently there is a mechanism to refresh Druid metadata per the following menu item:

thus the plan would be to also include an option for refreshing SQLA table metadata. We could optionally also provide a mechanism to scan for new SQLA tables though we should be aware that there scale of datasources (and their corresponding columns/metrics) could negatively impact the performance of Superset.
Refreshing
Regarding the term "refreshing" I proposed the following is instrumented for both Druid and SQLA datasources.
Columns/Metrics

New entities are added
Existing entities are updated
Obsolete entities are deleted†

†Note derived entities which reference an obsolete entity will also be deleted.
For reference here's the current UI behavior. Starting with a baseline:

when one drops a column (sum_boys in this instance) from the underlying table the UI state remains unchanged and the query fails to execute (as expected) with the no such column: sum_boys error:

Finally if one deletes the column and corresponding metrics from the Superset datasource, the UI state remains unchanged (also expected as the state is defined in its entirety from the form-data), though the query never runs as Superset rightfully raises an error stating that the metric is non-valid:

These behaviors seem correct and should remain unchanged.
Datasources
An unanswered question remains about what should happen when a Druid or SQLA datasource is defined in Superset but no longer resides in the underlying data warehouse. One school of thought is that if the underlying Druid datasource or SQLA table no longer exists we should:

Delete all slices which reference the datasource
Delete all empty dashboards (if appropriate)†
Delete the datasource

†Note I'm unsure what the current logic is for having a dashboard with no slices.
The concern with this approach is it is a fairly destructive process, i.e., significant IP is potentially lost when one deletes a slice. Would it make more sense that the underlying institution controls when/how datasources are deleted? For example one could define a policy that if said datasource has been deleted (and not restored) for n consecutive days then it's probably safe to delete it from Superset.
New dependencies
The following existing PRs are required which ensures we have uniqueness at the datasource/column/metric level and the appropriate fields are non-nullable:

~~https://github.com/apache/incubator-superset/pull/5445~~
~~https://github.com/apache/incubator-superset/pull/7084~~
https://github.com/apache/incubator-superset/pull/5449
~~https://github.com/apache/incubator-superset/pull/5451~~
~~https://github.com/apache/incubator-superset/pull/5452~~
~~https://github.com/apache/incubator-superset/pull/5453~~

Note many of these PRs require migrations which may need manual intervention as the lack of constraints and non-nullability may have resulted in a corrupted (and thus complex) database in which procedurally defined migration rules are non-viable.
Open Questions

Should we delete datasources which no longer exist in the data warehouse?
How do we deal with custom SQL definitions when the underlying datasource changes? I suspect this is out-of-scope.

to: @betodealmeida @michellethomas @mistercrunch @timifasubaa

Should be SIP-7

Note this is somewhat related to https://github.com/apache/superset/issues/8639 with regards to the auto-cleanup.

@john-bodley Closing this due to inactivity as part of SIP board cleaning. Please re-open if you think it's still active.


Frontend containerization

Closes #49
PS1: @guidanoli there is only one comment in .eslintrc.json that I would like to keep because I added a "hack". If you create an empty project it is not needed so I would like to register why there is "next/babel" on that file, ok? But if you still don't like it, I can remove.
PS2: @endersonmaia now it can be easier if you need to make any change on the branch and update the PR. 😉

there is only one comment in .eslintrc.json that I would like to keep because I added a "hack".

I didn't know it was a hack. Let's keep it then.

now it can be easier if you need to make any change on the branch and update the PR. 😉

The branch is still under your repository, though.
I assume you gave @endersonmaia access to your personal repository, then?

now it can be easier if you need to make any change on the branch and update the PR. 😉

The branch is still under your repository, though. I assume you gave @endersonmaia access to your personal repository, then?

I didn't @guidanoli ... 😳 But I did now just to not close this PR also.
The next ones I will be aware to create branches from the original repo. 😉
@endersonmaia please let me know if it worked for you, ok? 🙏

Here we go!!!


Add pt-BR README translated from en

I'm brazillian so pt-BR is my native language.

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

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Move postgres messages above query in output.

Is your feature request related to a problem? Please describe.
Potentially related to: https://github.com/mtxr/vscode-sqltools/issues/437 and https://github.com/mtxr/vscode-sqltools/issues/417 which you fixed awesomely.
In the latest version of SQL Tools, If I run a very long query, such as adding a function, it used to show a blank result to let you know there was a success. However, now it shows the slide-out from the side overlapping results with a DO successfully executed.
Could we consider moving MESSAGES up above QUERY? In most cases, the message is what is important, vs having to scroll through the entire query.
Describe the solution you'd like
Move MESSAGES up above QUERY.
Describe alternatives you've considered
A lot of scrolling.
Additional context

Thanks @Fmstrat. Messages were moved to another place in vnext branch.
Keep following the project to get updates on that, ok?
Thanks.


fix: set credential as password

A very naive fix for https://github.com/1Password/terraform-provider-onepassword/issues/52

Could either @volodymyrZotov or @jpcoenen please review/merge this? This is a major hindrance whilst item type parity is lacking.

@SMillerDev I looked at it; unfortunately, this implementation doesn't work. The field.Purpose value is always empty for api_credential fields.
You can use field.ID to check that there is a credential field:
if field.ID == "credential" {
     data.Set("password", field.Value)
}

However, that would fix the problem only partially.
The thing is that the ID value for credential field is mutable, purpose is always empty and there is no other field that we can rely on to make sure the right field is found. If you ever change the ID (for example using 1Passsword Connect), the provider won't be able to find it. We need to make some improvements on our side to fix that.
To summarise, let's check field.ID == "credential" for now to make it work and I'll raise that within the team so we decide what we'll do on our end.

Fixed, let me know what else

@SMillerDev The change is inside switch statement, which checks f.Purpose. The f.Purpose for the credentials field will always be an empty string. So, it'll never reach if f.ID == "credential" { statement.
Let's put if f.ID == "credential" { after the switch.

@SMillerDev Thinking more about this, there are several questions I have.

Why set the credential into password schema field? Why not create a new credentials property in the schema? aka

"credential": {
    Description:  "Some description",
    Type:         schema.TypeString,
    Optional:     true,
    Computed:     true,
    Sensitive:    true,
},

So then in to code toy would set it like data.Set("credential", f.Value)

I'd extend the if statement a bit more, to make sure we are looking for credential field only if item category is api_credential, aka

if f.ID == "credential" && item.Category == "API_CREDENTIAL" {
    data.Set("credential", f.Value)
}

Those all sound fine. I didn't really want to get too invasive with my changes so I kept them small, but happy to tweak them like that

I'd appreciate it if you could do that.
I have another question though. The changes are only for data-source. Is it possible to do the same for the resource types?
I'd like to include that also to keep the consistency between data-source and resources.

If this one works for you I'll add it to the resource too.

@SMillerDev, sorry for the delay. A migration happened in the background https://github.com/1Password/terraform-provider-onepassword/pull/167 that led to conflicts with your work. Do have a chance to resolve them, please?

No problem, it's really not that much time I've spend on this. I'll check again later

Okay, updated with the latest changes. Let me know if this is good and I'll add it to the resource.

Hi @SMillerDev . Had a chance to test it. It works fine.👍
I see that ci job for documentation doesn't pass. You can fix it by running the following command.
go run github.com/hashicorp/terraform-plugin-docs/cmd/tfplugindocs generate -provider-name onepassword

Then just push updated documentation.

I think it would certainly make it faster if you can give some instructions, I don't have that much time (or interest) in a deep dive into tf provider testing.
You're also very welcome to push to my branch or just copy my changes if that's easier. I'm not doing this for my portfolio/credits, I'd just like to use credentials in the data source 😁


Graceful handling after restoring

Task description
A user creates a backup with the old app and then restores after the new app is released. The app must handle this situation gracefully.

Versions

1.1.27
Mainnet

During some tests, after seeing the pop-up with the options “Remind me later”, “Keep as read-only” and “Import keys from backup”, and after choosing "Remind me later", the app crashed. Not immediately, but after a minute or so, while watching the account list containing one or more read-only accounts. I'm not sure if it's related to the pop-up at all, but nonetheless it happened more than once, after pressing "Remind me later" and looking at the account list for a bit.
App Version: 1.2(29),
iPhone SE (from 2016) running iOS 13.5.1.

Jens couldn't reproduced. Might be indirectly fixed by fix for https://github.com/Concordium/concordium-reference-wallet-ios/issues/130.


Feature request: Customizable dimensions

First of all, great idea!
I'd like to share some code on Instagram, but it requires me to share only square images.
A greate feature would be the ability to set/change dimensions of the image.
Cheers!

Sweet! see issue #1 for updates on this issue

We are going to track future progress for this issue at #1. In the mean time you have full control of the width and length of your code with the "Width Adjustment" setting. See PR #102


[Doc]to fix syntax err in the sample code: close producer async

Motivation

to fix syntax err in the sample code: 'Close operations can also be asynchronous’, the error are as below:

Cannot resolve method 'exceptionally(<lambda expression>)
Bad return type in lambda expression: Throwable cannot be converted to Void

Modifications

delete the unexpected semicolon
adjust return clause.

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)

Documentation

Does this pull request introduce a new feature? (no)

run integration tests
run cpp tests

run integration tests

org.apache.pulsar.tests.integration.functions.PulsarFunctionsProcessTest.pulsar-test-suite

run integration tests

run integration tests


Db connection management - performances

Hi guys,
this is based on the discussion here: https://github.com/tiangolo/full-stack-fastapi-postgresql/issues/104 .
I tried a few benchmarks, with:

Depends
SessionManager
Another implementation with scoped_session that I'll describe

I found out that - as seen by others - Depends doesn't work under load, it just exhausts the connections. So I'll leave out this solution.
SessionManager works, but i found out an (apparently) more performant solution:
SessionManager solution (only relevant parts):
# models.py
engine = create_engine(
    os.getenv('DATABASE_URL'), pool_recycle=3600, pool_size=5)

SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)

# SQLAlchemy model definitions....

class SessionManager:
    def __init__(self):
        self.db = SessionLocal()

    def __enter__(self):
        return self.db

    def __exit__(self, exc_type, exc_val, exc_tb):
        self.db.close()

# api.py
from models import SessionManager

@app.post("/bla")
def create_bla(bla: BlaModel):
    with SessionManager() as conn:
        # use conn with SQLAlchemy models - insert

@app.get("/bla")
def index_bla():
    with SessionManager() as conn:
         # use conn with SQLAlchemy models - select

scoped_session solution (only relevant parts):
# models.py
from sqlalchemy.orm import sessionmaker, scoped_session

# SQLAlchemy model definitions....

def database_setup():
    engine = create_engine(
        os.getenv('DATABASE_URL'), pool_recycle=3600, pool_size=5)
    Base.metadata.bind = engine
    return scoped_session(sessionmaker(autocommit=False, autoflush=False, bind=engine))

# api.py
from models import database_setup

conn = database_setup()

@app.post("/bla")
def create_bla(bla: BlaModel):
    # directly use conn with SQLAlchemy models - insert

@app.get("/bla")
def index_bla():
    # directly use conn with SQLAlchemy models - select

Results:
Doing some tests with jmeter, the second implementation seems faster, for example, with 20 concurrent threads in jmeter (left: SessionMaker, right: scopedsession):
"throughput" : 46.987912232295365, vs "throughput" : 49.83682907230358,
"meanResTime" :<PHONE_NUMBER>930802, vs "meanResTime" :<PHONE_NUMBER>298123,

My question is: is the approach with scopedsession correct or is there some flaw that i can't see? I didn't get any connection errors during tests even with 100 threads. I am not a fastapi expert (or an expert python programmer), so if there is some silly error, I would be happy to know! Thanks in advance!

A couple of things i want to point out:

poll_size has a default value of 5 connections it will keep open at all times, and max_overflow has a default value of 10 connections it can open on demand beyond the pool's capacity, so your SQLAlchemy engine runs out of connections to use for serving requests after 15 and will block the thread of any other requests that need one while waiting for connections to be freed.
The ThreadPoolExecutor fastapi is using to run non-async routes and dependencies without blocking the main thread (the one handling connections and running async code) has a capacity equal to the amount of cores (both logical and physical) you have plus 4, maxing out at 32, and since all your functions are def and not async def (they kind of have to be, since SQLAlchemy won't get async support until version 1.4, and making blocking functions async def would only serve to block your event loop and stall your whole application instead of blocking one thread in your threadpool), each of these requests will occupy one thread in this thread pool for the entirety of its lifetime, so it may have trouble handling more requests than however many threads your ThreadPoolExecutor has.
SqlAlchemy's sessionmaker works using thread-local storage to keep track of sessions, which will work when using FastAPI in a fully sync way, but might cause you to run into issues when async gets thrown in because requests and threads aren't mapped 1:1 in FastAPI, which is why I usually reccomend using Session(bind=db_conn) directly with a setup like you have.

You can't really handle more requests at any given time than the amount of connections your thread pool can give you (15 conns for 100 requests means you have to start with 85 requests queued up), nor can you have more sync requests waiting than you have threads in your thread pool (if you have a 4 core + 4 "logical threads" machine, that's 12 threads in your thread pool).

Hi, @sm-Fifteen, thank you very much for the additional information (I am not a Fastapi expert, I was just trying to make sense of these results).
So, basically:

Using sessionmaker is correct as long as everything is sync
If I go over 32 simultaneous connections with my last example, I will get an error because there wouldn't be available threads to keep the requests waiting?

Thanks and kind regards!

So, basically:
* Using `sessionmaker` is correct as long as everything is sync

Given how FastAPI works (with dependencies being injected on demand, so ideally wou wouldn't have a whole lof of stuff existing outside of functions, which is sessionmaker's case), I would say that using Session directly is better regardless of whether your requests are sync or async. Again, see my comment on https://github.com/tiangolo/fastapi/issues/726#issuecomment-584371305.

* If I go over 32 simultaneous connections with my last example, I will get an error because there wouldn't be available threads to keep the requests waiting?

No, if you go over that limit, the async thread will most likely stall and pause handling incoming connections while waiting for a thread to be freed. If your application is behind a reverse proxy, that would likely cause you to run into 502 errors (bad gateway) after a few seconds as the proxy tried contecting the application and the application didn't even "pickup the phone" (if you want a more visual metaphor), which is what usually happens when the event loop blocks.

Given how FastAPI works (with dependencies being injected on demand, so ideally wou wouldn't have a whole lof of stuff existing outside of functions, which is sessionmaker's case), I would say that using Session directly is better regardless of whether your requests are sync or async. Again, see my comment on tiangolo/fastapi#726 (comment).

Thanks, do you have an example of using Session directly? I see this in one of your comments in the thread you linked: https://github.com/tiangolo/fastapi/issues/726#issuecomment-557687526

No, if you go over that limit, the async thread will most likely stall and pause handling incoming connections while waiting for a thread to be freed. If your application is behind a reverse proxy, that would likely cause you to run into 502 errors (bad gateway) after a few seconds as the proxy tried contecting the application and the application didn't even "pickup the phone" (if you want a more visual metaphor), which is what usually happens when the event loop blocks.

Ok, this is clear, thank you!

Bringing over my comments from https://github.com/tiangolo/fastapi/issues/726:
Starlette discourages on_* handlers, and @sm-Fifteen sessionmaker is a thin wrapper around Session that passes through default arguments, including the engine.
There seems to be a lot of confusion about the best way to setup and teardown DB connections, and whether some of these ways are performance traps.
@tiangolo Could you provide guidance on how to best manage DB lifecycle in consideration of all the various factors (--reload, only opening a session on routes that need it, using a connection pool, cleanly closing/rolling back transactions, etc)?
A few additional questions:

Why is autoflush=False included in FastAPI examples? What goes wrong when setting this to true?
Assuming that yielding a new session is the recommendation method, is there a reason to prefer async def vs def?
Is there any reason to prefer Depends(get_db) vs with get_db() as session (or async with)?
Should the generator do a session.commit() and except: session.rollback() or should this be explicit in the endpoints or handled implicitly by SQLAlchemy's teardown?

Putting it all together, we have something like this:
from contextlib import contextmanager

from sqlalchemy import create_engine
from sqlalchemy.orm import Session, sessionmaker

from .core import settings

# Does not connect until used, pool_pre_ping applies when requesting a pool connection
engine = create_engine(settings.DB.DSN, pool_pre_ping=True)

_Session = sessionmaker(
    bind=engine,
    autoflush=False,
)

def close_engine():
    # For lifecycle shutdown
    engine.dispose()

@ccontextmanager
def get_db():
    # Explicit type because sessionmaker.__call__ stub is Any
    session: Session = _Session(bind=engine)

    try:
        yield session
        session.commit()
    except:
        session.rollback()
        raise
    finally:
        session.close()

Bringing over my comments from https://github.com/tiangolo/fastapi/issues/726:
Starlette discourages on_* handlers, and @sm-Fifteen sessionmaker is a thin wrapper around Session that passes through default arguments, including the engine.
There seems to be a lot of confusion about the best way to setup and teardown DB connections, and whether some of these ways are performance traps.
@tiangolo Could you provide guidance on how to best manage DB lifecycle in consideration of all the various factors (--reload, only opening a session on routes that need it, using a connection pool, cleanly closing/rolling back transactions, etc)?
A few additional questions:

Why is autoflush=False included in FastAPI examples? What goes wrong when setting this to true?
Assuming that yielding a new session is the recommendation method, is there a reason to prefer async def vs def?
Is there any reason to prefer Depends(get_db) vs with get_db() as session (or async with)?
Should the generator do a session.commit() and except: session.rollback() or should this be explicit in the endpoints or handled implicitly by SQLAlchemy's teardown?

Putting it all together, we have something like this:
from contextlib import contextmanager

from sqlalchemy import create_engine
from sqlalchemy.orm import Session, sessionmaker

from .core import settings

# Does not connect until used, pool_pre_ping applies when requesting a pool connection
engine = create_engine(settings.DB.DSN, pool_pre_ping=True)

_Session = sessionmaker(
    bind=engine,
    autoflush=False,
)

def close_engine():
    # For lifecycle shutdown
    engine.dispose()

@ccontextmanager
def get_db():
    # Explicit type because sessionmaker.__call__ stub is Any
    session: Session = _Session(bind=engine)

    try:
        yield session
        session.commit()
    except:
        session.rollback()
        raise
    finally:
        session.close()

Starlette discourages on_* handlers, and @sm-Fifteen sessionmaker is a thin wrapper around Session that passes through default arguments, including the engine.

@pikeas : on_* events aren't discouraged yet to my knowledge, they're now hooks for the lifespan context manager (see encode/starlette#799) which replaces them. As of right now, though, that new feature hasn't been integrated to FastAPI yet (I'm not even sure if it's documented at all in Starlette), which would be the ideal solution for tiangolo/fastapi#617 . In the meantime, unless you want to try using the context manager approach by hand, I'd still reccomend to use the approach from https://github.com/tiangolo/fastapi/issues/726#issuecomment-557687526.

A few additional questions:
2. Assuming that yielding a new session is the recommendation method, is there a reason to prefer `async def` vs `def`?

FastAPI tries to run routes and dependencies that are defined as def functions in a threadpool to avoid blocking, but if you can guarantee that a dependency will never block for any amount of time under any circumstances, it's slightly faster to use async def since that makes it run on the same thread. If you look at my example comment on the other issue, the only such async def I use is this one:
_db_conn: Optional[Database]

async def get_db_conn() -> Database:
    assert _db_conn is not None
    return _db_conn

...because spinning up a thread for such a trivial getter would definitely be wasteful, and it's not calling any function I can't tell for sure wouldn't block.
If that's what you were asking, given how your get_db() calls a lot of potentially blocking session methods, I would advise against using async def.

3. Is there any reason to prefer `Depends(get_db)` vs `with get_db() as session` (or `async with`)?

It means you can actually call the function that takes Depends(get_db) as a default parameter with a different database by passing the object in parameter and not have to change the code anywhere else, which can be useful for things like testing with a mock DB or function reuse.

4. Should the generator do a `session.commit()` and `except: session.rollback()` or should this be explicit in the endpoints or handled implicitly by SQLAlchemy's teardown?

I believe rollback is always implicit if you exit a sesion without comitting, and I personnally prefer explicit commits on the caller's side than doing it implicitly via the context manager because it gives the caller more control on how the transaction flows (and it lets you do quick and dirty smoke tests by running a route that writes to DB and just commenting the part that commits the transaction).

Starlette discourages on_* handlers, and @sm-Fifteen sessionmaker is a thin wrapper around Session that passes through default arguments, including the engine.

@pikeas : on_* events aren't discouraged yet to my knowledge, they're now hooks for the lifespan context manager (see encode/starlette#799) which replaces them. As of right now, though, that new feature hasn't been integrated to FastAPI yet (I'm not even sure if it's documented at all in Starlette), which would be the ideal solution for tiangolo/fastapi#617 . In the meantime, unless you want to try using the context manager approach by hand, I'd still reccomend to use the approach from https://github.com/tiangolo/fastapi/issues/726#issuecomment-557687526.

A few additional questions:
2. Assuming that yielding a new session is the recommendation method, is there a reason to prefer `async def` vs `def`?

FastAPI tries to run routes and dependencies that are defined as def functions in a threadpool to avoid blocking, but if you can guarantee that a dependency will never block for any amount of time under any circumstances, it's slightly faster to use async def since that makes it run on the same thread. If you look at my example comment on the other issue, the only such async def I use is this one:
_db_conn: Optional[Database]

async def get_db_conn() -> Database:
    assert _db_conn is not None
    return _db_conn

...because spinning up a thread for such a trivial getter would definitely be wasteful, and it's not calling any function I can't tell for sure wouldn't block.
If that's what you were asking, given how your get_db() calls a lot of potentially blocking session methods, I would advise against using async def.

3. Is there any reason to prefer `Depends(get_db)` vs `with get_db() as session` (or `async with`)?

It means you can actually call the function that takes Depends(get_db) as a default parameter with a different database by passing the object in parameter and not have to change the code anywhere else, which can be useful for things like testing with a mock DB or function reuse.

4. Should the generator do a `session.commit()` and `except: session.rollback()` or should this be explicit in the endpoints or handled implicitly by SQLAlchemy's teardown?

I believe rollback is always implicit if you exit a sesion without comitting, and I personnally prefer explicit commits on the caller's side than doing it implicitly via the context manager because it gives the caller more control on how the transaction flows (and it lets you do quick and dirty smoke tests by running a route that writes to DB and just commenting the part that commits the transaction).

Thanks for the thorough reply! One quick note about on_*...

Starlette discourages on_* handlers, and @sm-Fifteen sessionmaker is a thin wrapper around Session that passes through default arguments, including the engine.

@pikeas : on_* events aren't discouraged yet to my knowledge, they're now hooks for the lifespan context manager (see encode/starlette#799) which replaces them. As of right now, though, that new feature hasn't been integrated to FastAPI yet (I'm not even sure if it's documented at all in Starlette), which would be the ideal solution for tiangolo/fastapi#617 . In the meantime, unless you want to try using the context manager approach by hand, I'd still reccomend to use the approach from tiangolo/fastapi#726 (comment).

It's actually right in Starlette's code at https://github.com/encode/starlette/blob/master/starlette/applications.py#L114-L115

The following usages are now discouraged in favour of configuration during Starlette.init(...)

Thanks for the thorough reply! One quick note about on_*...

Starlette discourages on_* handlers, and @sm-Fifteen sessionmaker is a thin wrapper around Session that passes through default arguments, including the engine.

@pikeas : on_* events aren't discouraged yet to my knowledge, they're now hooks for the lifespan context manager (see encode/starlette#799) which replaces them. As of right now, though, that new feature hasn't been integrated to FastAPI yet (I'm not even sure if it's documented at all in Starlette), which would be the ideal solution for tiangolo/fastapi#617 . In the meantime, unless you want to try using the context manager approach by hand, I'd still reccomend to use the approach from tiangolo/fastapi#726 (comment).

It's actually right in Starlette's code at https://github.com/encode/starlette/blob/master/starlette/applications.py#L114-L115

The following usages are now discouraged in favour of configuration during Starlette.init(...)

@sm-Fifteen is your recommendation here benchmarked ? https://github.com/tiangolo/fastapi/issues/726#issuecomment-557687526
will it work in all situations without lockups, etc. we are planning to use this with postgresql

@sm-Fifteen is your recommendation here benchmarked ? tiangolo/fastapi#726 (comment)

I have not run any load-testing benchmarks on it, no. When I posted this comment, it was more of an architectural suggestion to avoid initializing anything in the global scope (outside the ASGI application's scope) to take advantage of dependency injection everywhere.

will it work in all situations without lockups, etc. we are planning to use this with postgresql

@sandys : open_database_connection_pools and close_database_connection_pools are blocking, but run outside the normal flow of your server, async get_db_conn is just a "scoped singleton" getter so it will not block. You may want to check get_db_sess, since I'm not sure whether or not the Session constructor blocks or not, and sess.close() probably does (at least when it needs to rollback), so it would probably be safer to mark this as def rather than async def.
Like I mentionned in https://github.com/tiangolo/full-stack-fastapi-postgresql/issues/290#issuecomment-723155540, another thing you'll want to check when testing for performance is whether FastAPI's threadpool ends up filling itself up when trying to offload the main thread of all the non-async defs you're running, which may also run you into performance problems, though I'm saying that from having looked at the code, not from having explicitly tested that case myself.

It's actually right in Starlette's code at https://github.com/encode/starlette/blob/master/starlette/applications.py#L114-L115

The following usages are now discouraged in favour of configuration during Starlette.init(...)

@pikeas: Ah, that's regarding the declarative API Starlette added in 0.13 (see encode/starlette#704), which was agreed to be a poor fit for FastAPI when the question of supporting that was brought up in tiangolo/fastapi#687 (I think the API is great, it's just that it wouldn't mesh very well with FastAPI's very function-oriented style, see my comment in that thread). The best approach in the future would be the context manager approach I mentionned earlier, which has been implemented in Starlette, but still needs support to be added on the FastAPI side of things.

@sm-Fifteen thanks for this! your comment here is pure gold ... as well as the previous one  https://github.com/tiangolo/full-stack-fastapi-postgresql/issues/290#issuecomment-723155540
Its the first discussion of production performance tuning of fastapi!
would you be willing to add some color to this ? i know it may be asking too much, but how to turn the knobs would be very welcome (and there seem to be quite a few of them)

@sandys: Unfortunately, that's about the limit of what I know regarding FastAPI performance. You can probably further reduce blocking by starting uvicorn with --workers=4 or some other number to spawn multiple processes of your application, but that's not something I've tested myself either, I'm just aware of it.
FastAPI could really use a "Performance" section in its documentation, you might want to mention that in the FastAPI developper survey if you haven't taken it yet.

@sm-Fifteen I have a new way to do it instead of global sqlalchemy.engine - to use lru_cache and contextmanager.
https://gist.github.com/sandys/671b8b86ba913e6436d4cb22d04b135f
do you think it works just as well ?

@lru_cache() on a function with no parameters only creates the database engine as a (lazily initialized) global singleton, which is basically equivalent to just running create_engine in the global scope. The main difference between the two is that the first request calling get_engine() will have to wait for the connection to be established, as that step is no longer performed before the FastAPI application goes up.
The reason I was recommending on_event() lifecycle managers earlier is because these make sure that your SQL Alchemy engine is initialized before the application runs, but also that it's managed with the lifecycle of the application, which enables your ASGI server life Uvicorn to run proper cleanup of both sync and async ressources before exiting, instead of assuming that it will happen automatically (which it might not, Python tries to run garbage collection before exiting but this is not perfect, and uvicorn's --reload option causes means your code gets reloaded without exiting from the python interpreter first). lru_cache doesn't really resolve that, it merely makes sure that get_engine() only runs once.
With that said, the exact way you initialize your DB engine isn't how you're going to improve performance, I was pointing that out more for the sake of correctness.

Thank you. That makes total sense.
However what is your opinion on the contextmanager stuff ?
On Wed, 17 Mar, 2021, 20:44 sm-Fifteen, @.***> wrote:

@lru_cache() on a function with no parameters only creates the database
engine as a (lazily initialized) global singleton, which is basically
equivalent to just running create_engine in the global scope. The main
difference between the two is that the first request calling get_engine()
will have to wait for the connection to be established, as that step is no
longer performed before the FastAPI application goes up.
The reason I was recommending on_event() lifecycle managers earlier is
because these make sure that your SQL Alchemy engine is initialized before
the application runs, but also that it's managed with the lifecycle of the
application, which enables your ASGI server life Uvicorn to run proper
cleanup of both sync and async ressources before exiting, instead of
assuming that it will happen automatically (which it might not, Python
tries to run garbage collection before exiting but this is not perfect
https://github.com/tiangolo/fastapi/issues/617#issuecomment-562307021,
and uvicorn's --reload option causes means your code gets reloaded
without exiting from the python interpreter first). lru_cache doesn't
really resolve that, it merely makes sure that get_engine() only runs
once.
With that said, the exact way you initialize your DB engine isn't how
you're going to improve performance, I was pointing that out more for the
sake of correctness.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tiangolo/full-stack-fastapi-postgresql/issues/290#issuecomment-801165838,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAASYU3TULO6I34VOGVJPIDTEDBNTANCNFSM4SIR67IA
.

However what is your opinion on the contextmanager stuff ?

It's a documented feature of FastAPI and generally a pretty good pattern to adopt for request-scoped resources.
However:

You probably don't need the @contextmanager annotation, FastAPI can handle them just fine without.
You may not want to automatically commit and rollback as part of your context manager, as that tends to get in the way when attempting to make more serious use of transactions. Closing a session without committing implicitly rolls back, so what you probably want instead is to do an explicit sess.commit() at the end of your route function and keep get_db() as a simple try-yield-finally like in the doc.

A couple of things i want to point out:

poll_size has a default value of 5 connections it will keep open at all times, and max_overflow has a default value of 10 connections it can open on demand beyond the pool's capacity, so your SQLAlchemy engine runs out of connections to use for serving requests after 15 and will block the thread of any other requests that need one while waiting for connections to be freed.
The ThreadPoolExecutor fastapi is using to run non-async routes and dependencies without blocking the main thread (the one handling connections and running async code) has a capacity equal to the amount of cores (both logical and physical) you have plus 4, maxing out at 32, and since all your functions are def and not async def (they kind of have to be, since SQLAlchemy won't get async support until version 1.4, and making blocking functions async def would only serve to block your event loop and stall your whole application instead of blocking one thread in your threadpool), each of these requests will occupy one thread in this thread pool for the entirety of its lifetime, so it may have trouble handling more requests than however many threads your ThreadPoolExecutor has.
SqlAlchemy's sessionmaker works using thread-local storage to keep track of sessions, which will work when using FastAPI in a fully sync way, but might cause you to run into issues when async gets thrown in because requests and threads aren't mapped 1:1 in FastAPI, which is why I usually reccomend using Session(bind=db_conn) directly with a setup like you have.

You can't really handle more requests at any given time than the amount of connections your thread pool can give you (15 conns for 100 requests means you have to start with 85 requests queued up), nor can you have more sync requests waiting than you have threads in your thread pool (if you have a 4 core + 4 "logical threads" machine, that's 12 threads in your thread pool).

@sm-Fifteen now that SQLAlchemy has async support, I was wondering if Sessionmaker still uses thread locals when creating AsyncSession objects. I'd hope not, at least. Has your recommendation around using Session(bind=db_conn) in favour of Sessionmaker changed at all for async sessions? I'm asking this question under the assumption that:

I do not have a global engine, and I instead tie it to app.state
I do not use a global Sessionmaker, but use some kind of caching to ensure it is not created more than once

@sm-Fifteen now that SQLAlchemy has async support, I was wondering if Sessionmaker still uses thread locals when creating AsyncSession objects. I'd hope not, at least. Has your recommendation around using Session(bind=db_conn) in favour of Sessionmaker changed at all for async sessions? I'm asking this question under the assumption that:
1. I do not have a global engine, and I instead tie it to app.state

2. I do not use a global Sessionmaker, but use some kind of caching to ensure it is not created more than once

What my question amounts to, is whether you still recommend following your approach from tiangolo/fastapi#726 and your comment in this issue on using Session directly instead of sessoinmaker still apply today, if I"m doing the stuff I mentioned above.
Edit: Added some more references, and a summary.

@bharathanr: sessionmaker's main use case seems to be pre-configuring an alternate Session() constructors so you don't need to repeat that configuration elsewhere in the code. That wouldn't really be needed if you're using dependency injection for that, I don't think, so I would stick to recommending creating a Session directly inside  your session dependency. Also, the documentation for scoped_session now actively discourages using it with async frameworks, so definitely don't use that.

Hi !
I don't get it completely.
Is this repo's session setup is the way to go ?
Is there still performance issues ?
I see that https://github.com/tiangolo/fastapi/issues/726 issue is closed.
I had to use FastAPI-utils get rid of the famous error:
sqlalchemy.exc.TimeoutError: QueuePool limit of size 5 overflow 0 reached, connection timed out, timeout 30 (Background on this error at: http://sqlalche.me/e/3o7r)
But I got some trouble to bind declarative_base with the engine.
If this repo still have performance issues, is there a way to make a application without this limitation thanks to FastAPI doc or must we wait for update on FastAPI side ?

Adding to this discussion:
I have performed some tests with synchronous SQLAlchemy session and have documented my findings in this repo:
https://github.com/harsh8398/fastapi-dependency-issue
I haven't tested with the async SQLAlchemy session but from my tests, it seems that the problem is within how fastapi implements Depends .

@sm-Fifteen in ur comment in the other bug, u mentioned sqlalchemy 1.4 as one of the solutions to this issue.
In that context, and given SA 1.4 is stable, what would be the recommended configuration for fastapi now ?

I have performed some tests with synchronous SQLAlchemy session and have documented my findings in this repo: https://github.com/harsh8398/fastapi-dependency-issue
I haven't tested with the async SQLAlchemy session but from my tests, it seems that the problem is within how fastapi implements Depends .

@harsh8398: Those are some pretty troubling numbers, I wasn't aware that this cocktail of CM dependencies, thread pools and blocking I/O could kill server performance this badly. That certainly makes a convincing argument against using sync SQLAlchemy in FastAPI application, and makes the discussion in tiangolo/fastapi#3205 extra important. I was going to say that "the docs are luckily unaffected by this as they still keep their SQLA objects in the global namespace", but on closer look, the docs do use CM deps to initialize ORM sessions, and haven't been updated to use async SQLA, so this problem also affects official documentation.

In that context, and given SA 1.4 is stable, what would be the recommended configuration for fastapi now ?

@sandys: I haven't had the opportunity to try async SQLA on the FastAPI applications I'm working on at the moment, so I can't say.

https://github.com/harsh8398/fastapi-dependency-issue/pull/2
The issue seems to be hitting the default capacity limiter for the threadpool used to run sync dependencies/endpoints.

@adriangb on the other hand, i want to submit my code
https://gist.github.com/sandys/671b8b86ba913e6436d4cb22d04b135f
with a single gunicorn worker, im getting the following performance.
 docker run --network=host --rm skandyla/wrk -t12 -c400 -d30s http://localhost:8000/users/
Running 30s test @ http://localhost:8000/users/
  12 threads and 400 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   161.88ms   64.40ms   1.93s    88.77%
    Req/Sec    49.64     29.37   131.00     49.80%
  8700 requests in 30.09s, 2.17MB read
  Socket errors: connect 0, read 0, write 0, timeout 1
Requests/sec:    289.16
Transfer/sec:     73.70KB

P.S. this uses Depends.

There's way too much code in there for me to go through. I'm also not sure why you're tagging me?


Some unexpected error with cattrs 22.1.0

Hi dear @JWCook, I am experimenting an unexpected behaviour, not necessarily a bug, but
The problem
python-cattrs 22.1.0 was upgraded today and now we can not run the check() function here
==> Starting check()...
No module named 'exceptiongroup'
Traceback (most recent call last):
  File "/tmp/makepkg/python-requests-cache-git/src/requests-cache/requests_cache/__init__.py", line 9, in <module>
    from .backends import *
  File "/tmp/makepkg/python-requests-cache-git/src/requests-cache/requests_cache/backends/__init__.py", line 7, in <module>
    from .base import BaseCache, BaseStorage, DictStorage
  File "/tmp/makepkg/python-requests-cache-git/src/requests-cache/requests_cache/backends/base.py", line 22, in <module>
    from ..serializers import SERIALIZERS, SerializerPipeline, SerializerType, pickle_serializer
  File "/tmp/makepkg/python-requests-cache-git/src/requests-cache/requests_cache/serializers/__init__.py", line 24, in <module>
    from .cattrs import CattrStage
  File "/tmp/makepkg/python-requests-cache-git/src/requests-cache/requests_cache/serializers/cattrs.py", line 19, in <module>
    from cattr import GenConverter
  File "/usr/lib/python3.10/site-packages/cattr/__init__.py", line 1, in <module>
    from .converters import Converter, GenConverter, UnstructureStrategy
  File "/usr/lib/python3.10/site-packages/cattr/converters.py", line 1, in <module>
    from cattrs.converters import Converter, GenConverter, UnstructureStrategy
  File "/usr/lib/python3.10/site-packages/cattrs/__init__.py", line 1, in <module>
    from cattr import converters, disambiguators, dispatch, errors, gen, preconf
  File "/usr/lib/python3.10/site-packages/cattr/disambiguators.py", line 15, in <module>
    from cattrs._compat import get_origin
  File "/usr/lib/python3.10/site-packages/cattrs/_compat.py", line 38, in <module>
    from exceptiongroup import ExceptionGroup
ModuleNotFoundError: No module named 'exceptiongroup'
ImportError while loading conftest '/tmp/makepkg/python-requests-cache-git/src/requests-cache/tests/conftest.py'.
tests/conftest.py:28: in <module>
    from requests_cache import ALL_METHODS, CachedSession, install_cache, uninstall_cache
E   ImportError: cannot import name 'ALL_METHODS' from 'requests_cache' (/tmp/makepkg/python-requests-cache-git/src/requests-cache/requests_cache/__init__.py)
==> ERROR: A failure occurred in check().
    Aborting...

Expected behavior
All the test must be pass excepted integration tests that were disabled.
Steps to reproduce the behavior
Install python-url-normalize from AUR, then
git clone https://aur.archlinux.org/python-requests-cache-git.git
cd python-requests-cache-git
makepkg -s

Workarounds
Researching, not workaround from my side.
Environment

requests-cache version: main (last commit) python-requests-cache-git from AUR.
Python version: 3.10.5
Platform: Arch Linux

Meow.
Thanks.

Also added an issue on the cattrs repo for reference: https://github.com/python-attrs/cattrs/issues/280


Add extraValueFiles to operator-install chart

I think you also want:
diff --git a/clustergroup/values.schema.json b/clustergroup/values.schema.json
index e64a812..b1359a3 100644
--- a/clustergroup/values.schema.json
+++ b/clustergroup/values.schema.json
@@ -83,6 +83,10 @@
             }
           }
         },
+        "extraValueFiles": {
+          "type": "array",
+          "description": "List of extra values files that will be passed to ArgoCD."
+        },
         "gitops": {
           "type": "object",
           "additionalProperties": false,

Or something along those lines anyways

Do we need to talk about whether those extraValueFiles are "main" or "global"? It seems like they should be "global" since I think we intend them to also apply to subsequent applications?

Yeah so I never really liked that main vs global distinction. In theory as far as I understood things 'main' is for things passed at 'make install' time and global is everything else. I think the distinction is non obvious so I would not hate the idea of just moving towards global


Refactor RedshiftSourceSuite to remove use of ScalaMock

This patch refactors RedshiftSourceSuite to remove the use of ScalaMock.
The motivation for this is the fact that our previous ScalaMock code became a bit of an impediment when trying to refactor codepaths which created and closed new JDBC connections.

Current coverage is 94.81%

Merging #88 into master will increase coverage by +0.22% as of 4c46884

@@            master     #88   diff @@
======================================
  Files           11      11
  Stmts          444     444
  Branches       105     107     +2
  Methods          0       0
======================================
+ Hit            420     421     +1
  Partial          0       0
+ Missed          24      23     -1

Review entire Coverage Diff as of 4c46884

Powered by Codecov. Updated on successful CI builds.

Discussed offline; will merge now to unblock another PR.


[CARBONDATA-1574] No_Inverted is applied for all newly added column i…

If No_Inverted_Index is specified in tableproperties during addition of several column.
All columns are being considered as No_Inverted_Index, even if few of them is marked as No_Inverted_Index in tableproperties .
Solution : Match column name before applying the property.

SDV Build Fail , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1073/

Build Success with Spark 1.6, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder/320/

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/445/

retest this please

SDV Build Fail , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1074/

Build Success with Spark 1.6, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder/321/

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/446/

retest this please

SDV Build Fail , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1081/

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/453/

Build Success with Spark 1.6, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder/328/

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/456/

Build Success with Spark 1.6, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder/331/

SDV Build Success , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1084/

SDV Build Fail , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1176/

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/545/

SDV Build Fail , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1281/

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/651/

retest this please

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/721/

SDV Build Success , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1358/

@jackylk  please review and merge

@jackylk please merge this PR

Build Success with Spark 2.1.0, Please check CI http://<IP_ADDRESS>:8080/job/ApacheCarbonPRBuilder1/1095/

SDV Build Success , Please check CI http://<IP_ADDRESS>:8080/job/ApacheSDVTests/1696/

LGTM


Link to callback booking from ML completion page

Trello card
Trello-1745
Context
When a candidate has finished signing up for the mailing list we want to highlight the callback booking form in case they wish to speak to an adviser.
Only enabled in dev/test - disabled in production for now.
Changes proposed in this pull request

Link to callback booking from ML completion page

Guidance to review

Desktop
Mobile

LGTM - guess we'll do a new PR to amend the env rules once we're ready to go?

Yep, we can remove those conditionals when its ready - I've got a ticket to enable the feature with what needs doing in ready for dev just pending a green light


${EnvironmentVariable} expansion in windows does not work

To quote the wiki:

Environment variable expansion is also supported when passed to rex functions. The syntaxes $FOO and ${FOO} are supported, regardless of the syntax supported by the target shell.

However in my testing the environment variables do not get expanded on windows in CMD.
Only the "Python" interpreter does expand_env_vars = True, while in bash the system takes over and does the expanion.
Since on windows ${} is not supported I suggest that expand_env_vars should be set True in the CMD shell.
Is there a risk doing this?

@bfloch Can you provide reproducible steps to what you tried, please? I am trying to understand where you're running into this issue.

I will have to try again and see if this still applies.
But I do not have access to that environment till April.

Closing in favor of the new issue.


"Train your own model" Release date

Hi, can you please let us know when is Training code scheduled to be released?

10 months show everything.   Maybe the  answer is never.

Had a chat with the author. Due to time constraints, it is not possible. I may try to recreate the training code.

@INF800 Please read ther paper <One-shot Talking Face Generation from Single-speaker Audio-Visual Correlation Learning>  and maybe you will change your mind....
If you finished the training part of PC-AVS , maybe  I'd like to try this pile of code.

@INF800 Thanks for your reply. @yaleimeng I'm not sure what is the relationship between PCAVS and the paper of Suzhen's. Sorry that perhaps I may never have time to organize the training code, nevertheless, I think I have done enough by releasing the inference code.

@INF800 Do post an update in case you're able to recreate the training code! That would be awesome!
@Hangz-nju-cuhk Thanks for your work!

Hi, I have tested this repo and it works awesome. However I want to train my own model, if @INF800 or anyone can help recreating the training code.

Hey @saharsh0612 thankyou for reaching out, you can follow the steps in readme and reproduce in colab. I lost my notebook and source code files.


Segmentation fault on incorrect case for parameter name

Hi,
The following code:
# ...

components:
  parameters:
    CategoryFilter:
      in: query
      name: category
      description: Filter by product category.
      required: false
      schema:
        type: string
      examples:
        single:
          value: laptops
          summary: A sample category to filter products.

paths:
  /v1/products:
    get:
      summary: Get products
      description: Returns a list of all products.
      operationId: products.index
      tags: [products]
      parameters:
        - $ref: '#/components/parameters/categoryFilter'

# ...

leads to segmentation fault as show bellow:
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x20 pc=0x10134f3d0]

goroutine 59 [running]:
github.com/daveshanley/vacuum/functions/openapi.OperationParameters.RunRule({}, {0x14000124048?, 0x8?, 0x14000088000?}, {0x14000114150, 0x140001048f0, {0x10196de00, 0x101a4b378}, {0x0, 0x0}, ...})
	/Users/egrep/go/pkg/mod/github.com/daveshanley/vacuum@v0.0.47/functions/openapi/operation_parameters.go:73 +0xa00
github.com/daveshanley/vacuum/motor.buildResults({0x140001048f0, 0x140003e8aa0, {0x101a50670, 0x140003e0410}, 0x140000bff08, 0x140003caa00, 0x140004960c0, 0x14000539800, 0x140003e8b40, 0x0, ...}, ...)
	/Users/egrep/go/pkg/mod/github.com/daveshanley/vacuum@v0.0.47/motor/rule_applicator.go:400 +0x55c
github.com/daveshanley/vacuum/motor.runRule({0x140001048f0, 0x140003e8aa0, {0x101a50670, 0x140003e0410}, 0x140000bff08, 0x140003caa00, 0x140004960c0, 0x14000539800, 0x140003e8b40, 0x0, ...})
	/Users/egrep/go/pkg/mod/github.com/daveshanley/vacuum@v0.0.47/motor/rule_applicator.go:327 +0x36c
created by github.com/daveshanley/vacuum/motor.ApplyRulesToRuleSet
	/Users/egrep/go/pkg/mod/github.com/daveshanley/vacuum@v0.0.47/motor/rule_applicator.go:162 +0x628

Of course this can be fixed in the following way:
# ...

components:
  parameters:
    CategoryFilter:
      in: query
      name: category
      description: Filter by product category.
      required: false
      schema:
        type: string
      examples:
        single:
          value: laptops
          summary: A sample category to filter products.

paths:
  /v1/products:
    get:
      summary: Get products
      description: Returns a list of all products.
      operationId: products.index
      tags: [products]
      parameters:
-       - $ref: '#/components/parameters/categoryFilter'
+       - $ref: '#/components/parameters/CategoryFilter'

# ...

However, finding this error in automatic mode at CI/CD phase is almost impossible because vacuum crashes without specifying on which line you have an error.
Steps to reproduce: https://github.com/sergeyklay/specmatic-testing-example/pull/8 (see "Lint OpenAPI" check)

Nice find, I will look into this for you ASAP.

Resolved in v0.0.52

Thank you!