HEAD and OPTIONS requests

Documentation says:

// Matching NO HTTP methods
// Pass an empty array to the HTTP methods. HEAD and OPTIONS will still be
// honored. (In FastRoute, GET is also honored.)
$app->route('/post', 'WillThisHandlePost', []);
https://github.com/zendframework/zend-expressive/blob/master/doc/book/reference/usage-examples.md

As I understand HEAD and OPTIONS requests should be available when "no http" methods are set (an empty array), and also GET request when using FastRoute.
I've added test that shows it's not working correctly.
Also added two other tests:

check if HEAD/OPTIONS requests are allowed when list of allowed methods is set,
check if GET*/POST/PUT/DELETE/PATCH request are not allowed when "no http" methods is set.

GET* - as docs says in FastRoute GET method is honored (as we can see in my tests it's not working as well).
We can discuss what is the correct way and what should contain body for HEAD and OPTIONS requests (probably we need more tests for it). My tests shows that for different route providers it works different way. IMHO, it should be consistent, doesn't matter what router is used.

as docs says in FastRoute GET method is honored (as we can see in my tests it's not working as well).

I'd argue this is incorrect behavior on the part of FastRoute. While it's unusual to not honor GET for a given path, the various HTTP specifications do not say that servers MUST honor it. As such, I'd argue that we should not include the test case for that in this set.

I'm looking at the code now, and thinking about how this should work.
First, I agree that HEAD and OPTIONS requests should always be honored. The question is, what should that look like?
When you do a HEAD request, the request is generally expected to send the same headers as if you had made a GET request to the resource (though this may not be true if the server does not support GET to that particular resource; in that case, it may be something else entirely). This might then imply that the routing middleware should consider this a successful route match, delegate to the next, and, once a response is received, return the response with an empty body.
When you do an OPTIONS request, you generally are mainly interested in emitting the Allow header, which is what we do on a routing failure, but not what we do currently on a routing success, even when it's based on an OPTIONS method. However, OPTIONS can be handled in many ways, and provide additional detail; Zac Stewart has a nice post on using it for self-documenting APIs. So, then, the question becomes: should we just go ahead and pass OPTIONS requests on to the next middleware? or should we return a response immediately with an Allow header and 200 status, and be done?
In reviewing the existing routers, we have an interesting situation:

FastRoute always honors HEAD and OPTIONS requests for a matched route; this means, however, that whatever middleware is mapped will be invoked for these types of requests, always.
Neither the zend-router or aura.router support HEAD or OPTIONS requests out of the box, unless HTTP_METHOD_ANY is specified. As such, specifying either of these methods currently results in either a 405 or a 500 response.

Here are my thoughts:

We should honor HEAD and OPTIONS requests always.

For HEAD:

In individual routers, if the matched route DOES NOT ALLOW the HEAD OR GET methods, we would need to flag the returned route result somehow to indicate this, and the routing middleware would need to return a 200 response immediately.  This would ensure we do not have a 404.

If the matched route DOES ALLOW the HEAD method, we would delegate handling on to the next middleware, and then need to ensure that the returned body is empty. (More on that on the next point.)

In individual routers, if the matched route DOES NOT ALLOW the HEAD method, but DOES ALLOW the GET method, we would need to flag the returned route result somehow to indicate this, and we would then proceed as normal for a routing match, potentially altering the request method sent to the next layer to GET. Once the response is returned, we would replace the response body with an empty stream. I would argue that we should do this via new middleware, which users can OPTIONALLY register in an outer layer (and which, in 1.1, we'd register by default), and which would look something like the following:
use Zend\Diactoros\Stream;

function ($request, $delegate) {
    $response = $delegate->process($request);
    if ('HEAD' !== $request->getMethod()) {
        return $response;
    }

    return $response->withBody(new Stream('php://temp'));
}

Note, this also means that the middleware author would be required to test the incoming method, particularly if they allow more than just GET to reach the middleware.

For OPTIONS:

In individual routers, if the matched route DOES NOT ALLOW the OPTIONS method, we would need to flag the route result somehow to indicate this, and the routing middleware would then return a 200 response with an Allow header. (Essentially, this is the same as our current 405 handling, but with a 200 status instead.)

If a route DOES ALLOW the OPTIONS method, we would delegate to the next middleware as-is, and simply return the response.

My inclination is to handle this via a combination of new functionality in zend-expressive-router, the invididual router implementations, and in Expressive itself.
First, we'd update the constructor logic of Zend\Expressive\Router\Route to add HEAD and OPTIONS to the list of accepted HTTP methods for the route if they are not provided; we'd then also provide the following additional methods on that class:

implicitHead()
implicitOptions()

These would allow the router implementations to check if a match for a HEAD or an OPTIONS request was due to the user specifying these as acceptable methods, or the Route class adding them during instantiation.
Additionally, Zend\Expressive\Router\RouteResult would need to be modified to allow storing the actual matched Route instance in the RouteResult, instead of the matched route name, middleware, and params. This could be done via a new method fromRoute() (vs the current fromRouteMatch()), and it would inject a $route property.  The existing methods getMatchedRouteName(), getMatchedMiddleware(), and getMatchedParams() would be updated to proxy to the underlying Route instance if present, and a new method, getMatchedRoute() would be added to retrieve the Route instance, allowing consumers to test for implictHead() and/or implicitOptions().
Once those changes were made, individual routing implementations could then be updated to use RouteResult::fromRoute() with the matched Route instance.

For FastRouteRouter, this is relatively straight-forward, as we are already pulling data from the Route instance when creating the RouteResult.
For AuraRouter, we could store the Route instance as one of the params provided to the generated AuraRoute instance; when marshaling the RouteResult, we'd use that value from the AuraRoute.
For ZendRouter, we could store the Route instance as one of the defaults, and then access it via the matched route params.

These changes could be done in an immediate minor release of zend-expressive-router, and subsequent minor releases of the individual router implementations, without BC concerns as the current RouteResult API would still work for existing Expressive users; a new minor release of Expressive (I'd like to target 1.1 with this, as I think it's quite important!) could then depend on the new zend-expressive-router version, and update the routing and/or dispatch middleware as noted above in order to provide implicit OPTIONS and HEAD handling.
So, consider this a request for comments. Pinging @michaelmoussa and @xtreamwayz.

Closing in favor of #413, which incorporates the commits from this pull request.


The Reach Menu isn't being triggered inside a React Aria Dialog.

Provide a general summary of the issue here
We recently started using the react-aria library for our overlays, and this has caused a bug that prevents the Reach menus from being triggered in the dialogs. The issue arises because the menus are rendered within portals, and my best guess is that react-aria maintains a focus lock on elements outside of the currently active overlay element.
Any ideas how to solve this?
🤔 Expected Behavior?
The menu can be opened in the modal/dialog.
😯 Current Behavior
Menu list is not visible after clicking on a menu button.
💁 Possible Solution
No response
🔦 Context
No response
🖥 Steps to Reproduce
https://codesandbox.io/p/devbox/jrt8hx
Version
latest
What browsers are you seeing the problem on?
Chrome
If other, please specify.
No response
What operating system are you using?
Mac OS X
🧢 Your Company/Team
No response
🕷 Tracking Issue
No response

This is a tough one since many of our overlays manage/trap focus within themselves as you've discovered for accessibility reasons. Usually child overlays would be registered as child FocusScopes and thus focus could move from a parent overlay into the child overlay but this relies on said child calling/using FocusScope. 3rd party overlays won't do this by default of course and may have their own focus management that may clash with our own. We've discussed in the team in the past about perhaps allowing users to disable our focus management or register arbitrary elements as "ok to focus"  but didn't land on anything yet. I don't suppose you can control where the Reach Menu gets portalled to?


[IDL] remove reportable

Problem
Reportable tag is unstable in idl generation (zap has different settings on reportable for client and server because reportable is generally not used (and reportable is a zcl term: matter has subscriptions and everything is subscribable but some may not generate values)
Change overview
Remove the reportable flag. Does not help code review and generates instability.
Testing
ZAP gen still works.

fast track: IDL only change: this is to help code reviewers, has no functionality difference.

Closing: knowing reportable or not seems useful, but should be named as 'subscribable' and the 'not' variant is the interesting one.


Publisher cards are not properly aligned

The numbers of datasets go up and down...

This is more of an issue of the design system.
On one hand we can pin the number of datasets to the bottom, but still the issue is that some headlines spans two lines. So we would actually need to pin down also the buttons leaving only the heading at the top.

Which is how bootstrap does it....

I agree it makes sense, thus I belive we should implement it to Design System, at least our version.


Cannot toggle text searching when using ctrl + c to exit insert mode

If a use ctrl + c to exit insert mode, the text searching will not toggle any more.
https://user-images.githubusercontent.com/44799572/164244675-0b071393-1d88-4202-aae3-51148ff1bc9c.mov

sorry  the only way to do that is esc. because  leave insert without event
you can map  to  on file type spectre_panel


Lasso Improvements

The lasso should now work more reliably
It supports the two modes

pencil selection + one finger scroll
finger selection + two finger scroll

In the video, you can see in the first part the "finger mode" and in the second part "pencil mode"
https://user-images.githubusercontent.com/16856573/212888904-80bcc00f-4c22-4457-9858-97f9661d5894.MP4

Looks good!
A small improvement: I think it's best when the button changes based on the mode you're in to indicate the mode you're switching to. So pencil if you're in finger mode and the touch hand when you're in pencil mode.

Looks good! A small improvement: I think it's best when the button changes based on the mode you're in to indicate the mode you're switching to. So pencil if you're in finger mode and the touch hand when you're in pencil mode.

@botrufus should look better now
https://user-images.githubusercontent.com/16856573/212987197-7e37b2fe-499e-46fc-8d68-1af1a1b5caa4.MP4


use HTTPS for homepage URL in gemspec

This pull request updates the thinking-sphinx gemspec metadata to use an encrypted HTTPS URL for the gem's homepage.

Thanks Ken, appreciated - just cherry-picked this into the develop branch.

Oh, shoot, develop, right :) I'll remember next time.


Download issues in preview

The currect preview has some OSM data download issues of visible tiles, even without selecting anything.
I have tested with Firefox 68.0.1 on Windows 10.
Btw, when a tile download has failed a reload button should be offered in case the download is not periodically repeated.

@slhh Can you point us to some locations that exhibit this behavior? I don't think I've seen it.
Also, if a tile fails then we should just make sure iD keeps requesting it rather than bugging the user.

Closing this since I haven't seen it happen and no followup occurred. @slhh Let us know if you see this again!


Use getpass for password collection (#9)

I also moved initialisation of PASSPHRASE to within if __name__ == '__main__' to avoid prompting the user for a password if postoffice.py ever gets imported from another module

Sorry, force of habit! I usually separate builtin imports from third party dependencies, but here wasn't the right place to do it


Add support for ARG instruction

Ref: http://docs.docker.com/engine/reference/builder/#arg

Thanks!


Added pullSecrets override

What this PR does / why we need it:
Allow the ability to pull images from a private registry
Which issue this PR fixes
Special notes for your reviewer:
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

[x] DCO signed
[x] Chart Version bumped
[x] Variables are documented in the README.md

/assign @foxish

/assign unguiculus

/ok-to-test

/lgtm


HDDS-10144. Zero-Copy in replication

What changes were proposed in this pull request?
Currently zero-copy is supported in EC (write).
This PR, which is similar to PR-5621, implements zero copy for replication.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10144
How was this patch tested?
UT

Thanks, @kerneltime @errose28 @szetszwo  for the review, everyone agrees that zerocopy.enabled should be a default enabled in configuration, let me change it!

@guohao-rosicky do you plan to continue working on this?

@guohao-rosicky do you plan to continue working on this?

I'm modifying UT. Will continue this work. @kerneltime


Documentation workflow based on Kindly & Clay

edit: corrected parhs
New experimental dev workflow
This PR is part of our work on a new dev workflow for Tablecloth:

namespace-as-a-notebook documentation using Kindly and Clay -- this PR
testing the documentation using note-to-test -- coming soon

Status
Not to be merged yet.
There are still some rendering issues (e.g. in the layout of R code output).
Relevant files

notebooks/draft.clj - the tutorial as a Kindly notebook (developed with Clay)

dev/conversion.clj - the script used to generate the notebook from the original Rmarkdown tutorial (up to a few additional manual edits)

docs/draft.html - the tutorial rendered using Clay and Quarto -- temporarily served at https://daslu.github.io/tablecloth/draft

Actions

to render the notebook using Clay (assuming you have the Quarto CLI installed):

(require '[scicloj.clay.v2.api :as clay])
(clay/make! {:format [:quarto :html]
             :source-path "notebooks/index.clj"})

The Tablecloth docs are now rendering mostly correctly through the proposed namespace-as-a-notebook approach -- served temporarily here: https://daslu.github.io/tablecloth/
Since we are rendering it through Quarto, the R code blocks are still evaluated as they used to be in the Rmarkdown docs.
E.g. here: https://daslu.github.io/tablecloth/#data.table
We cannot merge the PR yet since the tests are failing. I'll update soon.

The tests pass now, so the PR can be merged from my perspective.

Quoting @genmeblog on the Zulip conversation:
since my time is limited this week I can't move forward. I don't want to stop you so here is my proposal what to do with merging PR:

I want to keep current docs reachable somewhere (all artifacts) - so let's move it to the "old" subfolder and update all links (readme + github itself)
then merge Daniel's branch to enable new docs, also link to it somewhere in Readme.
since the merge we do not update RMarkdown anymore, changes in docs will be done only in Daniel's version

You have all rights to make the merge - feel free to do this


Will Docker Hub images be released?

WHAT
Docker Hub release is stuck at 1.0.4.
https://hub.docker.com/r/mercari/wrench/tags
Will this be part of the release process going forward?
WHY

because the README points there
https://github.com/cloudspannerecosystem/wrench/blob/d04d088bfb4cb581d84c9402ce15c146e3c6990c/README.md?plain=1#L49

I think that @110y manually pushed https://hub.docker.com/r/mercari/wrench, but what should we do next?

We should use GitHub Docker registry instead. Please help me to setup GitHub action for that if anyone can.


Tokenisers

Two scripts related to tokenisers:

tokeniser_config.py, containing the TokeniserConfig object for storing the exact configuration file for our tokenisers, to ensure reproducibility. Can be saved to a json file with the save method and loaded with the load class method.
train_tokeniser.py, containing a train_tokeniser function for, guess what, training a tokeniser. Takes in a TokeniserConfig object as input, or a dictionary which is converted to such an object. Saves both tokeniser and config upon completion.

I did not include the full flexibility of the tokenisers (there are thousands of combinations), but included a decent chunk of such flexibility. It supports three kinds of tokenisers (BPE, WordPiece and Unigram), and you can enable SentencePiece for them, to ensure that the tokens can be decoded as well (in the case of BPE and Unigram), which we'd need for generator models.
I took the liberty to add some "sensible defaults":

Maximum sequence length defaults to 512 tokens.
By default use NFKC Unicode normalisation, which converts weird symbols (like the triple elipsis symbol "…") to something more sensible (like three full stops "...")
Special tokens are by default <pad>, <s>, </s>, <unk> and <mask>.
Default is to add the CLS and SEP tokens (we can adjust what these tokens are in the config, defaults to <s> and </s>, as is the new standard).
Add both padding and truncation by default

Further required configurations are:

tokeniser_type  (str), can be 'bpe', 'wordpiece' or 'unigram'
vocab_size (int)
lower_case (bool)
sentence_piece (bool)
byte_level (bool)
add_prefix_space (bool), whether to add a space before all words, including the first one. Used by, e.g., RoBERTa.

The tests folder contains a tokenisers_test.py, with unit tests for both scripts, which all pass on my end (but you should of course check if they work on your end as well!). For this reason I also included a pytest.ini file in the root, containing the pytest configuration.
I could set up a CLI as well, but I'm not sure how useful that is, compared to simply calling train_tokeniser from a notebook or a script. If you feel like such a thing would be useful I'll add it 🙂

Incredible work @saattrupdan! The speed at which you work is quite amazing.
I do not know how I feel about the british spelling, and I actually think that we should change it to "tokenizer" with "z" for streamlinings sake.
Any other thoughts on this? @HLasse @KennethEnevoldsen

Thanks @MalteHB! Haha yeah, I guess I've been indoctrinated. We could all just do what the Americans are doing, sure. I'll change that ASAP.


Add copy to clipboard button next to target branch

Description
Context
It is possible to copy the source branch name to clipboard, but not the target branch.
Feature
Add a button next to target branch to copy its name to clipboard.
Screenshot

Example URL
https://github.com/refined-github/refined-github/pull/5217

I think the reason why only the head branch has a copy button is that the target branch is usually the default branch, whose name people seldom need to copy.
If you do want to copy it, just hold Option/Alt and you can easily select the target branch.

Duplicate of https://github.com/refined-github/refined-github/issues/77

Can you show your step-by-step process for using this value? Like, what do you do after you copy it?

Simply copying and pasting to the terminal in order to switch to the branch and test locally.
This might be the same reason they added the copy next to source branch.
However reading #77, it seems you are not fond of this change, so up to you 🙌

This would be also great for pasting into Slack for review. Having that extra description with line change numbers next to the link is helpful. I know there's Github preview in Slack, but that give way too much context sometimes.
We use a GreaseMonkey script at my company to generate put a markdown string into our clipboard when clicking on the title like this: https://github.com/BlaInc/important-service/pull/2347/files Add external GQL module (+38/−18)

for review requests

Why not share the PR url? It sounds like it would make the most sense since it's where reviews should happen

Simply copying and pasting to the terminal in order to switch to the branch and test locally.

I'm not sure I get it. Why do you want to switch to the base of a PR? The branch isn't even necessarily the actual base of the PR.
It sounds like both arguments are for the target branch, which I don't agree with but make sense. I don't see how the base branch is useful in either situation.


upgrade gradle 4.10.2

upgrade spotbugs-gradle-plugin to 1.6.5 because of https://github.com/spotbugs/spotbugs-gradle-plugin/issues/39

upgrade lombok to 1.18.2 because of https://github.com/rzwitserloot/lombok/issues/1716


Problem with install

Following the current README, I got various errors when running make devready, the first being
a problem with install pysam.
I ended up manually running:
pip install -r
on etc/develop.reqs and etc/install.reqs
and then make devready
after that I could run
python app.py &
successfully.
Has anyone else had success with the README in a fresh virtualenv

It works for me as advertised on Ubuntu 16.04, w/Python 2.7.12.
snafu$ (set -x; make cleanest; make devready) 2>&1 | tee log.txt
snafu$ tail log
Searching for wcwidth==0.1.7
Best match: wcwidth 0.1.7
Adding wcwidth 0.1.7 to easy-install.pth file

Using /home/reece/projects/biocommons/hgvs-eval/venv/lib/python2.7/site-packages
Finished processing dependencies for hgvseval==0.0.1.dev18+naf64e28469ad
make[1]: Leaving directory '/home/reece/projects/biocommons/hgvs-eval'
#############################################################################
###  Do not forget to `source venv/bin/activate` to use this environment  ###
#############################################################################

Details at log.txt
That's not to say that the devready scheme isn't broken somewhere, but rather that it does work in at least one context.

So, I was able to follow Reece's instructions above, and get server running and the example curl to work once I

removed anaconda from my PATH
realized that the code starts its own venv that I need to activate as per its output, not just use one I create ahead of time

I'm running ubutnu 16.04
HOWEVER, running:
hgvs-eval --html report.html --json report.json http://<IP_ADDRESS>:8000/

produces all kinds of error messages that I excerpt below
 hgvs-eval --html report.html --json report.json http://<IP_ADDRESS>:8000/
Starting HGVS-eval tests for endpoint: http://<IP_ADDRESS>:8000/

HGVS Endpoint: http://<IP_ADDRESS>:8000/
rootdir: /home/paschallj/hgvs_eval/v5/hgvs-eval, inifile: pytest.ini
plugins: json-0.4.0, html-1.11.0, cov-2.4.0
collected 18 items

tests/test_hgvs.py Testing HGVS service info endpoint on: http://<IP_ADDRESS>:8000/info
<IP_ADDRESS> - - [09/Nov/2016 08:32:43] "GET /info HTTP/1.1" 200 -
{u'packageVersion': u'0.5.0a6', u'nomenclatureVersion': u'yeah, right', u'evalVersion': u'0.0.1.dev23+ng015a294'}
.Testing HGVS operation on: http://<IP_ADDRESS>:8000/project_g_to_t
project_g_to_t(None, None)
<IP_ADDRESS> - - [09/Nov/2016 08:32:43] "POST /project_g_to_t HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 2000, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 1991, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask_cors/extension.py", line 161, in wrapped_function
    return cors_after_request(app.make_response(f(*args, **kwargs)))
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 1567, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 1988, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 1641, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask_cors/extension.py", line 161, in wrapped_function
    return cors_after_request(app.make_response(f(*args, **kwargs)))
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 1544, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 1639, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/paschallj/hgvs_eval/v5/hgvs-eval/venv/lib/python2.7/site-packages/flask/app.py", line 1625, in dispat

and

self = <json.decoder.JSONDecoder object at 0x7f40a677a810>, s = '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
  "http://www.w3.org/TR/html4/loose.dtd">
<html>
  <he...", line 232, in head
    if self.position >= len(self.data):
TypeError: object of type \'NoneType\' has no len()

-->
', idx = 0

    def raw_decode(self, s, idx=0):
        """Decode a JSON document from ``s`` (a ``str`` or ``unicode``
            beginning with a JSON document) and return a 2-tuple of the Python
            representation and the index in ``s`` where the document ended.

            This can be used to decode a JSON document from a string that may
            have extraneous data at the end.

            """
        try:
            obj, end = self.scan_once(s, idx)
        except StopIteration:
>           raise ValueError("No JSON object could be decoded")
E           ValueError: No JSON object could be decoded

/usr/lib/python2.7/json/decoder.py:382: ValueError

the output for me of
curl -d hgvs_string="NC_000020.10:g.278701_278703delGGC" -d ac=NM_033089.6  http://<IP_ADDRESS>:8000/project_g_to_t

is
project_g_to_t(NC_000020.10:g.278701_278703delGGC, NM_033089.6)
<IP_ADDRESS> - - [09/Nov/2016 08:37:56] "POST /project_g_to_t HTTP/1.1" 200 -
{
  "hgvsString": "NM_033089.6:c.471_473delGGC"
}

and http://<IP_ADDRESS>:8000/info works
so I think the server is running, but not sure if these results from
hgvs-eval --html report.html --json report.json http://<IP_ADDRESS>:8000/ are expected

When I installed virtualenv using the preinstalled Python version and then installed this tool, I was able to run the example curl command and get the same output as you did. I also got errors when running the hgvs-eval command.

I got those errors too, I think because the endpoints are still just stubs.
Since we've got confirmation that this works as described (modulo the anaconda issue), let's close this issue.
I'm open to considering changes to the current scheme. If any of you would like to do that, let's open a new issue to discuss what we want before we make changes.


feat(controller): health checks for multi-source argo cd apps

Fixes #1399
Changes in PR:

Fix determinining desired revision when application has multiple sources
Fix detecting revision for apps with multiple sources targeting the same repository
Add debug log messages to help diagnose issues
Add missing fields to prevent destroying application settings in Helm sources such as valueFiles, valuesObject, etc.

@krancour , this is a WIP but I've made some good progress fixing the issues I found.

@hiddeco , any other concerns with this one, other than having to wait until #2156 is merged?

Is there anything else to address for this one @krancour or @hiddeco ?

This is on our list to look at, but given the (upstream) complexity it may take a while before we have time to validate this end-to-end. Please bear with us until then, thanks 🙇

That's quite likely, as I think what you are working on compliments what someone else tried to do before in #2088.

Added health check fix and tests for multi-source app support.

Hello @hiddeco / @krancour  , can one of you review please?

Review please?

@gnadaban we are working at a furious pace to get v0.8.0 released and this isn't something we need for v0.8.0, so we need to ask for some patience please. 🙏

Hey @krancour , thanks for the update!
Any chance to include this in v0.8.0? If there's anything I can do to help to make that happen let me know.
It's literally fixing previously broken/unavailable paths, so I don't think it would bother anyone.

@gnadaban, we have already frozen 0.8.0 and are only accepting critical bug fixes now. The main goal is to get 0.8.0 out the door as stable as possible, and if we keep adding new items to it, it will require that we start our testing all over again.
The good news is that we release new versions every five weeks, so your change (if ready) should be included in the next release (0.9.0) and will be available before you know it. Thank you for understanding.

Hey @kencochrane , thanks for your reply, I was not aware that 0.8 was frozen, and can appreciate wanting to avoid scope creep.
At the same time, I can't help but feel disappointed that this also means we'll have to maintain this PR for potentially weeks more, and wait 5+ to get it officially merged, all this for a trivial addition.
Aside from adding debug log messages, which could arguably be invisible, I added
		if len(app.Status.Sync.Revisions) > 0 {
			// App has multiple sources, so we need to check the list of revisions.
			// Note: Apps with multiple sources pointed at the same Git repository can only have the same revision
			// 		 	 for all sources because ArgoCD does not support the alternative, so we only need to check
			//				the first revision match.
			for _, r := range app.Status.Sync.Revisions {
				if r == revision {
					logger.Debug("Found desired revision in list of revisions of multi-source application, app is healthy.",
						"desiredRevision", revision, "currentRevisions", app.Status.Sync.Revisions)
					return kargoapi.HealthStateHealthy, nil
				}
			}
		}

I removed
	// TODO: We should re-evaluate this soon. It may have been fixed in recent
	//       versions.
	// TODO(hidde): Do we have an upstream reference for this?
	if len(app.Spec.Sources) > 0 {
		err := fmt.Errorf(
			"bugs in Argo CD currently prevent a comprehensive assessment of "+
				"the health of multi-source Application %q in namespace %q",
			key.Name,
			key.Namespace,
		)
		return kargoapi.HealthStateUnknown, healthStatus, syncStatus, err
	}

And added
	// An application may have more than one source that points to the same Git repository,
	// eg. a Helm and a vanilla manifest source.
	// In that situation it doesn't matter which source's target revision is returned as ArgoCD does not support
	// different target revisions for sources targeting the same repository.
	for i := range app.Spec.Sources {
		desiredRevision := GetDesiredRevisionFromSource(ctx, &app.Spec.Sources[i], freight)
		if desiredRevision != "" {
			return desiredRevision
		}
	}
``` to ` GetDesiredRevision`.

The most frustrating is that every time any of these functions are changed I have to resolve the conflicts and the clock starts again.

You shouldn’t have to wait 5 weeks to get this merged. If all goes as planned, 0.8.0 goes out the door this week, and then next week we can merge the PRs that are ready, that didn’t make it into 0.8.0. Then it will be available in the nightly builds.

I have had to essentially reimplement my fixes due to the extensive changes done to the affected files before merging this.
Please try to review it soon so I don't have to again.

ArgoCD does not support cloning different revisions of the same repository, hence the single desired revision.

The current implementation only supports a single source per application to sync/subscribe to, which may arguably be the most common/important use-case where everything from chart to values files are in a single location. This implementation does not support the scenario where manifests/charts/resources of a single ArgoCD app are NOT tracked in a single repository, which IMO would be an anti-pattern.
This PR extends that with the case where one might have 2 or more sources, of which we are still interested in changes to a single one being synced.
For the sake of your example, let's assume there's an app with 2 sources, one from an official chart or Git repository that we don't have control over, and another from some repository that we do. Let's call the one we have control over "internal" and the other one "external".
Normally, I'd think users track target revisions of external sources in the same repository as the app whose repo changes we are interested in, eg. via a Chart.yaml dependencies target revision, or a values or kustomization file with specific target revisions.
In my opinion, we don't care about the external source or what it is synced to, as we don't subscribe to freight changes from that source, we care about the internal repository changes, which might very well have eg. values files or manifests that change the revision of the external source as well.
Because of this, I don't think we should care about checking beyond one Git repository match for a single app.
Either way, this PR already unblocks a large set of users who would benefit from at least this level of multi-source app support.

As I understand it, subscribing to an official chart repository for templates and default values and to a git repo for value overrides was the main driver behind multi-source apps. Those umbrella charts were often regarded as a workaround.
You may not agree with that and may see it as an anti-pattern, and I won't argue that point. It may largely be a matter of preference. But I will say, as I've told you before, we are not keen on merging functionality that narrowly addresses your use cases, to the exclusion of the general case.
The general case for multiple source is that for every source whose desired revision could be known, we ought to be checking that that is what it is synced to.

As I understand it, subscribing to an official chart repository for templates and default values and to a git repo for value overrides was the main driver behind multi-source apps. Those umbrella charts were often regarded as a workaround.

I'm not sure we're on the same page, let me give you an example and let you decide. This I think would be a typical example of using an upstream chart + an internal repo with the values file for it:
apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  labels:
    application-group: aws-controllers-k8s
  name: ack-iam-controller
  namespace: argocd
spec:
  project: myproject
  destination:
    namespace: ack-system
    server: https://kubernetes.default.svc
  sources:
    - chart: iam-chart
      repoURL: public.ecr.aws/aws-controllers-k8s
      targetRevision: 1.3.1
      helm:
        releaseName: ack-iam-controller
        valueFiles:
          - $values/config/default/ack-iam-controller.values.yaml
        valuesObject:
          aws:
            region: us-east-1
    - ref: values
      repoURL: git@mygithubserver.net:myorg/myrepo.git
      targetRevision: 3.4.6
  syncPolicy:
    automated:
      selfHeal: true
    syncOptions:
      - CreateNamespace=true

I'm not sure one would call this a workaround, or what you mean by "umbrella chart".

You may not agree with that and may see it as an anti-pattern, and I won't argue that point. It may largely be a matter of preference. But I will say, as I've told you before, we are not keen on merging functionality that narrowly addresses your use cases to the exclusion of the general case.

I would say this approach of using open-source charts from whatever source with your company's specific configuration is very much a de-facto standard, not some esoteric use-case specific to our example. Think of how many are using eg. cert-manager, a very popular open source app, for which you need to include eg. Issuer resources with your own configuration, and perhaps some IRSA annotations for an IAM role that grants access to your DNS records. In such a case, you wouldn't want to subscribe to updates to eg. the cert-manager chart itself, because it removes determinism from your infrastructure to blindly give up control over when and what to upgrade to. Instead, one would update the revision in their own cadence in the same Git repo the application's values files are in. This is not only my use-case, and I find it difficult to present it as such.

The general case for multiple source is that for every source whose desired revision could be known, we ought to be checking that that is what it is synced to.

Can you explain to me through an example why would you have 2 separate Git repository sources you want to subscribe changes for in the same application, given my example above?

@akolacz , can you provide an example for your use-case, and let me know if what @krancour is advocating for is a a requirement for you, eg. do you have an example app with multiple sources where more than one source is from a different repository you control and would want to subscribe to?

@gnadaban
Apologies for the confusion. I might have misunderstood what you meant by all of the following:

This implementation does not support the scenario where manifests/charts/resources of a single ArgoCD app are NOT tracked in a single repository, which IMO would be an anti-pattern.
For the sake of your example, let's assume there's an app with 2 sources, one from an official chart or Git repository that we don't have control over, and another from some repository that we do. Let's call the one we have control over "internal" and the other one "external".
Normally, I'd think users track target revisions of external sources in the same repository as the app whose repo changes we are interested in, eg. via a Chart.yaml dependencies target revision, or a values or kustomization file with specific target revisions.
In my opinion, we don't care about the external source or what it is synced to, as we don't subscribe to freight changes from that source, we care about the internal repository changes, which might very well have eg. values files or manifests that change the revision of the external source as well.
Because of this, I don't think we should care about checking beyond one Git repository match for a single app.
Either way, this PR already unblocks a large set of users who would benefit from at least this level of multi-source app support.

Isn't the example you gave, which you say is common (and I agree, it is) an instance of what you claimed to be an anti-pattern? Namely, "where manifests/charts/resources of a single ArgoCD app are NOT tracked in a single repository?"
Let's not quibble about patterns or anti-patterns. Let's just run with this use case.
Why do you not care about verifying that you are synced to the correct revisions of both the chart in public.ecr.aws/aws-controllers-k8s and values in git@mygithubserver.net:myorg/myrepo.git?

Yep. I know we're checking for general problems first and moving on to looking at what everything is synced to only once we get past that.

In the context of Kargo, we check if a Freight from our internal repo is in the synced sources list of the app, because that's one signal we care about.

Do you think that everyone has this same expectation? Or do you think a lot of people might also care that the other source is synced as they expect as well?

In the context of subscribing to a single Git Warehouse that represents that Git repository, yes, in addition to also having established that the other source is already synced as verified by having no conditions.

Do you think that everyone has this same expectation? Or do you think a lot of people might also care that the other source is synced as they expect as well?

I expect that very few people today even have Kargo, and that no one today uses 2 Warehouse subscriptions for the same app, yes.

@krancour I understand your position of wanting to enforce good, robust solutions, I'm just not sure why this particular PR is expected to address every single edge-case all at once. I'm not against improving things, but given my comparably limited time for this I'd hope you or someone from the maintainers can add/update things as you see fit if necessary, like you've done with multi-pipeline support.
I reached out on Discord in June, I'd be more than happy to do a quick call and discuss and try to find common ground to move things forward.

I expect that very few people today even have Kargo, and that no one today uses 2 Git Warehouse subscriptions for the same app, yes.

I don't understand the relevance of this argument. That's not the case we were using as an example. The example involved one source pointed at a Helm chart repo and one pointed at a Git repo.

I'm just not sure why this particular PR is expected to address every single edge-case all at once

The case we have been discussing is far from an edge case.
If I were subscribed to a Helm chart repo and a Git repo, and updating two corresponding sources of a multi-source App, I would want to know that both updates had the desired effect. In the general case, if any source were worth updating, why would the user not care that the update worked? If you happen not to care about all but one of the sources being synced correctly, it seems to me that your case may actually be the edge case, with the general case still poorly accounted for.
We were previously honest that we couldn't comprehensively assess the health of Stages with associated multi-source Apps. If we were to merge your change as is, there is a real danger that we begin mis-reporting things as healthy when they are not.
You can either figure accept that @hiddeco and I -- the two most senior maintainers of this project -- share this concern and you can work toward remedying it, or you can just close the PR.
This is the second time we've clashed over a PR that suits your specific use case, but does not generalize well. In the previous case, we worked hard to arrive at a compromise, but please let me be clear that pressuring maintainers into merging PRs they are not comfortable with is poor etiquette in the open source community and if you make a habit of it, you may find that future contributions are not welcome.
With that behind us, why don't we talk about if or how it is possible to check whether all sources with known desired revisions are synced according to expectations?

@krancour  Allow me to respond slightly out-of-order this time.

We were previously honest that we couldn't comprehensively assess the health of Stages with associated multi-source Apps. We returned unknown with an explanation. If we were to merge your change as is, there is a real danger that we begin mis-reporting things as healthy when they are not.
You can either accept that @hiddeco and I -- two senior maintainers of this project -- share this concern and you can work toward remedying it, or you can just close the PR.

I understand both your concerns and I also understand that as you are senior maintainers of this project, you have the say in what goes and what doesn't, and as before, will do what I can to accommodate that.
That said, I, and the community that needs this feature may not be convinced that not supporting multi-source apps for fear of potential misrepresentation of health status is better than supporting it at all.

This is the second time we've clashed over a PR that suits your specific use case, but does not generalize well. In the previous case, we worked hard to arrive at a compromise, but please let me be clear that pressuring maintainers into merging PRs they are not comfortable with is poor etiquette in the open source community and if you make a habit of it, you may find that future contributions are not welcome.

I did not intend to pressure you into accepting something broken, but while I was trying to make this much-needed feature acceptable I did start to feel like being given the runaround for over a month now.
That said, it would be remiss of me to blatantly disregard etiquette in any situation, and if my sense of urgency was pressuring I owe you both an apology. I completely understand if you don't want to accept my contribution for this, or any reason.
I do not think that this PR is the best implementation, by far. Me and our team would obviously benefit from this, but since there are many other issues with Kargo at this moment eg. #2178 , I was hoping to improve things even a bit, one step at a time, without having to rely on a forked version for long.
Respectfully, we are however a potential customer in the process of trying to evaluate Kargo, and by submitting PRs like this, are actively trying to improve things not just for ourselves and the community, but through this project, Akuity. With that in mind, let me try to speak to the use-case you mentioned, with hopes of finding an acceptable solution to this.

If I were subscribed to a Helm chart repo and a Git repo, and updating two corresponding sources of a multi-source App, I would want to know that both updates had the desired effect. In the general case, if any source were worth updating, why would the user not care that the update worked? If you happen not to care about all but one of the sources being synced correctly, it seems to me that your case may actually be the edge case, with the general case still poorly accounted for.

If we had an app that has a Helm chart repo and a Git repo, IMO an Application would not be subscribed to both changes at the same time because one would want to control the overall target revision of even the Helm chart via the Git repo that contains the application, and any values files. In my mind at least, a single source of truth for an app is always a single Git repository, regardless of how other sources like Helm repos change.

With that behind us, why don't we talk about if or how it is possible to check whether all sources with known desired revisions are synced according to expectations?

That said, if we want to support multiple sources being synced, in health.go we could iterate on a slice of desired revisions returned from GetDesiredRevision(s) if need be, provided that we can match sources we care about to FreightReference instances within that function. What do you think?

@gnadaban thank you. Let's work through the issues now. 😄

That said, I, and the community that needs this feature may not be convinced that not supporting multi-source apps for fear of potential misrepresentation of health status is better than supporting it at all.

I promise you, we want full support for multi-source apps as badly as anyone. Since the very early days of the project, Argo CD App updates can each involve multiple source updates. Granted, without the corresponding health checks also working, using more than one source update per App update has never been practical, but those updates do work, and I hope that reinforces that this has always been on our radar. It's just those pesky health checks -- and we appreciate your willingness to help with that. With this said, any fear we have of misrepresenting health status isn't merely hypothetical and the notion that misrepresenting health may be preferable to not supporting multi-source at all is probably not shared by all users, especially those who may be contemplating Kargo for the delivery of mission critical applications. Any one or more sources not synced to what you think they should be could be a pretty serious problem.

I completely understand if you don't want to accept my contribution

We're not there. I promise you that I see us as being back on track now.

I do not think that this PR is the best implementation

The code review process is just meant to tease out the snarls so this can evolve into the right implementation and I'm confident we can get it there together.

we could iterate on a slice of desired revisions returned from GetDesiredRevision(s) if need be

This is pretty much what we've been wanting...

provided that we can match sources we care about to FreightReference instances

This is exactly right. On the Kargo end of things, there's definitely some effort involved, but should be relatively straightforward. The FreightCollection at the top of the Stage's Status. FreightHistory stack either will or will not contain a reference to a specific revision of an artifact from the repo that the source syncs to. For any source where there's nothing corresponding in the FreightCollection, we don't care what that source is synced to. For the others, we do.
It is on the Argo CD end of things where I have been skeptical all along (having nothing to do with your PR) that the information we need is present in the App's Status. A failure of mine is that in Feb 2023 I didn't more explicitly document what information was either missing or unreliable that lead me to treat a comprehensive snapshot of health for a multi-source app as indeterminable. I'm digging into Argo CD right now to refresh my own memory and it was Status.History[n].Revisions, which is a []string, that I had found perplexing because this doesn't tell you anything about what revision goes with which source. To the best of my recollection (which is probably not super reliable), this field also didn't seem that is was being updated correctly. This was in the Argo CD 2.6 time frame. Multi-source Apps were brand new and far from stable then.
Fast forward to the present day and @hiddeco and I have both refactored parts of the health checks numerous times now, for various reasons, and we're no longer looking at Status.History[n].Revision at all. Now we're looking at Status.OperationState.SyncResult.Revision. There's a corresponding Revisions field that was absent from our "knock-off" SyncOperationResult type. Your PR (correctly) adds it. I looked at the docs for the genuine SyncOperationResult type just now and found this:
	// Revisions holds the revision this sync operation was performed for respective indexed source in sources field
	Revisions []string `json:"revisions,omitempty" protobuf:"bytes,5,opt,name=revisions"`

So it's parallel to (indexed the same as) Status.OperationState.SyncResult.Sources! This is surprising because parallel arrays are usually regarded as rather brittle. idk why they did it this way, but it solves the riddle of how revisions in that array can be correlated to individual sources.
So I am fairly convinced at this point that what you are looking to do is possible and mainly hinges on iterating over Status.OperationState.SyncResult.Sources (unless len(sources) == 0, in which case we only care about Status.OperationState.SyncResult.Source), finding the desired revision for that source, and as long as we don't come up empty-handed, comparing that to the corresponding entry in Status.OperationState.SyncResult.Revisions (or Status.OperationState.SyncResult.Revision).
In the end, I believe this to be a slight refactoring of GetDesiredRevision(...) in internal/argocd/revision.go to accept a *kargoapi.ArgoCDSourceUpdate instead of a *kargoapi.ArgoCDAppUpdate and a new loop around the two places where  GetDesiredRevision(...) is currently invoked, so that it is invoked for every source in Status.OperationState.SyncResult.Sources (unless len(sources) == 0, in which case it only needs to be called once for Status.OperationState.SyncResult.Source).
I hope these details are what is needed to get this across the line.

Hey @krancour , thanks for the elaborate writeup!
I looked at the Status.OperationState.SyncResult.Revisions and found that Status.Sync.Revisions to be more accurate.
In my example, the former is actually an incorrect SHA for some reason, while Status.Sync.Revisions holds the correct hash.
The Status.Sync field definition confirms that it is indeed the current status, so I'm going to check this in my implementation first.
	// Sync contains information about the application's current sync status
	Sync SyncStatus `json:"sync,omitempty" protobuf:"bytes,2,opt,name=sync"`

Can you provide more information on why that SHA is incorrect, or how it may relate to past and/or other state?
As I feel this may relate to the reflections made by Argo not being truthful, which is why we disabled support in the beginning.

I looked at the Status.OperationState.SyncResult.Revisions and found that Status.Sync.Revisions to be more accurate.
In my example, the former is actually an incorrect SHA for some reason, while Status.Sync.Revisions holds the correct hash.

Can you provide more information on why that SHA is incorrect, or how it may relate to past and/or other state?
As I feel this may relate to the reflections made by Argo not being truthful, which is why we disabled support in the beginning.

To the best of my recollection (which is probably not super reliable), this field also didn't seem that is was being updated correctly.

Maybe my recall isn't so bad after all.

Hey guys, so in my specific case the SHA is incorrect in Status.OperationState.SyncResult.Revisions because it's pointing to a commit from eg. May which has since been squashed I believe, whereas the hash in Status.Sync.Revisions correctly points to the appropriate hash. I don't think I can give you more than that without giving out proprietary information, but I hope it helps.
I suspect that after ArgoCD does an initial sync, it doesn't actually update that information again in those fields.

Ok, I've an initial implementation of supporting multiple desired revisions, but I could really use your help with a few things:

I don't like the way this block is duplicated in revision.go getRevisionFromSource:
    // If there is a source update that targets app.Spec.Source, it might
    // have its own ideas about the desired revision.
    var targetPromoMechanism any
    for i := range update.SourceUpdates {
      sourceUpdate := &update.SourceUpdates[i]
      if sourceUpdate.RepoURL == s.RepoURL && sourceUpdate.Chart == s.Chart {
        targetPromoMechanism = sourceUpdate
        break
      }
    }
    if targetPromoMechanism == nil {
      targetPromoMechanism = update
    }
    desiredOrigin := freight.GetDesiredOrigin(stage, targetPromoMechanism)

If I extract this out of the switch, it could be rewritten to only consider RepoURL, which might be enough. Am I missing something here?

I refactored TestGetDesiredRevisions to allow specifying multi-warehouse test cases, can you sanity-check it pls? I am having trouble figuring out why the one test-case is not returning the correct results, but for the love of me I can't output debug logs when running tests that would help identify the codepath. Any chance you could provide a code-snippet as to how to output log messages for testcases, even temporarily? It'd be nice if this was a flag, but I don't know how much complexity it would add to this PR.

Something about how ArgoCD has separate Revision and Revisions fields makes me want to abstract the whole thing away with some helper functions. Would that be bad? It feels too repetitive to iterate and evaluate multiple times in different code paths. I'm thinking some functions that would eg. return all sources with corresponding Freights and current/desired revisions in a single call, that could then be reused. Thoughts?

I suspect that after ArgoCD does an initial sync, it doesn't actually update that information again in those fields.

Ugh. I wonder if we may need to resort to getting certain information from Status.OperationState and other info from Status.Sync. 🤦♂

I don't like the way this block is duplicated in...

Ya. At a glance, it looks pretty repetitious, but I think the similarities may be superficial.

it could be rewritten to only consider RepoURL, which might be enough

For a source pointed at a chart repo, chart name matters. Taking that into consideration, the only true commonality between the two cases is desiredOrigin := freight.GetDesiredOrigin(stage, targetPromoMechanism). The logic for identifying the target promotion mechanism and the logic for finding the commit/chart version remain different across the two cases.
But I am never opposed to refactoring code for better efficiency or readability. I tried once before in this spot and although I was able to avoid the superficial appearance of repetition, the resulting code was a bit more convoluted to actually understand. Invoking "clear over clever," I scrapped that change. You are more than welcome to try. If you come up with something better than I did, I'll be happy.

I refactored TestGetDesiredRevisions to allow specifying multi-warehouse test cases, can you sanity-check it pls?

Happy to.

Any chance you could provide a code-snippet as to how to output log messages for testcases, even temporarily?

You're supposed to be able to control the log level with the LOG_LEVEL env var, but something is fishy with the initialization. I'm seeing more logs than I expect during testing. Let me get back to you on this.

Something about how ArgoCD has separate...

Yeah. I think there are several spots now where, because of the old 🤷♂ behavior for multi-source apps, we only ever looked at singular Source, Revision, etc. If we now have several spots where it's "look at Sources if it's not empty, otherwise look at Souce, and similar for Revisions vs Revisions, I absolutely agree about finding an efficient way to keep those conditionals in one place if you can.

You're supposed to be able to control the log level with the LOG_LEVEL env var

This does work as intended.

but something is fishy with the initialization. I'm seeing more logs than I expect during testing. Let me get back to you on this.

My bad. I failed to notice that the logs I was seeing were error messages and I was using the default LOG_LEVEL of INFO.
I also forgot that successful test results are cached and that changing an env var doesn't invalidate the cache.
I'm using VS Code and have now added this in .vscode/settings.json and can control the log level to my satisfaction quite easily:
{
  "go.testFlags": ["-count=1", "-v"], // Disable test result caching, but keep verbose output
  "go.testEnvVars": {
    "LOG_LEVEL": "ERROR" // Change as needed
  }
}

The equivalent should be possible for other editors/IDEs as well.

Ah, marvellous, thanks @krancour !
Now, I took a stab at properly supporting multiple source updates in addition to allowing multiple apps, as well as correctly reporting health status of multi-source apps, let me know what you think.

Now, I took a stab at properly supporting multiple source updates in addition to allowing multiple apps, as well as correctly reporting health status of multi-source apps, let me know what you think.

Sure thing. At this stage I really need to test drive because of that whole "do we trust what Argo CD's telling us argument."
I should be able to get to this tomorrow.

Hey @krancour do you have an ETA on when this will be reviewed?

I feel we should trim down the number of logged messages

tbh, I would prefer seeing all the new log lines removed. The signal to noise ratio in this PR is low mostly on account of the logging. To be frank, the noise has been a hindrance in reviewing the relatively few substantive changes in this PR.

@krancour , @hiddeco : I've addressed the majority of the asks, and would like your help to finally complete this.
The debug logging messages added in this PR were essential for debugging what was preventing successful sync operations or causing issues, and to understand what parts of the call chain were involved. Understanding what the app does and how it reacts to things is crucial for cluster admins using Kargo. Since there's no equivalent, I'm strongly against complete removal of detailed logging. I'm not opposed to using Trace level for some, albeit I can't recall if I ever had to use such for any open source project, in my experience "debug" is usually what people use for well, debugging.
Unless there's something broken I'm hesitant to change or refactor anything else at this point, and would ask to forgo changes to parts of the code that are non-essential to this improvement.
As always, your suggestions are greatly appreciated, and I would again like to ask you to help expedite completion.

Superseded by #2552


[iOS] Remove accessibility root ID assumptions

iOS assumes there is a single accessibility root node with ID 0. In a multi-window world, there may be multiple root nodes each with their own unique ID. Thus, iOS must not rely on the root node having ID 0.
See: https://github.com/search?q=repo%3Aflutter%2Fengine+kRootNodeId+path%3A%2F^shell\%2Fplatform\%2Fdarwin\%2Fios\%2F%2F&type=code

This is no longer necessary as the framework will no longer require that embedders use unique IDs across all accessibility trees. iOS can continue assuming ID 0 is the root of its accessibility trees.


[BugFix] fix incorrect result of yearweek

Why I'm doing:
What I'm doing:
mysql> select yearweek('2023-12-31', 2), yearweek('2024-01-01', 2);
+---------------------------+---------------------------+
| yearweek('2023-12-31', 2) | yearweek('2024-01-01', 2) |
+---------------------------+---------------------------+
|                    202353 |                    202353 |
+---------------------------+---------------------------+
Fixes #51034
What type of PR is this:

[X] BugFix
[ ] Feature
[ ] Enhancement
[ ] Refactor
[ ] UT
[ ] Doc
[ ] Tool

Does this PR entail a change in behavior?

[ ] Yes, this PR will result in a change in behavior.
[X] No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

[ ] Interface/UI changes: syntax, type conversion, expression evaluation, display information
[ ] Parameter changes: default values, similar parameters but with different default values
[ ] Policy changes: use new policy to replace old one, functionality automatically enabled
[ ] Feature removed
[ ] Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

[X] I have added test cases for my bug fix or my new feature
[ ] This pr needs user documentation (for new or modified features or behaviors)

[ ] I have added documentation for my new feature or new function

[ ] This is a backport pr

Bugfix cherry-pick branch check:

[X] I have checked the version labels which the pr will be auto-backported to the target branch

[X] 3.3
[ ] 3.2
[ ] 3.1
[ ] 3.0
[ ] 2.5

@rohitrs1983 thanks for fixing this. but I'd like maybe you can add more test cases, to make sure this function's behaviour is consistent with mysql's.
maybe you can refer to this PR's comment: https://github.com/StarRocks/starrocks/pull/47365/files#diff-fc27f5c55e94b69f881f2184c34d746a5495999f61b05c724b58e9259ca132e0R1457-R1463

you can run that sql in mysql, to generate outputs of mysql
and add these outputs into your test cases.

@rohitrs1983 you can use following mysql statement to generate output for your test cases
create temporary table D(day int);
create temporary table Y(delta int);
create temporary table M(mode int);

insert into D values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9),(10),(11),(12),(13),(14);
insert into Y values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9),(10),(11),(12),(13),(14),(15),(16),(17),(18),(19),(20),(21),(22),(23),(24),(25),(26),(27),(28),(29),(30),(31),(32),(33),(34),(35),(36),(37),(38),(39),(40);
insert into M values (0),(1),(2),(3),(4),(5),(6),(7);

    with T as (
  select
   date_add(date_add('1900-01-01', interval Y.delta year), interval D.day day) as start_day,
   date_sub(date_add('1900-12-31', interval Y.delta year ), interval D.day day) as end_day,
   date_add(date_add('2000-01-01', interval Y.delta year), interval D.day day) as start_day2,
   date_sub(date_add('2000-12-31', interval Y.delta year), interval D.day day) as end_day2,
   M.mode
  from D join Y join M
 )
 select mode, start_day, yearweek(start_day, mode), end_day, yearweek(end_day, mode) ,
   start_day2, yearweek(start_day2, mode), end_day2, yearweek(end_day2, mode) from T;

@rohitrs1983 again, thanks for  your contribution


Removed dummy definitions in sample-detail-type.

My first Vocab! So be nice.
Added in four sample-detail types relating to cores, "Pallet weight, pallet condition, core condition and core tray condition". Definitions might be a bit vague, so feel free to change.

I was the only reviewer, and the commit has been merged by David in to the Master. So, I take it from everyone that they will be happy if I delete the branch?


doc: add directions for using sops

doc:

what is sops
directions on generating age keypair
directions on encrypting and editing a yaml file with `sops

Signed-off-by: Vibhav Bobade<EMAIL_ADDRESS>

@waveywaves. I ran the linter locally:
git:(patch-1) markdownlint -c .markdownlint.yaml docs/**/*.md
docs/core-concepts/1204-secrets.md:57 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]
docs/core-concepts/1204-secrets.md:58 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "1. Create an `age` key"]
docs/core-concepts/1204-secrets.md:59 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
docs/core-concepts/1204-secrets.md:59 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
docs/core-concepts/1204-secrets.md:64:1 MD029/ol-prefix Ordered list item prefix [Expected: 1; Actual: 2; Style: 1/1/1]
docs/core-concepts/1204-secrets.md:64 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "2. Use the public key to encry..."]
docs/core-concepts/1204-secrets.md:65 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
docs/core-concepts/1204-secrets.md:65 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
docs/core-concepts/1204-secrets.md:69:1 MD029/ol-prefix Ordered list item prefix [Expected: 1; Actual: 3; Style: 1/1/1]
docs/core-concepts/1204-secrets.md:69 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "3. Edit the `secrets_sops.yaml..."]
docs/core-concepts/1204-secrets.md:70 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
docs/core-concepts/1204-secrets.md:70 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]

