Only first line of figure description extracted if distance between lines deemed too large

After having re-trained the fulltext and figure GROBID models, I noticed the extracted figure description / caption may sometimes only contain the first line despite the (DL) model correctly tagging the full description.
482984v1 (DOI: 10.1101/482984) (from the bioRxiv 10k validation set, not to be added to training)

PDF

target xml
<fig xmlns:mml="http://www.w3.org/1998/Math/MathML" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="fig1" position="float" fig-type="figure">
Figure 1.

<title>Regression analysis of mesophyll conductance (<italic>g</italic>m) with photosynthetic-N use efficiency (PNUE) in four species seedling leaves. The determination coefficient (<italic>R</italic>2) and <italic>P</italic>-value are also shown. The lines fitted separately for four species were significantly different (<italic>P</italic> < 0.05) according to the result of a one-way ANCOVA with PNUE as a dependent variable, tree species as fixed factors, and <italic>g</italic>m as a covariate.</title>

<graphic xlink:href="482984_fig1.tif"/>
</fig>

<fig xmlns:mml="http://www.w3.org/1998/Math/MathML" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="fig2" position="float" fig-type="figure">
Figure 2.

<title>Regression analysis of the fraction of leaf N allocated to (a) Rubisco (<italic>P</italic>R), (b) bioenergetics (<italic>P</italic>B), (c) light-harvesting components (<italic>P</italic>L), and (d) the photosynthetic apparatus (<italic>P</italic>P) with photosynthetic-N use efficiency (PNUE) in four species seedling leaves. The determination coefficient (<italic>R</italic>2) and <italic>P</italic>-value are also shown. Only one line is fitted for four species, because there was no significant difference (<italic>P</italic> > 0.05) according to the result of a one-way ANCOVA with PNUE as a dependent variable, tree species as fixed factors, and <italic>P</italic>R<italic>, P</italic>B<italic>, P</italic>L, or <italic>P</italic>P as a covariate.</title>

<graphic xlink:href="482984_fig2.tif"/>
</fig>

<fig xmlns:mml="http://www.w3.org/1998/Math/MathML" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="fig3" position="float" fig-type="figure">
Figure 3.

<title>Regression analysis of the fraction of leaf N allocated to the cell wall (<italic>P</italic>CW) with leaf N allocated to Rubisco (<italic>P</italic>R) in four species seedling leaves. The determination coefficient (<italic>R</italic>2) and <italic>P</italic>-value are also shown. The shaded zone represents the distribution area of <italic>P</italic>CW and <italic>P</italic>R when a trade-off exists, for more information see Harrison <italic>et al</italic>. [<xref rid="c18" ref-type="bibr">18</xref>]. The lines fitted separately for four species were significantly different (<italic>P</italic> < 0.05) according to the result of a one-way ANCOVA with <italic>P</italic>R as a dependent variable, tree species as fixed factors, and <italic>P</italic>CW as a covariate.</title>

<graphic xlink:href="482984_fig3.tif"/>
</fig>

GROBID TEI XML

  Figure 1.
  Figure 1.
  <figDesc>Figure 1. Regression analysis of mesophyll conductance (g m ) with photosynthetic-N use efficiency (PNUE) in</figDesc>

  Figure 2.
  Figure 2.
  <figDesc>Figure 2. Regression analysis of the fraction of leaf N allocated to (a) Rubisco (P R ), (b) bioenergetics (P B ), (c)</figDesc>

  Figure 3.
  Figure 3.
  <figDesc>Figure 3. Regression analysis of the fraction of leaf N allocated to the cell wall (P CW ) with leaf N allocated to</figDesc>

The DL model actually seem to receive and label more text as the description, e.g.

ScienceBeam DeLFT tagging output
<xml>

    Figure 1 .
    <figDesc>Regression analysis of mesophyll conductance ( g m ) with photosynthetic - N use efficiency ( PNUE ) in four species seedling leaves . The determination coefficient ( R 2 ) and P - value are also shown . The lines fitted separately for four species were significantly different ( P < 0 . 05 ) according to the result of a one - way ANCOVA with PNUE as a dependent variable , tree species as fixed factors , and g m as a covariate .</figDesc>

</xml>

This seem to be down to org.grobid.core.document.Document.getFigureLayoutTokens (https://github.com/kermitt2/grobid/blob/0.6.1/grobid-core/src/main/java/org/grobid/core/document/Document.java#L1489).
In the case of the first extracted figure (see above), the following expression
BoundingBox.fromPointAndDimensions(b.getPageNumber(), b.getX(), b.getY(), b.getWidth(), b.getHeight()).distanceTo(prevBlock) equates to 21.698000.
Perhaps it would be good if there was an option to disable that logic? (in cases where there is enough training data)

Hi Daniel !
Yes this rule was introduced as a "quick" security to avoid frequent errors and it works well in general for standard publication. At the time we were not really considering review papers with extra-large line spacing, which are a low proportion of papers.
This is easy to fix and get it a bit more general, we simply need to use the average/median line space separation of the document instead of the fixed value 15 (it will be much faster actually to add this than such an obscure option that will need to be documented/maintained, which does not make sense imho). It was anyway supposed to be a temporary hack because of the lack of training data and reliability of the full text model. The more medium term objective would be to have this median line spacing value as a feature in the full text model to prevent this situation earlier.

Hi Patrice, thank you for your response.

This is easy to fix and get it a bit more general, we simply need to use the average/median line space separation of the document

It seems like it should be easy. I am slightly hesitant putting all my faith into it being simple given the issues with the character spacing. (e.g. with a mixture of font sizes)
Adding it as a feature to the model seems to be a good way.
Perhaps what I would propose is to:

first add a feature flag to disable that safety net altogether.
Then we could do more tests, whether we would get much value out of adding that additional feature.

I used feature flags in other circumstances, as they are quite light-weight and allow a step-wise transition to a new feature / behaviour, or just a trial. Feature flags do not have a long-term commitment.
In general it would probably be good if most models just received the same data. (e.g. the same feature could be useful to the fulltext model).

Hi Patrice, thank you for your response.

This is easy to fix and get it a bit more general, we simply need to use the average/median line space separation of the document

It seems like it should be easy. I am slightly hesitant putting all my faith into it being simple given the issues with the character spacing. (e.g. with a mixture of font sizes)
Adding it as a feature to the model seems to be a good way.
Perhaps what I would propose is to:

first add a feature flag to disable that safety net altogether.
Then we could do more tests, whether we would get much value out of adding that additional feature.

I used feature flags in other circumstances, as they are quite light-weight and allow a step-wise transition to a new feature / behaviour, or just a trial. Feature flags do not have a long-term commitment.
In general it would probably be good if most models just received the same data. (e.g. the same feature could be useful to the fulltext model).

BTW The similar area of code (assignGraphicObjectsToFigures: https://github.com/kermitt2/grobid/blob/0.6.1/grobid-core/src/main/java/org/grobid/core/document/Document.java#L1153-L1159) seems to also be responsible for, in my case, include the label / header in the description. I haven't yet reproduced that with the default models, I guess there is some code handling it. In my model the label and header are the same (e.g. Figure 1).
There doesn't seem to be the same logic for tables even though they seem to be very similar.

BTW The similar area of code (assignGraphicObjectsToFigures: https://github.com/kermitt2/grobid/blob/0.6.1/grobid-core/src/main/java/org/grobid/core/document/Document.java#L1153-L1159) seems to also be responsible for, in my case, include the label / header in the description. I haven't yet reproduced that with the default models, I guess there is some code handling it. In my model the label and header are the same (e.g. Figure 1).
There doesn't seem to be the same logic for tables even though they seem to be very similar.

Just to get an idea what difference this can make, an evaluation over the bioRxiv 10k validation dataset (first 200 samples):

The last two bars are using the same models. The difference being that for the last bar, the rules relating to updating the figure caption have been disabled (via https://github.com/elifesciences/grobid/pull/32)

Just to get an idea what difference this can make, an evaluation over the bioRxiv 10k validation dataset (first 200 samples):

The last two bars are using the same models. The difference being that for the last bar, the rules relating to updating the figure caption have been disabled (via https://github.com/elifesciences/grobid/pull/32)

I guess one counter example, where rules or an additional model input could help:
475335v1 (DOI: 10.1101/475335) (bioRxiv 10k train example)

PDF

bioRxiv JATS XML
<sec id="s3c">
<title>Epidemic synchrony and annual phase coherence</title>

We explored correlations between dengue time series in different regions. Both epidemic synchrony and phase coherence were higher for closer regions and declined with distance (
<xref rid="fig3" ref-type="fig">Fig 3</xref>
). For the Urban-2 (n = 161) spatial level, epidemic synchrony reached the average countrywide correlation at approximately 1,260 kilometres (
<xref rid="fig3" ref-type="fig">Fig 3A</xref>
). This synchrony length represents a substantial part of Brazil’s dimensions as the country extends 4,395 kilometres north to south and 4,319 kilometres west to east. The coherence length had a higher value of 1,590 kilometres (
<xref rid="fig3" ref-type="fig">Fig 3B</xref>
), suggesting that agreement in dengue seasonality spreads further than correlations of epidemic curves.

<fig id="fig3" position="float" fig-type="figure">
Fig 3.

<title>
Epidemic synchrony and annual phase coherence between Brazilian Urban-2 regions.
</title>

Epidemic synchrony (A) and annual phase coherence (B) summarised using nonparametric spline covariance function. Solid blue line describes the mean pairwise correlation from the data and the dotted lines represent the 95% envelope for bootstrapped correlations of case and annual phase angle time series, respectively. Red line indicates global countrywide correlation.

<graphic xlink:href="475335_fig3.tif"/>
</fig>

We also looked at epidemic synchrony and annual phase coherence at other spatial levels (S4 and S5 Figs, respectively) and found that both synchrony and coherence lengths tend to decrease for smaller spatial resolutions and stabilise at 1,240 km and 1,500 km.

</sec>

GROBID TEI XML (with re-trained models)

  Fig 3.
  Fig 3.
  <figDesc>Epidemic synchrony and annual phase coherence between Brazilian Urban-2 regions. Epidemic synchrony (A) and annual phase coherence (B) summarised using nonparametric spline covariance function. Solid blue line describes the mean pairwise correlation from the data and the dotted lines represent the 95% envelope for bootstrapped correlations of case and annual phase angle time series, respectively. Red line indicates global countrywide correlation. We also looked at epidemic synchrony and annual phase coherence at other spatial levels (S4 and S5 Figs, respectively) and found that both synchrony and coherence lengths tend to decrease for smaller spatial resolutions and stabilise at 1,240 km and 1,500 km.</figDesc>

I guess one counter example, where rules or an additional model input could help:
475335v1 (DOI: 10.1101/475335) (bioRxiv 10k train example)

PDF

bioRxiv JATS XML
<sec id="s3c">
<title>Epidemic synchrony and annual phase coherence</title>

We explored correlations between dengue time series in different regions. Both epidemic synchrony and phase coherence were higher for closer regions and declined with distance (
<xref rid="fig3" ref-type="fig">Fig 3</xref>
). For the Urban-2 (n = 161) spatial level, epidemic synchrony reached the average countrywide correlation at approximately 1,260 kilometres (
<xref rid="fig3" ref-type="fig">Fig 3A</xref>
). This synchrony length represents a substantial part of Brazil’s dimensions as the country extends 4,395 kilometres north to south and 4,319 kilometres west to east. The coherence length had a higher value of 1,590 kilometres (
<xref rid="fig3" ref-type="fig">Fig 3B</xref>
), suggesting that agreement in dengue seasonality spreads further than correlations of epidemic curves.

<fig id="fig3" position="float" fig-type="figure">
Fig 3.

<title>
Epidemic synchrony and annual phase coherence between Brazilian Urban-2 regions.
</title>

Epidemic synchrony (A) and annual phase coherence (B) summarised using nonparametric spline covariance function. Solid blue line describes the mean pairwise correlation from the data and the dotted lines represent the 95% envelope for bootstrapped correlations of case and annual phase angle time series, respectively. Red line indicates global countrywide correlation.

<graphic xlink:href="475335_fig3.tif"/>
</fig>

We also looked at epidemic synchrony and annual phase coherence at other spatial levels (S4 and S5 Figs, respectively) and found that both synchrony and coherence lengths tend to decrease for smaller spatial resolutions and stabilise at 1,240 km and 1,500 km.

</sec>

GROBID TEI XML (with re-trained models)

  Fig 3.
  Fig 3.
  <figDesc>Epidemic synchrony and annual phase coherence between Brazilian Urban-2 regions. Epidemic synchrony (A) and annual phase coherence (B) summarised using nonparametric spline covariance function. Solid blue line describes the mean pairwise correlation from the data and the dotted lines represent the 95% envelope for bootstrapped correlations of case and annual phase angle time series, respectively. Red line indicates global countrywide correlation. We also looked at epidemic synchrony and annual phase coherence at other spatial levels (S4 and S5 Figs, respectively) and found that both synchrony and coherence lengths tend to decrease for smaller spatial resolutions and stabilise at 1,240 km and 1,500 km.</figDesc>

As a complementary information, I removed the logic and ran the end 2 end evaluation. I saw no changes in both evaluations. I've tested on a slightly different data set, though.

As a complementary information, I removed the logic and ran the end 2 end evaluation. I saw no changes in both evaluations. I've tested on a slightly different data set, though.

As a complementary information, I removed the logic and ran the end 2 end evaluation. I saw no changes in both evaluations. I've tested on a slightly different data set, though.

The end-2-end evaluation only covers the figure title extraction, not the figure caption, so no difference is to be expected.
This "security" was introduced from the feedback of ResearchGate on processing very large volume of figure extractions (several millions), this remains not so frequent, but having incorrect extra-long figure caption has a very bad impact on users.
The figure label added in the figure description was also a decision from ResearchGate feedback, making the extraction in general more readable (although I was not found of that personally).

As a complementary information, I removed the logic and ran the end 2 end evaluation. I saw no changes in both evaluations. I've tested on a slightly different data set, though.

The end-2-end evaluation only covers the figure title extraction, not the figure caption, so no difference is to be expected.
This "security" was introduced from the feedback of ResearchGate on processing very large volume of figure extractions (several millions), this remains not so frequent, but having incorrect extra-long figure caption has a very bad impact on users.
The figure label added in the figure description was also a decision from ResearchGate feedback, making the extraction in general more readable (although I was not found of that personally).

The end-2-end evaluation only covers the figure title extraction, not the figure caption, so no difference is to be expected.

oh, I missed this completely. What is the difference between figure title and figure caption?

The end-2-end evaluation only covers the figure title extraction, not the figure caption, so no difference is to be expected.

oh, I missed this completely. What is the difference between figure title and figure caption?

oh, I missed this completely. What is the difference between figure title and figure caption?

There is often a title to the figure (often in bold) and a distinct description blabla (caption), and usually some sort of label/identifier for the cross-reference in the text. This is the general pattern considered in the annotations for the training data.
The difference between figure title and figure caption is generally relatively clear, but not always. For instance below... no real title in this case, except Fig. 1 (is in this case redundant with the label) and some caption, which could easily be interpreted as figure title as well. In the PMC JATS, if I remember correctly, this is not encoded in a consistent manner, so I ignored figure captions in the end-2-end eval.

                Fig 1 .
                1
                <figDesc>

Fig 1. CXR patterns. A) left lower zone consolidation; B) bilateral lower zone interstitial infiltrates; F) scattered alveolar infiltrates in right lung; H) bilateral diffuse consolidation. CXR Severity Scoring (Right + Left): (A) 0+1 = 1; (B) 1+1 = 2; (C) 2+1 = 3; (D) 2+2 = 4; (E) 2+3 = 5; (F) 4+2 = 6; (G) 4+3 = 7; (H) 4 +4 = 8.

                </figDesc>

oh, I missed this completely. What is the difference between figure title and figure caption?

There is often a title to the figure (often in bold) and a distinct description blabla (caption), and usually some sort of label/identifier for the cross-reference in the text. This is the general pattern considered in the annotations for the training data.
The difference between figure title and figure caption is generally relatively clear, but not always. For instance below... no real title in this case, except Fig. 1 (is in this case redundant with the label) and some caption, which could easily be interpreted as figure title as well. In the PMC JATS, if I remember correctly, this is not encoded in a consistent manner, so I ignored figure captions in the end-2-end eval.

                Fig 1 .
                1
                <figDesc>

Fig 1. CXR patterns. A) left lower zone consolidation; B) bilateral lower zone interstitial infiltrates; F) scattered alveolar infiltrates in right lung; H) bilateral diffuse consolidation. CXR Severity Scoring (Right + Left): (A) 0+1 = 1; (B) 1+1 = 2; (C) 2+1 = 3; (D) 2+2 = 4; (E) 2+3 = 5; (F) 4+2 = 6; (G) 4+3 = 7; (H) 4 +4 = 8.

                </figDesc>

