Request Twitter email

#436

@samtstern The twitter tests are failing because we are waiting for a callback that will never come (the ok from the user in twitter's request email activity). Because of that, I don't think it's possible to test the TwitterProvider, but it also doesn't look like we really need the tests: we manually call twitterProvider.success(result); and don't check anything after that:
@Override
public void onSuccess(IdpResponse idpResponse) {
    assertTrue(mAssertSuccess);
    mCountDownLatch.countDown();
}

Is it ok for me to remove the test? Or how could I get it to work with requestEmail?
Here's the test in question: https://github.com/firebase/FirebaseUI-Android/blob/master/auth/src/test/java/com/firebase/ui/auth/ui/provider/TwitterProviderTest.java#L68

OK I just checked out this PR at the latest commit and tried a number of situations:

Twitter account that has email

When collision, it linked properly
When I signed in and clicked "deny" on the Twitter email screen FirebaseUI still seemed to get my email.  Deny and allow behaved the same for this account.

Twitter account that only has phone number

The allow/deny email screen still showed up
No matter which one I clicked, I obviously could not get an email from Twitter

So now I am wondering: when does this request email screen ever do anything?  What sort of account setup would result in Firebase Auth only getting the email when that screen is presented.

When I signed in and clicked "deny" on the Twitter email screen FirebaseUI still seemed to get my email. Deny and allow behaved the same for this account.

@samtstern do you mean the welcome back screen still showed up? The account linking/welcome back screen isn't showing up for me when I press deny on Twitter's email screen. If account linking didn't occur, but FirebaseUser#getEmail() still works, that's expected behavior because firebase auth is getting the Twitter email some other way.

The allow/deny email screen still shows up if the twitter user doesn't have an email

This is working according to Twitter's documentation: we have to show the email screen to know if the Twitter user has an email.

What sort of account setup would result in Firebase Auth only getting the email when that screen is presented.

That was one of the reasons I chose to use the retrofit hack, we can't. Firebase auth doesn't care what we do because they get the Twitter email some other way.
@samtstern Do you want to proceed with this PR or go back to the retrofit hack? Twitter does want us to use their request email activity, but it seems redundant in most cases because Firebase Auth gets the email anyway.

@SUPERCILEX sorry maybe I am confused but I think my question is why do we need either the retrofit hack or the request email screen?  It seems to me that Firebase Auth is already going to get us the email address whenever possible and all we need to do in FirebaseUI is handle the case where twitter email is null (which you've done)

@samtstern Now I'm all confused too! 😄 From what I understand, we need the email in IdpResponse because of this code in CredentialSignInHandler:
if (!task.isSuccessful() && task.getException() instanceof FirebaseAuthUserCollisionException) {
    // At this point, the user isn't signed in right?
    mHelper.getFirebaseAuth()
            // If the user isn't signed in, this will always fail whether or not the Twitter user has an email because we can't use FirebaseUser#getEmail() yet.
            .fetchProvidersForEmail(mIdpResponse.getEmail())
            // ...
}

@samtstern For account linking, we need the email from IdpResponse before the user is logged in, right?

@SUPERCILEX I think the account linking cases is not relevant here, right?  Would our flow ever initiate account linking for an account with no email?

@samtstern Well no, but this PR was originally created to handle the case where a Twitter user with an email tries to link their account: user creates email account -> user tries to sign into Twitter with the same email they created their email account with -> we need the email before the user is signed in to perform account linking.

I finally get it!  Your last comment turned on the lightbulb for me.  I'm going to summarize below just in case I forget again:

User signs up with email<EMAIL_ADDRESS>using either email/pw or some non-Twitter IDP
User tries to sign up with a Twitter account with that email.
We acquire a Twitter auth credential.  This does not have the email address so we can't check for providers.
We launch into the sign in flow and get a FirebaseAuthUserCollisionException because the Firebase auth backend used our Twitter token to get the email address and saw a collision that we didn't see.

But with your change we have:
3a. Use the twitter SDK to request the email and check for collisions, launching the welcome back flow if necessary.
Now that I finally understand the edge case here I think the way you are doing it now (launching the twitter email screen) is the correct approach.

@samtstern do you want to wait for your email refactor to go in before merging this PR or are there a few changes that still need to be addressed before merging?

Nope this LGTM!

Awesome!


Include new material icons in asset studio

This commit added a bunch of new icons, but they are not in Android Asset Studio yet.
I am uploading all the svg production icons into this Google Drive folder, if that proves useful. I renamed them according to the naming conventions the asset studio currently uses.

Updated


[0.29.x] Add support of const fused type memory views (GH-3118)

Cherry-pick candidate for 0.29.x - the actual change outside the tests is really pretty tiny. Would fix #3783

Nice! I’ve been looking forward to using these

Thank you a lot for back-porting this fix to 0.29.X. This will ease scikit-learn's Cython implementations and maintenance significantly. :100:


Alternative citation for pre-DOI downloads

For downloads from before we started issuing DOIs the citation should probably include a link to the download page rather than "https://doi.org/"
e.g. https://www.gbif.org/occurrence/download/0001232-150116162929234

I'm not sure I understand the example:

Sorry, my mistake. I assigned a DOI to this example after creating this issue.
Here's another one: https://www.gbif.org/occurrence/download/0000859-140827132448942

To ensure that people actually do cite when using an old download, could we just include the url of the download page itself in the citation—in the cases when no DOI exists?
So basically:

Citation: GBIF.org (31 August 2014) GBIF Occurrence Download https://www.gbif.org/occurrence/download/0000859-140827132448942

(they'll still be encouraged to contact us for a DOI, but I'd rather they cite something identifiable than simple "GBIF.org")

closed by https://github.com/gbif/portal16/commit/c6054fb954e7f1baa292f7a17e71426a93ac62fb


Quick fix beam-integration-benchmark

FLAGS.beam_it_options is optional currently. Exception will through if a beam benchmark job doesn't specify this flag.
In the future, we want to migrate to FLAGS.beam_it_options from FLAGS.beam_it_args.
Test is done by running beam_integration_benchmark on GCP and pipeline can be submitted successfully.
+R: @asaksena
+cc: @ssisk

LGTM, sorry I didn't catch that.

ping @asaksena

LGTM


Minor README correction: wiki link for google spreadsheets.

It linked to the google maps page before the correction.
Thank you for the time you are putting into AlaSQL. Really appreciate it.

Codecov Report

Merging #928 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #928   +/-   ##
========================================
  Coverage    67.99%   67.99%
========================================
  Files            1        1
  Lines         9915     9915
  Branches      2965     2965
========================================
  Hits          6742     6742
  Misses        3173     3173

Continue to review full report at Codecov.

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

Awesome! - thanks :)


Stop using Astropy test runner. Use pytest-runner instead

The main motivation for this is that the Astropy test runner was no longer running schema tests. I have no interest in making this work. In fact, I'd ultimately like to deprecate the setup.py test option entirely, but for now this seems like a good compromise.

Coverage increased (+1.0%) to 95.293% when pulling 030d820bc36328e4535c8846d1194524f8c23f17 on drdavella:fix-test-runner into 584e17bc43c5ba0a1d750f33f5162279e83636db on spacetelescope:master.


Cannot resolve dependency

Error message:
Could not find io.realm.kotlin:cinterop-jvm:0.5.0-SNAPSHOT.
Required by:
    project :desktop > project :common > io.realm.kotlin:library:0.5.0-SNAPSHOT:20210715.121820-5 > io.realm.kotlin:library-jvm:0.5.0-SNAPSHOT:20210715.121820-5 > io.realm.kotlin:cinterop:0.5.0-SNAPSHOT:20210715.121820-5

Possible solution:
 - Declare repository providing the artifact, see the documentation at https://docs.gradle.org/current/userguide/declaring_repositories.html

I'm using realm-kotlin 0.5.0-SNAPSHOT. And here is my project structure:

We currently don't support pure JVM targets. It is on our roadmap, but we don't have an ETA yet. Closing this issue as it is already tracked through #62.


Warning: Deprecated field [pattern] used, replaced by [Put patterns directly under the [include] or [exclude]]

NEST/Elasticsearch.Net version:5.2.0
Elasticsearch version:5.2.0
Steps to reproduce:
Using TermsAggregationDescriptor Include(string includePattern, null)
I have this warning
Deprecated field [pattern] used, replaced by [Put patterns directly under the [include] or [exclude]]

Thank you for reporting this its one of the breaking changes in 6.0 (our master branch)
We still have to apply at some point: https://www.elastic.co/guide/en/elasticsearch/reference/master/breaking_60_aggregations_changes.html


Adds oviewers() implementation

If you'd rather I do this in C#, tell me and I will.
https://secure.byond.com/docs/ref/#/proc/oviewers
Closes: #83

writing one of the most expensive procs in DM instead of C#

😩

I'll rewrite it in the morning


The extension is not working with RES infinite scroll.

The awards are only being hidden in the first page. When the second page is loaded the awards show up.

Fixed in v1.1


Resource NOFILE rlimit support

Sets the NOFILE rlimit if not set to the minimal required value.
Offers a commandline option for setting a custom NOFILE rlimit value.
Signed-off-by: Philipp Deppenwiese<EMAIL_ADDRESS>
1. What does this change do, exactly?
Sets the NOFILE rlimit for unix operating systems. Normally this could be done as well by
the init system. The problem comes with non systemd OS and privilege separation if used.
This patch sets the default rlimit to 8192 instead of log a warning and offers the ability to set
a custom NOFILE rlimit value via "rlimit" commandline option.
It does checks for the UID in order to figure out if the resource capability is enabled. In general
golang does not support capabilities and privilege separation (see issue https://github.com/golang/go/issues/1435 ) until now.
Remove unneccessary NOFILE limits in init scripts for posix operating systems.
2. Please link to the relevant issues.

3. Which documentation changes (if any) need to be made because of this PR?
An additional commandline option needs to be documented. See https://caddyserver.com/docs/cli
-rlimit
Sets the open file descriptors limit for the caddy process. Default limit is 8192. For non-root user
usage please refer to setcap. The resource capability can be set by executing:
sudo setcap cap_sys_resource+ep caddy

4. Checklist

[ ] I have written tests and verified that they fail without my change
[x] I have squashed any insignificant commits
[x] This change has comments for package types, values, functions, and non-obvious lines of code
[x] I am willing to help maintain this change if there are issues with it later

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

No problem!. I planned to write tests but I am not sure how to do it in this case. Maybe you can give me a hint. :)

Re: tests - is there a way to do a system call of some sort to verify that the new value is set properly?

@zaolin Regarding tests, you could use the Getrlimit syscall the same as the previous checkFdlimit function to verify the limit has been set. I suggest you also check how other command flags are tested and you can do the same test on yours. Perhaps a test to make sure a value smaller than the minimum is correctly rejected.

My plan wasn't to test the SetRlimit syscall.(golang has already tests for it) :) Instead I planned to test the setrlimit feature with/without root (caps) access. Is that possible via travis ci ?

Probably not without modifying the Travis script. Doable, but something to consider as we implement a better test suite in the future.
However, while we're on this topic, any harm in increasing the limit even higher than 8192? Would setting it to double that work well?
The way I'm seeing this change so far is, instead of the user running ulimit -n they have to run setcap (which isn't available in some constrained systems apparently). Wouldn't it be better for the sysadmin to just configure the system themselves instead, since they have to run a command either way?