Could you please update that ?


Update AmazonOrderItemList.php

Will add the AmazonOrderId from the Xml to the OrderItem. This way checks can be created to ensure OrderItem belongs to the Order.

It looks like a simple enough change. I am not able to test it or anything at this time, so I will get to it whenever I can do so.

I have implemented this in a product, which is under development, and I saw it worked. Do you need a test case for it?

I don't need one, thanks.

The issue was resolved with a different implementation. Thank you for the attempt, though.


Fix bugfix

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Pull request type

Please check the type of change your PR introduces:

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, renaming)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] Documentation content changes
[ ] Other (please describe):

What is the current behavior?

Issue Number: N/A
What is the new behavior?

Other information

Codecov Report
Patch coverage: 51.85% and project coverage change: -18.65 :warning:

Comparison is base (b1dc8fc) 64.83% compared to head (629e109) 46.18%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
Additional details and impacted files
@@                Coverage Diff                @@
##           release-3.44     #714       +/-   ##
=================================================
- Coverage         64.83%   46.18%   -18.65%
=================================================
  Files               228      154       -74
  Lines             22282    16036     -6246
=================================================
- Hits              14447     7407     -7040
- Misses             7065     8302     +1237
+ Partials            770      327      -443

Flag
Coverage Δ

?

1.17.x
45.96% <51.85%> (-18.45%)
:arrow_down:

1.19.x
46.27% <51.85%> (-18.38%)
:arrow_down:

1.20.x
46.23% <51.85%> (-18.45%)
:arrow_down:

integration
?

macOS
46.15% <51.85%> (-0.13%)
:arrow_down:

secure
?

ubuntu
46.18% <51.85%> (-0.10%)
:arrow_down:

unit
46.18% <51.85%> (-0.19%)
:arrow_down:

windows
?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

connection.go
3.71% <ø> (-74.81%)
:arrow_down:

internal/balancer/balancer.go
26.72% <0.00%> (-59.06%)
:arrow_down:

internal/table/client.go
79.81% <68.42%> (-7.71%)
:arrow_down:

internal/table/session.go
45.03% <100.00%> (-38.03%)
:arrow_down:

... and 169 files with indirect coverage changes
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


remove gauges tracking code

heyo! I saw that you are using my blog as a template for yours. That's great! However could you make sure to remove my gauges tracking code so your page views don't show up in my tracker?
Thanks! And happy blogging! :)

have done;
Sorry for the inconvenience.

thanks a lot for your template, I like it.

awesome, thanks! Glad you like it :)


Misc actor data issues

[ ] Fenix from SOA calldown doesn't play the spinning animation properly when spinning. Fenix in Templar's Return works fine
[ ] Zerg air advanced morphs don't play an "entering cocoon" animation when starting their morph. Observed on Viper and morphling -> devourer, others should be checked too
[ ] Protoss Stargate units do a warping-in spell animation on being built from a (non-warp) stargate. This doesn't work for all units. Skylord should be fine after WC PR, Arbiter was observed to be missing this animation.

