text:
Should Transformers be recursive?

The current implementation of cmp.Equal recursively applies any Transformers that remain after applying all the filters. For a trivial Transformer of the type func(T) T, this will be recursively apply to the output since it will always match. For this reason, the cmpopts.SortSlices is wrapped with a FilterValues that acts as a "base-case" to prevent infinite recursion. One alteration to the algorithm is to say that a given Transformer cannot be recursively applied on a value that was transformed at some point by that very Transformer.
The major advantage of non-recursive Transformers is:

A naive use of func(T) T works as expected without any need for a "base-case" filter.

The disadvantages of non-recursive Transformer is:

The additional documentation needed to explain the special-case where Transformers are not executed. The implementation of this is essentially a set of "spent" Transformers, where transformers are added to the set when descending down the tree, and popped when ascending up the tree.
In some rare use-cases you may actually want Transformers to be recursive. Imagine you have a non-binary tree, where each node is essentially a set of children. When comparing this data-structure, you may want to sort each node using a transformer. You would need the property of recursive transformers to ensure each node may get transformed.
The need for "base-case" filters to prevent recursive cycles.

Thoughts? I do see the value in non-recursive Transformers. It simplifies use of them, but prevents some very rare use-cases.
\cc @bcmills @jba

FWIW my first experiment with this package, was to test that transformers were repeatedly applied, as I think that makes things more scalable, always transforming towards the goal of comparable types.
I think the current implementation is both intuitive and useful.

I'm probably confused, but as an unlikely working hypothesis I'm going to assume instead that both of you are confused.
Here's how I see it: At a given call of cmp.Equal, you have a value x (well, actually, two values x and y, but they behave independently w.r.t. transformers) and a set S of transformers that have passed the filters.  You then do the following repeatedly:

Choose a transformer t in S that can be applied to x. If none, terminate. If more than one, fail.
Apply t to x, giving a new x.
Remove t from S.

Step 3 ensures that this algorithm will terminate.
Now in comparing the x' and y' you get from applying the transforms, cmp.Equal may invoke itself recursively. This is the only recursion that should exist (unless a transformer has some internal recursion, which should be irrelevant to this discussion). In that recursive call, the above algorithm will be applied again to a new x and y, with a fresh S that may include the same transformers.
@dsnet, your tree example is handled by my algorithm. There is a transformer from *Node to *Node that sorts Node.Children and returns. When cmp.Equal recurses on the children, they each in turn will be transformed. In a sense the same transformer is being used recursively, but there is no danger of infinite recursion in transformer invocations.
@rogpeppe, if by "repeatedly applied" you mean at each recursive call of cmp.Equal, then that is what I'm suggesting. If, however, you mean that the same transformer is applied repeatedly at the same level, then I don't understand why that would be useful. Why wouldn't I just write the transformer to do all the work on the first call? (I suspect this is the step I'm confused about, and there is some case where it's useful to do that.)

Imagine you have a non-binary tree, where each node is essentially a set of children. When comparing this data-structure, you may want to sort each node using a transformer.

Why wouldn't you implement that as a single Transformer that returns the sorted tree? (The user's transformer code can always use recursion internally...)
The other logical behavior we could consider would be to apply Transformers recursively until we either reach a fixed-point or a cycle (where a cycle that is not a fixed-point would cause a panic). But that would require us to have some way to reliably analyze the T to determine when it converges.

@jba The three steps you're describing are essentially what we're doing today, except that we don't do step (3); instead, we rely on the user's filter to indicate that the Transformer has reached a fixed-point.

@bcmills Why is the current choice better than mine? I certainly see the advantage to having step 3—no need to filter in the case where the transformer's input and output types are the same. Are there good examples that argue for the current design?

The current API is strictly more restrictive: every program which is correct under the current API is also correct under both of the possible alternatives (non-recursive Transformers, and iteration until fixed points), so absent a compelling use-case, it's a better default position to start from.

so absent a compelling use-case...

I think we have two compelling use-cases for the non-recursive design:

sorting a slice or part of a data structure to achieve order-independence (which currently requires an expensive filter that checks whether the values are already sorted)
rounding values because the SUT only supports reduced precision (e.g., rounding times to the nearest microsecond because bigquery timestamps are only to the microsecond)

I haven't seen any use-cases for the current design. All the alleged examples are handled by the recursion of cmp.Equal itself.
I can certainly imagine examples. Like algebraic simplification, where each simplification rule is expressed as a separate transformer. But wouldn't the author be more likely to use a single transformer that does all the work?
All that said, if you're not convinced I don't see a problem with waiting a few months to gather more data. It's unlikely that the change will cause much breakage.

@jba, I don't see how step 3 in your algorithm accomplishes anything as you say that a recursive call to Equal uses "a fresh S". In which case, your algorithm is exactly as what we have today (i.e., one that leads to infinite recursion).
Un-rolling your example, we end up with this:

1a:

@jba, I don't see how step 3 in your algorithm accomplishes anything since you say that a recursive call to Equal uses "a fresh S". In which case, your algorithm is exactly as what we have today (i.e., one that leads to infinite recursion).
Un-rolling your example, we end up with this:

First call to cmp.Equal
1a: select t from S where S := {t}
2a: obtain x' := t(x)
3a: remove t from S such that S := {}
Second recursive call to cmp.Equal
1b: select t from S where S := {t} (according to your description this has a fresh S)
2b: obtain x' := t(x)
3b: remove t from S such that S := {}
Third recursive call to cmp.Equal
... (infinite recursion)

The algorithm I had in mind for preventing recursion was to basically say that every Transformer has an implicit filter on it where it is applicable only if that Transformer does not already exist within the current Path. I prototyped an implementation of such here: #26
The crucial implementation details are:

In the applyOption function when we use a transformer, we add it to the usedTrans set (with a defer to remove it from the set when returning)
In the applyFilters function, we check if a given transformer is already in usedTrans. If yes, then we exclude it from S.

@bcmills said:

[Edit: actually, that's not true. Programs that are currently correct using filters to stop at fixed-points would terminate early with non-recursive transformers.]

Correct. A change in this behavior is subtle and unlikely to break someone unless they were explicitly depending on the behavior. At least inside google, there are no use cases that would be broken by the new policy. If we want to switch this policy, I'd rather do it sooner than later.

@rogpeppe, you mentioned that you find the current implementation useful. Did you have a real use-case in mind? So far all use-cases for recursive transformers have been theoretical.

every Transformer has an implicit filter on it where it is applicable only if that Transformer does not already exist within the current Path

I think that's too surprising in general: what happens if you have a Transformer that flattens the top level of a recursive type?
I think the rule would have to be that the Transformer only applies if that Transformer does not already exist within the Transform tail of the current Path. But that seems pretty subtle to me.

I think the rule would have to be that the Transformer only applies if that Transformer does not already exist within the Transform tail of the current Path.

Agreed; that seems subtle to me as well.
An alternative would be to add cmpopts.NonReetrantTransformer (better name required) that has the implicit filter added where it can't be called if the transformer itself exists in the Path (or last segment of the path as you suggest).

Here's my evaluation of an approach based on @jba's algorithm where:

Every Transformer has an implicit filter on it where it is only applicable if that specific Transformer does not already exist within the tail of the current Path since the last non-Transform step.

The prototype is here: #29. The actual logic to add this behavior is only ~10 lines long.
The benefits:

Most transformers do not require a user defined filter. For example, ExampleOption.SortedSlice simply becomes:

trans := cmp.Transformer("Sort", func(in []int) []int {
	out := append([]int(nil), in...) // Copy input to avoid mutating it
	sort.Ints(out)
	return out
})

It does not prevent more advanced usages of Transformers that may actually want to rely on the recursive property of Transformers. Thus for the example shown above:

type Node struct {
    Key int
    Children []*Node
}

// This does not require a filter because Sort is not allowed to trigger again
// until cmp.Equal has descending into the []*Node type.
// However, it still triggers for each sub-Node containing []*Node.
trans := cmp.Transformer("Sort", func(in []*Node) []*Node {
	out := append([]int(nil), in...) // Copy input to avoid mutating it
	sort.Slice(out, ...)
	return out
})

It is very very very unlikely to break anyone already using cmp unless they were already relying on the same Transformers being recursively applied to itself immediately.

The neutral:

This does not prevent all possible cycles. Go's lack of generics forces us to rely heavily on interface{} for generic transformers that operate on any slice for example. The interface{} forces cmp.Equal to recursively descend into the interface, adding a TypeAssertion step to the Path and thus allowing the Transformer to again apply on the underlying concrete type. However, this is not any different that what we have to do today. The cases where you have to rely on interface{} are probably only going to be helper libraries like cmpopts, which are only written once and used by many. I'm okay if these cases are not easily handled.

We could adjust the rules to be special for TypeAssertion and Indirect steps, but I am opposed to this since they are still technically a natural part of the structure of a Go value. Thus, it is easier to justify specially handling the Transform step since it is the only step that does not directly correspond with recursive calls dictated by the structure of the value itself, while Indirect, TypeAssertion, SliceIndex, MapIndex, and StructField correspond directly pointers, interfaces, slices, maps, and structs.

In the very very very rare case where someone really cared about Transformers recursively applying immediately to itself (I cannot think of any use case for this), this can be trivially achieved by simply returning the value in an interface{}. Thus, this approach does not prevent any use-case that was possible today.

The detriments:

There is additional logic to handle this implicit filter. This adds complexity to cmp, but the complexity is cheap: 10 lines.
There is additional cognitive load on the user using Transformers to know about the implicit filter (the documentation in my PR could be improved). However, it could be argued that its easier on users since they generally don't have to think about the base-case filter that is currently required. In the event that the user writes a infinitely-recursive Transformer, the failure is obvious from the stack trace, and the solution is the same as today: add a base-case filter.

Conclusion: I believe the benefits out-weight the detriments and worth doing. Thoughts?

I disagree that a TypeAssertion corresponds to a change in structure. It's more of a change of viewpoint. But usability is more important than our philosophical differences. What are some examples where a type assertion naturally comes into play, and which rule makes them easier to write?

There is additional cognitive load on the user using Transformers to know about the implicit filter

I'm willing to bet that most people will think about it in the terms I used: each transformer is applied once per value per call, and the recursion is structural. So I think this way is easier to think about.

What are some examples where a type assertion naturally comes into play, and which rule makes them easier to write?

Generally, I don't see anticipate people creating transformers that return interfaces because transformers require you to create a copy of the input. Unless you use reflect, creating a copy usually requires you to create a value of a concrete type. You might as well return that concrete value.
Again, it's only in the cases of generic algorithms like cmpopts.SortSlices and cmpopts.SortMaps that this is becomes a problem. However, those are the exception and not the norm.

I'm willing to bet that most people will think about it in the terms I used

Perhaps? I'm an example of someone who thinks of it the other way around, but I could be biased since I wrote the implementation. One problem I have with your high-level description is that "recursion is structural" is not well defined (again, someone may wonder whether TypeAssertion and Indirect is considered "structural" or not). Do you have succinct description of the algorithm that is unambiguous as to exactly how it operates?

authors: bcmills dsnet jba rogpeppe

repo: google/go-cmp


text:
Add test cases for JSON resource marshaling

Resources Covered :

HeadCommit
PushEventRepository
PushEventRepoOwner

Helps with issue : #55

Friendly ping @wesleimp

@sagar23sj - either you can resolve the conflicts or I can - either way is fine with me, please just let me know.

source: gharchive/pull-request

repo: google/go-github

license_type: permissive

license_source: github-api


text:
Generate new eight-digit emergency codes for Authenticator for PAM.

From @ThomasHabets on October 10, 2014 8:6
Original issue 248 created by tim@timpollock.info on 2013-02-08T00:05:10.000Z:
What steps will reproduce the problem?

Wish to generate new emergency codes for times when I don't have Authenticator on me.
Setting up Authenticator only gives me five codes.
I would like more.

What version of the product are you using? On what operating system?
pam_google_authenticator.so
Please provide any additional information below.
When setting up Google Authenticator for PAM, it gives me five emergency codes. I'd like to generate more as a just-in-case. Is this at all possible?
Copied from original issue: google/google-authenticator#247

Comment #1 originally posted by belotton on 2013-02-09T09:47:43.000Z:
You can change the SCRATCHCODES value (file google-authenticator.c, line 41).
Then, build and install by running "make install".
Run google-authenticator.
Is it possible to generate new emergency codes without changing the secret key ?

Comment #2 originally posted by belotton on 2013-02-09T11:01:20.000Z:
You can change the SCRATCHCODES value (file google-authenticator.c, line 41).
Then, build and install by running "make install".
Then, run google-authenticator.
I think google-authenticator should be able to generate new emergency codes without generating a new secret.

Comment #3 originally posted by belotton on 2013-02-09T13:47:28.000Z:
Here is a patch allowing you to specify the number of emergency codes (max 10) you want to generate.
Usage :
google-authenticator --emergency-code 10
google-authenticator -e 10

Hi Thomas, it seems like @Lo0k has done his patch in #60.
Is it possible for it to merged into the master branch?
Thanks!

It was already merged a year ago.

source: gharchive/issue

authors: ThomasHabets fartbagxp

repo: google/google-authenticator-libpam

license_source: bigquery


text:
Modify GTMOAuth2Authentication.h to optionally import GTMSessionFetcher via a framework-like path

This supports the Cocoapods use_frameworks! use case, and is enabled in the Podspec, but is disabled by default, therefore making this change a no-op for non-CocoaPods use cases.

Squash into one commit?

authors: perotinus thomasvl


text:
Unsupport global language

when change the language, captions display error. and It happens on textbox too.

It looks like the default font Roboto-Regular.ttf does not contain the glyphs you are trying to use.
Please try loading a true-type font you know to contain your language glyphs and pass this to Driver.CreateFont.
Please reopen if this does not work for you.

authors: ben-clayton justay

repo: google/gxui


text:
Code coverage error

Hello,
I've tried to fuzz x64 target on ubuntu 14.04 lts with code coverage.
Target is properly configured and build with asan and code coverage.
When I run honggfuzz I get this error:
[2016-03-28T10:44:11+0200][E][11737] sancov_queryBitmap():131 bitmap overflow (67156064)
[2016-03-28T10:44:11+0200][E][11737] sancov_setBitmap():144 Out of range index (67156064 > 67108863)
Segmentation fault

It was my fault.

repo: google/honggfuzz


text:
Add API and launcher for NonEditableBasicTextField

WHAT
Adding NonEditableBasicTextField composable to open the keyboard on click.
Currently a draft as I need to get it to work. Opening a draft PR here so I can get some feedback on the solution :)
WHY
Ref. #1563
HOW
Checklist :clipboard:

[x] Add explicit visibility modifier and explicit return types for public declarations
[ ] Run spotless check
[ ] Run tests
[ ] Update metalava's signature text files

Thanks for working on this!
I will do another round of review tomorrow, just leaving these comments for now.
Could you please add a video showing the usage of the component to the PR description? A sequence of screenshots is also fine.

Thank you for checking this out and giving valuable feedback 🙏  Currently very in draft stage. I can provide a video on how it's working now. From what I understood in the issue we want to have a wrapper on BasicTextField exposing mostly the same API, that when clicking it it launches this intent instead to go to another screen to do the input. Is that the correct assumption? :)

I think maybe I'll close this for now until we get a bit more input from the UX. Not quite sure how this component should work to provide most value to the users

Thanks for the patience, Odin. I will ping you here once I get more information.

repo: google/horologist


text:
Switching to using our own flatcc compiler library.

We were already doing our own runtime library slicing, and we have to do our own compiler build for bazel, so this is consistent and lets us avoid including their cmake file (and the issues that creates).

@GMNGeoffrey

Yeah this has bugged me. The latest weirdness was their CMake touching a file in the source tree on every configure:

I can't follow that discord link, but are you referring to CMake in general at that version, or something in the flatcc configuration? I would also like to not have the source tree touched. (Author of flatcc speaking.)
In either case, flatcc was intended to be easily integrated in end users builds more than necessarily consuming the existing CMake project as is.

Thanks for pointing this out, but this touches a file in the build directory, not in the source tree, and it is covered by .gitignore.
There is probably a more sane way to configure this, but the problem being solved here is that flatcc depends on its own output, and if the output somehow becomes invalid during development, it cannot bootstrap itself to fix the problem without removing the feature that depends on its own output (being able to read binary schema files in flatbuffer format).
There is actually another file scripts/build.cfg potentially being touched to remember the build backend (ninja or make) when using build scripts in the scripts folder, but if you use CMake directly this will not happen, and the file is also covered by .gitignore.

Thanks for pointing this out, but this touches a file in the build directory, not in the source tree, and it is covered by .gitignore.

Only if the build directory is located within the source directory and called "build", which is not always the case. If you want to use the build directory, you should use PROJECT_BINARY_DIR instead of PROJECT_SOURCE_DIR and a hardcoded "build"

OK, I removed the offensive files - they were only used to flag test scripts in that needed to know, but they were largely obsolete anyway. All the shell script build and test drivers still make assumptions about build location, but CMake itself shouldn't, I agree.

authors: GMNGeoffrey benvanik mikkelfj

repo: google/iree


text:
Closes: #1

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

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

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

@googlebot I signed it!

repo: google/it-cert-automation-practice


text:
Closes: #1

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

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

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

@googlebot  I signed it !

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
📝 Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
What to do if you already signed the CLA
Individual signers
* It's possible we don't have your GitHub username or you're using a different email address on your commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).

Corporate signers
* Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to [go/cla#troubleshoot](http://go/cla#troubleshoot) ([Public version](https://opensource.google/docs/cla/#troubleshoot)).

* The email used to register you as an authorized contributor must be the email used for the Git commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).