Increasing limit to 16384 shouldn't be a problem.
I think it's an issue on non systemd OS because systemd supports setting resources. Normally on server systems there is no systemd and the sysadmin wants to do privilege separation in order to keep his machine safe. But you can only set ulimit user based which means there is no way setting ulimit if you use a no shell user, e.g. www-data user.
Capabilities are in my opinion the best solution. It's also easy for linux package maintainers. The solution doesn't work a cross distributions but that's how it is.

@zaolin Any chance you could finish up this PR? (Sorry for the merge conflict. Shouldn't be too bad.)

Closing due to inactivity. :(


Thank you!

I'm totally impressed how quick this API is adjusted to the latest changes of Porsche.
Thank you all for your great work.
Andreas
BTW: I'm using this API to run a docker container providing soc to openwb.de

Thanks, @fredriklj deserves credits for that :)


Does not support Unity 2018.4 or later versions

I am trying to get this environment working with mlagents 0.6 . This would be useful to be able to use the new brain configuration introduced in mlagents from version 0.6 with the ActiveRagdoll project concerned with Style Transfer, which is based on this marathon-envs project.
To get started on this, I have simply downloaded the marathon repository. Realizing the files from ml-agents were already included in the marathon repo, all I did was open the project and try to run some scenses. However, both the deepmindwalker and the deepmind hopper simply fall to the ground, which is suprising given the fact that in the past I have been able to run these projects on an earlier version of ml-agents.
I have downloaded and opened the latest commit on the marathon repo, imported the tensorflow-csharp unity package, and opened one of the existing scenes, but the characters fall to the ground almost immediately.
Any advice on what I might getting wrong would be welcome.
Regards,
Joan

by the way, I am using this in unity 2019.1 (since it worked for ActiveRagdollStyleTransfer with mlagents 0.5, I thought it would also work here)

@joanllobera hmm sounds very strange - not sure why it would be the Unity version - I will delete my local and pull and see if it is happening for me
Note: my dev branch is more up to date (i think on ml-agents 7) - https://github.com/Sohojoe/marathon-envs - but to give some context:  I did quite a bit of new work on supporting spawning of agents (so one can set the number of agents within an environment). Prior to this, one had to set the number of agents by hand which became a pain to manage. However, in parallel, the unity team updated ml-agents to support spawning multiple instances of the Unity environment (i.e. the executable) and they also improved how to extend ml-agents on the python side. So this makes it a little more complex I had made some changes to the ml-agents code base.

Quick question - which OS are you developing on?

@joanllobera I was able to reproduce the issue with Unity 2018.4; it works fine with 2018.3 - the problem is that the Unity physics changed around 2018.3
My dev branch was also built against 2018.3 (so has the same problem on 2018.4 and later
ActiveRagdollStyleTransfer had the same problem with and I was able to do a simple scaler to get StyleTransfer working on 2018.4 and later without retraining. However, this approach does not seem to work on MarathonEnvs and I will need to retrain them all. I am working on fine-tuning the scaler in ActiveRagdollStyleTransfer (https://github.com/Sohojoe/ActiveRagdollStyleTransfer/issues/4) and so will aim to find a value that works with MarathonEnvs also

Hi,
I will check with your development branch.
From my side I think the first thing to sort out is that the
activeragdollstyletransfer project works on something like mlagents 0.6, or
later.
By now i will try on 2018.3, and once this works will check if retraining
allows upgrading the unity version for that particular project.
This is not specifically from this project, but I thought you might like to
know where i am trying to go over here.
On Wed, 17 Jul 2019 at 07:39, Joe Booth<EMAIL_ADDRESS>wrote:

@joanllobera https://github.com/joanllobera I was able to reproduce the
issue with Unity 2018.4; it works fine with 2018.3 - the problem is that
the Unity physics changed around 2018.3
My dev branch was also built against 2018.3 (so has the same problem on
2018.4 and later
ActiveRagdollStyleTransfer had the same problem with and I was able to do
a simple scaler to get StyleTransfer working on 2018.4 and later without
retraining. However, this approach does not seem to work on MarathonEnvs
and I will need to retrain them all. I am working on fine-tuning the scaler
in ActiveRagdollStyleTransfer (Sohojoe/ActiveRagdollStyleTransfer#4
https://github.com/Sohojoe/ActiveRagdollStyleTransfer/issues/4) and so
will aim to find a value that works with MarathonEnvs also
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Unity-Technologies/marathon-envs/issues/20?email_source=notifications&email_token=AA5H4TGMUU77STOSZQUHMQTP72V7LA5CNFSM4IDWQED2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2DC32Q#issuecomment-512110058,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA5H4TA4Y7WD63B3LAU2RF3P72V7LANCNFSM4IDWQEDQ
.

--
Joan Llobera - http://joan.llobera.name
<EMAIL_ADDRESS>

OK - sounds good. I've started to look into it and it shouldn't be too difficult to get it working with ml-agents 0.8

Solved with current dev branch


feat: Added app icon assets

Fixes #19
Changes: Added corresponding app icon assets as of apple app icon guideliness
Screenshots for the change:

@chashmeetsingh oh yeah I also noticed that but I double checked the sizes of the icons and there are no any issues with the sizes of the icons. I think we see a black background because of the icon shape. I will work on that and submit it back

@chashmeetsingh I have already pushed the fix and I squashed the commits. Now the app icon appears correctly.


Feature: Floating Cube Animation

Is your feature request related to a problem? Please describe.
I would like to add an animation that provides an 3D effect to the web page
Describe the solution you'd like
Adding an Animation under the animation section
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
screen-recording.webm
Additional context
Add any other context or screenshots about the feature request here.

Go for it !