Fixed Fenix from SoA calldown, fixed protoss warping-in spell animations

Should be already patched


feat: store graphs on hub and not proejct

There are a few remaining project strings. And I would love to do it naively without signals. :+1:


fix Emmanuelle info + mobile img

Description
Two tiny issues:
Emmanuelle did her Master's in reverse :p

The images aren't great on mobile

After fix
It should look something like this once merged

Thank you @VincentBeaud, Antoine just added a new framework. I'm going to fix em info in a new PR.


Updated broken links in documentation.

The spaces removed in CONTRIBUTING.md were due to the trailing-whitespace pre-commit hook.

Maintainer merge checklist

[x] Title is descriptive/clear for inclusion in release notes.
[x] Applied a Component: xxx label.
[ ] Applied the api-deprecation or api-break label.
[ ] Applied the release-highlight label to be highlighted in release notes.
[x] Added to the milestone version it was merged into.
[ ] Unittests are included in PR.
[ ] Properly documented, including versionadded, versionchanged as needed.

@Lee-Rice thanks 👍

Not uploaded as of now: https://kivy.org/doc/stable/

Another half a year to update main docs links website? https://kivy.org/doc/stable/
Typo introduced here
https://github.com/kivy/kivy/commit/9c8528df2ef871d36c151f0c4ab96c851da7641d