For what it's worth, in the bioRxiv dataset there doesn't seem to be a separate title as far as I am aware. I am just evaluating label and caption / description (where I use the full label, e.g. Fig 1).

The figure label added in the figure description was also a decision from ResearchGate feedback, making the extraction in general more readable (although I was not found of that personally).

But since it is a separate field, this seems like a display decision? Perhaps it's to also include the separator, e.g. . or :? Although that seems to be part of the head.
It's not done for tables anyway. (I am not sure why the figures and tables are not just sharing more code).

For what it's worth, in the bioRxiv dataset there doesn't seem to be a separate title as far as I am aware. I am just evaluating label and caption / description (where I use the full label, e.g. Fig 1).

The figure label added in the figure description was also a decision from ResearchGate feedback, making the extraction in general more readable (although I was not found of that personally).

But since it is a separate field, this seems like a display decision? Perhaps it's to also include the separator, e.g. . or :? Although that seems to be part of the head.
It's not done for tables anyway. (I am not sure why the figures and tables are not just sharing more code).

I could look into adding something to the training data...
In general I prefer more flexibility in terms of data being used by the model, closer to the model itself. Or in other words make more raw data available to the model and do the preprocessing then (see #543 and #544). Although that is probably more up for debate and shouldn't be rushed through.
We could for example add a relative line spacing feature. e.g. 2.0 if the line space between the current line and the previous line is twice as big. I would add it as a float. Although the current feature support in DeLFT is geared towards categorical values. So maybe it could be rounded to something like 1.0 or less, 1.5 and 2.0 or more.
I am not sure how well that would work for two column layouts or text across pages.
The model could then still come up with his own answer and ignore that parameter.
Any thoughts?

I could look into adding something to the training data...
In general I prefer more flexibility in terms of data being used by the model, closer to the model itself. Or in other words make more raw data available to the model and do the preprocessing then (see #543 and #544). Although that is probably more up for debate and shouldn't be rushed through.
We could for example add a relative line spacing feature. e.g. 2.0 if the line space between the current line and the previous line is twice as big. I would add it as a float. Although the current feature support in DeLFT is geared towards categorical values. So maybe it could be rounded to something like 1.0 or less, 1.5 and 2.0 or more.
I am not sure how well that would work for two column layouts or text across pages.
The model could then still come up with his own answer and ignore that parameter.
Any thoughts?


bench: go/nogo 1.35-rcLAST Value

Run 2022-06-19-<IP_ADDRESS>.0.6196.k51-5ep-360kTx-4000kU-1000kD-64kbs

Run data discarded -- we'll have a ledger bump in https://github.com/input-output-hk/cardano-node/pull/4063


docker-py breaks with some configurations of Requests v2.12.2

Reported by @graingert.
The patch filed by @tiran in #3713 seems to have broken docker-py. They're using a custom URL scheme (http+docker), which we previously applied URL preparation to but now do not. This has therefore busted what they were up to in v2.12.2.
I'm not immediately sure that we have a good way out of this. Prior to this patch docker-py users were probably at risk of encountering issues because the HTTP requests to the Docker API can be routed across unix domain sockets, which may entirely fail to contain hostnames and also fail to IDNA-encode. So I'm not sure that we don't need docker-py to route around this a different way.

Corresponding docker-py issue is #1321.

What is unclear to me is exactly why this skip is happening. @graingert, if you can run your example under pdb can you set a breakpoint at requests.sessions.Session.request and run print url when it breaks?

@Lukasa can do

@Lukasa it's 'http+docker://localunixsocket/v1.24/images/create' but that gets handled by the mounted docker.transport.unixconn.UnixAdapter

Heh, nevermind, I think I know what this is.
The only difference between the two Requests versions is this. In 2.12.1:
Python 2.7.12 (default, Oct 13 2016, 13:16:42)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.35)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests
>>> r = requests.Request('GET', 'http+docker://localunixsocket/v1.24/images/create')
>>> p = r.prepare()
>>> p.url
'http+docker://localunixsocket/v1.24/images/create'

In 2.12.2
Python 2.7.12 (default, Oct 13 2016, 13:16:42)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.35)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests
>>> r = requests.Request('GET', 'http+docker://localunixsocket/v1.24/images/create')
>>> p = r.prepare()
>>> p.url
u'http+docker://localunixsocket/v1.24/images/create'

Specifically, the issue is that when Requests processes the URL it ends up as a bytestring. When it does not, it ends up as whatever type was passed in: in the case of Python 3, unicode. That then trips something up lower down in the stack (probably in the docker-py adapter) which expects only bytes there.