* The email used to register you as an authorized contributor must also be [attached to your GitHub account](https://github.com/settings/emails).

ℹ Googlers: Go here for more info.

@googlebot  I signed it !


text:
Update validations.py python script.

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

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

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

great

outdated

authors: Mahmoud-naj gaymertian


text:
Updated validation.py python script.

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

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

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

Changed isnumeric into isalpha


text:
Equivalent to TensorArray for while_loop

A large class of algorithms require maintaining an arbitrary length list of arrays over loops.
What is the flow for collecting a list of tensors (all of the same shape and dtype) in a while_loop?
This should be the same notion as TensorArray in tensorflow.
One obvious way is to use python while loops, but what I'm looking for is while_loop support.
Is there a way to do it without unrolling the graph?
Also, if it can only be done with python while loops what are the constraints and performance effects? Is it still jittable? Is it feasible for really long loops?

Most likely, you're looking for jax.lax.scan:
https://jax.readthedocs.io/en/latest/_autosummary/jax.lax.scan.html#jax.lax.scan
e.g., if you were writing, say, an RNN, that's what you would want to use.
Does that help?

TensorArray lets your grow tensors in each loop iteration, but that isn't possible to in XLA (i.e., under jit).
Instead, you should preallocate the full sized arrays you need, and update them using primitives such as while_loop and scan. This may seem extremely inefficient, but is actually only a 2x cost in memory (the difference between the upper triangular half of a matrix and the full matrix).

Thanks for the pointer to lax.scan. Solves the problem for most problems except those with a dynamic length. Thanks for the info about XLA too.

@shoyer re-opening this thread to ask another related question. Is there a way to have both a dynamic halting condition and a dynamically allocated result. Basically, I need scan-like behaviour in that it collects results, but the termination condition is dynamic as in a while_loop. I was wondering how you would implement scan under the hood, and noted that in TF it's with a while_loop. In JAX are you essentially just preallocating a result array tree and then using dynamic_update_slice?

Looks like the _scan_impl in JAX also uses a while_loop using dynamic_update_index_in_dim. Makes sense. So I'm guessing the answer to my problem is to do a similar thing.

Clear

Extending on the question, I wonder: Without sth like TensorArray, how can that be efficient with backprop? Wouldn't it keep a copy of ys of each iteration for backprop? StackOverflow question with more details.

authors: Joshuaalbert albertz hawkinsp shoyer

repo: google/jax


text:
[typing] overloaded type declaration for variadic lax.sort

Part of #12049
Whether the return value is a tuple or an array depends on the input; leaving as a straight union causes type errors for patterns like:
x = jnp.arange(10)
lax.sort(x).shape

because Tuple doesn't have a shape attribute.

This was Approved as a subset of #12421 so I'm going to merge this first


text:
Add optional size argument to jnp.nonzero to allow use in JIT

This changes the implementation of jnp.nonzero to allow it to be used within JIT by optionally passing a static size argument:
>>> import jax.numpy as jnp
>>> from jax import jit
>>> x = jnp.array([0, 1, 1, 0, 0, 1])

# normal calling pattern still works
>>> jnp.nonzero(x)
(DeviceArray([1, 2, 5], dtype=int32),)

# pass a static size and it works with jit()
>>> jit(jnp.nonzero, static_argnums=1)(x, 3)
(DeviceArray([1, 2, 5], dtype=int32),)

# if the size is too large, the result is zero-padded
>>> jit(jnp.nonzero, static_argnums=1)(x, 5)
(DeviceArray([1, 2, 5, 0, 0], dtype=int32),)

Performance-wise, this appears to be comparable to the previous approach; using the following test code:
import jax.numpy as jnp
import numpy as np
x = np.random.randn(200, 300, 400)
x[x < 0] = 0
x = jnp.asarray(x)
%timeit [i.block_until_ready() for i in jnp.nonzero(x)]

on master gives:
760 ms ± 4.71 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

and in this branch gives:
772 ms ± 7.67 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

this works toward #3614

authors: froystig jakevdp


text:
Levenberg Marquardt with box constraints?

Hello. I am wondering if there is any workarounds in the library to support box constraints during the Levenberg Marquardt optimization. Any guidance would help tremendously. Thanks in advance!

You could try a change of variable, min f(x) s.t. L <= x <= U <-> min f(min(max(x, L), U)), potentially replacing min and max with soft approximations.

As a suggesting, you can use these constraints to/from unit interval. It's faster than using something like sigmoid. https://github.com/Joshuaalbert/jaxns/blob/main/jaxns/internals/constraint_bijections.py#L11

authors: Joshuaalbert N9-Tails mblondel

repo: google/jaxopt


text:
Fixup formatting [phab:D1404]

Created by salguarnieri at 2017-03-02 18:59:50:
Run clang-format on a .h file that has a few lines that are too long.

fromberger accepted this revision at 2017-03-02 19:01:07

salguarnieri closed via commit 05ab996853d0256c76152def916f67a5228f71d1 at 2017-03-02 19:03:57

repo: google/kythe


text:
add more xrefs in object literals [phab:D2120]

Created by evmar at 2018-01-16 22:55:51:
An object literal, of the form
let foo = { bar: 3 };
should define a "foo.bar" variable.
At the AST level, we parse this as
(1) a variable declaration of foo, which
(2) contains an anonymous object literal
So the full name ends up as something like "foo.anon0.bar".
But this is fine as long as the references to foo.bar also use
that name.
In fixing this, it revealed a bug in the function.ts test --
we were conflating the function param named 'num' with the property
named 'num' on the anonymous object within it.  Mark the problem
with a comment for now, as the new behavior is more correct.

evmar wrote at 2018-01-16 22:58:13:
simpler

zarko accepted this revision at 2018-01-16 23:14:57

zarko closed via commit 851e2ba942c029e4904ff20288a95b9919ea133b at 2018-01-17 18:32:48


text:
First attempt at implementing a generalized extraction configuration schema and parsing logic. [phab:D2243]

Created by craigbarber at 2018-04-05 00:22:21:
(This is an incremental CL)
The goal here was to implement an intermediate configuration schema which can be used to generate a customized extraction container tailored for the individual repo. This schema would be the output of both the automated repo examination logic as well as a parser for a separate user friendly schema which could live in the repo itself. In both cases, the common patterns you see in the mvn_config.json file could be stored in a mapping used to generate this intermediate form.

craigbarber wrote at 2018-04-05 00:26:38:

incorporated linter feedback

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
Hmm maybe is there some way to indicate what the format of the output should be?  (Is there some file or webpage or something that dictates the format?)

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
Is this intended to ever run in a directory where we don't have complete control of the contents?  (Could the name collide?)

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
My go is still weak - don't suppose there's a joiner that does this?

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
ah maybe a slightly more descriptive name (or at least not such a generic name) - I had already forgotten what the regex did 6 lines later when reading this

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
why is this (and the ones above) log.Fatalf instead of t.Fatalf?

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
I don't know if we follow style guide that says you must have newline at end of file or not

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
Is it confusing to have both of these files named config_parser.go?  I dunno.

danielmoy-google commented inline (context lost during migration) at 2018-04-05 16:48:32:
Should we be reusing analysis.proto or make a separate proto?

craigbarber wrote at 2018-04-05 17:03:22:

incorporated linter feedback

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
My goal is to make the factor images as orthogonal as possible, however in some situations some cross deps become necessary. Having a mvn/java cross dep seems reasonable in this case IMHO.

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
The goal here is to have this schema be the format outputted by automation which examines the repo teasing out extraction requirements.

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
My thoughts here are to have a separate mapping containing some of these common config patterns. This mapping can be utilized by the automation which examines repo contents as well as the parser which consumes the more user friendly manual config. Both these systems are to follow.

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
In this case this is for a container image we have complete control over. The volumes themselves are basically mount points for I/O when running the container. The names of the volumes are arbitrary and don't correspond with real directories outside of the container. See: https://docs.docker.com/storage/volumes/

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
There is a join, however not one that also provides for the sandwiching of the values within the parens above, that I know of.

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
My understanding of the go convention, (which could be wrong), is log.Fatal is for unexpected errors, whereas t.Fatal is meant for actual test failures.

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
Added for good measure.

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
Yeah I'm not sure what the convention is in Go for naming executables and their corresponding binaries. Will have to defer to our local golang guru for some guidance on that.

craigbarber commented inline (context lost during migration) at 2018-04-05 17:04:23:
Good question, I'm not sure. Would appreciate guidance from others on this as well.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Blank above, please.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Each package should have a package comment, e.g.,
// Package config defines configuration settings for extracting compilation records from
// a repository using the Maven build system.
… etc. as needed …

This becomes the package description in http://godoc.org/, so it's helpful to have this be explanatory.
N.B.: Only one of the files in the package needs to have such a comment.
https://github.com/golang/go/wiki/CodeReviewComments#package-comments

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Protobuf message types should be passed by their pointer. The interesting methods on the generated code are attached to the pointer, not the bare type: config *apb.ExtractionConfigSchema

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Group related values together:
const (
    // DefaultBaseImage is the name of the default base Docker image to use for extraction.
   DefaultBaseImage = "debian:jessie"

   ...
)

Please also make comments into complete sentences, per https://github.com/golang/go/wiki/CodeReviewComments#comment-sentences, rather than JavaDoc-style "name Telegraphic description".

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
I think you could leave off the "Var" suffix here.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Here and throughout: Variable names living in small scopes need not be quite so verbosely-named. I've made some specific suggestions below, but as a rule of thumb try to avoid echoing type information in the variable names.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
A typical naming convention in Go for a constructor-type function is to start with New…. Here, I'd suggest NewImage (or NewExtractionImage if you expect there to be other kinds of images in the config as well).

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
factorImage → image

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
This is fine as written, but could probably be a single top-level string, since all the substituents are constant:
var baseConfig = fmt.Sprintf(`FROM %[1]s
VOLUME %[2]s
ENV %[3]s=%[2]s
VOLUME %[4]s
ENV %[5]s=%[4]s`, DefaultBaseImage, DefaultRepoVolume, RepoVolumeEnv, DefaultOutputVolume, OutputVolumeEnv)

…
fmt.Fprintln(&buf, baseConfig)

Also: If these config values aren't referenced outside the package, they could be unexported (e.g., defaultBaseImage, etc.)

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
imageCopyArtifact → artifact

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
fmt.Errorf("missing source for copy artifact: %s", image.ImageUri)

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
imageEnvVar → env

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
runCmd → cmd

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
I'm not sure why this comment is here, since this is only extracting some bytes from the buffer—was this meant to be part of the function doc?

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
I think we should create a separate proto file for these, rather than putting them in analysis.proto.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Why "Schema" vs., say, "ExtractionConfiguration"?

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
I recommend nesting most of these subordinate messages inside the top-level config message, unless they are going to be used separately.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
I suggest destination or target (but not to abbreviate the field name in this case; it's not that long).

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
The names of these messages should be simplified: If they are at the top-level, they should be generic enough not to reference FactorImage, and if they're specific to a FactorImage they can be nested inside it to avoid the need for repeating the prefix:
Suggestions

FactorImageEnvVar → EnvVar, or even just Env (which is what we used in CompilationUnit, for example)
FactorImageCopyArtifact → CopyArtifact, or even just Copy (since Artifact doesn't really add any information; maybe "Spec" would be better?)
FactorImage → Image (what does "Factor" mean here; these are just ordinary Docker images, no?)

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Let's just use the proto package name here: kythe.proto.ExtractionConfiguration or whatever you wind up calling it.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
extractionConfig → config

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
if err := ioutil.WriteFile(…); err != nil {

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Unexport?

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
This function doesn't load anything.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
filepath.Join(testDataDir, fileName)

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Does this actually work in bazel test? Ordinarily we need to plumb through TEST_SRCDIR to find the root of the test execution path. The Blaze docs should say what the assumptions are.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
In a test, it's OK to have setup functions crash if they don't work, which simplifies the test logic:
func mustLoadConfig(path string) *apb.ExtractionConfiguration {
    data, err := ioutil.ReadFile(filepath.Join(testDir, path))
    if err != nil {
       log.Panicf("Unable to read test file: %v", err)
    }
    …
    return config
}

This isn't kosher for a package API, but is fine in a test where a test setup is a failure you want to catch anyway.

fromberger commented inline (context lost during migration) at 2018-04-05 17:32:26:
Don't echo types in the name, please.
This can also be boosted to the top level:
var multipleNewlines = regexp.MustCompile(`\n{2,}`)

then
multipleNewlines.ReplaceAllString(image, "\n")

fromberger commented inline (context lost during migration) at 2018-04-05 17:45:19:
Oh, another related question: If these are command-lines, should we separate the arguments? If not, maybe include a note in the doc about what the escaping rules are (e.g., "these command lines are passed literally to the docker RUN atom, so values must be escaped appropriately inline")

danielmoy-google commented inline (context lost during migration) at 2018-04-05 17:46:00:
Yea I mean you could make a new type or such thing where its toString surrounds itself with " ", and then join on thing mapped to those, but maybe that's overkill compared to just doing this.

craigbarber wrote at 2018-04-06 21:27:04:

incorporated CL feedback

craigbarber wrote at 2018-04-06 21:30:24:

incorporated CL feedback

craigbarber commented inline (context lost during migration) at 2018-04-06 21:30:41:
The test currently works as is within bazel, however if this will be an issue with blaze and google3, I'm all for taking pro-active measures. Added $TEST_SRCDIR as a path prefix to testDataDir. PTAL

craigbarber commented inline (context lost during migration) at 2018-04-06 21:30:41:
Added logic to escape the args.

craigbarber commented inline (context lost during migration) at 2018-04-06 21:30:41:
Changed all but FactorImage. "Factor" in this case indicates that these images are going to be used as factors which are combined to form a composite image as the output of the this library.

craigbarber commented inline (context lost during migration) at 2018-04-06 21:30:41:
Agreed that is an approach I could take, but yeah it feels like overkill in this case.

danielmoy-google commented inline (context lost during migration) at 2018-04-06 22:09:55:
(nit on no newline)

danielmoy-google commented inline (context lost during migration) at 2018-04-06 22:09:55:
Is it worth linking to some doc on how docker images are set up, or would that not make sense here?  I think it might be worthy to ground some of the terminology (factor image, run, their uri spec, etc)

danielmoy-google commented inline (context lost during migration) at 2018-04-06 22:09:55:
I see this was marked as done, but is it not done?
imageString
expectedImageString
both still using Hungarian notation
I don't remember the original anymore (yay phabricator), so maybe this is fine now?

craigbarber wrote at 2018-04-06 22:29:55:

incorporated CL feedback

craigbarber commented inline (context lost during migration) at 2018-04-06 22:30:12:
Good catch, I had noticed the renaming of the regexp but not the string vars.

craigbarber wrote at 2018-04-09 18:22:28:
Pinging this. Are there any outstanding issues for this?

danielmoy-google wrote at 2018-04-09 21:10:25:
Fine by me, dunno if anyone else still has open concerns.

danielmoy-google accepted this revision at 2018-04-09 21:10:25

danielmoy-google wrote at 2018-04-09 21:11:08:
(I'm also kinda assuming that since this is still WIP, we can address other concerns in near-future subsequent changes, since it'll be awhile before this goes all the way to production)

fromberger commented inline (context lost during migration) at 2018-04-09 21:27:29:
This can be boosted to the package level (so it need only happen once).

fromberger commented inline (context lost during migration) at 2018-04-09 21:27:29:
Actually, do these constants need to be exported from the package at all? If not: defaultBaseImage, defaultRepoVolume, etc.

fromberger commented inline (context lost during migration) at 2018-04-09 21:27:29:
extractionConfig → config

fromberger commented inline (context lost during migration) at 2018-04-09 21:27:29:
What does "Factor" mean in this context? Is there some reason this can't simply be "Image"?

fromberger commented inline (context lost during migration) at 2018-04-09 21:27:29:
Dead?

fromberger commented inline (context lost during migration) at 2018-04-09 21:27:29:
Within a test use the t.* methods. It's OK for a test to have some helper functions that log.Fatal, but generally the stuff in scope of a t should use it.

craigbarber wrote at 2018-04-10 00:34:59:

incorporated CL feedback

craigbarber commented inline (context lost during migration) at 2018-04-10 00:36:25:
If we're able to realize the plan of having either automation logic and/or "user friendly" config logic produce these configs, I think it'll be useful to have these constants exported.

craigbarber commented inline (context lost during migration) at 2018-04-10 00:38:53:
Factor in the composite->factor sense. It's meant to denote that each of these images are meant to be a factor composing the generated custom composite image.

craigbarber wrote at 2018-04-10 00:39:52:

incorporated CL feedback

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
It wouldn't be hard to export them later, if that happens, but YMMV.
A useful trick for that, if you wish:
gofmt -w -r 'DefaultBaseImage->defaultBaseImage' kythe/go/extractors/config/config_parser.go

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
I think if you want the variable expanded you'll need to pass it through http://godoc.org/os#ExpandEnv before calling Open on it.

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
Not quite done—note := here (it happens to work without because there's already an err in the enclosing scope, but the point was not to reuse it)

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
Nothing requires the names to match up, but generally I like to follow the convention that package foo resides in foo.go (assuming it's a single-file package) and that package main resides in binary_name.go (corresponding to whatever the name of the Bazel go_binary rule is called).
It's fine to have them overlap, though since library packages are usually //not// named with underscores, that convention avoids the issue.
None of this is graven in stone, though.

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
If you indent the example a couple extra spaces inside the comment, godoc will format it verbatim.

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
A plea for flowing:
// Binary config_parser provides a tool to parse a configuration file the the
// schema defined in kythe.proto.ExtractionConfiguration. Its output is a
// Dockerfile defining an image which can be used to perform an extraction on a
// repository according to the given configuration.

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
if err := jsonpb.Unmarshal(configFile, &config); err != nil {

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
I see. Do we really need that qualification? I think it's not really adding any information—we have a configuration that refers to some docker images, and their intended use doesn't really seem to matter. (In particular, we don't have FactorImage vs. NonFactorImage for some other purpose—and even if we did, would they be shaped differently?)
On that basis, what do you think about
FactorImage → Image
required_factor_image → required_image

?

fromberger commented inline (context lost during migration) at 2018-04-10 02:03:17:
Please add a newline.

craigbarber wrote at 2018-04-11 20:21:40:

incorporated CL feedback

craigbarber wrote at 2018-04-11 20:22:45:
Feedback incorporated. PTAL

danielmoy-google commented inline (context lost during migration) at 2018-04-11 20:56:37:
^ I think this comment was getting at, we'd rather imports be grouped as:
"bytes"
"fmt"
"kythe.io/some/package/we/own"
"kythe.io/another/package/we/own"
"bitbucket.org/something/we/don't/own"
"google.com/even/stuff/from/el/goog"
apb "kythe.io/some/proto/at/bottom"
opb "github.com/but/also/with/non-kythe/pb"
So you want to reorder:
"bytes"
"bitbucket.org/stuff"
ecb "kythe.io/kythe/stuff"

danielmoy-google commented inline (context lost during migration) at 2018-04-11 20:57:30:
\n

fromberger commented inline (context lost during migration) at 2018-04-11 21:35:08:
log.Fatalf (because you have %v here)

fromberger commented inline (context lost during migration) at 2018-04-11 21:35:08:
extractionImageContent → image

fromberger commented inline (context lost during migration) at 2018-04-11 21:35:09:
Yeah, that. It's changed a few times since I made my original comment, so there's more to do now. :)

fromberger commented inline (context lost during migration) at 2018-04-11 21:35:09:
t.Fatalf

fromberger commented inline (context lost during migration) at 2018-04-11 21:35:09:
t.Fatalf

fromberger commented inline (context lost during migration) at 2018-04-11 21:35:09:
Avoid "assert" style in Go tests (see https://github.com/golang/go/wiki/CodeReviewComments#useful-test-failures for some context).
It's fine to have a function that reports an error that the test can log, but avoid passing the *testing.T value into helper functions that actually report test failures. (Logging stuff is fine)
Better: imagesEqual(got, want []byte) (bool, string) {
if eq, diff := imagesEqual(got, test.want); !eq {
    t.Errorf("Bootstrap images not equal: diff: %s", diff)
}

or words to that effect.

fromberger commented inline (context lost during migration) at 2018-04-11 21:35:09:
This is entirely an optional nit that you should feel free to disregard.
Most Unix binaries are verb-like: ls, cat, pwd, sort, etc. It's nice for reading command lines if ours do that too, instead of naming them like objects. So how about parse_binary_config instead of binary_config_parser. After all, what the user cares about is that the tool does so, rather than being so. :)

craigbarber commented inline (context lost during migration) at 2018-04-11 23:20:31:
Understood. I added the "binary" to the name as I interpreted a previous comment to mean that using the suffix "binary_" was a golang convention in regards to a corresponding library . If that's not the case, then I'd rather ditch the "binary" part entirely as that is not in fact what this program is doing, rather it's parsing a JSON config :)

fromberger commented inline (context lost during migration) at 2018-04-12 00:41:56:
My apologies, I was probably not clear enough in my comment. It's not necessary to include binary_* in the name at all.
There is a convention that the package comment in package main should mention the name of the binary, e.g.,
// Binary parse_config converts a text-format proto config for an extractor into
// wire format in the specified output file.

But that's not meant to constrain what you call the binary itself, it's only meant to ensure the comment winds up being a complete sentence.
So yeah: parse_config or even just parseconfig would be a fine name for this tool if you like that.

craigbarber wrote at 2018-04-12 17:43:44:

incorporated CL feedback

craigbarber commented inline (context lost during migration) at 2018-04-12 17:44:04:
That definitely helps to clear things up. Thanks :)

craigbarber wrote at 2018-04-12 17:44:04:
Incorporated latest feedback. PTAL

fromberger wrote at 2018-04-12 17:47:18:
Thanks, you've been very patient with all these rounds of comments and I'm very grateful for that.

fromberger accepted this revision at 2018-04-12 17:47:18

craigbarber wrote at 2018-04-12 18:20:48:
As scottg is fond of saying, good feedback is a gift. I'm grateful for you all lending me your time and energy on this :)

craigbarber closed via commit c5eadb6ed307db9725d1d48c8435e1dde38efd97 at 2018-04-12 21:59:17


text:
Update LLVM and Clang to deal with upstream API changes [phab:D229]

Created by zarko at 2015-05-05 23:48:30:

schroederc accepted this revision at 2015-05-06 16:06:02

zarko closed via commit 6356be052a0ce4968917eb62883c1494e98d4882 at 2015-05-06 16:40:57


text:
Add a Gitter chat badge to README.md

google/material-design-lite now has a Chat Room on Gitter
@addyosmani has just created a chat room. You can visit it here: https://gitter.im/google/material-design-lite.
This pull-request adds this badge to your README.md:

If my aim is a little off, please let me know.
Happy chatting.
PS: Click here if you would prefer not to receive automatic pull-requests from Gitter in future.

Erm, nope. Thanks.

authors: addyosmani gitter-badger

repo: google/material-design-lite


text:
How to pass in new video packet?

Have I written custom code (as opposed to using a stock example script provided in MediaPipe)
None
OS Platform and Distribution
iOS
MediaPipe Tasks SDK version
No response
Task name (e.g. Image classification, Gesture recognition etc.)
Selfie Segmentation
Programming Language and version (e.g. C++, Python, Java)
Objective C, swift
Describe the actual behavior
Currently the selfie segmentation example graph only have one input stream that takes in the camera input and one output stream that outputs the recolored image.

Describe the expected behaviour
I would like to add a new image/video input stream to the graph. I know how to add it to the graph, however, I am not sure where and how to pass in the image/video input. Could you please provide an example of how to do this? Thank you.

Standalone code/steps you may have used to try to get what you need
I tried to edit the CommonViewController files, but it seems like that wasn't enough. More changes had to be done in the MPPPlayer files?

Other info / Complete Logs
No response

Never mind, I think I figured it out

repo: google/mediapipe


text:
Disambiguate injected Cloud SQL parameter names

This allows us to also inject the BeamJpaModule into RegistryTool, which
allows us to use the SocketJpaTransactionManager in Beam pipelines.
Some side effects of this include:

duplication of KMS connections -- one standard, one Beam
duplication of the creation of the partial Hibernate SQL configs
removal of ambiguity between credentialFileName, credentialFilename,
and credentialFilePath -- we now use the latter.
Performing the credential null check when instantiating the SQL
connection rather than when instantiating the module object. See the code
comments for more details on this.

I verified that this compiles and the tests run successfully when
injecting a @SocketFactoryJpaTm into a Beam pipeline.

This change is

Note: the failing tests are seemingly unrelated to this issue (flaky WebDriver test(?) and javadoc generation) because it succeeds locally

repo: google/nomulus


text:
django_sample issue

Tried following the django sample, upon logging in, django automatically redirects to accounts/profile/ following this line:
https://github.com/google/oauth2client/blob/master/samples/django/django_user/myoauth/urls.py#L28
The sample won't work as expected.

@waprin any thoughts?

Thanks for report, can I see your settings.py config, models you're using, and version of Python and Django?

For settings.py, just the parts relevant to the oauth2 config.

I just downloaded the current master branch and ran the sample code, so its the same as what is available in the repo. Here are the steps to replicate it the issue:
Django Version: 1.10

$ pip install -r requirements.txt
$ python manage.py migrate
$ python mange.py createsuperuser // Create a user
$ python manage.py runserver

Once you start the server, go to http://<server>:8000/login
Login with the user we just created

This should replicate the issue.

Very sorry for delay, was traveling ,will investigate today.

@jayanth1991 sorry again for delay - what did you expect to have happen? I guess I could make the sample redirect the login page to index by default, but really the point of the sample is to demonstrate the oauth decorator. Logging in directly and handling the views after that is not what the sample  are trying to demonstrate. If you look at the views, you see there are ones that are protected by oauth_decorators and which should kick off both the oauth and login flows. What happens after that flow is after you.
http://stackoverflow.com/questions/29082682/django-redirect-after-successful-login
Let me know if I'm misunderstanding your problem and we can re-open the issue, or feel free to ask any more questions.

authors: jayanth1991 jonparrott waprin


text:
BIBD implementation using Python CP-SAT solver: Dot prod of BoolVar list

Hi,
I am new to OR tools and trying to implement BIBD using CP-SAT solver through Python API. As part of the code, have two lists: each of form [model.NewBoolVar("i") for i in range(k)].
The constraint to be specified is that dot product of the two lists has to be a constant. Using model.Add gives an error "Not an Integer Linear expression". Is there a way to implement this.
Thanks,

I think you have to create a intermediate var, for each i in range(k) using
AddProdEquality and then compute its sum
On Sun, 11 Aug 2019, 06:41 Shishir Suman, notifications@github.com wrote:

Hi,
I am new to OR tools and trying to implement BIBD using CP-SAT solver
through Python API. As part of the code, have two lists: each of form
[model.NewBoolVar("i") for i in range(k)].
The constraint to be specified is that dot product of the two lists has to
be a constant. Using model.Add gives an error "Not an Integer Linear
expression". Is there a way to implement this.
Thanks,
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/or-tools/issues/1483?email_source=notifications&email_token=AEWWNCVCAT4JHZEXFTK3VG3QD47Y5A5CNFSM4IK2P5T2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HERXKZA,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEWWNCWUIBVWN4QP7Z4V5F3QD47Y5ANCNFSM4IK2P5TQ
.

AddProdEquality seems to require a target value which will be varying in this case depending on the values.

You have to create k new vars so that AddProdEquality(new[i], [x[i], y[i]])
On Sun, 11 Aug 2019, 06:47 Shishir Suman, notifications@github.com wrote:

AddProdEquality seems to require a target value which will be varying in
this case depending on the values.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/google/or-tools/issues/1483?email_source=notifications&email_token=AEWWNCQAWT3MKCIIDCZXVDDQD5ARBA5CNFSM4IK2P5T2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4AWJWI#issuecomment-520185049,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEWWNCRYXYL5M6WTKXTFDV3QD5ARBANCNFSM4IK2P5TQ
.

Use model.AddMultiplicationEquality
Le sam. 10 août 2019 à 15:41, Shishir Suman notifications@github.com a
écrit :

Hi,
I am new to OR tools and trying to implement BIBD using CP-SAT solver
through Python API. As part of the code, have two lists: each of form
[model.NewBoolVar("i") for i in range(k)].
The constraint to be specified is that dot product of the two lists has to
be a constant. Using model.Add gives an error "Not an Integer Linear
expression". Is there a way to implement this.
Thanks,
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/or-tools/issues/1483?email_source=notifications&email_token=ACUPL3OAEQ454N5335L4STTQD47YZA5CNFSM4IK2P5T2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HERXKZA,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACUPL3IDIQZRKWKKIU5KAW3QD47YZANCNFSM4IK2P5TQ
.

authors: Shishir-Suman lperron stradivari96

repo: google/or-tools


text:
Add top-down/bottom-up inspections

When a user chooses to "Select these callstacks" in the context menu in
top-down/bottom-up tabs, this creates the selection within the tab
itself.  It also adds a banner above to show the user that they are
currently in an inspection and give them the option to leave the
inspection.
The effect on the samples tab will be done in an upcoming PR.
These changes are hidden behind the time_range_selection flag.

I meant to add a screenshot: https://screenshot.googleplex.com/6xgU94Wm8qapqyR

Bug: http://b/170370933

repo: google/orbit


text:
Simpler bucket url access for result_string_search.py

Now we can get the direct URLs to the report, e.g.,
https://llm-exp.oss-fuzz.com/Result-reports/ofg-pr//2024-07-11-465-dg-comparison/sample/output-ada-url-ada_can_parse_with_base/08.html

via:
python helper/result_string_search.py -s "undefined reference" -r 2024-07-11-465-dg-comparison/sample/ -u https://llm-exp.oss-fuzz.com/Result-reports/ofg-pr/

/gcbrun skip

/gcbrun skip

TBR.

repo: google/oss-fuzz-gen


text:
URL Checker

Is there a tool to check if a URL is Physical Web-compatible, particularly within the Chrome implementation?
I have a beacon with endpoint URL https://mobile.twitter.com/search?q=sbincubator and it is not showing on Chrome for Android.
Also, can anyone see why this URL would not show on Chrome for Android?

I think this is because https://twitter.com/robots.txt prevents GoogleBot (used by Physical Web Service) to crawl some URLs such as /search?q=.
@mmocny will know more for sure though

Thanks François. I tried with just https://www.twitter.com and it also does
not work. Does this mean that we won't be able to broadcast any Twitter
URLs?
On Tue, May 31, 2016 at 1:52 PM François Beaufort notifications@github.com
wrote:

I think this is because https://twitter.com/robots.txt prevents GoogleBot
(used by Physical Web Service) to crawl some URLs such as /search?q=.
@mmocny https://github.com/mmocny will know more for sure though
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/google/physical-web/issues/671#issuecomment-222666345,
or mute the thread
https://github.com/notifications/unsubscribe/AG6lvExYm-MfiDcZDMVSiTqYsKRda-s5ks5qHCDogaJpZM4IqU2A
.

--
Regards,
Russell M Morton

You may want to try https://mobile.twitter.com/ ;)

Thanks, that works. I will wait for Michel to confirm, but it seems you are
saying there is no way to get my original URL to work on Chrome for
Android? Interestingly, it works on iOS.
On Tue, May 31, 2016 at 2:08 PM François Beaufort notifications@github.com
wrote:

You may want to try https://mobile.twitter.com/ ;)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/google/physical-web/issues/671#issuecomment-222669550,
or mute the thread
https://github.com/notifications/unsubscribe/AG6lvDhdRnFRJoTbxjUWUxugRC5Qi9Ljks5qHCTYgaJpZM4IqU2A
.

--
Regards,
Russell M Morton

It's because Chrome for iOS and Android implementations differ for now: https://github.com/google/physical-web/blob/master/implementation-status.md#chrome
It will be the same soon though.

@mmocny any further information on how I could get the link in my original post to work?

@russellmorton As @beaufortfrancois says, we respect the website robots.txt file and do not crawl it.
However, looking at the twitter robots file, it looks like they have a way for google to crawl which we are not taking advantage of.  I will investigate if this is not something we can fix server-side, and I can update you when I know more.  (Apologies if this takes some number of days.)
As per the iOS question -- also as @beaufortfrancois says, Chrome for iOS is already updated to provide the same results that Chrome for Android does, but it hasn't shipped to the stable version just yet.  You can expect that in version m52.
Additionally, we have a new team member working on updating the OSS sample apps (those within this repo) to also be more in line with the Google product integrations, so as to not be so confusing.  Today we have a lot of variability in our scanners because of history and experimental variations, and its time to clean up!
Thanks for bringing up the issue.
(Also, sneak peak, I am trying to help @beaufortfrancois build a diagnostic tool to help give you insight into why URLs are failing to resolve.  Will share more when that is ready, but hopefully you won't need a github issue for every question.)
Cheers.

Thanks Michel, I will wait for your feedback on this Twitter robots.txt query.

Guys, we're developing our own forwarder and have experienced an issue. Here's a URL to test the issue:
https://beeem.co/em00004
The really odd part is that the resolved URL (https://beeem.co/p/GB/Bushey/WatfordBathroomandKitchens/watfordbath) shows up in Chrome for Android (see attached screenshots) but not on the nearby list.

It looks like the shortened URL that you've linked to doesn't resolve in the browser either (I see this when I go to the URL directly in Chrome). Are you experiencing this too?

I think you meant https://beeem.co/em000004 (added a zero), which redirects
as expected.
I configured a beacon with the URL and was able to see it in Chrome for
Android and the Today view widget in Chrome for iOS, but not in Nearby.
Other URLs were showing in Chrome but not in Nearby (including
https://www.google.com) so I think this is a Nearby issue and not related
to your URL.
On Fri, Jul 22, 2016 at 11:21 AM, anirudhcmohan notifications@github.com
wrote:

It looks like the shortened URL that you've linked to doesn't resolve in
the browser either (I see this when I go to the URL directly in Chrome).
Are you experiencing this too?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/physical-web/issues/671#issuecomment-234618336,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANckc_lO2pJzQPww3eVx105d_Kk9_Okkks5qYQpFgaJpZM4IqU2A
.

It looks like the metadata service isn't finding a description for your
page and is omitting the field from the returned JSON, which might be
what's tripping up Nearby.  As a workaround, could you try adding a
description to your page?
Example:

On Fri, Jul 22, 2016 at 11:37 AM, Matt Reynolds mattreynolds@google.com
wrote:

I think you meant https://beeem.co/em000004 (added a zero), which
redirects as expected.
I configured a beacon with the URL and was able to see it in Chrome for
Android and the Today view widget in Chrome for iOS, but not in Nearby.
Other URLs were showing in Chrome but not in Nearby (including
https://www.google.com) so I think this is a Nearby issue and not related
to your URL.
On Fri, Jul 22, 2016 at 11:21 AM, anirudhcmohan notifications@github.com
wrote:

It looks like the shortened URL that you've linked to doesn't resolve in
the browser either (I see this when I go to the URL directly in Chrome).
Are you experiencing this too?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/physical-web/issues/671#issuecomment-234618336,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANckc_lO2pJzQPww3eVx105d_Kk9_Okkks5qYQpFgaJpZM4IqU2A
.

Thanks! Will do!

I think some of the confusion stems from the fact that if Nearby takes over beacon discovery some beacon URLs working in the "past" get filtered out in Nearby.

There appears to be a bug in Nearby where websites without a description
are filtered out. This is clearly a bug there there is an easy workaround.
On Tue, Jul 26, 2016 at 10:47 AM, Ferenc Brachmann <notifications@github.com

wrote:

I think some of the confusion stems from the fact that if Nearby takes
over beacon discovery some beacon URLs working in the "past" get filtered
out in Nearby.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/physical-web/issues/671#issuecomment-235348387,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABAbuq8iPgfdIQ1vkZdBcgohgmekTVP0ks5qZkhCgaJpZM4IqU2A
.

you can now check URLs against the PWS by going here: verify.physical-web.org

Awesome!! Any chance of a Chrome extension or something, so sites behind a firewall can be checked? Would that even make sense, or can only public-facing sites be broadcast? That would be unfortunate as it cuts off a broad swath of potential use cases..

The whole point of the Physical Web Service is to cache and vet websites we're showing to users. We are, structurally, just like google.com. Your request is much like asking of Google.com can show sites behind a firewall. I get why you'd like to do this of course.
If you are willing to make people wait just a bit, you could take them to a public landing page and then use a JS redirect into your page. Not perfect but you could try things out.

Fair enough. I'll have to experiment, but won't the url verifier try to follow the redirect and then 404? Is there a timeout value that would be safe?

Unfortunately, there is no safe value, this is a bit of a hack you can use to try something out. If it isn't too much trouble, I'd just put up a public landing page and have people click through. I realize this isn't perfect but you could get that working 100% right now.

authors: anirudhcmohan beaufortfrancois ferencbrachmann jugaltheshah mmocny nondebug russellmorton scottjenson

repo: google/physical-web


text:
Fix broken run.py script

Delimiter was missing following the seed argument that was added in
run.py a couple of commits back.

I have uploaded the CLA. Please rescan.

repo: google/riscv-dv


text:
Replace manually defined links in the doc

Description
Fixes #3260
Type of change

[ ] New feature (non-breaking change which adds functionality)
[ ] This change requires a documentation update

Checklist:

[ ] I have signed the Google CLA https://cla.developers.google.com
[ ] My code follows the style guidelines of this project
[ ] I have made corresponding changes to the documentation
[ ] I have verified my change on multiple browsers on different platforms
[ ] I have run ./build/all.py and the build passes

@googlebot I consent.

As discussed here(https://github.com/google/shaka-player/pull/3278#issuecomment-806025506) I have updated link pointing to docs.

Updated all the changes mam.

repo: google/shaka-player


text:
Pirate question

Initial check in of Pirate question. Note that I checked with Brad and he confirmed that punctuation and case are not relevant to this question.
I unfortunately got this code and the findMostCommonRepeatedCharacter code together in this branch.
Please review only the pirate stuff but tips on how to separate out the other question are appreciated.

Trying again on a clean branch

repo: google/tie


text:
GCS failures with latest version

Hopefully just needs a minor refactor.  Note that PSQ also uses Datastore (transparently) to return task results.
It's broken with:
gapic-google-cloud-datastore-v1 (0.15.3)
google-cloud-datastore (1.4.0)
proto-google-cloud-datastore-v1 (0.90.4)
But works with:
gapic-google-cloud-datastore-v1 (0.15.3)
google-cloud-datastore (1.0.0)
grpc-google-cloud-datastore-v1 (0.14.0)
proto-google-cloud-datastore-v1 (0.90.4)
[INFO] Task failed with exception: [request() got an unexpected keyword argument 'data']
[[INFO] Writing /var/tmp/1511642221-817a08dc1d334e4587df1739da220996-PlasoTask/worker-log.txt to GCS path output/1511642221-817a08dc1d334e4587df1739da220996-PlasoTask/worker-log.txt
DEBUG: traceback.format_exc(): Traceback (most recent call last):
File "/usr/local/google/home/aaronpeterson/src/turbinia/turbinia/workers/init.py", line 312, in run_wrapper
result = self.run(evidence, result)
File "/usr/local/google/home/aaronpeterson/src/turbinia/turbinia/workers/plaso.py", line 49, in run
ret, result = self.execute(cmd, result, save_files=[plaso_log], close=True)
File "/usr/local/google/home/aaronpeterson/src/turbinia/turbinia/workers/init.py", line 257, in execute
result.close(success=True)
File "/usr/local/google/home/aaronpeterson/src/turbinia/turbinia/workers/init.py", line 114, in close
self.save_local_file(logfile)
File "/usr/local/google/home/aaronpeterson/src/turbinia/turbinia/workers/init.py", line 177, in save_local_file
new_path = writer.write(file_)
File "/usr/local/google/home/aaronpeterson/src/turbinia/turbinia/output_writers.py", line 176, in write
blob.upload_from_filename(file_, client=self.client)
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google/cloud/storage/blob.py", line 992, in upload_from_filename
size=total_bytes)
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google/cloud/storage/blob.py", line 946, in upload_from_file
client, file_obj, content_type, size, num_retries)
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google/cloud/storage/blob.py", line 867, in _do_upload
client, stream, content_type, size, num_retries)
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google/cloud/storage/blob.py", line 700, in _do_multipart_upload
transport, data, object_metadata, content_type)
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google/resumable_media/requests/upload.py", line 97, in transmit
retry_strategy=self._retry_strategy)
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google/resumable_media/requests/_helpers.py", line 101, in http_request
func, RequestsMixin._get_status_code, retry_strategy)
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google/resumable_media/_helpers.py", line 146, in wait_and_retry
response = func()
File "/usr/local/google/home/aaronpeterson/src/turbinia-old-pubsub-env/local/lib/python2.7/site-packages/google_auth_httplib2.py", line 198, in request
uri, method, body=body, headers=request_headers, **kwargs)
TypeError: request() got an unexpected keyword argument 'data'
[INFO] Task failed with exception: [request() got an unexpected keyword argument 'data']

Probably related to:
https://github.com/GoogleCloudPlatform/google-cloud-python/issues/3736

FWIW, same error with 1.2.0

Turns out this is likely only GCS (I guess I had confused some of the datastore package names), and the packages that are (were?) having dependency problems are:
google-cloud
google-cloud-core
google-cloud-storage
requests
Hopefully we can resolve these issues without breaking our pinning of the google-cloud-pubsub package to 0.26.

Stub script to reproduce this:
"""
#!/usr/bin/env python
from google.cloud import storage
file_ = '/tmp/test.txt'
open(file_, 'w').write('testing')
c = storage.Client(project='MYPROJECT')
b = c.get_bucket('MYBUCKET')
blob = storage.Blob('output/test.txt', b)
blob.upload_from_filename(file_)
"""

So, it appears that pinning the pubsub version is what is causing problems (indirectly).  When removing the pubsub pinning, the above code runs just fine.  Below is the pip freeze diff between two virtualenvs setup with the only difference being the pubsub version being pinned to 0.26.  I think the google-cloud-core package is the issue, and is mentioned in the bug referenced above.
$ diff ~/pip.head.freeze.txt ~/pip.pubsub-0.26.freeze.txt
24a26

google-auth-httplib2==0.0.3
28c30
< google-cloud-core==0.28.0

google-cloud-core==0.25.0
36c38
< google-cloud-pubsub==0.29.2

google-cloud-pubsub==0.26.0

Ugh, now after pinning google-cloud-core>=0.28.0, something is wrong with psq.  Looks like something in the pubsub library?  It seems to be the correct version though:
"""
$ pip list | grep google-cloud
gapic-google-cloud-datastore-v1 (0.15.3)
gapic-google-cloud-error-reporting-v1beta1 (0.15.3)
gapic-google-cloud-logging-v2 (0.91.3)
gapic-google-cloud-pubsub-v1 (0.15.4)
google-cloud (0.31.0)
google-cloud-bigquery (0.28.0)
google-cloud-bigtable (0.28.1)
google-cloud-core (0.28.0)
google-cloud-datastore (1.4.0)
google-cloud-dns (0.28.0)
google-cloud-error-reporting (0.28.0)
google-cloud-firestore (0.28.0)
google-cloud-language (1.0.0)
google-cloud-logging (1.4.0)
google-cloud-monitoring (0.28.0)
google-cloud-pubsub (0.26.0)
google-cloud-resource-manager (0.28.0)
google-cloud-runtimeconfig (0.28.0)
google-cloud-spanner (0.29.0)
google-cloud-speech (0.30.0)
google-cloud-storage (1.6.0)
google-cloud-trace (0.16.0)
google-cloud-translate (1.3.0)
google-cloud-videointelligence (1.0.0)
google-cloud-vision (0.28.0)
proto-google-cloud-datastore-v1 (0.90.4)
proto-google-cloud-error-reporting-v1beta1 (0.15.3)
proto-google-cloud-logging-v2 (0.91.3)
proto-google-cloud-pubsub-v1 (0.15.4)
$ pip list | grep requests
requests (2.18.4)
$ python -c 'import psq'
Traceback (most recent call last):
File "", line 1, in
File "/usr/local/buildtools/current/sitecustomize/sitecustomize.py", line 152, in SetupPathsAndImport
return real_import(name, globals, locals, fromlist, level)
File "/lib/python2.7/site-packages/psq/init.py", line 24, in
from .broadcast_queue import BroadcastQueue
File "lib/python2.7/site-packages/psq/broadcast_queue.py", line 18, in
from google.cloud import pubsub
File "lib/python2.7/site-packages/google/cloud/pubsub/init.py", line 30, in
from google.cloud.pubsub.client import Client
File "lib/python2.7/site-packages/google/cloud/pubsub/client.py", line 21, in
from google.cloud.pubsub._http import Connection
File "lib/python2.7/site-packages/google/cloud/pubsub/_http.py", line 25, in
from google.cloud.iterator import HTTPIterator
ImportError: No module named iterator
"""

Ok, it seems like this file got moved in this PR:
https://github.com/GoogleCloudPlatform/google-cloud-python/pull/3770
And then the version is updated to 0.27.0, so I guess I need to try to pin 0.26.0.  It breaks with 0.25.0 though, so I don't have high hopes.

repo: google/turbinia


text:
Redesign: Update tabs within the 3rd party Media panel

Feature Description
See Figma

[x] Redesign the tabs in the 3rd party media panel (Images, Video, GIFs).
[x] Make sure not to leave any unused components/styles to the codebase.

Implementation Brief

Verified in QA

authors: csossi miina

repo: google/web-stories-wp


text:
Provide filename instead of diff to stdout

When integrating this into our repo we run a wrapped yamlfmt as part of a suite of misc checks. In our case the diff piped to stdout can be quite large and not very interesting in my opinion, the more helpful approach in our case would be to just provide the path of the files with incorrect formatting. Would it be possible to add this behavior under some flag?

I didn't consider this use case, seems like a good feature. I will try and get something for this in the next release.

@mnil I merged #90 which adds a -quiet flag. I am just working on one more thing before releasing 0.8.0, but you can pull github.com/google/yamlfmt@main to try the feature early. Thank you for the feature idea!

thanks, I will try it out!

authors: braydonk mnil

repo: google/yamlfmt


text:
Add CMake script

CMake is a cross-platform build system generator that is "not too bad". It is used by projects like libpng, mozjpeg, and zlib.
This CMake script works similar to the Makefile included, and was tested with GCC and Clang on Linux, and with mingw-w64 and Visual C++ 2013 on Windows.
Typical use:
mkdir build
cd build
cmake ..
cmake --build .

Building shared libraries with MSVC does not work. Unlike GCC, MSVC does not export global symbols, so when building shared libraries with MSVC, the resulting dll files have no exports.
One solution would be to add a conditional macro to add __declspec(dllexport) to all symbols that need to be exported in the header files.

Yes please. Zopfli is the last library in my particular project that still requires a ./bootstrap step before I build with CMake.

+1

@jibsen why close this? It's perfectly valid, and Google should be pushing for more interoperable building since native build systems have been a mess for decades.

I was cleaning up my open pull requests, and this one had been open for three years, so I figured there was no interest from the maintainers. Also, I think it would need an update to work with the current Zopfli version, plus it should have an install option added. Today, I might have gone with Meson instead, or at least a more modern CMake version.
I agree it would be nice if more Google projects supported a cross platform build generator (which isn't Bazel).

Better late than never, merged it in. Thank you for making this script!

Oh, thanks!

authors: Qix- jibsen lvandeve rogersachan

repo: google/zopfli


text:
App Open Ads doesn't take the full screen when another view is presented

App Open Ad Doesn't Occupy the Full Screen When an Overlay View is Present
Issue
When attempting to present an App Open Ad with a nil value for the root view controller, the ad does not occupy the full screen as expected.
Example:
appOpenAd.present(fromRootViewController: nil)

This issue occurs when another view, such as an overlay, is already being presented.

Use Case
This problem is particularly evident when using In-App Messaging through OneSignal. If an In-App Message is displayed as an overlay and you attempt to present the App Open Ad simultaneously, the ad only occupies the area within the overlay instead of taking up the entire screen as intended.
Currently Working Solution
Passing a specific view controller to the present(fromRootViewController:) method resolves this issue. The following code snippet demonstrates a workaround:
if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene {
    if let rootViewController = windowScene.windows.first?.rootViewController {
        appOpenAd.present(fromRootViewController: rootViewController)
    }
}

This ensures that the App Open Ad is presented over the entire screen, even when an overlay is present.

Hi @sheikhbayazid, can you a create a new thread in the AdMob Developers Forum? This issue is related to the ad implementation which the Swift Package Manager doesn't control. From there our support team can begin the troubleshooting process
Closing this out - thanks!


text:
Regenerate AndroidPublisher client

This PR was generated using Autosynth. :rainbow:
Here's the log from Synthtool:
synthtool > Executing /tmpfs/src/git/autosynth/working_repo/synth.py.
synthtool > Cloning https://github.com/googleapis/elixir-google-api.git.
synthtool > Running: docker run --rm -v/home/kbuilder/.cache/synthtool/elixir-google-api:/workspace -v/var/run/docker.sock:/var/run/docker.sock -w /workspace gcr.io/cloud-devrel-public-resources/elixir16 scripts/generate_client.sh AndroidPublisher
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/tmpfs/src/git/autosynth/env/lib/python3.6/site-packages/synthtool/metadata.py", line 63, in write
    with open(outfile, "w") as fh:
FileNotFoundError: [Errno 2] No such file or directory: 'clients/AndroidPublisher/synth.metadata'

Not sure why this didn't bump the versions

repo: googleapis/elixir-google-api


text:
FlowContorller for new v2.Batcher

This is a part of series of PR for v2/Batcher, and following up with #744
What this PR contains
This PR introduces FlowContorller in v2.BatchingSettings, Whose interface is copied over from existing FlowController.
Note: The implementation is still under discussion.
cc: @igorbernstein2 @kolea2

Change of plan: using existing

repo: googleapis/gax-java


text:
Fix: Uncaught Runtime Exception: Unable to read from stream

[Fri Mar 17 20:23:17.946922 2023] [php:error] [pid 1703] [client 172.17.0.1:53962] PHP Fatal error:  Uncaught RuntimeException: Unable to read from stream in /var/www/html/vendor/guzzlehttp/psr7/src/Stream.php:237
Stack trace:
#0 /var/www/html/vendor/guzzlehttp/psr7/src/Utils.php(53): GuzzleHttp\\Psr7\\Stream->read()
#1 /var/www/html/vendor/guzzlehttp/guzzle/src/Handler/StreamHandler.php(210): GuzzleHttp\\Psr7\\Utils::copyToStream()
#2 /var/www/html/vendor/guzzlehttp/guzzle/src/Handler/StreamHandler.php(140): GuzzleHttp\\Handler\\StreamHandler->drain()
#3 /var/www/html/vendor/guzzlehttp/guzzle/src/Handler/StreamHandler.php(56): GuzzleHttp\\Handler\\StreamHandler->createResponse()
#4 /var/www/html/vendor/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php(35): GuzzleHttp\\Handler\\StreamHandler->__invoke()
#5 /var/www/html/vendor/guzzlehttp/guzzle/src/Middleware.php(31): GuzzleHttp\\PrepareBodyMiddleware->__invoke()
#6 /var/www/html/vendor/guzzlehttp/guzzle/src/RedirectMiddleware.php(71): GuzzleHttp\\Middleware::GuzzleHttp\\{closure}()
#7 /var/www/html/vendor/guzzlehttp/guzzle/src/Middleware.php(61): GuzzleHttp\\RedirectMiddleware->__invoke()
#8 /var/www/html/vendor/guzzlehttp/guzzle/src/HandlerStack.php(75): GuzzleHttp\\Middleware::GuzzleHttp\\{closure}()
#9 /var/www/html/vendor/guzzlehttp/guzzle/src/Client.php(331): GuzzleHttp\\HandlerStack->__invoke()
#10 /var/www/html/vendor/guzzlehttp/guzzle/src/Client.php(168): GuzzleHttp\\Client->transfer()
#11 /var/www/html/vendor/guzzlehttp/guzzle/src/Client.php(187): GuzzleHttp\\Client->requestAsync()
#12 /var/www/html/vendor/guzzlehttp/guzzle/src/ClientTrait.php(44): GuzzleHttp\\Client->request()
#13 /var/www/html/vendor/google/apiclient/src/AccessToken/Verify.php(173): GuzzleHttp\\Client->get()
#14 /var/www/html/vendor/google/apiclient/src/AccessToken/Verify.php(201): Google\\AccessToken\\Verify->retrieveCertsFromLocation()
#15 /var/www/html/vendor/google/apiclient/src/AccessToken/Verify.php(105): Google\\AccessToken\\Verify->getFederatedSignOnCerts()
#16 /var/www/html/vendor/google/apiclient/src/Client.php(815): Google\\AccessToken\\Verify->verifyIdToken()
#17 /var/www/html/tryout-google-auth/web/idtoken.php(179): Google\\Client->verifyIdToken()
#18 {main}

Next GuzzleHttp\\Exception\\RequestException: Unable to read from stream in /var/www/html/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php:52
Stack trace:
#0 /var/www/html/vendor/guzzlehttp/guzzle/src/Handler/StreamHandler.php(74): GuzzleHttp\\Exception\\RequestException::wrapException()
#1 /var/www/html/vendor/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php(35): GuzzleHttp\\Handler\\StreamHandler->__invoke()
#2 /var/www/html/vendor/guzzlehttp/guzzle/src/Middleware.php(31): GuzzleHttp\\PrepareBodyMiddleware->__invoke()
#3 /var/www/html/vendor/guzzlehttp/guzzle/src/RedirectMiddleware.php(71): GuzzleHttp\\Middleware::GuzzleHttp\\{closure}()
#4 /var/www/html/vendor/guzzlehttp/guzzle/src/Middleware.php(61): GuzzleHttp\\RedirectMiddleware->__invoke()
#5 /var/www/html/vendor/guzzlehttp/guzzle/src/HandlerStack.php(75): GuzzleHttp\\Middleware::GuzzleHttp\\{closure}()
#6 /var/www/html/vendor/guzzlehttp/guzzle/src/Client.php(331): GuzzleHttp\\HandlerStack->__invoke()
#7 /var/www/html/vendor/guzzlehttp/guzzle/src/Client.php(168): GuzzleHttp\\Client->transfer()
#8 /var/www/html/vendor/guzzlehttp/guzzle/src/Client.php(187): GuzzleHttp\\Client->requestAsync()
#9 /var/www/html/vendor/guzzlehttp/guzzle/src/ClientTrait.php(44): GuzzleHttp\\Client->request()
#10 /var/www/html/vendor/google/apiclient/src/AccessToken/Verify.php(173): GuzzleHttp\\Client->get()
#11 /var/www/html/vendor/google/apiclient/src/AccessToken/Verify.php(201): Google\\AccessToken\\Verify->retrieveCertsFromLocation()
#12 /var/www/html/vendor/google/apiclient/src/AccessToken/Verify.php(105): Google\\AccessToken\\Verify->getFederatedSignOnCerts()
#13 /var/www/html/vendor/google/apiclient/src/Client.php(815): Google\\AccessToken\\Verify->verifyIdToken()
#14 /var/www/html/tryout-google-auth/web/idtoken.php(179): Google\\Client->verifyIdToken()
#15 {main}
  thrown in /var/www/html/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php on line 52

We default to not using streams. To use streams in guzzle and override our default, you can do the following:
$googleClient = new Google\Client();
$googleClient->setHttpClient(new GuzzleHttp\Client(['stream' => true]));

Thanks for the feedback and will use the suggested workaround.
FYI, I noticed that the Guzzle behaviour is to make an asynch call  (see trace snippet below) and that means it will open a stream, or at least that is my understanding. So if the default should be not to use Streams than this does not seem to line up.

In the Guzzle Documentation there seems to be an explanation why guzzle switches to async mode. It says Guzzle will use the PHP stream wrapper to send HTTP requests if cURL is not installed.. Worth considering this in how the Google Client works.
Thanks.

We do not decide the default here, Guzzle does. I don't know why Guzzle doesn't default to streams, but for whichever reason they don't is the same reason we don't.

repo: googleapis/google-api-php-client


text:
Cloud Functions - GoogleAuth.getIdTokenClient() reports Error: Could not load the default credentials. Browse to https://cloud.google.com/docs/authentication/getting-started for more information.

Thanks for stopping by to let us know something could be better!
PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Is this a client library issue or a product issue?
client library

Did someone already solve this?
I have followed the thread https://github.com/googleapis/google-auth-library-nodejs/issues/798 and trying the recommendation https://github.com/googleapis/google-auth-library-nodejs/issues/798#issuecomment-591622283 with the below code already

Do you have a support contract?
no

If the support paths suggested above still do not result in a resolution, please provide the following details.
Environment details

OS:
Node.js version: 8
npm version:
google-auth-library version: latest - "google-auth-library": "^6.1.3",

Steps to reproduce

Cloud Function doing the following inside the async function (specific one I am modifying is https://github.com/GoogleCloudPlatform/docker-clamav-malware-scanner/blob/4d020bdf24219dd194ab0b9a30c2b6cd16a7f8bb/function-scantrigger-node/index.js#L27 to invoke IAP Protected Resource)

const url = process.env.SCAN_SERVICE_URL;
const targetAudience = '<abracadabra>.apps.googleusercontent.com';

const {GoogleAuth} = require('google-auth-library');
const auth = new GoogleAuth();

console.info(`request IAP**** ${url} with target audience**** ${targetAudience}`);
const client = await auth.getIdTokenClient(targetAudience);

Making sure to follow these steps will guarantee the quickest resolution possible.
Thanks!

Logs
request IAP**** https://gb-plat-poc-spaces-pt-poc-v3.ts.r.appspot.com/scan with target audience**** .apps.googleusercontent.com
Error: Could not load the default credentials. Browse to https://cloud.google.com/docs/authentication/getting-started for more information. at GoogleAuth.getApplicationDefaultAsync (/srv/node_modules/google-auth-library/build/src/auth/googleauth.js:157:19) at  at process._tickDomainCallback (internal/process/next_tick.js:229:7)
(node:2) MetadataLookupWarning: received unexpected error = URL is not defined code = UNKNOWN

Hi @pankajmt, would you be able to set an environment variable to get your service account credentials? Or, could you set your  service account when you create a new instance, like so:
const {Storage} = require('@google-cloud/storage');

// Instantiates a client. If you don't specify credentials when constructing
// the client, the client library will look for credentials in the
// environment.
const storage = new Storage({
   keyfile: <pathToYourServiceaccount>
});

Could you also clarify what endpoint you're calling with this? SCAN_SERVICE_URL
Thanks!

would you be able to set an environment variable to get your service account credentials
but the cloud function already runs as a service account and should have the credentials available at runtime for the library to use? https://cloud.google.com/functions/docs/securing/function-identity - when making requests to Google Cloud Platform services using the Google Cloud Client Libraries, Cloud Functions can automatically obtain and use tokens to authorize the services this identity has permissions to use.

@pankajmt could you share the source of your Cloud Function in a gist?

Sure.
So its this code https://github.com/GoogleCloudPlatform/docker-clamav-malware-scanner/blob/4d020bdf24219dd194ab0b9a30c2b6cd16a7f8bb/function-scantrigger-node/index.js being modified to work with an Identity-Aware Proxy protected Public App Engine Endpoint. "CHANGES" below is where I am trying to do what is documented here and here
`
/*

Copyright 2019 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");

you may not use this file except in compliance with the License.

You may obtain a copy of the License at

https://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software

distributed under the License is distributed on an "AS IS" BASIS,

WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

See the License for the specific language governing permissions and

limitations under the License.
*/

const request = require('request-promise');
/**

Background Cloud Function that handles the 'google.storage.object.finalize'
event. It invokes the Malware Scanner service running in App Engine Flex
requesting a scan for the uploaded document.

@param {object} data The event payload.
@param {object} context The event metadata.
*/
exports.requestMalwareScan = async (data, context) => {

const file = data;
console.log(  Event ${context.eventId});
console.log(  Event Type: ${context.eventType});
console.log(  Bucket: ${file.bucket});
console.log(  File: ${file.name});
let options = {
method: 'POST',
uri: process.env.SCAN_SERVICE_URL,
body: {
location: gs://${file.bucket}/${file.name},
filename: file.name,
bucketname: file.bucket
},
json: true
}
// CHANGES
const url = process.env.SCAN_SERVICE_URL;
const targetAudience = 'hidden.apps.googleusercontent.com';
const {GoogleAuth} = require('google-auth-library');
const auth = new GoogleAuth();
console.info(request IAP**** ${url} with target audience**** ${targetAudience});
const client = await auth.getIdTokenClient(targetAudience);
console.info(1);
const res = await client.request({url});
console.info(2);
console.info(res.data);
try {
if(context.eventType === "google.storage.object.finalize") {
await request(options);
console.log(Malware scan succeeded for: ${file.name});
} else {
console.log('Malware scanning is only invoked when documents are uploaded or updated');
}
} catch(e) {
console.error(Error occurred while scanning ${file.name}, e);
}
}
`

Any thoughts on this one?

Any thoughts on this one?

@pankajmt thank you for providing the example, nothing is immediately jumping out at me. We'll do our best to reproduce on our end, and will keep this thread updated.

@pankajmt thank you for providing the example, nothing is immediately jumping out at me. We'll do our best to reproduce on our end, and will keep this thread updated.

Thanks!
On Fri, Jan 8, 2021 at 3:54 PM Benjamin E. Coe notifications@github.com
wrote:

@pankajmt https://github.com/pankajmt thank you for providing the
example, nothing is immediately jumping out at me. We'll do our best to
reproduce on our end, and will keep this thread updated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/googleapis/google-auth-library-nodejs/issues/1107#issuecomment-756545500,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AHVJHSVZUQCFBKRMZCNEVG3SY2FZTANCNFSM4U65VQXQ
.

--
पंकज एम तोलानी

Thanks!
On Fri, Jan 8, 2021 at 3:54 PM Benjamin E. Coe notifications@github.com
wrote:

@pankajmt https://github.com/pankajmt thank you for providing the
example, nothing is immediately jumping out at me. We'll do our best to
reproduce on our end, and will keep this thread updated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/googleapis/google-auth-library-nodejs/issues/1107#issuecomment-756545500,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AHVJHSVZUQCFBKRMZCNEVG3SY2FZTANCNFSM4U65VQXQ
.

--
पंकज एम तोलानी

This issue will fall OoSLO soon.

Hi @pankajmt,
When recreating your sample, I noticed that you weren't importing @google-auth-library in your dependencies. When recreating just the following of your code snippet, I was able to successfully print out the client object, like so:
 exports.bug = async(req, res) => {
    const targetAudience = 'hidden.apps.googleusercontent.com';

    const {GoogleAuth} = require('google-auth-library');
    const auth = new GoogleAuth();

    const client = await auth.getIdTokenClient(targetAudience);
    console.log(JSON.stringify(client));
  };

With at least these dependencies in your package.json:
  "dependencies": {
    "google-auth-library": "^7.1.2"
  }

Please do follow up if this doesn't resolve your issue.

Closing for now as I'm pretty sure that was the issue, but feel free to reopen @pankajmt

authors: SurferJeffAtGoogle bcoe pankajmt sofisl

repo: googleapis/google-auth-library-nodejs