Virtualbox 5 memory dump command change

Virtualbox 5 command for memory dump changed,
https://www.virtualbox.org/manual/ch08.html#vboxmanage-debugvm
VBoxManage debugvm <uuid|vmname> dumpvmcore [--filename=name]
For proper memory dumping
https://github.com/cuckoobox/cuckoo/blob/master/modules/machinery/virtualbox.py#L206
should read
label, "dumpvmcore", "--filename", path],
for Virtualbox 5 support

I made a fix for this. See this pull request: #628.

 Thanks for your pull request.


JsonCodec cannot validate valid json code with quotes inside (the json itself is valid)

Steps to Reproduce

Run flutter create bug.
Update the files as follows: ... main.txt (could not upload .dart file)
Run the app
You will see an exception while decoding the json.

Expected results:
Decoded json
Actual results:
Exception because there are quotes in the json. The json is valid (checked with https://jsonformatter.curiousconcept.com/#).

Logs

The output is located in

[flutter_run_verbose.txt](https://github.com/flutter/flutter/files/6974291/flutter_run_verbose.txt)

No issues found! (ran in 2.3s)

[✓] Flutter (Channel stable, 2.2.2, on macOS 11.3.1 20E241 darwin-x64, locale
    es)
    • Flutter version 2.2.2 at /Users/ondrej/Documents/Programms/flutter
    • Framework revision d79295af24 (9 weeks ago), 2021-06-11 08:56:01 -0700
    • Engine revision 91c9fc8fe0
    • Dart version 2.13.3

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.2)
    • Android SDK at /Users/ondrej/Library/Android/sdk
    • Platform android-30, build-tools 30.0.2
    • ANDROID_HOME = /Users/ondrej/Library/Android/sdk
    • Java binary at: /Applications/Android
      Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build
      1.8.0_242-release-1644-b3-6915495)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 12.5, Build version 12E262
    • CocoaPods version 1.10.1

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 4.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build
      1.8.0_242-release-1644-b3-6915495)