Yup, the criminal is http.client: the auto-adding of Host headers is only done if url.startswith('http'), which in Python 3 is not true for bytes URLs.

Just as a little background here, #3713 was raised because unix sockets were hitting idna. The bypass (#3695) had addressed (at least from my tests) the socket path issues already. I didn't think it was worth noting because it seemed #3713 was providing some helpful constraint tightening, beyond fixing the immediate idna issue. As far as I can tell, avoiding that tightened bailout point would likely fix this issue.

So, Requests can potentially address this with a feature: we could have a registry of schemes that we should just treat like HTTP (that is, we should assume that all our URL-processing logic can handle). That would allow docker-py to register "http+docker" as something that makes sense.
However, given that docker-py are already lying to us by setting a hostname (hi there localunixsocket, why don't docker-py just use the URL http://localunixsocket and mount their adapter at that full-string?

@Lukasa someone might come along and register the gtld localunixsocket

@graingert That's fine, it won't matter. The adapter is selected based on a longest-prefix match, so the docker-py adapter will still be selected and knows it doesn't have to do a DNS lookup.

Ok, so here's the thing: now that we're not processing the URL everything is off the table. Query parameters for example, are not handled any longer. This also probably breaks some docker-py stuff.

@graingert has pointed out that in situations like this we should probably throw an exception if we were supposed to add any query parameters to the request.

Yup, the criminal is http.client: the auto-adding of Host headers is only done if url.startswith('http'), which in Python 3 is not true for bytes URLs.

It's not the missing Host header, the request still works fine without it. (currently it's set to 'Host: localhost' which doesn't mean anything to the docker process)
The issue is the params processing of the URL.

Yeah, so once again, docker-py can get out of this by changing the way they handle their adapters. However, I'm not sure Requests should be processing schemes it doesn't understand, and our previous escape hatch wasn't great.

@Lukasa should this handling be moved to the adaptor?

Which handling?

No, I don't think so. I think it's right where it is. We can discuss whether there is a sensible feature addition that allows for Requests adapters to register scheme-specific URL processing logic, but that can't be shoved in for a little while and has to clear quite a high bar.

Closing in favour of #3735.


Fix "No roles attached to instance profile"

Description
For some reason, the EC2 instances created by the launch template do not automatically associate with the appropriate IAM Role, even though the name matches correctly. This change fixes this by using the ARN of the created role directly.
Migrations required
NO
Verification
Changing this line and re-applying on Terraform allowed the EC2 instance to associate with the correct IAM Role as expected.

Seems there was an issue on my AWS configuration where the instance profile already existed with the same name, but didn't get linked to the IAM Role. I had to manually remove the instance profile (using the AWS CLI) and the IAM role and recreate everything. So this change can be ignored, sorry to bother you!

I ran into this after rebuilding a setup. If this fixes the issue with no regressions, it seems worth merging.


Add support for dryRun parameter

What type of PR is this?
/kind feature

What this PR does / why we need it:
This adds the ability to make dry run requests with the dynamic client.
Which issue(s) this PR fixes:

Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?

Support for the dryRun parameter has been added to the dynamic client.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/assign @fabianvf
Thanks!

/check-cla

I signed it

/approve


how to integrate  agrpc::GrpcContext  with existing asio::io_context

Thanks for a wonderful library, I was in the process of migration some code to agrpc.
how do I hookup existing boost::asio::io_context  with GrpcContext ??
some thing like "boost::asio::posix::stream_descriptor"
The api is not clear to me.
Please advise.

I have now tried to summary the possible approaches in this documentation article: https://tradias.github.io/asio-grpc/md_doc_using_asio_io_context.html
I hope that helps, let me know if you have further questions.

Thanks for the link.


Support Iceberg table and column statistics

This commit adds support for Iceberg table and
column statistics.  The commit includes tests
for constraints by TupleDomain as well as
Predicate.
Column statistics include null fractions and
min and max column values.  Column size statistics
are available for Parquet files, but are not yet
generated by the ORC column writers.
Rebased on tip master 2020/07/19.

From a code organization perspective, I dislike putting all the new code into PartitionTable, since it's unrelated to the job of the partition system table. It would be better to

As a first refactor commit, make Partition a top level class
Create a separate class for the new code in PartitionTable

This will also simplify PartitionTable.

I made these changed in the commits just now force-pushed.

We should fork and simplify this code, since the Hive code will return return the wrong result for strings equaling HIVE_DEFAULT_PARTITION, plus the conversion logic is more lax than we need for Iceberg. For example, the Hive code handles multiple representations for boolean values, etc.

You've mentioned this twice before, and I spaced on doing.  Doh!
Done in the latest force-push.  The new method is just:
    private NullableValue makeNullableValue(io.prestosql.spi.type.Type type, Object value)
    {
        return value == null ? NullableValue.asNull(type) : NullableValue.of(type, value);
    }

Merged with master eliminating merge conflicts.


Add native pdfium text extract/search support in Pdfiumcore

Why not adding native pdfium text extract /search support for using with AndroidPdfViewer Beta 3.2.0? Please do it. Thank you.

pdfium should have this function, the problem is to implement it for android.  I remember the old app "google pdf viewer" which however was based on pdfium and had this function
I did some research and found some libraries that have implemented the search, but I don't know if they work, they should be tested.
It would take someone to test them and maybe fork and keep an updated fork.
Test them and let me know if they work in any way
https://github.com/gulshan98185/AndroidPdfViewer
https://github.com/beeline09/AdvancedPdfView
https://github.com/beeline09/android-support-pdfium
https://github.com/benjinus/android-support-pdfium


feat: add classes for disabling and activating user highlight selection

This will;
add classes for disabling and activating user highlight selection

As show in screenshot; there is universal functionality for highlighting and selecting content. In some cases this is wanted behavior, and in other cases not.
I added two classes which can be utilized in components for activating and deactivating this behavior.

Coverage remained the same at 84.949% when pulling 4c885d12ca1a20e83ce91712a00ab0bce5215d7a on johann1301h:selection into 333251fe5974b102e21f4d2f433b840c154a5d30 on SpareBank1:develop.


Spaceship: support getting and setting pricing of recurring subscriptions.

Checklist

[x] I've run bundle exec rspec from the root directory to see all new and existing tests pass
[x] I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
[x] I've read the Contribution Guidelines
[x] I've updated the documentation if necessary.

Motivation and Context
Since Apple introduced the introductory prices, the API for getting and setting the pricing for auto-renewable has changed. In order to set or get the pricing, a request to a new URL is needed.
This PR solves this issue by adding functionality to send requests to the new URL.
The changes were tested by creating new auto-renewable subscription using Spaceship, and getting the pricing of existing auto-renewable subscription. In addition, I verified that creating a consumable products still works as before.
Solves #11189.
Description

tunes_client was added two methods: one for setting and another for getting auto-renewable subscriptions pricing.
The API of iap, iap_detail and iap_list hasn't changed. If there is a need to set or get the pricing information of an auto-renewable subscription, these classes perform an additional call to the methods added to tunes_client. That way the user of these classes will not have to take extra steps to get or set the pricing, as it was before Apple changed the API.

@untitaker @luongm thanks for the comments! After we finish resolving the raw_pricing_data issue I'll update the PR.
@luongm I think that handling intro prices should be in another PR.

I pushed a fix with the review fixes.

This looks pretty good to me. What do you think @untitaker @luongm?

No, you just wait :)
On Sat, Feb 17, 2018 at 01:50:11AM -0800, Minh Luong wrote:

luongm approved this pull request.
LGTM
is there a way to escalate this PR to one of the people with write access?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/fastlane/fastlane/pull/11627#pullrequestreview-97370288


/github.com/azure-samples/azure-sdk-for-go-samples/quickstarts/deploy-vm$ go run main.go

failed to create group: resources.GroupsClient#CreateOrUpdate: Failure responding to request: StatusCode=404 -- Original Error: autorest/azure: Service returned an error. Status=404 Code="SubscriptionNotFound" Message="The subscription '47000f32-1714-4b72-b022-aa1b21cfa03f' could not be found."

Make sure to check your subscription ID. You can find this in the Portal or using the Azure CLI by running:
az login

or
az account list

thank you I have changed azure.PublicCloud.ResourceManagerEndpoint to azure.ChinaCloud.ResourceManagerEndpoint


github pages actions fails to build

Do you want to request a feature or report a bug?
This is a bug. The github pages fails to build because some wrong symlink. In the action log we get:
github-pages 225 | Error:  No such file or directory @ rb_check_realpath_internal - /github/workspace/three.js/examples/multi-markers/threex-armultimarkerlearning.js
To fix this we need to change those symlinks.

What is the current behavior?
see the action log: https://github.com/AR-js-org/AR.js/runs/5495431975?check_suite_focus=true
If the current behavior is a bug, please provide the steps to reproduce.
It fails every time we push in the master branch.

Please mention other relevant information such as the browser version, Operating System and Device Name
What is the expected behavior?
It should build the gh-page.
If this is a feature request, what is motivation or use case for changing the behavior?

This is fixed. Now all the examples should be online. Test the three.js basic example:
https://ar-js-org.github.io/AR.js/three.js/examples/basic.html

There are other improvements to do but are not related to this issue: example some example fails to load resources, i will try to fix them in a next release.


TagBot trigger issue

This issue is used to trigger TagBot; feel free to unsubscribe.
If you haven't already, you should update your TagBot.yml to include issue comment triggers.
Please see this post on Discourse for instructions and more details.
If you'd like for me to do this for you, comment TagBot fix on this issue.
I'll open a PR within a few hours, please be patient!

Triggering TagBot for merged registry pull request: https://github.com/JuliaRegistries/General/pull/49044

Triggering TagBot for merged registry pull request: https://github.com/JuliaRegistries/General/pull/49674


host block devices can be seen inside the container

System Info
I have this issue in archlinux and ubuntu.

Distribution: ArchLinux/Ubuntu
Distribution version: - / 16.04
lxc info:

Archlinux
apiextensions:

storage_zfs_remove_snapshots
container_host_shutdown_timeout
container_syscall_filtering
auth_pki
container_last_used_at
etag
patch
usb_devices
https_allowed_credentials
image_compression_algorithm
directory_manipulation
container_cpu_time
storage_zfs_use_refquota
storage_lvm_mount_options
network
profile_usedby
container_push
container_exec_recording
certificate_update
container_exec_signal_handling
gpu_devices
container_image_properties
migration_progress
id_map
network_firewall_filtering
network_routes
apistatus: stable
apiversion: "1.0"
auth: trusted
environment:
addresses:

<IP_ADDRESS>:8443
<IP_ADDRESS>:8443
<IP_ADDRESS>:8443
architectures:
x86_64
i686
certificate: |
-----BEGIN CERTIFICATE-----
-----END CERTIFICATE-----
certificatefingerprint: d8c745cfede9ded4befe182ed924757761d464f20861576dd1e8843a5dc03e6f
driver: lxc
driverversion: 2.0.6
kernel: Linux
kernelarchitecture: x86_64
kernelversion: 4.8.13-1-ARCH
server: lxd
serverpid: 916
serverversion: "2.7"
storage: btrfs
storageversion: "4.9"
config:
core.https_address: '[::]:8443'
core.https_allowed_headers: Content-Type
core.https_allowed_methods: GET, POST, PUT, DELETE, OPTIONS
core.https_allowed_origin: '*'
public: false

Ubuntu
apiextensions:

id_map
apistatus: stable
apiversion: "1.0"
auth: trusted
environment:
addresses:

<IP_ADDRESS>:8443
<IP_ADDRESS>:8443
<IP_ADDRESS>:8443
<IP_ADDRESS>:8443
architectures:
x86_64
i686
certificate: |
-----BEGIN CERTIFICATE-----
-----END CERTIFICATE-----
certificatefingerprint: e5af923e8761cee98e815442ae22ba4c47dd80c0caa66761a3e6b4f22d20d371
driver: lxc
driverversion: 2.0.6
kernel: Linux
kernelarchitecture: x86_64
kernelversion: 4.4.0-59-generic
server: lxd
serverpid: 2236
serverversion: 2.0.8
storage: btrfs
storageversion: "4.4"
config:
core.https_address: <IP_ADDRESS>:8443
core.trust_password: true
public: false