[✓] VS Code (version 1.59.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.25.0

[✓] Connected device (2 available)
    • iPhone 11 Pro (mobile) • 01C71F07-7591-4FEE-AE3D-28343C5F456F • ios
      • com.apple.CoreSimulator.SimRuntime.iOS-14-5 (simulator)
    • Chrome (web)           • chrome                               •
      web-javascript • Google Chrome 92.0.4515.131
    ! Error: iPhone is not connected. Xcode will continue
      iPhone is connected. (code -13)

• No issues found!

In the future, please file issues with Dart packages against Dart, not Flutter. That code isn't part of the Flutter project.
However, in this case the problem is with your code, not convert. You have not accounted for the fact that \ is the Dart string escape character, which is resolved at compile time; \" in your source resolves to " in the string itself, so what you are passing is in fact invalid JSON. You need to either escape the backslash or use a raw string literal, so that the \" is actually pay of the runtime string.

@stuartmorgan Thank you for your quick response! I apologize for posting to Flutter package, did not realize that 🤦♂.


In windows 7, I can't get DataGridViewCell value.

Can anyone help me?

I had also some issues with datagrid recently when updated to .NET 4.8 framework (implements changes of WPF an UIAutomation). I cannot cast my automation element anymore to a datagrid object. I only can find the object with ID and then use the Grid pattern to get the values. @Roemer maybe a good idea when updating to new version (which i am still waiting on wink wink), to keep the new .NET version(s) in mind


fix: fix dataSource not beign required in fetcher

🚨 IMPORTANT: Please do not create a Pull Request without creating an issue first.
Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of the pull request.
Description
Provide fix, so that dataSource is required param in situations where user have not provided custom endpoint because fetch must get it from somewhere.
#6633
Type of change
Please delete options that are not relevant.

[x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

[x] Run build-and-test with all tests passed (had to update snapshot)

Test Environment:

OS: Linux
@graphql-codegen/...: latest
NodeJS: 14

Checklist:

[x] I have followed the CONTRIBUTING doc and the style guidelines of this project
[x] I have performed a self-review of my own code
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes
[ ] Any dependent changes have been merged and published in downstream modules

Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

@nenadfilipovic Can you please add a changeset?

@nenadfilipovic Awesome, thanks 🔥

@n1ru4l thanks


[Suspense] Select suspense component to be handled by (possible key value as well)

It might be possible by throwing promise with a key param

Hi, thanks for your suggestion. RFCs should be submitted as pull requests, not issues. I will close this issue but feel free to resubmit in the PR format.


Row select and row clicked event not working?

Thank you for a really nice grid component.
We have started working with this along with bootstrap 3 and your helper @Html.AjaxGrid and everything is working nicely, but we have two issues that are unresolved.

In your example mvc-grid.azurewebsites.net/Javascript/RowClicked there is an effect in the grid that marks a row as "selected". How is this accomplished? We have  used the exact same razor configuration and javascript but when we click a row, no effect is visible. Is it a bootstrap issue?  Or what have we missed?

When binding the  rowClicked event as in your example, the "data" structure is undefined. In your example, there is a json object with data. Any idea why we can't get the data like that?

Also, it would be nice with an example of how to edit, delete rows. We plan to use the row double click event along with a bootstrap custom modal to edit row values. Also, we would like to add a delete button on each row that call by ajax a controller action that deletes a row.
Best regards

Selected is achieved by $(row).toggleClass('selected');
Could you share a small repro of that, because I can't reproduce that kind of behavior.

As for edit/delete rows you will have to use your own javascript event bindings to do that.

Hi and thank your for such a quick reply.
I have added this line of code $(row).toggleClass('selected'); to the rowClicked event and can confirm that the class="selected" is added. However, this happens on every row that I click on, I also need to perform the toggleClass on the previous row that was selected, so there are not multiple rows visually selected. Also, I guess we have to write custom css for the "selected" class on the   to give it some kind of visual appearance., since by default it looks just the same as a row that doesn't have the selected class.
As for #2, I can confirm that it is working in Chrome (F12->Console). In IE11 it is just blank, no clue why.
Thank you for a really nice component!

Does it also happen in row clicked example (maybe example is outdated)?
If it works there, you can download and see the sources, maybe you are doing something differently.

Thank you for the answer. The example seems to be working correctly. But when running the exact same code on my dev environment with bootstrap style, the rows are selected but are not unselected when I click another row. If you can, can you verify that the examples are working as the production code? The behaviour I get corresponts to "multi select" where u click multiple rows and get multiple rows selected. This is nice in some ways, but not what we are looking for here.

Selection is controller by you, just like browser's click event, rowclicked get's called on every grid row click, everything else is up to the developer.
If you want single selection you will have to unselected other rows and select a new one, I usually do
$(row).addClass('selected').siblings().removeClass('selected');

Thank you, I think I have a better understanding now on how to get things done with this grid! Problem solved.


Added trace type to Web Chat log

Description
This PR adds the trace type next to trace items in the Web Chat log.
Task Item
Fixes #6749
Screenshots
BEFORE:

AFTER:

@mewa1024 @cwhitten

Coverage decreased (-6.0e-05%) to 51.154% when pulling<PHONE_NUMBER>0284965b229a740586b698a2f0148b on toanzian/trace-type into ddc2a6784b362bf54f62a65f48886eb2eb8688f1 on main.


ERROR: tvservice-client: Failed to connect to TV service: -1 pngview: pngview.c:177: main: Assertion `display != 0' failed.

hi, wonderful tool, this pngview. thanks.
if i run it first, i have no error.
i can than run omxplayer on top of my pngview-image. no error in omxplayer.
i got an error the other way round:
if i run my omxplayer first, an than pngview, i get this error from pngview:
root@raspberry2-www:/usr/src/pngview# ./pngview -l 5 Monsters30.png
tvservice-client: Failed to connect to TV service: -1
pngview: pngview.c:177: main: Assertion `display != 0' failed.
Abgebrochen
root@raspberry2-www:/usr/src/pngview#
i dont know if you can fix this?

sorry for putting this here in.
i investigated a little bit, because the image did not vanish, so i found a process still running, wich must have lead to this error. i found it with 'ps" and killed it. now it works normal....
BUT i can not start a pngview a second time, to put an image on an higher layer.
That woud be nice, to stack images over each other.
is pngview allowed only on one single instance?

It works for me both ways, however looks like pngview is either limited to 8 instances, since I can only display 8 images at the same time, on any layer. When I try to run the 9th instance, I get the same error message:
tvservice-client: Failed to connect to TV service: -1
s5: pngview.c:184: main: Assertion `display != 0' failed.
Aborted

@AttilaSVK I've confirm. I have only 8 instances too. Also sometime after killing pngview I've empty black screen instead XServer, but application started in XServer continue to work as well

if you set dispmanx to offline to config.txt you will pas this limit, but things will slow down !

It works for me both ways, however looks like pngview is either limited to 8 instances, since I can only display 8 images at the same time, on any layer. When I try to run the 9th instance, I get the same error message:
tvservice-client: Failed to connect to TV service: -1
s5: pngview.c:184: main: Assertion `display != 0' failed.
Aborted

I have the same issue. Running on a raspberry pi 4 with 8G of ram. Is this a limit of the gpu? And why hasn't this limit changed, I can also run 8 pngviews on my raspberry pi 2. I had hoped newer hardware would have lifted this limit to a higher level. Or is this really a limitation on how pngview was written?
Thanks

There is not problem with this limit. You need to code your software. Raw tools like pngview are not suposed to be used as a desktop window manager.


idl RPC names can override Service object fields?!?  where are idl objects/fields validated?

https://github.com/oasislabs/oasis-client/blob/afd2e12d3a25b6b50d1997fa3bbc76a3073fe17c/packages/service/src/service.ts#L67
it looks like from
    // Attach the rpcs directly onto the Service object so that we can have
    // the nice service.myMethod() syntax in JavaScript.
    Object.assign(this, this.rpc);

that if the idl object has RpcFn with a name field that collides with any property in Service, the Object.assign will just blindly override.
https://github.com/oasislabs/oasis-client/blob/afd2e12d3a25b6b50d1997fa3bbc76a3073fe17c/packages/service/src/rpc.ts#L47
does not do any validity checks on fn.name.  So minimally the idl and options properties of Service objects are vulnerable, since those were set prior to the Object.assign.  I could not find via a quick grep of this repo where the idl argument originates and where validation, if any, were done.  (Please reply w/ pointers to use sites.)  In any case, doing Object.assign seems quite dangerous, since it requires an exhaustive enumeration of properties in Service objects that we wouldn't want to get mangled, and it limits the future evolution of Service objects since adding a new property later can cause existing code to malfunction.  Smashing two namespaces together is, in general, a bad idea, even if it saves some typing.

Believed to be addressed by https://github.com/oasislabs/oasis.js/pull/179. Please re-open if there are still concerns.

In addition, an idl check was added here https://github.com/oasislabs/oasis.js/commit/421a0e0f02d785dd93cb880cbe9387c3a0324583


AutoHeight with unknown maxHeight?

Right now autoHeightMin, autoHeightMax take parameter of type number, which means number of pixels. How can I pass something like 'calc(100vh - 120px)' as my max height in scroll container?
Is it possible right now?

Not 100% sure, but it might be as folloing:
autoHeightMin={`calc(100vh - 120px)`}

I can confirm @Dmitry-N-Medvedev's solution works. Would be good to add to docs.

Yeah, it works, but what if I would like to have the height to be 100% of the container?

Setting it ` autoHeightMin={100%} , makes it about 200px high,
setting it  autoHeightMin={calc(100vh)} disables scroll events, because height than, is indeed 100vh, not fitting the container.

same problem here, i'd like it to be 100% height of it's container but nothing that I use works

well i was hopping there was a way to set a real auto height that fit to the container or the first positioned relative parent. No luck so far.

Wow, I found
<Scrollbars style={{ height: '100%' }}></Scrollbars>

does not work.
But
<Scrollbars autoHeight autoHeightMin="100%" autoHeightMax="100%"></Scrollbars>

works!

Also having this issue... unknown maxHeight 🤷♂

Had the opposite effect that @Hongbo-Miao had.
style={{ height: '100%' }}
Worked great for me, where autoHeight, didn't work. Not quite sure why.

Hmm... style={{ height: '100%' }} worked for me as well... I'm not positive, but I think it really depends on how your parent container elements are defined. If your parent element defines it's height according to it's content, then you'll probably need to use autoHeight somehow. On the other hand, if your parent element defines it's height according to it's own space available, then style={{ height: '100%' }} will probably be the way to go.
In my case, I had a flexbox column container with setting auto. This was not working, apparently because of the auto flex-basis, which sets the initial height of the flex container to the size of its content -- the result was a 0 height scrollbar and content. Changing this to flex 1 1 100% (same as auto, but with a 100% initial height flex-basis) fixed the problem.
It's likely this just needs some documentation and some samples to know how to use it properly in various situations.

Good point @rocketraman!
style={{ height: '100%' }} works well to me when I set height: '100%' for the parent container.

i use
<Scrollbars                                     autoHeight                                     autoHeightMin={100}                                     autoHeightMax="calc(80vh - 36px - 35px - 75px)"> content scroll </Scrollbars> it work for me

Hi everyone, I had the same issue.
As my parent has display: flex; I  use <Scrollbars style={{ flex: 1 }}>{children}</Scrollbars>
It works like a charm ;)


EMBED FORM

let's go ahead and embed form on easter page.

oh yea we talked about that huh?
no worries, i’ll continue to link directly to breeze form. good catch adam!

On Mar 13, 2018, at 11:22 AM, Adam Garrett-Harris<EMAIL_ADDRESS>wrote:
My only concern is that it would seem like the page would be more about members than visitors...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/Mountainview-WebDesign/lifestonechurch/issues/264#issuecomment-372747685, or mute the thread https://github.com/notifications/unsubscribe-auth/AXfBraFrT5NmPvAdgk5KicMN4rdnDJTJks5teAA4gaJpZM4SlY-1.

All For Jesus,
Keely Williams
Administrative Assistant
Lifestone Church Riverton
817.598.9567
lifestonechurch.net


Ensure that dttm is of same timezone as epoch_with_tz

When querying columns with timezone on postgres and redshift, psycopg2 returns datetime objects withtzinfo of type psycopg2.tz.FixedOffsetTimezone which is incompatible with pytz.utc. This PR ensures that dttm is always of type pytz.utc. Should have no adverse effects on other engines, as subtraction is only possible if both datetimes are of same timezone. Closes #5910, closes #6284

LGTM

Codecov Report

Merging #6453 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #6453      +/-   ##
==========================================
+ Coverage   73.31%   73.31%   +<.01%
==========================================
  Files          67       67
  Lines        9588     9589       +1
==========================================
+ Hits         7029     7030       +1
  Misses       2559     2559

Impacted Files
Coverage Δ

superset/utils/core.py
88.38% <100%> (+0.02%)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2931baa...c7e60a5. Read the comment docs.


Use time.monotonic instead of time.time

Solves: #42

Solved in https://github.com/xhluca/bm25s/pull/44


move change user info to different component (route: user-edit-profile)

Generate new component
Move code from user-profile to new component
Add new menu "edit profile" with route to page "user-edit-profile"

won't fix