Description
If you execute the lsblk command, which displays block devices, inside the container the hosts block devices can be seen. This is no problem for the container execution, /dev/  directory doesn't have these devices but I think the container is not supposed to be able to see this info.
lsblk
NAME   MAJ:MIN RM   SIZE RO TYPE MOUNTPOINT
sda      8:0    0 894.3G  0 disk
|-sda4   8:4    0   7.3G  0 part [SWAP]
|-sda2   8:2    0  18.6G  0 part
|-sda3   8:3    0 867.8G  0 part /dev/.lxd-mounts
`-sda1   8:1    0   512M  0 part

That's unfortunately how the kernel works. /sys isn't namespaced so any information available in it will be seen by all containers. They won't be able to do anything with it as the kernel does enforce permissions though.
We've been discussing a fuse overlay for /sys as part of lxcfs, but doing so would be very complex and come with a pretty high performance impact too.
Anyway, closing this LXD issue as it's not something LXD can do anything about. This is either a kernel issue (as part of a new device namespace, which has so far failed to gain any traction) or something that can possibly be papered over by lxcfs in userspace.


Upgrade to DropWizard 5.0.0-alpha. Also, minor imports cleanup

The fun continues as DropWizard 5.0.0-alpha has dropped!  I've incorporated these changes and tested them accordingly, and things are looking pretty great!  I really appreciate this awesome project.

And.. oh, if you are ever interested in seeing where I am using this, see here! https://www.youtube.com/watch?v=Mi7kCxV_ZaU

@TomCools any chance of this getting merged and released?


feat!: new @scalar/openapi-types package

We modified and extended the openapi-types package for our use case. We’re basically allowing any user input, but still maintaining the types based on the OpenAPI specification.
With this PR, we’re exposing those types as a separate, dependency-free package called @scalar/openapi-types. Even comes with basic type tests. :)
Note: This is a breaking change. Instead of importing types from @scalar/openapi-parser, users now have to import from @scalar/openapi-types.
Usage
npm add @scalar/openapi-types

And use it like this:
import type { OpenAPI } from '@scalar/openapi-types'

const file: OpenAPI.Document = {
  openapi: '3.1.0',
  info: {
    title: 'Hello World',
    version: '1.0.0',
  },
  paths: {},
}

hey @hanspagel we are going to move the types into the monorepo so I'll copy them over, then there should be no breaking changes here. We can also just re-export them after for a single source of truth


chore(master): release 1.1.48

:robot: I have created a release beep boop
1.1.48 (2023-08-29)
Bug Fixes

deps: update dependency wayofdev/laravel-cycle-orm-adapter to v4.0.78 (c570400)

This PR was generated with Release Please. See documentation.

:robot: Release is at https://github.com/wayofdev/laravel-webhook-client/releases/tag/v1.1.48 :sunflower:


How webpack to resolve dataSources in graphpack.config.js

How webpack to buildle './src/services/client' ?
const Client = require('./src/services/client')
module.exports = {
    ...
    dataSources: () => ({
	client: new Client(),
   }),
}

Hey! What are you trying to achieve by this?


Access Token is NULL for Firebase Messaging

I setuped WEB Firebace Cloud Messaging and collected tokens from clients.
I try to send first message by this instruction https://firebase.google.com/docs/cloud-messaging/js/first-message like this:
POST https://fcm.googleapis.com/v1/projects/myproject-b5ae1/messages:send HTTP/1.1

Content-Type: application/json
Authorization: Bearer ya29.ElqKBGN2Ri_Uz...HnS_uNreA

{
  "message":{
    "token" : "bk3RNwTe3H0:CI2k_HHwgIpoDKCIZvvDMExUdFQ3P1...",
    "notification" : {
      "body" : "This is an FCM notification message!",
      "title" : "FCM Message",
      }
   }
}

But I need Bearer

Content-type: application/json
Authorization: Bearer <valid Oauth 2.0 token for the service account of the Firebase project>

I need access_token to do it.

I found exampe on nodeJS, how to get access_token
I try to convert this nodeJS code to PHP (https://firebase.google.com/docs/cloud-messaging/auth-server#authorize_http_v1_send_requests)
function getAccessToken() {
  return new Promise(function(resolve, reject) {
    var key = require('./service-account.json');
    var jwtClient = new google.auth.JWT(
      key.client_email,
      null,
      key.private_key,
      SCOPES,
      null
    );
    jwtClient.authorize(function(err, tokens) {
      if (err) {
        reject(err);
        return;
      }
      resolve(tokens.access_token);
    });
  });
}

My code:
$SCOPE = "https://www.googleapis.com/auth/firebase.messaging";

$client = new Google_Client();
$client->setApplicationName('Play Cool Math Notifications');
$client->setAuthConfig("token/play-cool-math-notifications-firebase-adminsdk.json");
$client->setScopes($SCOPE);
$client->authorize();
$token = $client->getAccessToken();
echo $token;

But I got NULL, what did I do wrong?

getting same issue……

I found way to send w/o Access Token
private function sendNotification($_token, $_title, $_body, $_link)
    {
        $SERVER_KEY = "AAAA....";

        $data = array(
            "to" => $_token,
            //"content_available"   => false,
            "notification" => array
            (
                "title"         => $_title,
                "body"          => $_body,
                "icon"          => "https://...../icons/android-icon-192x192.png",
                "click_action"  => $_link."?".(new DateTime())->getTimestamp()
            )
        );
        $data_string = json_encode($data);

        $ch = curl_init('https://fcm.googleapis.com/fcm/send');

        curl_setopt($ch, CURLOPT_CUSTOMREQUEST,     "POST");
        curl_setopt($ch, CURLOPT_POSTFIELDS,        $data_string);
        curl_setopt($ch, CURLOPT_RETURNTRANSFER,    true);
        curl_setopt($ch, CURLOPT_HTTPHEADER,
            array(
                'Content-Type: application/json',
                'Authorization: key='.$SERVER_KEY
            ))  ;
        $result = curl_exec($ch);
        var_dump($result);
        curl_close ($ch);

        return $result;
    }

Thanks for creating this issue.

See this: https://gist.github.com/Repox/64ac4b3582f8ac42a6a1b41667db7440


feat: update readme for v 5.0.0

Your development orb has been published. It will expire in 30 days.
You can preview what this will look like on the CircleCI Orb Registry at the following link:
https://circleci.com/developer/orbs/orb/circleci/windows?version=dev:2132d43ce2a7174ddbd66e6d5c7c840a1109d68e

Your development orb has been published. It will expire in 30 days.
You can preview what this will look like on the CircleCI Orb Registry at the following link:
https://circleci.com/developer/orbs/orb/circleci/windows?version=dev:2132d43ce2a7174ddbd66e6d5c7c840a1109d68e

Your orb has been published to the CircleCI Orb Registry.
You can view your published orb on the CircleCI Orb Registry at the following link:
https://circleci.com/developer/orbs/orb/circleci/windows?version=5.0.0


Kill LRO total timeout.

Florence reported a bug in Node where the LRO times out with a sufficiently large video.
After investigating, it turns out that this is because we have a hard-coded 10 minute total timeout. This is reasonable for most languages where this is a poll in the background that you can easily restart, but in Node, the operation provides op.promise() which is a one-and-done. Once the Promise is rejected, that is it. This makes even a very long timeout problematic.
While eventually we need to approach these settings differently as a whole, for the moment, this PR simply makes the timeout be null so that we can unblock video intelligence.
Assigned to @geigerj or @swcloud for review. Once either of you has approved, please feel free to merge and push to npm.

Codecov Report

Merging #140 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
+ Coverage   92.72%   92.73%   +0.01%
==========================================
  Files          11       11
  Lines        1305     1307       +2
  Branches      269      270       +1
==========================================
+ Hits         1210     1212       +2
  Misses         95       95

Impacted Files
Coverage Δ

lib/longrunning.js
95.33% <100%> (+0.06%)
: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 002e240...3624476. Read the comment docs.

LGTM. Will leave it to @swcloud to merge and push to npm.


[FEAT] support groq.com

Is your feature request related to a problem? / 你想要的功能和什么问题相关？
Most models are still too slow.
Describe the solution you'd like. / 你想要的解决方案是什么？
Groq can serve 500 TPS
Describe alternatives you've considered. / 你考虑过的其他方案是什么？
No response
Additional context / 其他信息
No response

I've prepared a PR and will send it once #791 is merged.

PR sent at #794, FYR.


URL

According to google docs, the HTTP URL should be 'https://gcm-http.googleapis.com/gcm/send'

Indeed, according to https://developers.google.com/cloud-messaging/http the Google Cloud Messaging (GCM) HTTP connection server should be https://gcm-http.googleapis.com/gcm/send. I'll make a pull request regarding this issue.

Thanks Alex


feat(lib): support usage as a library

Use generator-node-mdl to bring many features
Allow importing and using as a library
Provide bin to run the server
Adds docs

: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:


Injected Privly apps more like Github

If I use Privly inside Github and use a coding reference, it doesn't show.
For eg:
If I type <title>Privly · <i class="fa fa-github"></i> Open Source</title>
Privly renders it without ``
@smcgregor details the following problem:
The screen capture below shows the HTML for the injected content above. It is wrapping the code in the code block, but since the injected iframe does not inherit the stylesheets of GitHub it does not display code blocks the same as GitHub.

Stylesheet needs to be updated.

I would say this is more something that could look better than an actual bug. The current design choice was made because we want the injected content to be as seamless with the surrounding content as possible. GitHub is only one site the content is displayed within, so saying it should change means you are saying that there is a way for it to look better on all websites-- not just GitHub.

@smcgregor This could extend the User Experience project in a minute way as far as I can tell. To increase more use-cases for Privly in that context.

This can even be extended to emails. Right now every field requires to input info in Privly. It should be one application to send all data. Data indifference could be applied here I guess, since all email apps require to:, subject:., body:.


No gradient defined for operation 'SparseSlice'

i am working on solve a indeterminate equation optimization with sparse_slice functions and met this error.
it's strange all sparse functions have gradient except this one. any suggestion how can i avoid this problem?
Source code / logs
import tensorflow as tf
sparse_values = tf.identity(tf.Variable(tf.constant([1., 2., 3.])))
sparse_indices = tf.constant([[0, 0], [1, 1], [2, 2]], dtype=tf.int64)
sparse_matrix = tf.SparseTensor(sparse_indices, sparse_values, [3, 3])
sparse_slice = tf.sparse_slice(sparse_matrix,[0,0], [2,2])
multiplied = tf.sparse_tensor_dense_matmul(sparse_slice, tf.eye(2))
loss = tf.reduce_sum(multiplied)
gradients = tf.gradients(loss, [sparse_values])
with tf.Session() as session:
tf.global_variables_initializer().run()
print(session.run(gradients))

I'm also running into this issue with a different use case. No workaround I'm afraid but if possible to solve I'd much appreciate it!

/CC @ebrevdo any plans for a sparse slice gradient? Mark as contributions welcome if not.

Hi, I write a gradient op for SparseSlice in #19663. Could you take a look?

Nagging Assignee @ebrevdo: It has been 15 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.


Genesis block

Store all the data required to instantiate the genesis block (eg. signature must not be calculated at runtime)

Done


Dragon Auras are not computed correctly

While working on #305 , I noticed a strange behaviour on Chocolate Egg Cookies value, leading to incorrect simulated value.
This is most likely caused by the "Reality bending" Krumblor aura not being handled properly while simulating building selling in CM.Sim.BuildingSell.
I think I'll be able to look a bit further into this issue in the next days.

Test save for import (click to expand)
[encoded data, 5007 characters]%3D%21END%21

Context + how to reproduce
To keep the testing as simple as possible, this save doesn't have wrinklers / stock market goods.
The 2 initial auras are not relevant for chocolate egg computation (Dragon Cursor and Breath of Milk).
Here is a simple summary of the chocolate egg simulated value upon aura switch :

Action
Earth Shatterer
Reality Bending
Sim chocolate egg
Real chocolate egg

Initial
No
No
7.693 Sexdec
7.793 Sexdec

Switch 1 aura to Earth Shatterer from Initial
Yes
No
7.693 Sexdec
7.793 Sexdec

Switch 1 aura to Reality Bending from Initial
No
Yes
7.689 Sexdec
7.793 Sexdec

Switch both auras to Earth Shatterer + Reality Bending from Initial
Yes
Yes
7.793 Sexdec
7.793 Sexdec

The last simulation is the only one correct.
Both initial and switch to ES have the same computed value, whereas switch to RB has a lower value (probably because of the building sold to switch auras).

I believe this is similar to the problem @Aktanusa mentions in #266.
Perhaps a general review of the calculation of aura's is needed. I have not looked at that part of the code much since assuming maintainer-ship, so I am not quite sure about the state of it. However, as @Aktanusa mentioned elsewhere, we need to tread carefully here, as it is very easy to make crucial mistakes within the Sim code.

You're right, this is indeed caused by aura calculations within the CM.Sim.BuildingSell method.
I was able to identify 2 issues in it:

The sell building multiplier (giveBack) can be emulated for Earth Shatterer aura only, not for Reality Bending or ES + RB auras;
The Game.modifyBuildingPrice called to calculate the individual selling price of a building uses the "Fierce Hoarder" aura multiplier in its calculations, which is wrong unless the player's current auras are Reality Bending + anything but Fierce Hoarder.
The first issue is quite simple to solve, however the second one is way more tricky. I've been finally able to have the correct choco egg value by hardcoding values to bypass the Fierce Hoarder aura problem in my test case, but I'll need to work a bit more on it to find a generic and elegant way to have these calculations right in any case.

As per the Sim code, I am well aware of how critical it is to the whole plugin.
You (and @Aktanusa ) may be reassured to know that I took the time to fully understand the CM.Sim.BuildingSell method before making any changes (mostly by comparing it with the game's Game.Object.sell method on which this sim is based) 😉
I'll keep on working on this issue tomorrow!

It has been a bit more complicated than I thought, but it should be OK now.
However, you might want to rename the issue as it doesn't solve any other issue that could be related to dragon auras, sadly.

I will take a look a your fix in a minute and then use it check all other auras to fully squash this issue.
Thanks for your hard work!

I have gone through CM.Sim.CalculateGains and all functions and variables it calls.
I have not found any other mistakes other than the ones fixed in #361, and are therefore closing this issue.

Thanks for the hard work both of you!


Added Valorant VCT Masters Tokyo Player Performance data (Playoffs)

Player data from VCT Masters Tokyo Playoffs scraped from VLR.gg. Closes #117

closes #117

@MercMayhem Thanks for your contribution. Make sure to give a 🌟  to the repo


Recall

Before submitting

[x] Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
[x] Did you read the contributor guideline, Pull Request section?
[x] Did you make sure to update the docs?
[x] Did you write any new necessary tests?

What does this PR do?
Fixes #106 .
Added classification and functional for specificity. Also added tests
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃

Hello @arvindmuralie77! Thanks for opening this PR.

In the file tests/classification/test_specificity.py:

Line 88:121: E501 line too long (122 > 120 characters)
Line 91:11: E222 multiple spaces after operator
Line 122:121: E501 line too long (132 > 120 characters)

Do see the Hitchhiker's guide to code style

Can we make the testing simpler by flipping the labels and predictions and calculating regular recall?


Feature/375 plansummarylink

Related Issues:

EQ-375

Main Changes:

Added the new planSummaryLink column (for actions and tmdl profiles) to the ETL and export.

Steps To Test:

Look over the code changes and verify everything looks good. This column isn't available yet, so we can't do any real testing.

TODO:

[ ] Hold off on merging this in until after the new column is available in the S3 bucket.
[x] Manually add the new column to the production S3 bucket (i.e., update tableConfig.json in S3 bucket and restage the ETL). This part can be done before the ETL is scheduled to run.
[ ] After the production ETL successfully runs, manually update the profiles.json and api-public.json files in the S3 bucket and restage the app.

Good catch! I'll add that in later today.


KAFKA-7385: Fix log cleaner behavior when empty batches are retained

@dhruvilshah3 can you please add a description to the PR explaining the fix?


ability to install inspec as a package

As noted by @stephenlauck, there are times (at enterprise or other) when accessing a gem is not an option. We should support installation via package by means of an attribute that decides whether we need to run the inspec recipe or an alternative recipe, e.g. inspec_package

InSpec is part of Chef Client 13, therefore an additional package is not required anymore.


Double-Precision Floating Point Currency?

For real? (narf!) Given the very many well-known and widely reported, monumental issues with floating-point math and precision vis a vis money calculations, why aren't the currency-related fields in this project declared as type Decimal?
Using Double (or Single) instead seems to just be BEGGING for inaccurate calculations to crop up for, as far as I can tell, no benefit whatsoever.  What's the thinking behind this?

 I'm going to apply formatting to hopefully constrain the horror show, but I cannot help but expect that I'm going to have erroneous calculations eventually.
Looking at this StackOverflow thread, the commenters state repeatedly that you should never use floating point types for currency...
http://stackoverflow.com/questions/890100/how-do-i-format-a-double-to-currency-rounded-to-the-nearst-dollar


Print a notice when compiling with target=debug

Debug builds are considerably slower than release builds or even release_debug builds. target=debug is still the default SCons target option, so unsuspecting users may be compiling unoptimized debug builds for their personal use.

I don't think it reaches the place of the warnings if you are changing it before to release_debug.

@santouits The default target is debug, not release_debug. I don't think we should warn people if building a release_debug export template, since it still has some optimizations enabled.

Also, maybe we should alias target=release to target=release_debug when building the editor. Instead of failing like it currently does, the build would carry on after printing a warning message. What do you think?

I agree. target=release is never going to work with the editor because it's in the nature of the editor to rely on debug error checking. Although probably print a message notifying of the "fallback" (not a warning).

I don't think we should warn people when building a release_debug export template, since it still has some optimizations enabled.

What optimizations are lacking compared with release? I thought it was pretty much the same as release in optimizations but with DEBUG_ENABLED defined for extra checks in code.

@neikeq There, I made tools=yes target=release redirect to target=release_debug with a warning message.

Nevermind, I get many build errors due to modules not getting the corrected target value:
modules/gdscript/gdscript_editor.cpp: In member function 'virtual Error GDScriptLanguage::lookup_code(const String&, const String&, const String&, Object*, ScriptLanguage::LookupResult&)':
modules/gdscript/gdscript_editor.cpp:3282:2: error: 'GDScriptCompletionContext' was not declared in this scope
 3282 |  GDScriptCompletionContext context;
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~
modules/gdscript/gdscript_editor.cpp:3283:2: error: 'context' was not declared in this scope
 3283 |  context._class = parser.get_completion_class();
      |  ^~~~~~~
modules/gdscript/gdscript_editor.cpp:3449:4: error: 'GDScriptCompletionIdentifier' was not declared in this scope
 3449 |    GDScriptCompletionIdentifier base;
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
modules/gdscript/gdscript_editor.cpp:3450:112: error: 'base' was not declared in this scope
 3450 |    if (!_guess_expression_type(context, static_cast<const GDScriptParser::OperatorNode *>(node)->arguments[0], base)) {
      |                                                                                ^~~~
modules/gdscript/gdscript_editor.cpp:3450:9: error: '_guess_expression_type' was not declared in this scope
 3450 |    if (!_guess_expression_type(context, static_cast<const GDScriptParser::OperatorNode *>(node)->arguments[0], base)) {
      |         ^~~~~~~~~~~~~~~~~~~~~~
modules/gdscript/gdscript_editor.cpp:3454:33: error: 'base' was not declared in this scope
 3454 |    if (_lookup_symbol_from_base(base.type, p_symbol, is_function, r_result) == OK) {
      |

It's not modules. It's the platform's detect. Those errors are because DEBUG_METHOD_ENABLED is not being defined, which only happens if the platform's detect defines DEBUG_ENABLED.
Modules are already configured after the line which contains your changes. detect.configure on the other hand is called some lines before.

Don't know about mono, but I have build an editor with -O3(release) and not debug symbols and it seemed it worked fine. Of course I enabled the DEBUG_ENABLED so it could be built.
I'm not sure -O3(release) is faster than -O2(release_debug) to have both as an option. I would just leave -O2 and -Os and if anyone wants -O3 they could add it by the CFLAGS. I should make the scons files cleaner.

Now that production=yes has been added, I changed this PR to only print a notice when using target=debug.

Thanks!

There, I made tools=yes target=release redirect to target=release_debug with a warning message.

This could be an option for godotengine/godot-proposals#1632, but of course this would entail a lot of work.


Can not order by DESC??

when i use to select with “$orderBy”:"id desc"
it become to id desc!!!

Please browse the docs in den /sql directory. Everything is well documented with test, examples for wach case.
Try $orderBy: { columname: false | true | 1 | -1 }
In the docs you will find additional Syntax for the use of NULLS FIRST option etc.


Allow reaching the home page by clicking the title (currently reaches a broken state)

When opening a direct link (here: https://web3isgoinggreat.com/?id=2022-02-09-2 from February), the timeline starts at the specified card.
If the user wants to go to the top of the timeline, they have the option of clicking "Start from the top", but this button can be easily missed. What the user may try first instead is clicking on the website title, which is almost universally a shortcut to a website's home page.
What currently happens when clicking the title is that

the URL changes to the bare domain (:heavy_check_mark: good)
the timeline remains where it was (:confused: unexpected), but with a difference:
now the ability to "start from the top" has also disappeared (:lady_beetle: bug?)

This way, if the user had followed the common internet convention of clicking the header, they would have lost access to the top of the timeline, and they would have been left with no option but to refresh (or trigger a refetch by sorting Ascending then Descending again).

Thanks for the bug report! This should be all fixed now.

Thank you for the fix! Works great (on mobile at least)


Enable preemption of jobs in a queue

Preemption can be done on a Store object if an item of higher priority arrives. The amount of time the preempted object was served is stored.
This change adds a method to put that requires that the item have the following fields:

:priority::Int: Integer specifying the priority of that item (the more
negative, the higher the priority).
:process::Process: The storage process
of the item. This is used to know which Process to
interrupt when preempting.

This addresses #61
See example below:
using ResumableFunctions, SimJulia

#Define Job (item) to send to the queue (Store)
abstract type AbstractJob end
mutable struct Job <: AbstractJob
    id::Int
    arrival::Number
    priority::Int
    start_service::Number
    time_in_service::Number
    process::Union{Missing, SimJulia.Process}
    size::Number
end

#define a function to serve jobs that arrive at a queue and allow preemption
@resumable function serve(env, server, job)
    @yield timeout(env, job.arrival) #job arrives
    println("job $(job.id) enters queue at t = $(now(env))")
    while true
        try
            @yield put(server, job, true) #add job to queue, allow preemption
            println("Starting service on job $(job.id) at t = $(now(env)).")
            job.start_service = now(env) #save time service started on job
            @yield timeout(env, job.size - job.time_in_service) #service duration excludes any time already spent in service (if preempted)
            println("Service complete on job $(job.id) at t = $(now(env)).")
            job.time_in_service = now(env) - job.start_service #update time in serice
            get(server, x -> x.id == job.id) #remove job from queue
            break
        catch exc
            pre = exc.cause #find cause of preemption
            job.time_in_service = now(env) - job.start_service #update time in service
            println("Service on job $(job.id) interrupted (preempted) by job $(pre.id) at t = $(now(env)).")
        end
    end
end

#create simulation
sim = Simulation() #simulation object
server = Store{Job}(sim, capacity = UInt(1)) #server that processes jobs
job1 = Job(1,0,-1,0,0,missing,3) #add 3 jobs
job2 = Job(2,1,0,0,0,missing,3)
job3 = Job(3,2,-2,0,0,missing,3)

#create and store service processes
job1.process = @process serve(sim, server, job1)
job2.process = @process serve(sim, server, job2)
job3.process = @process serve(sim, server, job3)

#run simulation
run(sim)

#output
#=
job 1 enters queue at t = 0.0
Starting service on job 1 at t = 0.0.
job 2 enters queue at t = 1.0
job 3 enters queue at t = 2.0
Service on job 1 interrupted (preempted) by job 3 at t = 2.0.
Starting service on job 3 at t = 2.0.
Service complete on job 3 at t = 5.0.
Starting service on job 1 at t = 5.0.
Service complete on job 1 at t = 6.0.
Starting service on job 2 at t = 6.0.
Service complete on job 2 at t = 9.0.
=#

@hdavid16 , should we consider reopening this pull request (or creating a new one if that is easier)?


cc.php: Added second missing time ./a.out

Follow-up of #21.

Additionally, I've noticed that the sample code uses the filename opt_main.c, but the command lines were using just plain main.c. For consistency, I've fixed all of them to opt_main.c.
There are still a few cases using hello.c and hello.o, and those cases don't seem to make sense to me. I'll leave them to you to fix, or possibly me in a later date.

Please have only one commit in a PR. Having several mixes issues and it harder to review. Look at this message which I need to itemize.
1/ While I agree commit #1 was necessary, I think this was done in 47fe001ce3274e26fe13226265beef3512fccdef.
2/ commit #2 is not something we discussed.  I don't understand what is the consistency you mention. Please open another issue if you want to discuss this further

commit 2 is not something we discussed. I don't understand what is the consistency you mention. Please open another issue if you want to discuss this further

Fair enough. I should have broken it up in multiple PRs. Commit https://github.com/denilsonsa/dc/commit/d9b28fe3239d0b31ec8bb48152fc37558b253770 was trying to address a new detail I had just noticed: around line 989 there is the listing for the opt_main.c file (containing a loop calling do_nothing()). Then, at the lines changed by that commit, the commands are compiling main.c instead. An alternative solution was to rename the file from line 989.
The commit was a suggestion, anyway, open for discussion. Let me know what you think about it.


delete_term is not deleting documents

Describe the bug

What did you do?
I created several documents and saved the index to a folder.
I then restarted my program, and, this time, attempted to use delete_term to delete the document.
I then restarted my program, and, this time, ran a query which showed the document was still there.
Each time the program restarts, it reads the index from the filesystem again.
What happened?
The new query shows the document which should've been deleted.
What was expected?
That the new query would not show the document which should've been deleted.

Which version of tantivy are you using?
0.19.0 with features mmap and lz4-compression
To Reproduce
If your bug is deterministic, can you give a minimal reproducing code?
Some bugs are not deterministic. Can you describe with precision in which context it happened?
If this is possible, can you share your code?
Here's a link to the code file in which this bug appears: https://github.com/Dirout/vaultist/blob/master/src/main.rs
Essentially, I'm building a secret manager (like an unstructured password manager of sorts for storing any type of secret string). Some of the functionality includes: adding secrets into a vault, reading secrets from a vault, and deleting secrets from a vault.
For the purposes of either reading or deleting secrets, I'm using Tantivy to store records of the secrets so the user can search and select which secret they'd like to either read or delete.
Here is how I'm creating the index when the vault is first made:
let mut schema_builder = Schema::builder();
let _name = schema_builder.add_text_field("title", TEXT | STORED);
let _id = schema_builder.add_text_field("id", TEXT | STORED);
let _last_modified = schema_builder.add_text_field("last_modified", TEXT | STORED);
let schema = schema_builder.build();
let index = Index::open_or_create(MmapDirectory::open(path_buf).unwrap(), schema).unwrap();
let mut index_writer = index.writer(100_000_000).unwrap();
index_writer.commit().unwrap();

Here is how I'm adding documents to the index:
let index = Index::open(MmapDirectory::open(path_buf).unwrap()).unwrap();
let schema = index.schema();
let name = schema.get_field("title").unwrap();
let id = schema.get_field("id").unwrap();
let last_modified = schema.get_field("last_modified").unwrap();
let mut index_writer = index.writer(100_000_000).unwrap();
index_writer
	.add_document(doc!(
		name => new_entry.clone().name,
		id => new_entry.id.to_string(),
		last_modified => new_entry.last_modified.to_rfc2822(),
	))
	.unwrap();
index_writer.commit().unwrap();

Here is how I'm reading documents from the index:
let index = Index::open(MmapDirectory::open(path_buf.clone()).unwrap()).unwrap();
let schema = index.schema();
let name = schema.get_field("title").unwrap();
let id = schema.get_field("id").unwrap();
let last_modified = schema.get_field("last_modified").unwrap();
let reader = index.reader().unwrap();
let searcher = reader.searcher();
let query_parser = QueryParser::for_index(&index, vec![name, id, last_modified]);

let search_query: String = Input::new()
	.with_prompt("❓ Search for the secret you're trying to view")
	.allow_empty(false)
	.interact_text_on(&Term::buffered_stdout())
	.unwrap();
let query = query_parser.parse_query(&search_query).unwrap();
let results: Vec<(Score, DocAddress)> = searcher
	.search(&query, &TopDocs::with_limit(deserialised_vault.items.len()))
	.unwrap();

Here is how I'm deleting documents from the index:
let index = Index::open(MmapDirectory::open(path_buf.clone()).unwrap()).unwrap();
let schema = index.schema();
let name = schema.get_field("title").unwrap();
let id = schema.get_field("id").unwrap();
let last_modified = schema.get_field("last_modified").unwrap();
let reader = index.reader().unwrap();
let searcher = reader.searcher();
let query_parser = QueryParser::for_index(&index, vec![name, id, last_modified]);

let search_query: String = Input::new()
	.with_prompt("❓ Search for the secret you're trying to remove")
	.allow_empty(false)
	.interact_text_on(&Term::buffered_stdout())
	.unwrap();
let query = query_parser.parse_query(&search_query).unwrap();
let results: Vec<(Score, DocAddress)> = searcher
	.search(&query, &TopDocs::with_limit(deserialised_vault.items.len()))
	.unwrap();

// Omitted code to select the document …

let secret_id_term =
	tantivy::schema::Term::from_field_text(id, &selected_item.clone().0.id.to_string());
let mut index_writer = index.writer(100_000_000).unwrap();
index_writer.delete_term(secret_id_term);
index_writer.commit().unwrap();
reader.reload().unwrap();

When I run my program and have it create a vault, the index is apparently created correctly.
When I run my program and have it add a secret, I encounter no issues. When I run my program to have it read a secret, the documents I've added show up.
However, when I run my program and have it delete a secret, I do encounter an issue. When I run my program to have it read a secret, the documents that should've been deleted still show up.
I read #835, which sounds very similar, but could not resolve my issue.
I read the example on deleting & updating documents (https://github.com/quickwit-inc/tantivy/blob/main/examples/deleting_updating_documents.rs), but still could not resolve my issue.
I would greatly appreciate any assistance which you could provide. Thanks!

Before digging deeper, I strongly suspect a tokenizer problem.
let _id = schema_builder.add_text_field("id", TEXT | STORED);

let _id = schema_builder.add_text_field("id", STRING | STORED);

I strongly suspect this is your problem.

(and thank you @emmyoh for taking the time to properly fill the issue, with instructions to reproduce and everything. It is super helpful!)

@fulmicoton That was it! Thank you so much for your help! Merry Christmas.


implement '--no-issues' flag to hide closed issues from the CHANGELOG

Add the options --issues (default) and --no-issues to show / hide the Closed Issues section in the CHANGELOG

Codecov Report
Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7503b3c) 50.38% compared to head (16be8ed) 50.51%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #102      +/-   ##
==========================================
+ Coverage   50.38%   50.51%   +0.12%
==========================================
  Files           7        7
  Lines         387      388       +1
==========================================
+ Hits          195      196       +1
  Misses        192      192

Files
Coverage Δ

github_changelog_md/config/settings.py
100.00% <100.00%> (ø)

github_changelog_md/main.py
47.82% <ø> (ø)

github_changelog_md/changelog/changelog.py
29.08% <0.00%> (ø)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


SC Managers need "Bucket Rank"

just like "SC Directors"

Role Created
[SC] Management
permissions will be updated later


Add pre-commit hook to validate staged files with JSCS

@cjohansen @mantoni you should test this locally before merging.

npm install
Modify a file, making an invalid change that JSCS looks for
Modify another file, making an invalid change that JSCS looks for
Stage the changes from one of the files
git commit

If all works as intended, then JSCS will complain about just the staged changes, ignoring the other file

I tried it, but I'm unable to come up with something JSCS finds unacceptable :) I first removed a couple of semicolons - nothing. I then ran npm run lint just to make sure it caught them, but that also reported OK.  Uhm?

I've pushed a better version of the pre-commit script, that is much, much simpler.
The pre-commit hook only does things for staged files, so if you didn't stage your changes, you won't see any errors.
Also, JSCS mostly cares about style, whitespace, indentation, etc. This is one of the reasons I'd like to also introduce ESLint to our toolchain.
Anyway, update, modify (try some whitespace in method signatures), stage, commit, fail.

Working great, nice work!


List table action columns update

Description
Describe what this change achieves.
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Codecov Report

:exclamation: No coverage uploaded for pull request base (2.7@1f071a8). Click here to learn what that means.
The diff coverage is n/a.

: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
@@          Coverage Diff           @@
##             2.7     #173   +/-   ##
======================================
  Coverage       ?   73.25%
======================================
  Files          ?       78
  Lines          ?     1331
  Branches       ?      231
======================================
  Hits           ?      975
  Misses         ?      302
  Partials       ?       54


Mac issues with coreutils

Hey, I'm trying to get some Nix basics working on MacOS, including home-manager. Turns out there are a few assumptions built-in to home-manager that break on Mac, but they should be fixable.

The major issue is that we can't assume the GNU versions of commands like readlink, ln, mkdir, etc. are the default ones. The GNU coreutils as packaged by Nix does compile fine on MacOS, so it seems like substituting in ${pkgs.coreutils}/bin/readlink etc. will fix that just fine. I'll attach a patch to illustrate. You might know of a better way to ensure that a coreutils environment is in the PATH before executing all this stuff. (But importantly, we can't assume that coreutils is in the user's path... he/she may prefer to access the BSD user-space commands by default.)

Next, I hit a problem linking into the gcroot, because /nix/var/nix/gcroots/per-user/$USER did not exist. On a Mac installation, all of /nix is owned by the user, not by root, so it didn't install a per-user subdir. I did a mkdir -p to create it anyway, and then it was fine, so we could add that to the script, assuming we're careful to ensure the same command works fine on NixOS.

Finally, we're almost there, but it dies Activating reloadSystemD because systemctl is not found. I'm pretty sure I'm not enabling any systemd services in home.nix on the Mac, but it does a daemon-reload anyway. We'd need to be able to skip that.

I could do a PR for this, but for now it's just an illustration...
diff --git a/modules/activation-init.sh b/modules/activation-init.sh
index a7011b9..4c76b80 100755
--- a/modules/activation-init.sh
+++ b/modules/activation-init.sh
@@ -17,7 +17,7 @@ function setupVars() {
     fi

     if [[ -e "$gcPath/current-home" ]] ; then
-        oldGenPath="$(readlink -e "$gcPath/current-home")"
+        oldGenPath="$(@READLINK@ -e "$gcPath/current-home")"
     fi

     genProfilePath="$profilesPath/home-manager"
diff --git a/modules/home-environment.nix b/modules/home-environment.nix
index 3b62b77..c47bf6d 100644
--- a/modules/home-environment.nix
+++ b/modules/home-environment.nix
@@ -7,6 +7,13 @@ let

   cfg = config.home;

+  cmd = {
+    bash = "${pkgs.bash}/bin/bash";
+    ln = "${pkgs.coreutils}/bin/ln";
+    mkdir = "${pkgs.coreutils}/bin/mkdir";
+    readlink = "${pkgs.coreutils}/bin/readlink";
+  };
+
   languageSubModule = types.submodule {
     options = {
       base = mkOption {
@@ -258,7 +265,7 @@ in
             relativePath="''${sourcePath#$newGenFiles/}"
             targetPath="$HOME/$relativePath"
             if [[ -e "$targetPath" \
-                && ! "$(readlink -e "$targetPath")" =~ "${pattern}" ]] ; then
+                && ! "$(${cmd.readlink} -e "$targetPath")" =~ "${pattern}" ]] ; then
               echo -e "Existing file '$targetPath' is in the way"
               collision=1
             fi
@@ -271,9 +278,9 @@ in
         '';
       in
       ''
-        newGenFiles="$(readlink -e "$newGenPath/home-files")"
+        newGenFiles="$(${cmd.readlink} -e "$newGenPath/home-files")"
         find "$newGenFiles" -type f -print0 -or -type l -print0 \
-                | xargs -0 bash ${check} "$newGenFiles"
+                | xargs -0 ${cmd.bash} ${check} "$newGenFiles"
       ''
     );

@@ -285,8 +292,8 @@ in
           for sourcePath in "$@" ; do
             relativePath="''${sourcePath#$newGenFiles/}"
             targetPath="$HOME/$relativePath"
-            $DRY_RUN_CMD mkdir -p $VERBOSE_ARG "$(dirname "$targetPath")"
-            $DRY_RUN_CMD ln -nsf $VERBOSE_ARG "$sourcePath" "$targetPath"
+            $DRY_RUN_CMD ${cmd.mkdir} -p $VERBOSE_ARG "$(dirname "$targetPath")"
+            $DRY_RUN_CMD ${cmd.ln} -nsf $VERBOSE_ARG "$sourcePath" "$targetPath"
           done
         '';

@@ -311,9 +318,9 @@ in
         ''
           function linkNewGen() {
             local newGenFiles
-            newGenFiles="$(readlink -e "$newGenPath/home-files")"
+            newGenFiles="$(${cmd.readlink} -e "$newGenPath/home-files")"
             find "$newGenFiles" -type f -print0 -or -type l -print0 \
-              | xargs -0 bash ${link} "$newGenFiles"
+              | xargs -0 ${cmd.bash} ${link} "$newGenFiles"
           }

           function cleanOldGen() {
@@ -324,17 +331,17 @@ in
             echo "Cleaning up orphan links from $HOME"

             local newGenFiles oldGenFiles
-            newGenFiles="$(readlink -e "$newGenPath/home-files")"
-            oldGenFiles="$(readlink -e "$oldGenPath/home-files")"
+            newGenFiles="$(${cmd.readlink} -e "$newGenPath/home-files")"
+            oldGenFiles="$(${cmd.readlink} -e "$oldGenPath/home-files")"
             find "$oldGenFiles" -type f -print0 -or -type l -print0 \
-              | xargs -0 bash ${cleanup} "$newGenFiles" "$oldGenFiles"
+              | xargs -0 ${cmd.bash} ${cleanup} "$newGenFiles" "$oldGenFiles"
           }

           if [[ ! -v oldGenPath || "$oldGenPath" != "$newGenPath" ]] ; then
             echo "Creating profile generation $newGenNum"
-            $DRY_RUN_CMD ln -Tsf $VERBOSE_ARG "$newGenPath" "$newGenProfilePath"
-            $DRY_RUN_CMD ln -Tsf $VERBOSE_ARG "$newGenProfilePath" "$genProfilePath"
-            $DRY_RUN_CMD ln -Tsf $VERBOSE_ARG "$newGenPath" "$newGenGcPath"
+            $DRY_RUN_CMD ${cmd.ln} -Tsf $VERBOSE_ARG "$newGenPath" "$newGenProfilePath"
+            $DRY_RUN_CMD ${cmd.ln} -Tsf $VERBOSE_ARG "$newGenProfilePath" "$genProfilePath"
+            $DRY_RUN_CMD ${cmd.ln} -Tsf $VERBOSE_ARG "$newGenPath" "$newGenGcPath"
           else
             echo "No change so reusing latest profile generation $oldGenNum"
           fi
@@ -379,13 +386,13 @@ in
           phases = [ "installPhase" ];

           installPhase =
-            "mkdir -p $out\n" +
+            "${cmd.mkdir} -p $out\n" +
             concatStringsSep "\n" (
               mapAttrsToList (n: v:
                 ''
                   if [ -d "${v.source}" ]; then
-                    mkdir -pv "$(dirname "$out/${v.target}")"
-                    ln -sv "${v.source}" "$out/${v.target}"
+                    ${cmd.mkdir} -pv "$(dirname "$out/${v.target}")"
+                    ${cmd.ln} -sv "${v.source}" "$out/${v.target}"
                   else
                     install -D -m${v.mode} "${v.source}" "$out/${v.target}"
                   fi
@@ -403,9 +410,10 @@ in
             install -D -m755 ${sf} $out/activate

             substituteInPlace $out/activate \
-              --subst-var-by GENERATION_DIR $out
+              --subst-var-by GENERATION_DIR $out \
+              --subst-var-by READLINK ${cmd.readlink}

-            ln -s ${home-files} $out/home-files
+            ${cmd.ln} -s ${home-files} $out/home-files
           '';
         };

I must admit I didn't even think the thought of Mac support so I'm very happy that it doesn't seem too far off ☺
For the first issue, would
diff --git a/modules/home-environment.nix b/modules/home-environment.nix
index 694a2aa..ddb838a 100644
--- a/modules/home-environment.nix
+++ b/modules/home-environment.nix
@@ -375,6 +375,10 @@ in
           set -eu
           set -o pipefail

+          # We assume coreutils command and Bash.
+          PATH="${pkgs.coreutils}/bin:${pkgs.bash}/bin/bash:$PATH"
+          export PATH
+
           . ${./lib-bash/color-echo.sh}

           ${builtins.readFile ./lib-bash/activation-init.sh}

be sufficient? It should cause the coreutils tools and bash to be available in the activation script (and the scripts it calls) and take priority over existing commands.

Yes, something along those lines should work. I wasn't sure if that block is the only entry point for all the scripts. I'll test that way when I get a minute; right now my Mac is rebuilding the world.
As for point 3 about systemd, it seems to me that if the list generated here:
    home.file =
      listToAttrs (
        (buildServices "service" config.systemd.user.services)
        ++
        (buildServices "target" config.systemd.user.targets)
        ++
        (buildServices "timer" config.systemd.user.timers)
      );

is empty, then we could skip omitting home.activation.reloadSystemD, and might that take care of it? Actually the workaround I have right now is I added a script called systemctl to the PATH; it just ignores its arguments and returns 0.

Oops! Sorry, I didn't mean to mark the first comment as coming from me. I just wanted to attach a few commits to this issue :-(
The first commit will just do the adding coreutils and bash to PATH. The second commit might sort out the systemd issue.
Unfortunately it wouldn't work to skip generating the systemd reload if no units are to be generated because we might be replacing a generation where units were present, then we need to reload systemd to flush them out.
The solution I have is to first exit with an error if running on a non-Linux system and systemd units are to be used. Second to only generate the systemd activation code if running on a Linux system.

I'm happy it works for you now. I'm still wondering about the gcroots directory I noticed that the shell.nix file from Nixpgs includes code that generates the profiles and gcroots directories. I'm thinking it should be OK to copy this into either the generated activation script or the home-manager script.
On the other hand, I'm kind of reluctant and wouldn't mind deferring the gcroots directory issue to "Make sure you have a working Nix installation" in the installation instructions ☺

I've merged these changes into master now. I will ponder the remaining issue with the gcroots directory a little bit more.

About the gcroots directory I ended up just adding a note in the installation section of the README file: dfb5bbddf571f0035a1a4559d4ad939b0491da0b
I think this will have to do for now. If anybody else complain will have to revisit it ☺
Many thanks for your report and interest.


SyncSession does not call error handler

This surely is a bug. The handle_error method does not take into consideration that there might be an error handler and silently skips errors
https://github.com/realm/realm-object-store/blob/master/src/sync/sync_session.cpp#L629
This happens when investigating ssl connection for this bug https://github.com/realm/realm-js/issues/1369
The server returns
Sync Error
336134278 error_code
message "certificate verify failed"

which is not recognized and the sync error handler is not called
This is ROS 1 and realm-js 1.13 still I think it should be fixed for 2.0 as well

Wouldn't that happen only if the error is unknown? I guess the bug is that the server returns an unknown error_code, not in the OS error handling logic.

I think both. The server should be using known errors but still the client should be redirecting this to user error handler. At least for debugging such cases it would help. Or the client can take appropriate actions to recover/show error message etc even when the error is not know

I'm not sure the application can take appropriate steps if we have no idea what the error code means - it could just as well be a bit flip due to gamma radiation :) At the very least, the server should set the appropriate category. @morten-krogh do you have any idea why the server would return an unrecognized error_code?

Yes exactly we can't be sure so we give the developer to opportunity to decide  by the error callback means. Not respecting the error handler means we are deciding for him. In this particular case I think we must call the error handler with the information we have ie unknown error code and the exact message.
If i am the developer in this case it will provide the means to tell the user something went wrong. Silencing this error means my app will still be showing a progress bar/spinner for example.
The server should return known errors but we have a precedent that it sometimes does not. So we should be respecting the error handler in the sync client code regardless.
IMO silencing the error should happen only when no error handler is set. Otherwise the error should be redirected to the developer to worry him at least :)

I agree that the errors should not be discarrded by object store.
I have actually seen the behavior before.
We have had various discussions about the sync error codes.
The system is that low level error codes go through the whole chain and can be captured by
any layer above.
This error
Sync Error
336134278 error_code
message "certificate verify failed"
originates from OpenSSL. There is something strange with some of the OpenSSL error codes.
Anyway, the message is quite informative.
If sync uses the principle of sending low level error codes through, it is intended that all bindings capture them and submit them as is to the user or transform them before giving them to the user.
I agree with you that the bindings should deliver it to the user. That is all there is to do.
The user will have to look a the SSL certificates. There is no point in continuing.
This means that Object store should pass the error code along, and the binding should report it to the user.  The current situation is not the intended behavior.

This is a straightforward change, and probably should have been made long ago.

Fixed by https://github.com/realm/realm-object-store/pull/596.


0.13.1 causing an error in project using git2

$ cargo build
...
   Compiling git2 v0.13.1
error[E0531]: cannot find unit struct, unit variant or constant `GIT_ERROR_HTTP` in crate `raw`
   --> /home/qtfkwk/.cargo/registry/src/github.com-1ecc6299db9ec823/git2-0.13.1/src/error.rs:159:18
    |
159 |             raw::GIT_ERROR_HTTP => super::ErrorClass::Http,
    |                  ^^^^^^^^^^^^^^ help: a constant with a similar name exists: `GIT_ERROR_NET`
    |
   ::: /home/qtfkwk/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.12.0+0.99.0/lib.rs:40:9
    |
40  |         pub const $variant: $name = $val;
    |         --------------------------------- similarly named constant `GIT_ERROR_NET` defined here

error[E0425]: cannot find value `GIT_ERROR_HTTP` in crate `raw`
   --> /home/qtfkwk/.cargo/registry/src/github.com-1ecc6299db9ec823/git2-0.13.1/src/error.rs:247:13
    |
247 |             GIT_ERROR_HTTP,
    |             ^^^^^^^^^^^^^^ help: a constant with a similar name exists: `GIT_ERROR_NET`
    |
   ::: /home/qtfkwk/.cargo/registry/src/github.com-1ecc6299db9ec823/libgit2-sys-0.12.0+0.99.0/lib.rs:40:9
    |
40  |         pub const $variant: $name = $val;
    |         --------------------------------- similarly named constant `GIT_ERROR_NET` defined here

Discovered 0.13.1 had released via cargo outdated --root-deps-only, then updated my Cargo.toml to have git2 = "0.13.1", then observed the above error during cargo build.
Short term workaround is to change Cargo.toml to have git2 = "<0.13.1", which builds successfully with 0.13.0.
Any ideas about the root cause and/or fix? Thanks in advance!
EDIT: OS: Debian Testing; libgit2-28, libgit2-dev not installed
EDIT:
$ rustup show
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/qtfkwk/.rustup

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu

active toolchain
----------------

stable-x86_64-unknown-linux-gnu (default)
rustc 1.42.0 (b8cedc004 2020-03-09)

You can update libgit2-sys to the latest version to fix this (cargo update -p libgit2-sys).
@alexcrichton when bumping versions, should we probably update this line as well? Like this:
diff --git a/Cargo.toml b/Cargo.toml
index c19830c..7232cf0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,7 +20,7 @@ url = "2.0"
 bitflags = "1.1.0"
 libc = "0.2"
 log = "0.4.8"
-libgit2-sys = { path = "libgit2-sys", version = "0.12.0" }
+libgit2-sys = { path = "libgit2-sys", version = "0.12.2" }

 [target."cfg(all(unix, not(target_os = \"macos\")))".dependencies]
 openssl-sys = { version = "0.9.0", optional = true }

@ehuss Perfect! cargo update -p libgit2-sys worked! Much appreciated! Please consider the original issue closed from my point of view, although will allow you or @alexcrichton to close it when that question has been resolved. Thanks again!

@ehuss Without also updating the libgit2-sys dependency, you can't use automatic dependency  management services like dependabot. Since that can't use cargo update.

Closing as I believe this is resolved. When updating the base crate to use new bindings, we'll be sure to also update the -sys dependency.


ews: Implement ExchangeSession with blocking connection pool

This tries to solve the issue where Exchange server refuses new connections with (a:ErrorExceededConnectionCount).
By default the underlying urllib3.HTTPConnectionPool allows to open more connections than the size of the pool. The newly implemented ExchangeSessionBlockingConnPool sets the pool to "blocking mode" where calls will block until a free connection is available.

The build fails with the following error:
E: Package 'postgresql-9.6-postgis-2.3' has no installation candidate

While that should be fixed at some point I think it can be considered as out-of-scope for this PR.

This change will accompanied by the following local settings in the production environment:
RESPA_EXCHANGE_EWS_SESSION_CLASS = "respa_exchange.ews.session.ExchangeSessionBlockingConnPool"
RESPA_EXCHANGE_EWS_SESSION_POOLSIZE = 8

If the purpose in the future is to have the ExchangeSession to not be used, then it might be worthy of a DeprecationWarning.

To be honest, I don't know what's the long-term plan. I'll just merge this and we'll see how it plays out.


[fix] Bump io.grpc from 1.56.0 to 1.56.1

Motivation
Bump io.grpc from 1.56.0 to 1.56.1 to address this important fix. https://github.com/grpc/grpc-java/pull/10328
Modifications

Bump io.grpc from 1.56.0 to 1.56.1

Verifying this change

[x] Make sure that the change passes the CI checks.

Documentation

[x] doc-not-needed

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 74.54%. Comparing base (bbc6224) to head (f4a498f).
Report is 570 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #23276      +/-   ##
============================================
+ Coverage     73.57%   74.54%   +0.97%
- Complexity    32624    34288    +1664
============================================
  Files          1877     1926      +49
  Lines        139502   145037    +5535
  Branches      15299    15856     +557
============================================
+ Hits         102638   108123    +5485
+ Misses        28908    28646     -262
- Partials       7956     8268     +312

Flag
Coverage Δ

inttests
27.95% <ø> (+3.37%)
:arrow_up:

systests
24.66% <ø> (+0.33%)
:arrow_up:

unittests
73.89% <ø> (+1.05%)
:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.
see 562 files with indirect coverage changes

Bump io.grpc from 1.56.0 to 1.56.1 to address this super important fix. grpc/grpc-java#10328

@mattisonchao Please share the context to others about why this is important for Pulsar.

Please share the context to others about why this is important for Pulsar.

Pulsar is pluggable messaging system. For some plugins that is using GRPC as the RPC framework will affected by this bug. (https://github.com/grpc/grpc-java/pull/10328)  the bug will cause the client can't not do reconnecting after hit UnknownHostException.