hotfix/loginError -> dev

This change is

Review status: 0 of 6 files reviewed, 7 unresolved discussions (waiting on @geogor37)

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 46 at r1 (raw file):

	public String getRefreshToken() {
		return refreshToken;
//		return "";

What are these?

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 71 at r1 (raw file):

	public boolean isExpired() {
//		return true;

?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 35 at r1 (raw file):

 */
public class ByuAccountAuthenticator extends AbstractAccountAuthenticator implements AuthConstants {
	private final static String DEFAULT_ERROR_MESSAGE = "The server encountered an error. Please try again later.";

Isn't this already defined somewhere else?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 114 at r1 (raw file):

	}

	private Bundle getAuthTokenInternal(Wso2Client.Api api, OAuthToken authToken, AccountManager accountManager, Account account, String authTokenType, int tries) {

You'll have to explain this one to me

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 117 at r1 (raw file):

		try {
			Response<OAuthToken> response = api.getOAuthToken(authToken.isSandbox() ? AuthConstants.SANDBOX_ENDPOINT : AuthConstants.PROD_ENDPOINT, new Wso2Client.RefreshWrapper(authToken.getRefreshToken())).execute();
//			response = Response.error(504, ResponseBody.create(MediaType.parse("Application/json"), "{\"readableMessage\": \"There was an error obtaining an authorization token for your request. Please try again later. If the problem persists, log out and back in.\"}"));

Remove?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 133 at r1 (raw file):

			} catch (RefreshException e) {
				if (tries < 3 && (response == null || response.code() < 300 || response.code() >= 500)) {
					// TODO: Ask if we want to keep these logs

Probably remove them. They aren't getting us anything right now.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 159 at r1 (raw file):

	}

	private Bundle createErrorBundle(String errorMessage) {

With how many changes we ended up making in this file, would it be worth going to Kotlin?

Comments from Reviewable

Review status: 0 of 9 files reviewed, 7 unresolved discussions (waiting on @geogor37 and @romrell4)

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 46 at r1 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
What are these?

I was using them to test error handling and refreshing. If you want to leave this unresolved until we decide this is ready to merge I'll remove them right before that happens.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 35 at r1 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Isn't this already defined somewhere else?

It's a protected static final in the ByuCallback class, so I couldn't use it unless we decided to make it public. I decided the better solution would be to just create a string resource for it and use that wherever I could.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 133 at r1 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Probably remove them. They aren't getting us anything right now.

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 159 at r1 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
With how many changes we ended up making in this file, would it be worth going to Kotlin?

I thought about converting it to Kotlin, and I would definitely be fine with doing that, but I think it would probably be a good idea to do all the authentication stuff at the same time which will take a little while. Do you think it would be worth doing now?

Comments from Reviewable

Reviewed 3 of 4 files at r2, 3 of 5 files at r3.
Review status: 6 of 12 files reviewed, 17 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/login/LoginActivity.java, line 82 at r3 (raw file):

				setAccountAuthenticatorResult(new Bundle());
				progressDialog.dismiss();
				Toast.makeText(LoginActivity.this, getString(R.string.failed_login_message), Toast.LENGTH_LONG).show();

See if we can delete this

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/Wso2Client.java, line 25 at r3 (raw file):

	@Override
	protected String getBaseUrl() {
		return "https://wl4pekqqac.execute-api.us-west-2.amazonaws.com/";

Remove. This is the old one

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/Wso2Client.java, line 25 at r3 (raw file):

	@Override
	protected String getBaseUrl() {
		return "https://wl4pekqqac.execute-api.us-west-2.amazonaws.com/";

Look into swapping between sandbox and prod here

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 133 at r1 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Done.

Only check for 500s

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 159 at r1 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
I thought about converting it to Kotlin, and I would definitely be fine with doing that, but I think it would probably be a good idea to do all the authentication stuff at the same time which will take a little while. Do you think it would be worth doing now?

Do it in Kotlin

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 118 at r3 (raw file):

			try {
				if (response == null || !response.isSuccessful()) {
					throw new RefreshException(context.getString(R.string.default_error_message));

Put RefreshException at the bottom of this class, and don't give it a message

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 119 at r3 (raw file):

				if (response == null || !response.isSuccessful()) {
					throw new RefreshException(context.getString(R.string.default_error_message));
				} else {

Remove else

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 121 at r3 (raw file):

				} else {
					OAuthToken token = response.body();
					if (token == null || !token.isValid()) {

Remove this check as well as isValid, error, and errorMessage from OauthToken class

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 131 at r3 (raw file):

			} catch (RefreshException e) {
				if (tries < 3 && (response == null || response.code() < 300 || response.code() >= 500)) {
					return getAuthTokenInternal(api, authToken, accountManager, account, authTokenType, tries + 1);

Change to countdown. Only retry once

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 138 at r3 (raw file):

					} catch (JSONException | IOException ignore) {
					}
					return createErrorBundle(errorMessage != null ? errorMessage : context.getString(R.string.default_error_message));

return createErrorBundle(new ByuError(response, context.getString(R.string.default_error_message), "readableMessage").getReadableMessage());

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 65 at r3 (raw file):

			onSuccess(call, response);
		} else {
			if (!response.errorBody().contentType().subtype().equals("json") || pathToReadableError == null) { // If the error isn't json we don't know what happened so try again

Look into moving 500+ logic here for retries

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 78 at r3 (raw file):

	private void retryCall(Call<T> call, Response<T> response, Throwable throwable) {
		if (retryCount > 0 && (response == null || response.code() >= 500)) {

Let's try to see all the 400 errors that we have

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 92 at r3 (raw file):

				return;
			}
			try {

Look through this guy

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 1 at r3 (raw file):

package edu.byu.support.retrofit;

Look through this guy

Comments from Reviewable

Reviewed 4 of 6 files at r1, 2 of 5 files at r3.
Review status: all files reviewed, 18 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/login/authentication/RefreshException.kt, line 6 at r3 (raw file):

 * Created by geogor37 on 6/6/18
 */
class RefreshException(message: String): Exception(message)

Remove the message

Comments from Reviewable

Review status: all files reviewed, 18 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 46 at r1 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
I was using them to test error handling and refreshing. If you want to leave this unresolved until we decide this is ready to merge I'll remove them right before that happens.

Sounds good. I'll leave this open until you're finished with other things

Comments from Reviewable

Review status: 11 of 13 files reviewed, 18 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 92 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Look through this guy

I moved the try catch statements out in order to keep the logic identical to how it was before I consolidated the parseReadableError function and the ByuError constructor.  Before, whenever the parseReadableError function threw an exception that would trigger a retry.  I'll talk to you about this, I'm just leaving myself a note so I don't forget.

Comments from Reviewable

Reviewed 1 of 2 files at r4.
Review status: 12 of 13 files reviewed, 28 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 28 at r4 (raw file):

 */
class ByuAccountAuthenticator(private val context: Context): AbstractAccountAuthenticator(context) {
	override fun addAccount(response: AccountAuthenticatorResponse?, accountType: String?, authTokenType: String?, requiredFeatures: Array<out String>?, options: Bundle?) = Bundle(2).apply {

What does the out mean?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 30 at r4 (raw file):

	override fun addAccount(response: AccountAuthenticatorResponse?, accountType: String?, authTokenType: String?, requiredFeatures: Array<out String>?, options: Bundle?) = Bundle(2).apply {
		if (accountType != AuthConstants.BYU_ACCOUNT_TYPE) {
			// TODO: These first two if blocks crash the app if they're ever true (both in this branch and in dev) I'm guessing we'd like to handle this a little more gracefully?

Wait, what? What's crashing?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 32 at r4 (raw file):

			// TODO: These first two if blocks crash the app if they're ever true (both in this branch and in dev) I'm guessing we'd like to handle this a little more gracefully?
			putInt(AccountManager.KEY_ERROR_CODE, AccountManager.ERROR_CODE_BAD_ARGUMENTS)
			putString(AccountManager.KEY_ERROR_MESSAGE, String.format("Invalid account type: '%s'", accountType))

Is this going to be displayed to a user? If so, can we put it in strings.xml?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 33 at r4 (raw file):

			putInt(AccountManager.KEY_ERROR_CODE, AccountManager.ERROR_CODE_BAD_ARGUMENTS)
			putString(AccountManager.KEY_ERROR_MESSAGE, String.format("Invalid account type: '%s'", accountType))
		} else if (authTokenType != null && authTokenType != AuthConstants.TYPE_BEARER_TOKEN) {

First check doesn't matter, right? (Kotlin)

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 40 at r4 (raw file):

		} else if (AccountManager.get(context).getAccountsByType(AuthConstants.BYU_ACCOUNT_TYPE).isNotEmpty()) {
			// If there's already an account they can't add another one
			Handler(context.mainLooper).post { Toast.makeText(context, R.string.lib__login_too_many_accounts, Toast.LENGTH_LONG).show() }

What's with the name of this string? Why the lib__ prefix?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 42 at r4 (raw file):

			Handler(context.mainLooper).post { Toast.makeText(context, R.string.lib__login_too_many_accounts, Toast.LENGTH_LONG).show() }
			putInt(AccountManager.KEY_ERROR_CODE, AccountManager.ERROR_CODE_BAD_REQUEST)
			putString(AccountManager.KEY_ERROR_MESSAGE, "Another account already exists")

strings.xml

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 62 at r4 (raw file):

			val peekAuthToken = accountManager.peekAuthToken(account, authTokenType)
			val authToken = Gson().fromJson(peekAuthToken, OAuthToken::class.java)
			if (!authToken.isExpired) createAuthBundle(account, peekAuthToken) else getAuthTokenInternal(Wso2Client().getApi(context), authToken, accountManager, account, authTokenType, 0)

default tries to 1, and then count down instead of up

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 64 at r4 (raw file):

			if (!authToken.isExpired) createAuthBundle(account, peekAuthToken) else getAuthTokenInternal(Wso2Client().getApi(context), authToken, accountManager, account, authTokenType, 0)
		} else {
			// TODO: This displays a really non-user-friendly message to the user, should we handle this better?

Let's handle it better (and with strings.xml)

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 69 at r4 (raw file):

	}

	// TODO: Kotlin-ify this function

Do it :) I'll wait to review this until it's done. Please look back at the comments now orphaned in the ByuAccountAuthenticator.java file above

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 115 at r4 (raw file):

			.apply { putString(AccountManager.KEY_ERROR_MESSAGE, errorMessage) }

	private fun createUnsupportedOperationBundle() = Bundle(1).apply { putInt(AccountManager.KEY_ERROR_CODE, AccountManager.ERROR_CODE_UNSUPPORTED_OPERATION) }

Can't this be saved as a variable, instead of a function?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 117 at r4 (raw file):

	private fun createUnsupportedOperationBundle() = Bundle(1).apply { putInt(AccountManager.KEY_ERROR_CODE, AccountManager.ERROR_CODE_UNSUPPORTED_OPERATION) }

	override fun getAuthTokenLabel(authTokenType: String) = "BYU Account $authTokenType"

strings.xml?

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 92 at r3 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
I moved the try catch statements out in order to keep the logic identical to how it was before I consolidated the parseReadableError function and the ByuError constructor.  Before, whenever the parseReadableError function threw an exception that would trigger a retry.  I'll talk to you about this, I'm just leaving myself a note so I don't forget.

Great. Talk to me

Comments from Reviewable

Reviewed 1 of 2 files at r4.
Review status: all files reviewed, 28 unresolved discussions (waiting on @geogor37)

Comments from Reviewable

Review status: 7 of 15 files reviewed, 28 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 159 at r1 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Do it in Kotlin

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 118 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Put RefreshException at the bottom of this class, and don't give it a message

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 119 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove else

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 121 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove this check as well as isValid, error, and errorMessage from OauthToken class

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.java, line 138 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
return createErrorBundle(new ByuError(response, context.getString(R.string.default_error_message), "readableMessage").getReadableMessage());

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 28 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
What does the out mean?

It's the way Kotlin does generics; it's equivalent to Array<? extends String> in Java meaning that all items in the array should be child classes of String

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 30 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Wait, what? What's crashing?

The first two if blocks

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 32 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Is this going to be displayed to a user? If so, can we put it in strings.xml?

I'm not sure what the original intention was behind this message, but currently nothing gets displayed to the user when we reach this line of code, the app just crashes. This is true in master, as well as in this branch.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 33 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
First check doesn't matter, right? (Kotlin)

Actually it does because we should still be able to add the account successfully even if authTokenType == null because in the FavoritesActivity we pass in null for the authTokenType. With that being said, I'm not sure why we don't just specify that authTokenType = AuthConstants.TYPE_BEARER_TOKEN in the FavoritesActivity. I don't see an advantage to passing in null, and I feel like it's better to specify the token type explicitly.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 40 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
What's with the name of this string? Why the lib__ prefix?

Haha I'm not sure, that's just how it was saved.  I'll go ahead and remove the prefix.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 42 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
strings.xml

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 62 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
default tries to 1, and then count down instead of up

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 64 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Let's handle it better (and with strings.xml)

Done. I changed the message to "Authentication failed, please try again." Do you think that's good enough?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 69 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Do it :) I'll wait to review this until it's done. Please look back at the comments now orphaned in the ByuAccountAuthenticator.java file above

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 115 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Can't this be saved as a variable, instead of a function?

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 117 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
strings.xml?

Done.

support/src/main/java/edu/byu/support/login/authentication/RefreshException.kt, line 6 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove the message

Done.

Comments from Reviewable

Reviewed 9 of 9 files at r5.
Review status: all files reviewed, 22 unresolved discussions (waiting on @geogor37 and @romrell4)

app/src/main/java/edu/byu/suite/mainScreen/FavoritesActivity.java, line 130 at r5 (raw file):

			AccountManager.get(this).addAccount(getString(R.string.account_type), null, null, bundle, this, new LoginCallback(this, result -> {
				onLogin();
				if (pendingActivityName != null && !result.containsKey(LoginCallback.LOGIN_FAILED)) {

Is this fixing a bug?

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 33 at r5 (raw file):

		} catch (e: Exception) {
			when (e) {
				// TODO: Should we display a Toast or an error dialog?

Good question. What do you think?

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 36 at r5 (raw file):

				// These exceptions are generated when the login request was accompanied by invalid parameters (such as accountType and authTokenType), in these cases e.message will contain
				// a readable message that we can display to the user.
				is AuthenticatorException, is IllegalArgumentException -> Toast.makeText(context, e.message, Toast.LENGTH_LONG).show()

I don't know if I'd use e.message here. What could be in that? Should we use a more generic default message?

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 38 at r5 (raw file):

				is AuthenticatorException, is IllegalArgumentException -> Toast.makeText(context, e.message, Toast.LENGTH_LONG).show()
				// I've personally never seen these exceptions occur, but I don't want to remove this catch statement in case it breaks something.
				is OperationCanceledException, is IOException -> Log.e(TAG, "run: Error getting result from future, in callback", e)

I would just combine this catch with the other one that displays the error. This one would swallow the error and confuse the user, right? Or maybe remove this so that it works like the else? It seems like they used to all just do a debug log...

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 40 at r5 (raw file):

				is OperationCanceledException, is IOException -> Log.e(TAG, "run: Error getting result from future, in callback", e)
				// Rethrow any other exceptions so that they will be handled as they normally would.
				else -> throw e

We didn't use to rethrow anything. Is this an improvement?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 30 at r4 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
The first two if blocks

Still? Can we fix them?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 33 at r4 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Actually it does because we should still be able to add the account successfully even if authTokenType == null because in the FavoritesActivity we pass in null for the authTokenType. With that being said, I'm not sure why we don't just specify that authTokenType = AuthConstants.TYPE_BEARER_TOKEN in the FavoritesActivity. I don't see an advantage to passing in null, and I feel like it's better to specify the token type explicitly.

Let's specify it explicitly. That way we can make that a non-nullable value

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 64 at r4 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Done. I changed the message to "Authentication failed, please try again." Do you think that's good enough?

Do we not already have a message somewhere that fits with this scenario (from the user's perspective)?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 34 at r5 (raw file):

		when {
			accountType != AuthConstants.BYU_ACCOUNT_TYPE -> {
//			true -> {

Remove?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 40 at r5 (raw file):

			}
			authTokenType != null && authTokenType != AuthConstants.TYPE_BEARER_TOKEN -> {
//			true -> {

Remove?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 47 at r5 (raw file):

			}
			AccountManager.get(context).getAccountsByType(AuthConstants.BYU_ACCOUNT_TYPE).isNotEmpty() -> {
//			true -> {

Remove?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 77 at r5 (raw file):

			if (!authToken.isExpired) createAuthBundle(account, peekAuthToken) else getAuthTokenInternal(Wso2Client().getApi(context), authToken, accountManager, account, authTokenType, 1)
		} else {
			// TODO: Make sure this functions as desired.

Do it

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 85 at r5 (raw file):

		try {
			val response: Response<OAuthToken>? = api.getOAuthToken(Wso2Client.getTokenUrl(), Wso2Client.RefreshWrapper(authToken.refreshToken)).execute()
			// response = Response.error(504, ResponseBody.create(MediaType.parse("Application/json"), "{\"readableMessage\": \"There was an error obtaining an authorization token for your request. Please try again later. If the problem persists, log out and back in.\"}"));

Remove?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 99 at r5 (raw file):

					getAuthTokenInternal(api, authToken, accountManager, account, authTokenType, tries - 1)
				} else {
					createNetworkErrorBundle(try {

Whoa. You can nest a try into a method call??

Comments from Reviewable

Review status: all files reviewed, 22 unresolved discussions (waiting on @geogor37 and @romrell4)

app/src/main/java/edu/byu/suite/mainScreen/FavoritesActivity.java, line 130 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Is this fixing a bug?

Yes

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 33 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Good question. What do you think?

I think I would lean towards an error dialog, especially because the messages that we're displaying are pretty long.

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 36 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
I don't know if I'd use e.message here. What could be in that? Should we use a more generic default message?

In both of these cases e.message will contain whatever message we set in the ByuAccountAuthenticator. I've included comments that I think explain how Android's authentication classes handle errors.

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 38 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
I would just combine this catch with the other one that displays the error. This one would swallow the error and confuse the user, right? Or maybe remove this so that it works like the else? It seems like they used to all just do a debug log...

As I said in the comment, I've never seen these exceptions get generated, so I'm not sure what the proper behavior should be.  I am certain that whatever message the exceptions contain won't be a message that we've set, so I don't think we'd want it to do the same thing as the other two.  Should I just display a generic message to the user?

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 40 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
We didn't use to rethrow anything. Is this an improvement?

No, this is just the closest thing in Kotlin to being able to mimic Java's ability to catch multiple exceptions in a single block. We only want it to catch the exception if it is one of those four types, otherwise we let it go.

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/Wso2Client.java, line 25 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove. This is the old one

Done.

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/Wso2Client.java, line 25 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Look into swapping between sandbox and prod here

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 30 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Still? Can we fix them?

Yeah, my last push should've fixed them. I meant to update my last comment here, but I forgot :)

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 33 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Let's specify it explicitly. That way we can make that a non-nullable value

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 64 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Do we not already have a message somewhere that fits with this scenario (from the user's perspective)?

Kind of, we had a message that said "Your credentials validated with CAS, but your authorization code was expired. Please try again. If the problem persists, submit feedback or contact the OIT Service Desk."  I could change that to a more generic message that says "Authentication failed. Please try again. If the problem persists..."

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 77 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Do it

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 99 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Whoa. You can nest a try into a method call??

Yup!

Comments from Reviewable

Reviewed 9 of 9 files at r6.
Review status: all files reviewed, 16 unresolved discussions (waiting on @geogor37 and @romrell4)

app/src/main/java/edu/byu/suite/mainScreen/FavoritesActivity.java, line 130 at r5 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Yes

Could you put a comment above this line about what it's doing?

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 40 at r5 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
No, this is just the closest thing in Kotlin to being able to mimic Java's ability to catch multiple exceptions in a single block. We only want it to catch the exception if it is one of those four types, otherwise we let it go.

Gotcha. How about we just catch all exceptions and display a generic message to the user.

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 36 at r6 (raw file):

				// These exceptions are generated when the login request was accompanied by invalid parameters (such as accountType and authTokenType), in these cases e.message will contain
				// a readable message that we can display to the user.
				is AuthenticatorException, is IllegalArgumentException -> activity.showErrorDialog(e.message, null)

Move showErrorDialog to extensions. Then default the button pressed listener

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 64 at r4 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Kind of, we had a message that said "Your credentials validated with CAS, but your authorization code was expired. Please try again. If the problem persists, submit feedback or contact the OIT Service Desk."  I could change that to a more generic message that says "Authentication failed. Please try again. If the problem persists..."

Good call. Do that

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 36 at r6 (raw file):

//			true -> {
				// The only account type we use in the app is "edu.byu.user," any attempt to create an account of a different type should be rejected. The same goes for any request
				// for a token that is not of type "Bearer."

Should the period in this comment be outside of the quotes? Or are we actually getting a token with the type of "Bearer." with the period?

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 93 at r6 (raw file):

				} else {
					createNetworkErrorBundle(try {
						ByuError(response, context.getString(R.string.default_error_message), "readableMessage").readableMessage

So why do we create the ByuError, just to get the readable message? Is it so that we can use the already baked in pathToReadableError stuff?

support/src/main/java/edu/byu/support/retrofit/ByuApiGenerator.java, line 100 at r6 (raw file):

		} catch (OperationCanceledException | AuthenticatorException | IllegalArgumentException e) {
			Log.e(TAG, "addBearerToken: ", e);
			throw new IOException(e.getMessage());

What is this fixing?

Comments from Reviewable

Review status: all files reviewed, 16 unresolved discussions (waiting on @geogor37 and @romrell4)

app/src/main/java/edu/byu/suite/mainScreen/FavoritesActivity.java, line 130 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Could you put a comment above this line about what it's doing?

Done.

support/src/main/java/edu/byu/support/login/LoginActivity.java, line 82 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
See if we can delete this

I don't remember what this was referring to

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 40 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Gotcha. How about we just catch all exceptions and display a generic message to the user.

Done.

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 36 at r6 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Move showErrorDialog to extensions. Then default the button pressed listener

Because this is already a member function of ByuActivity I'm getting a warning when trying to create an Extension with the same function signature. I think in order to do this safely we'd have to totally get rid of the showErrorDialog() method in ByuActivity and get rid of it everywhere it's currently being used, which might be beyond the scope of this branch.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 36 at r6 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Should the period in this comment be outside of the quotes? Or are we actually getting a token with the type of "Bearer." with the period?

Nope, good point

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 93 at r6 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
So why do we create the ByuError, just to get the readable message? Is it so that we can use the already baked in pathToReadableError stuff?

Bingo. We used to have a separate method that did the exact same thing without calling the constructor, but I figured that it would be better to just avoid duplicating the code and using ByuError's constructor instead. I'll add a comment explaining this.

support/src/main/java/edu/byu/support/retrofit/ByuApiGenerator.java, line 100 at r6 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
What is this fixing?

Before, whenever getAuthToken was throwing an exception, this line would create an IOException whose message looked something like this: IllegalArgumentException:  "WHATEVER THE ERROR MESSAGE SET IN GETAUTHTOKEN WAS" changing it to e.getMessage() keeps the exception name from showing up in the message that gets displayed to the user.

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 65 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Look into moving 500+ logic here for retries

Done. I also stopped checking whether or not we could read the response because I figured that the error code pretty much tells us whether or not we want to retry the call.  Let me know if that was the wrong call.

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 78 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Let's try to see all the 400 errors that we have

What did you want me to look at for this one? All the errors we could possibly see in the app?

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 92 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Great. Talk to me

After looking at the code more, I don't think it's necessary for the try catch statements to be moved outside of the constructor; though I'm still game to look at this with you if you want.

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 1 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Look through this guy

Done.

Comments from Reviewable

Reviewed 2 of 7 files at r7.
Review status: 12 of 17 files reviewed, 13 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 40 at r5 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Done.

can We do it for all of them? Including the ones above?

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 36 at r6 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Because this is already a member function of ByuActivity I'm getting a warning when trying to create an Extension with the same function signature. I think in order to do this safely we'd have to totally get rid of the showErrorDialog() method in ByuActivity and get rid of it everywhere it's currently being used, which might be beyond the scope of this branch.

Deal. Add it to the Trello board

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 47 at r7 (raw file):

	public String getAccessToken() {
		return accessToken;
//		return "";

Remove

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 78 at r3 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
What did you want me to look at for this one? All the errors we could possibly see in the app?

Anytime in the app we explicitly handle a 4XX code differently

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 74 at r7 (raw file):

				}
			} catch (JSONException e) {
				// TODO: Keep logs?

Eh. Remove

Comments from Reviewable

Reviewed 3 of 7 files at r7.
Review status: 15 of 17 files reviewed, 12 unresolved discussions (waiting on @romrell4 and @geogor37)

Comments from Reviewable

Review status: 15 of 17 files reviewed, 12 unresolved discussions (waiting on @romrell4 and @geogor37)

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 40 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
can We do it for all of them? Including the ones above?

Oh, right. Done.

support/src/main/java/edu/byu/support/login/LoginCallback.kt, line 36 at r6 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Deal. Add it to the Trello board

Done.

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 78 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Anytime in the app we explicitly handle a 4XX code differently

The only time we handle a 4XX code differently as far as retrying calls are concerned, is the one in MFC that you already knew about. There are only two other cases where we handle a 400 error differently and they both deal with UI stuff, such as changing the message that gets displayed to the user.

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 74 at r7 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Eh. Remove

Done.

Comments from Reviewable

Reviewed 1 of 7 files at r7, 2 of 2 files at r8.
Review status: all files reviewed, 10 unresolved discussions (waiting on @geogor37 and @romrell4)

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 64 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Good call. Do that

Is this done?

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 65 at r3 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Done. I also stopped checking whether or not we could read the response because I figured that the error code pretty much tells us whether or not we want to retry the call.  Let me know if that was the wrong call.

So if it's a 500, but has a readable message, then we don't want to retry

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 78 at r3 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
The only time we handle a 4XX code differently as far as retrying calls are concerned, is the one in MFC that you already knew about. There are only two other cases where we handle a 400 error differently and they both deal with UI stuff, such as changing the message that gets displayed to the user.

So we will we break that at all?

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 53 at r8 (raw file):

							if (i == paths.length - 1) {
								// If the path pointed to an array, concatenate the parts together
								JSONArray jsonArray = jsonBody.optJSONArray(paths[i]);

So is this supposed to be the same as it used to be? I doesn't look like it

Comments from Reviewable

Review status: all files reviewed, 10 unresolved discussions (waiting on @geogor37 and @romrell4)

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 64 at r4 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Is this done?

Yes

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 65 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
So if it's a 500, but has a readable message, then we don't want to retry

Are you saying that's what the logic should be? With the logic written as it currently is, we will retry a 500 whether or not it has a readable message.

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 78 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
So we will we break that at all?

Nope, that should still work fine

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 53 at r8 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
So is this supposed to be the same as it used to be? I doesn't look like it

No, the other function that duplicated this logic did have this bit of code that dealt with parsing a JSON Array that wasn't originally in the ByuError constructor, so I added it to the ByuError constructor so that the logic is all in the same place. It looks like my try statements aren't quite in the same place either, so I'll go ahead and move those.

Comments from Reviewable

Reviewed 4 of 4 files at r9.
Review status: all files reviewed, 8 unresolved discussions (waiting on @geogor37 and @romrell4)

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 65 at r3 (raw file):
Previously, geogor37 (Geoff Gorham) wrote…
Are you saying that's what the logic should be? With the logic written as it currently is, we will retry a 500 whether or not it has a readable message.

Yeah... I think it should be the other way. Is that difficult?

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 75 at r9 (raw file):

						}
					} catch (JSONException e) {
						populateByuError(defaultErrorMessage, response.errorBody().string());

Why did we switch this? Does this give us a better log?

Comments from Reviewable

Review status: all files reviewed, 8 unresolved discussions (waiting on @geogor37 and @romrell4)

support/src/main/java/edu/byu/support/retrofit/ByuCallback.java, line 65 at r3 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Yeah... I think it should be the other way. Is that difficult?

Nope, this should do the trick

support/src/main/java/edu/byu/support/retrofit/ByuError.java, line 75 at r9 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Why did we switch this? Does this give us a better log?

Actually this wasn't included at all before, so if there wasn't a valid String at the end of the path given by pathToReadableError the readableMessage in ByuError would just be null. I added this so that readableMessage will default to defaultMessage in the event that it never gets set in the constructor.

Comments from Reviewable

Reviewed 1 of 1 files at r10.
Review status: all files reviewed, 6 unresolved discussions (waiting on @geogor37)

Comments from Reviewable

Review status: all files reviewed, 6 unresolved discussions (waiting on @romrell4)

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 46 at r1 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Sounds good. I'll leave this open until you're finished with other things

Done.

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 71 at r1 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
?

Done.

support/src/main/java/edu/byu/support/login/authentication/clients/wso2/OAuthToken.java, line 47 at r7 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 34 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove?

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 47 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove?

Done.

support/src/main/java/edu/byu/support/login/authentication/providers/ByuAccountAuthenticator.kt, line 85 at r5 (raw file):
Previously, romrell4 (Eric Romrell) wrote…
Remove?

Done.

Comments from Reviewable

Reviewed 3 of 3 files at r11.
Review status: :shipit: complete! all files reviewed, all discussions resolved

Comments from Reviewable


feat: support PKCE and store next_url in the database

(Stacked PR)
Fixes #193, fixes #210.
Add PKCE support and store next_url in the database.

PKCE support is indicated by a new field, use_pkce, added to OAuth2 Clients. Note that only GitLab providers can support PKCE for now.
Stores the next_url parameter in the database instead of passing it around as a query parameter. This allows final redirects to be customized for GitHub which does not allow extra query parameters.

/deploy #notest renku=release-0.52.x

You can access the deployment of this PR at https://renku-ci-ds-216.dev.renku.ch

Thanks for adding this Flora.
My biggest concern is that we are saving things in the DB (like the code verifier and next_url) that are supposed to be unique for each authentication request.
I think here we have several options:

put these things in an encrypted cookie and do not store them in the DB
make a separate table keyed by the state parameter from the oauth flow and store these things there, but you have to make sure you delete things that are old and also delete things when the oauth flow completes
store things in the same table you have now but only allow one single authentication request to be in progress at a time, for this you can store the state parameter and fail if you get a request with a different state parameter, also in this case too you will have to cleanup/reset fields on a successful completion of the oauth flow

Number 3 is what already happens now. There is a uniqueness constraint on having a single connection per user per provider. The state is also used as an indexed key for the callback.
I can change the db fields and store the PKCE code with encryption.

Number 3 is what already happens now. There is a uniqueness constraint on having a single connection per user per provider. The state is also used as an indexed key for the callback.

Ah I see. I missed this earlier.

store the PKCE code with encryption.

You mean store the code_verifier value encrypted? I dont think there is a need for this.

I  can change the db fields

Yes it would be nice to have separate fields for next_url and the pkce code verifier in the db.

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

30 of 34 (88.24%)  changed or added relevant lines in 7 files are covered.
62 unchanged lines in 10 files lost coverage.
Overall coverage decreased (-0.02%) to 90.637%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

components/renku_data_services/connected_services/db.py
10
12
83.33%

components/renku_data_services/connected_services/utils.py
4
6
66.67%

Files with Coverage Reduction
New Missed Lines
%

components/renku_data_services/connected_services/blueprints.py
1
99.04%

components/renku_data_services/storage/blueprints.py
1
95.14%

components/renku_data_services/base_api/misc.py
1
97.37%

components/renku_data_services/message_queue/redis_queue.py
2
95.65%

components/renku_data_services/crc/blueprints.py
5
89.81%

components/renku_data_services/message_queue/db.py
5
88.64%

components/renku_data_services/namespace/db.py
10
88.11%

components/renku_data_services/app_config/config.py
12
95.65%

components/renku_data_services/connected_services/db.py
12
86.57%

components/renku_data_services/users/models.py
13
81.62%

Totals

Change from base Build<PHONE_NUMBER>:
-0.02%

Covered Lines:
7996

Relevant Lines:
8822

💛  - Coveralls


Use docker volume to store metric data

@discordianfish

In a real world deployment you usually override that anyways. But given that the image is good enough for a quick demo and in this case it should indeed write to the volume, I'm :+1: on this.

Thanks everybody.


add rudderstack to integration docs

🚀 Deployed on https://629657cf379ebb1d193f13a9--cozy-fox-0defec.netlify.app


ANSI escape prompt incorrectly rendered when using git in Powershell

Prerequisites

[X] Write a descriptive title.
[X] Make sure you are able to repro it on the latest released version
[X] Search the existing issues.
[X] Refer to the FAQ.
[X] Refer to Differences between Windows PowerShell 5.1 and PowerShell.

Steps to reproduce
I am using the lastest Powershell with some prompt customization utilities. When I attempted to use Git in Powershell, the ANSI escapes in prompt were outputed as raw text.
Steps:

Install Oh My Posh or Starship, and choose a theme or use the default one.

For example, use Oh My Posh:

>winget install oh-my-posh
>#restart terminal
>oh-my-posh init pwsh --config "$env:POSH_THEMES_PATH\jandedobbeleer.omp.json" | Invoke-Expression

>mkdir -p ps_debug; cd ps_debug; git init
>git remote add origin {REMOTE GITHUB REPO}
>git pull origin master # or maybe main
>git pull origin NOT_EXISTED_REMOTE_BRANCH
fatal: couldn't find remote ref NOT_EXISTED_REMOTE_BRANCH
←[38;2;195;134;241m←[0m←[48;2;195;134;241m←[38;2;255;255;255m username ←[0m←[38;2;195;134;241m←[0m←[38;2;255;71;156;49m←[7m←[0m←[48;2;255;71;156m←[38;2;255;255;255m   ps_debug ←[0m←[48;2;255;251;56m←[38;2;255;71;156m←[0m←[48;2;255;251;56m←[38;2;25;53;73m master ≢ ←[0m←[38;2;255;251;56m←[0m←[38;2;131;118;156;49m←[7m←[0m←[48;2;131;118;156m←[38;2;255;255;255m ﮫ626ms⠀←[0m←[48;2;233;30;99m←[38;2;131;118;156m←[0m←[48;2;233;30;99m←[38;2;255;255;255m  ←[0m←[38;2;233;30;99m←[0m←[K←[0J←[K←[0J←]0;pwsh in ps_debug←[0m

Related Issues:
https://github.com/JanDeDobbeleer/oh-my-posh/issues/2396
https://github.com/starship/starship/issues/4044
Expected behavior
ANSI escapes in prompt should be rendered correctly.

Actual behavior
The raw text was printed.

Error details
none

Environment data
Name                           Value
----                           -----
PSVersion                      7.2.4
PSEdition                      Core
GitCommitId                    7.2.4
OS                             Microsoft Windows 10.0.19043
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           <IP_ADDRESS>
WSManStackVersion              3.0

Visuals

I think this is a version of #16741
That has been fixed in 7.3 preview but remains in 7.2.4

Above 7.2.4 with the problem, below 7.3-preview4 with the fix.

It's worth trying on 7.3 preview to see if it is the same problem, or a new variant.

@xudyang1 Please give 7.3 preview a try and see if the issue you observed was fixed.

@xudyang1 Please give 7.3 preview a try and see if the issue you observed was fixed.

Yes, the prompt is rendered correctly in 7.3 preview .


"Ground" egg group has not been converted to "Field"

egg_groups.csv lists "ground" as an egg group. Since Diamond and Pearl, it is now "Field".

ground is just the identifier. The name is in egg_group_prose.csv and it has already been changed to Field.
https://github.com/veekun/pokedex/blob/7d5fbae/pokedex/data/csv/egg_group_prose.csv#L36

Gotcha will close. Thanks for informing.


Can this work from a .trx file?

Trying this library the resulting md file is empty.
Using .net core and run from command line:
´´´
dotnet test -l trx
which generates in TestResults folder a .trx file but which is basically an xml file.
When running afterwards xBehaveMarkdownReport it's empty.
Any idea?
Thanks a lot.

I don't even know what a .trx file is :-O
I only ever used it on full framework together with the xunit console runner.
The report generator probably can't parse the file. it looks for patterns like the test name and it's containing class.

Hi,
Thanks for the prompt feedback.
See also:
https://github.com/xbehave/xbehave.net/issues/483

Hi,
I just did some digging to achive something similar like @paulvanbladel is describing in
xbehave/xbehave.net#483.
I think it basically works for xml-reports generated by .NET-Full console-runner like this:

xunit.console.exe MyFullFxTestassemblyWithXBehaveScenario.dll -xml testresults.xml

Unfortunately the -xml - Switch doesn't exist for .NET Core's dotnet test command. So I looked into this project: https://github.com/spekt/xunit.testlogger and was able to do a small adjustment to write out the missing information from xbehave to the xml.
I will probably do a PR on this in the xunit.testlogger-project in the next few days. For now you can try it out when you change the XunitXmlTestLogger-Class to pass result.DisplayName to the TestResultInfo-Constructor in the TestResultHandler-Method instead of result.TestCase.DisplayName.
This way the generated xml by the following command contains the necessary information for the xBehaveMarkdownReport to work as expected:

dotnet test MyCoreFxTestassemblyWithXBehaveScenario.dll --logger:"xunit;LogFilePath=testresults.xml"

@paulvanbladel @Jazzman82 I wrote a new report generator that can directly generate a report from a .trx file, including some other improvements. You can find it here: https://github.com/ursenzler/xBehaveReportGenerator


Questions about model training.

I have a task to train a model for segmentation of satellite images. Initially I used tf because I know it better. I use iou_score from segmentation_models as an error. By training classic Unet + Adam I get iou_score > 0.9, for 256256 and a significant drop in iou_score for 512512. However, Unet is too slow. I tried training ddrnet_23_slim however iou_score was in the 0.7 range which is not satisfactory to me. Usually an acceptable result could be achieved in 20-30 epochs (within 2 hours on V100) for a dataset of about 10000-15000 images.
The question is how many epochs are usually needed for relatively small sets to get acceptable results using the segformer b0-b3 example.
What other models besides segformer can you recommend for real time applications on jetson type devices.
Thank you.

For real-time applications in transformer models, only segformer-b0 or b1 can be used. In my testing, HarDNet performs better than DDRNet. BiSeNetv2 is also good for real-time (check here https://github.com/CoinCheung/BiSeNet).
Your dataset of size 10k images is relatively large compared to cityscapes. So try tuning hyperparameters and see the tensorboard graphs to get an idea.

I'm now closing this issue. If you have any questions, re-open this issue.


Degree Radian Converter

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
Describe the solution you'd like
A clear and concise description of what you want to happen.
Screenshot

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.
What problem is this feature trying to solve?
How do we know when the feature is complete?

Closing this as exists #764


feat: initial route support

Only compatible with https://github.com/UnownHash/Golbat/pull/133
New routes permission needs to be added to use
Be sure to add route to the appropriate useFor array in your config
May want to force a user logout for changes to go through
Based on #780

After sleeping on the implementation. It might be a good idea to add the Google/Waze/Apple Map icon. This is a map after all and I expect there will be some users that need navigation in order to drive to new routes. Urban city sprawl 🦜 🥳

After sleeping on the implementation. It might be a good idea to add the Google/Waze/Apple Map icon. This is a map after all and I expect there will be some users that need navigation in order to drive to new routes. Urban city sprawl 🦜 🥳

Added in latest.


Improved readability of the Note

I added a table into the note regarding trial accounts to improve it's readability.

 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.

Hi @ceedee666 !
Thank you for your contribution! 👏
I will be happy to merge it, but I need you to sign the CLA agreement (above) before I can do that.
Regards,
Pao

Hey @ceedee666 and @Paolaufer I have seen the "Markdown linter" run has thrown up some errors, but they are not relevant to the changes in this PR. So we can ignore those errors in this particular case, and I will look into narrowing the scope of the lint target. In the meantime, I have disabled the [corresponding workflow]https://github.com/SAP-docs/btp-business-application-studio/actions/workflows/markdown-checks.yaml).

https://people.sap.com/christian.drumm


Fixing the behavior of --dep with --extract

See issue #2385
From CHANGES.md:

Issue #2385.
The behavior of the --extract option was changed so that it no
longer treats the OCaml and Kremlin targets
differently. Previously, when used with --dep full, F* would
disregard the --extract setting when emitting the
ALL_KRML_FILES variable.

From the new message associated with fstar --help:
  --extract One or more semicolon separated occurrences of '[TargetName:]ModuleSelector'
                Extract only those modules whose names or namespaces match the provided options.
                        'TargetName' ranges over {OCaml, Kremlin, FSharp, Plugin}.
                        A 'ModuleSelector' is a space or comma-separated list of '[+|-]( * | namespace | module)'.
                        For example --extract 'OCaml:A -A.B' --extract 'Kremlin:A -A.C' --extract '*' means
                                for OCaml, extract everything in the A namespace only except A.B;
                                for Kremlin, extract everything in the A namespace only except A.C;
                                for everything else, extract everything.
                        Note, the '+' is optional: --extract '+A' and --extract 'A' mean the same thing.
                        Note also that '--extract A' applies both to a module named 'A' and to any module in the 'A' namespace
                Multiple uses of this option accumulate, e.g., --extract A --extract B is interpreted as --extract 'A B'.

I also have patches to hacl-star, mitls-fstar and kremlin to accommodate this change.
With those patches, I have an everest green.

We discussed this extensively and I have open PRs in the other repos anticipating this change. Merging here followed by merges to the other everest repos.


The .NET dlls distributed with CodeGeneration.Roslyn.BuildTime are the wrong version

Create a new .NET Framework 4.6 project (which will become a code generator), add a nuget reference to CodeGeneration.Roslyn.0.3.9. This will add dependent packages to the project including, for example, Microsoft.CodeAnalysis.CSharp.1.3.2.
However, package CodeGeneration.Roslyn.BuildTime.0.3.9 includes several .NET dlls in its build directory, including, for example, build\net46\Microsoft.CodeAnalysis.CSharp.dll with version <IP_ADDRESS>621.
The problem really showed up for me when my generator also referenced Microsoft.CodeAnalysis.CSharp.Workspaces, which, due to the other version 1.3.2 packages in the generator project, must also be version 1.3.2. This situation causes a variety of confusing type system related exceptions at build time, as the build task side of things expects version 1.3.1 (etc) dlls, and the generator is built against and has local copies of version 1.3.2 (etc) dlls.
Replacing the 1.3.1 (etc) dlls in the extracted CodeGeneration.Roslyn.BuildTime.0.3.9 package with 1.3.2 (etc) dlls fixes the problem (all be it temporarily and unmaintainably).

Ouch. Thanks for the report. Version 0.4.6 is significantly different. Can you try that out and let me know whether the problem is fixed?

I have significant concerns about the new burden put on consumers of generator packages (see my comment on 185df8aa5cc9b6bc1eb41be63a571a3fd7a1b61a), but not withstanding that, I've tried installing the 0.4.9 packages but get the following error when installing CodeGeneration.Roslyn to my net46 generator project:

Could not install package 'CodeGeneration.Roslyn 0.4.9'. You are trying to install this package into a project that targets '.NETFramework,Version=v4.6', but the package does not contain any assembly references or content files that are compatible with that framework. For more information, contact the package author.

Ok, I made a fresh netstandard1.5 project for the generator, and I was able to add the package ref. But now my build fails with:
GenerateCodeFromAttributes:
  dotnet codegen -r "C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.0\Microsoft.CSharp.dll" -r "C
  :\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.0\mscorlib.dll" -r "C:\Program Files (x86)\Refere
  nce Assemblies\Microsoft\Framework\.NETFramework\v4.0\System.Core.dll" -r "C:\Program Files (x86)\Reference Assemblies\Microsoft\
  Framework\.NETFramework\v4.0\System.Data.DataSetExtensions.dll" -r "C:\Program Files (x86)\Reference Assemblies\Microsoft\Framewo
  rk\.NETFramework\v4.0\System.Data.dll" -r "C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.0\Sys
  tem.dll" -r "C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.0\System.Xml.dll" -r "C:\Program Fi
  les (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.0\System.Xml.Linq.dll" -r C:\dev\devtools-typedef\DevTools.Ty
  pedef.Attributes\bin\Debug\ZanyAnts.DevTools.Typedef.dll --generatorSearchPath C:\dev\devtools-typedef\DevTools.Typedef.CodeGener
  ation\bin\Debug\netstandard1.5 --out obj\Debug\ --generatedFilesList obj\Debug\hchfihid.bb4 -- Class1.cs Class2.cs Properties\Ass
  emblyInfo.cs
  No executable found matching command "dotnet-codegen"
C:\dev\devtools-typedef\packages\CodeGeneration.Roslyn.BuildTime.0.4.9\build\CodeGeneration.Roslyn.BuildTime.targets(20,5): error M
SB6006: "dotnet" exited with code 1. [C:\dev\devtools-typedef\DevTools.TypeDef.Tests\DevTools.TypeDef.Tests.csproj]

I have added the required DotNetCliToolReference to my test consuming project, but note that this is a .NET 4.0 project so I don't know if DotNetCliToolReference is meaningfully supported. Also, the test project uses packages.config, not <PackageReference>. Note that the extensive codebase with which I want to use my generator also uses packages.config, and cannot be changed to use <PackageReference>. Any such requirement would, I suggest, substantially restrict the audience for CodeGeneration.Roslyn.
I also tried dotnet restore from command line, which was not happy:
C:\dev\devtools-typedef>dotnet restore
  Restoring packages for C:\dev\devtools-typedef\DevTools.TypeDef.Tests\DevTools.TypeDef.Tests.csproj...
  Restoring packages for C:\dev\devtools-typedef\DevTools.Typedef.CodeGeneration\DevTools.Typedef.CodeGeneration.csproj...
  Generating MSBuild file C:\dev\devtools-typedef\DevTools.Typedef.CodeGeneration\obj\DevTools.Typedef.CodeGeneration.csproj.nuget.g.props.
  Writing lock file to disk. Path: C:\dev\devtools-typedef\DevTools.Typedef.CodeGeneration\obj\project.assets.json
  Restore completed in 274.6 ms for C:\dev\devtools-typedef\DevTools.Typedef.CodeGeneration\DevTools.Typedef.CodeGeneration.csproj.
C:\Program Files\dotnet\sdk\1.0.4\NuGet.targets(97,5): error : Unable to resolve 'dotnet-codegen (>= 0.4.6)' for '.NETCoreApp,Version=v1.0'. [C:\dev\devtools-typedef\DevTools.Typedef.sln]
C:\Program Files\dotnet\sdk\1.0.4\NuGet.targets(97,5): error : Value cannot be null.\r [C:\dev\devtools-typedef\DevTools.Typedef.sln]
C:\Program Files\dotnet\sdk\1.0.4\NuGet.targets(97,5): error : Parameter name: path [C:\dev\devtools-typedef\DevTools.Typedef.sln]

I'm afraid you're hitting both bugs in the dotnet CLI tooling (the ArgumentNullReference at the end of your message) and ya, some by (very unfortunate) design requirements for reliability.
But I hear your pain. And yes, this requires new .NET SDK projects to consume. You can (and I do) target net40 with the .NET SDK projects, just FYI. But porting is non-trivial and not possible for some projects (like those targeting WPF) at this point.
Here's what I'll do though: I've created a v0.3 branch at the point before I merged the changes that added these requirements. We can service that to fix bugs like this one till I find a better story in the master branch.

Argh! Your v0.3 branch sounds good. Does that correspond to an already-published set of nuget packages? If not, could you publish some? Also, if relevant, could you fix the binary versions in v0.3 as per the original report in this issue? Many thanks!

Does that correspond to an already-published set of nuget packages?

Yes: 0.3.9 is already on nuget.org and is nearly on-par with what is in the v0.3 branch.

could you fix the binary versions in v0.3 as per the original report in this issue?

Right: v0.3 would presumably still have the original issue you reported. So I'll keep this issue active and look for time to investigate.

I've got a working test using nuget 0.3.9 and SyntaxGenerator! Looks like the binary issue might have been a red herring: from what I can tell, Microsoft.CodeAnalysis.Common.1.3.2 and related packages confusingly contain dlls with version <IP_ADDRESS>621.

Great. Thanks for reporting.


Add yarn example command

We recommend user to use Yarn but Document is not available yet. That reason why I put this on this PR

Thanks for your contribution! Most of these changes apply to documentation that was originally copied over from https://github.com/vuejs-templates/webpack
Rather than have the docs diverge further, I'd ask that you submit an equivalent PR against https://github.com/vuejs-templates/webpack, and go through the review process there. If it's merged, we can make the same changes to this project's docs.


Not loading Log Viewer, because of route /

When I have route Route::get('/', 'PagesController@home')->name('pages.home'); then /log-viewer doesn't work. Any ideas how to workaround this?

@dkvadratu hey, what is the output of php artisan route:list ? You can remove sensitive any routes if you want.
Otherwise, I'd look into the PagesController class, especially the constructor to see if you're doing anything that would prevent other routes from being registered.
Also, check the 'route_path' value in the config/log-viewer.php if you have published it.

PagesController@home method just returns view blade with few variables. PagesController has no constructor.
I think in project that I got is something custom with routing in Kernel/Middleware, because if I remove first GET / PagesController@home, other custom links not working (log-viewer works). So for now its not an issue, I will update later.
GET|HEAD   / ---------------- pages.home › PagesController@home
GET|HEAD   _debugbar/assets/javascript ---------------- debugbar.assets.js › Barryvdh\Debugbar › AssetController@js
GET|HEAD   _debugbar/assets/stylesheets ---------------- debugbar.assets.css › Barryvdh\Debugbar › AssetController@css
DELETE     _debugbar/cache/{key}/{tags?} ----------------. debugbar.cache.delete › Barryvdh\Debugbar › CacheController@delete
GET|HEAD   _debugbar/clockwork/{id} ---------------- debugbar.clockwork › Barryvdh\Debugbar › OpenHandlerController@clockwork
GET|HEAD   _debugbar/open ----------------. debugbar.openhandler › Barryvdh\Debugbar › OpenHandlerController@handle
GET|HEAD   api/user ----------------..
GET|HEAD   complaints ----------------...... pages.complaints › PagesController@complaints
POST       contact-us ----------------. contact.post › ContactUsController@post
GET|HEAD   cookies ----------------.... pages.cookies › PagesController@cookies
GET|HEAD   livewire/livewire.js ----------------..  Controller@
POST       / ----------------......... Controller@
POST       /-**** ----------------....... Controller@
POST       /-- ----------------........... Controller@
GET|HEAD   //{****_id} ---------------- ****Controller@continue
GET|HEAD   /-pdf ----------------....... Controller@
POST       / ----------------..... Controller@
GET|HEAD   /-pdf ----------------..... Controller@
GET|HEAD   /-pdf ---------------- Controller@
POST       / ----------------. ****Controller@store
POST       / ---------------- ****ValidationController@store
GET|HEAD   ****.callback ---------------- ****.callback › Controller@callback
GET|HEAD   .failed ----------------...... .failed › Controller@failed
GET|HEAD   privacy-policy ---------------- pages.privacy › PagesController@privacy
GET|HEAD   **** ---------------- pages. › PagesController@
GET|HEAD   - ---------------- pages. › PagesController@
POST       /submit/{} ---------------- ****Controller@***Submit
GET|HEAD   translations ---------------- TranslationsController

Hm, the list of routes does contain both the Log Viewer and your PagesController@home route.
And what do you mean by "does not work" ? Does it return 404? Or some other error?

Sorry for unclear situation.
Not working - route /log-viewer loads home page, home.blade.php (PagesController@home)

That's so weird! I really want to help you, but I can't replicate the problem myself.
It really feels like the PagesController is catching all routes somehow.

Do you get a successful response even to a non-Log Viewer route? Such as /log-viewer-route-that-doesnt-exist or just /asdf
Do you have anything specific/special in your RouteServiceProvider or AppServiceProvider ?

Sorry for disturbing. I think it was Cache or Composer problems. Reinstalled project in local environment and all routes works fine.
p.s. thanks for great tool!


Corrected typo in issue description message

Changed "Jush" to "Just"
... very impressive, by the way.
On a related note, I have a tic-tac-toe engine I built in C++ just lying on my system. I'm considering refactoring it in TS and contributing to your project(I noticed your computer currently plays randomly generated moves). ...interested?

Oh! Thanks for pointing that out.  I had initially assumed that the readme was used as a template into which the dynamic info was filled, rather than being fully generated.  Nice.
Just fixed it now


Fixes deploy script incorrect tags #trivial

Mentioned in Slack: https://artsy.slack.com/archives/C02BAQ5K7/p1552918520142600
We saw the 4.4.1-2<IP_ADDRESS> build get pushed as 4.4.1-2<IP_ADDRESS> in this build. I took a look and we were pushing the tag based on latest_testflight_build_number, which is incorrect; it grabs the build number from the latest already-uploaded TestFlight build.
We set the build number in the Makefile, so it's already set by the time that Fastlane gets run. So I've modified the Fastfile to read the value out of the Info.plist file for the git tag.

Great, thanks! Merge on green


chore: bdd-spring-1585023862 to 0.0.1

chore: Promote bdd-spring-1585023862 to version 0.0.1

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approvers:
If they are not already assigned, you can assign the PR to them by writing /assign  in a comment when ready.
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment


Refactor  metrics data Composite

What is the purpose of the change
1、Extract the dataComposite common base class
2、Split different types of data （app、service、rt）
3、pkg opt

Codecov Report

Merging #12036 (e5c78d8) into 3.2 (bd57066) will decrease coverage by 0.23%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                3.2   #12036      +/-   ##
============================================
- Coverage     70.01%   69.79%   -0.23%
+ Complexity      104        2     -102
============================================
  Files          3369     1580    -1789
  Lines        157827    64911   -92916
  Branches      26081     9443   -16638
============================================
- Hits         110507    45306   -65201
+ Misses        37769    15261   -22508
+ Partials       9551     4344    -5207

see 1834 files with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

Please fix conflicts

Please fix conflicts
done


Add Table related value classes to JVM backend

Purpose
This PR contains implementation related to Tables JVM backend
Approach

Describe how you are implementing the solutions along with the design details.

Samples

Provide high-level details about the samples related to this feature.

Remarks

List any other known issues, releated PRs, TODO items or any other notes related to the PR.

Check List

[x] Read the Contributing Guide
[ ] Required Balo version update
[ ] Updated Change Log
[ ] Checked Tooling Support (#)
[x] Added necessary tests

[x] Unit Tests
[ ] Spec Conformance Tests
[ ] Integration Tests
[ ] Ballerina By Example Tests

[ ] Increased Test Coverage
[ ] Added necessary documentation

[ ] API documentation
[ ] Module documentation in Module.md files
[ ] Ballerina By Examples

Codecov Report

Merging #15226 into master will increase coverage by 46.15%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master   #15226       +/-   ##
=============================================
+ Coverage        11%   57.16%   +46.15%
- Complexity        0     3118     +3118
=============================================
  Files            38      277      +239
  Lines          1163    16964    +15801
  Branches        191     3106     +2915
=============================================
+ Hits            128     9697     +9569
- Misses         1022     6019     +4997
- Partials         13     1248     +1235

Impacted Files
Coverage Δ
Complexity Δ

tool-plugins/vscode/src/api-editor/index.ts

tool-plugins/vscode/src/utils/logger.ts

tool-plugins/vscode/src/core/index.ts

tool-plugins/vscode/src/debugger/index.ts

tool-plugins/vscode/src/debugger/manager.ts

tool-plugins/vscode/src/test-runner/activator.ts

...lugins/vscode/src/core/extended-language-client.ts

tool-plugins/vscode/src/extension.ts

tool-plugins/vscode/src/bbe/renderer.ts

tool-plugins/vscode/src/trace-logs/index.ts

... and 305 more

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 fb11ed8...9357add. Read the comment docs.


CLI: block query should wait until the specified block is available

Or do you mean that it should wait until that block is available?
If so, we might introduce a CLI flag - --wait, or --no-wait, depending on the default desired behavior. I don't have a strong opinion - let's discuss
Originally posted by @rphmeier in https://github.com/thrumdev/blobs/pull/164#discussion_r1438934895

I am leaning towards the default of not waiting and returning an error (and a non-zero error code). The default should be when the user is invoking the CLI (as opposed to a script). It feels confusing if the CLI tool just gets stuck (although we also can print a message to an stderr).


IE9 Saturation and Hue gradients not working after upgrade to 2.1.0

Hi,
After upgrading to 2.1.0, the gradients in my custom picker are not appearing for IE9.

I checked the react-color page (http://casesandberg.github.io/react-color/) and it breaks as well.

Unfortunately there are no errors being thrown, so I can't figure out how to fix it.
Every other browser works great, btw, good job!
Will there be a fix for this? Do you still support IE9?
Thanks

I will look into this shortly!


doc: list the actions available on the main README

For someone who is not aware of the actions Bonitasoft provides, it is hard to know what they will find here.
Just listing https://github.com/bonitasoft/actions/tree/main/packages is not enough.

Hi,
I guess what you suggest is to list all actions available in the packages directory and provide a short description for each available action.
We can do this but I cannot guarantee the list will be always up-to-date. This is probably why we put a link to the packages root directory. I suggest that we keep this link so that we can have the complete list.

Hi, I guess what you suggest is to list all actions available in the packages directory and provide a short description for each available action. We can do this but I cannot guarantee the list will be always up-to-date. This is probably why we put a link to the packages root directory. I suggest that we keep this link so that we can have the complete list.

Yes, this is what I suggest. Ok to keep the link to make sure we don't miss anything.
My idea is just to ease understanding what is the list of actions available.


Move integration tests out of controllers dir

In this PR, we move the integration tests that we have in the controllers dir out to a new integration tests dir. This doesn't change any of the tests behavior.
Later, we will refactor the util functions we have in these tests in a different package. Those functions will be used in two places:

E2E tests
Ramenctl and other cli tools

Abandoning this for now. Will revisit after working on simpler changes first.


improve blockquote rendering

http://docs.weave.works/weave/latest_release/weavedns.html#usage - the markdown blockquote in that is rendered rather oddly.
@davkal

@davkal ping


Bug was reintroduced, please check: User with no tokens

Description (what needs to be done) / Expected Behavior
User with no tokens should not be able to submit a proposal, and they should not be able to vote in a proposal (the button should be gray and not clickable).
A user with no tokens should not be able to click on the button to pull up a modal - the "new proposal" button on the Proposals page should should be grayed out and not clickable. And on the New Proposal modal, all buttons should be grayed out and not clickable.
This fixes a bug where they are able to.
Acceptance Criteria
User with no tokens cannot submit a proposal nor see an active button to do so. Same with voting.
Supporting information (Figma link, notes, etc)
When finished, please add Link to the PR or completed Figma / design asset, as a comment to the ticket.

We've moved this check from New proposal button to create proposal button.


direction?

Would love some elaboration on the documentation on how to get it up and running on my own machine.

Hi, I recommend you to use code from here: https://github.com/Metnew/neural-finance/tree/31a87647322302dbd133a9891827fd1ec7d52a3a
I currently have some plans on this repo, sure, that docs/info will be available in next few days.

Nice!
Are you aware of https://docs.gdax.com/#api ?

Yes, sure.

👍 I have just started looking into the concepts behind all this and playing around with the API and figuring out the environment. Looking forward to the docs update.

It's my bachelow degree project, so repo will be maintained and some positive results will be too.

Updates will be soon, guys :)
The project is targeting cryptocurrencies now


Send rsi submissions to dap as well as downstream

What? and Why?
https://trello.com/c/fRBkb2xA/753-copy-rsi-data-and-send-to-dap-as-a-dtrades-submission-2
RSI submissions need to go to DAP as well as to common software.
How to test
Submit an RSI survey via console.  Make sure that it ends up in both the FTP folder and ends up on the DAP queue in rabbit (you can see it via the management console)
Checklist

[ ] CHANGELOG.md updated? (if required)

Added label because there's a chance that extra metadata is required to be passed in the dap payload


Project query dataset onto umap of existing reference dataset

[ ] Additional function parameters / changed functionality / changed defaults?
[ ] New analysis tool: A simple analysis tool you have been using and are missing in sc.tools?
[ ] New plotting function: A kind of plot you would like to seein sc.pl?
[ ] External tools: Do you know an existing package that should go into sc.external.*?
[X] Other?

Would it be possible to project a query dataset onto an existing reference umap, such that the reference dataset is not altered? The readout would be the existing reference umap with perhaps the density of query cells overlaid on top. Perhaps also quantification of the percentage of query cells falling into specific clusters on the reference data set?  Apologies if this already exists, I would be very interested to know how to do this using the anndata framework. Thanks.

https://scanpy-tutorials.readthedocs.io/en/latest/integrating-data-using-ingest.html
Hi, this should be relevant.

Thank you.


Check $ACTION_ROOT

Adding check for HLS actions that $ACTION_ROOT ends in vhdl or in verilog

Deleting the non vhdl directories is fine for me too.


changed toolbar key color to cyan

Coverage remained the same at 82.656% when pulling bbfae9c6e84af2f963bdf37eaf823578ed6c4e2f on m/toolbar-key-color into c0ddd17908bee33b69a0bb95cd38a844f1aa191a on master.

Codecov Report

Merging #220 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #220   +/-   ##
=======================================
  Coverage   82.65%   82.65%
=======================================
  Files          17       17
  Lines        2116     2116
=======================================
  Hits         1749     1749
  Misses        367      367

Impacted Files
Coverage Δ

src/crate/crash/repl.py
57.86% <ø> (ø)
:arrow_up:

Continue to review full report at Codecov.

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


Schema command not working

I am not able to generate the schema for parquet file .
Below is the error.
parquet-tools schema Sample.parquet
usage: parquet-tools [-h] {show,csv,inspect} ...
parquet-tools: error: argument {show,csv,inspect}: invalid choice: 'schema' (choose from 'show', 'csv', 'inspect')

What do you want do with schema sub command?
If you'd like to see metadata of the parquet file, you can use inspect command.
I'm sorry to be confusing, this command is incompatible with original parquet-tools.

I need parquet schema file for specific use in Informatica ( ETL Tool ) Mapping . Informatica support demonstrated that they are able use Schema command in working on Unix environment.

But I am not able to use in windows based environment.

Is there any way , I can generate the schema file .


Mod Browser

Summary
Support in-game browsing and installing of mod content.

Notes
I'm thinking that the ModManager should have a list of ModSources from which it can find content. The ModSources are responsible for listing available mods and acquiring them. It might be useful for a source to provide it's own UI for the browser.
LocalFileSystemModSource
This is a basic mod source that looks at a specified directory. The functionality would largely be similar to the existing behavior. It lists all mods in the directory and allows you to enable/disable them.
GithubRepoModSource
This mod source references a url that point to a GitHub repo. It will list all available mods in the repo and will download mods to the local filesystem.

And as you mentioned this could be easily configured using a modsources.dat file with the following example content:
[
    {
        "class": "com.fqn.modsources.LocalFileSystem",
        "root": "C:\\Some\\path"
    },
    {
        "class": "com.fqn.modsources.GitHub",
        "uri": "https:\\\\www.github.com\\delver-community-mods"
    }
]

I feel like the decision of enabling/disabling a specific mod should be the responsibility not of the mod source but the mod manager. What do you think?

That is an interesting thought.
Also we are already serializing the modmanager to disk, so it doesn't need to be a separate.dat file.

The next step would be to copy over the existing functionality into the LocalFileSystemModSource class.


fix(RTL): align text to left

wip

Warnings

:warning:
There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

  Generated by :no_entry_sign: dangerJS


How to add custom gradian color add to config?

How to add this to the config file
inear-gradient(to right, #007991, #78ffd6)
    theme: {
        extend: {
            colors: {
                primary: "#164F63",,
                secondary: "#A3C0BA",
                neutral: "#FAFAFA"
            },
        }
    }

Wouldn't that work for you? Can you provide mode context about that? Thank you

Can show me the method of adding?
On Sat, 27 Mar, 2021, 2:53 PM Anthony Fu, @.***> wrote:

Wouldn't that work for you? Can you provide mode context about that? Thank
you
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/windicss/windicss/issues/202#issuecomment-808699339,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADTQJSIXIIFXKEEEMSV5BGTTFWPXZANCNFSM4Z4V6DKA
.

The way you have configured looks fine, you can use this in your classes
bg-gradient-to-r from-primary to-secondary

Actually I have applied primary class everywhere.
Now I wanna change the primary class value to a gradient colour.
Currently primary: 'a single color'.
Is it possible to have gradient value on single colour classe?🤔
On Sat, 27 Mar, 2021, 3:44 PM Anthony Fu, @.***> wrote:

The way you have configured looks fine, you can use this in your classes
bg-gradient-to-r from-primary to-secondary
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/windicss/windicss/issues/202#issuecomment-808706578,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADTQJSIS5LLJN5R7ZNAGAE3TFWVZFANCNFSM4Z4V6DKA
.

you can do so also via backgroundImage to change only bg-primary (other utilities shouldnt be affected)
{
  theme: {
    extend: {
      colors: {
          primary: "#164F63",,
          secondary: "#A3C0BA",
          neutral: "#FAFAFA"
      },
      backgroundImage: {
        primary: 'linear-gradient(to right, #007991, #78ffd6)',
      },
    },
  },
}

then you can use bg-primary

Is it possible to achieve gradation in this way?
theme: {
  extend: {
    colors: {
          'primary-default': 'var(--color-primary-default)',
          'primary-dark': 'var(--color-primary-dark)',
          'primary-light': 'var(--color-primary-light)',
    },
  }
}

bg-gradient-to-r from-primary-default to-primary-light


Cannot use with vue-class-component

Reporting a bug?
In case of Options API vue-i18n-loader parses the <i18n></i18n> section and generates code that puts the result into component object itself. It works for Options API + JS, but breaks when using vue-class-component because in that case component options are not the component object itself, but it's "__o" property.
Expected behavior
Code generated by vue-i18n-loader checks if __o prop exists on the component object and puts translations into it if it does.
Reproduction
https://github.com/SniperJoe/vue-i18n-loader-repro
Issue Package
vue-i18n-loader
System Info
System:
    OS: Linux 5.11 Ubuntu 21.04 (Hirsute Hippo)
    CPU: (16) x64 AMD Ryzen 7 3800X 8-Core Processor
    Memory: 2.01 GB / 31.35 GB
    Container: Yes
    Shell: 5.1.4 - /bin/bash
  Binaries:
    Node: 12.21.0 - /usr/bin/node
    npm: 8.3.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 93.0.4577.82
    Firefox: 92.0
  npmPackages:
    @intlify/vue-i18n-loader: ^3.2.0 => 3.3.0
    vue: ^3.2.0 => 3.2.21
    vue-i18n: ^9.2.0-beta.11 => 9.2.0-beta.17
    webpack: ^5.44.0 => 5.62.1

Screenshot
No response
Additional context
The workaround I use is to edit bundle-utils/lib/json.js:
<..>
                        generator.push(`${exportSyntax} function (Component) {`);
                        generator.indent();
                        generator.pushline(`const newI18N = (${componentNamespace}__o ? ${componentNamespace}__o.${variableName} : ${componentNamespace}${variableName}) || []`);
                        generator.pushline(`${componentNamespace}${variableName} = ${componentNamespace}${variableName} || []`);
                        generator.push(`newI18N.push({`);
                        generator.indent();
                        generator.pushline(`"locale": ${JSON.stringify(localeName)},`);
                        generator.push(`"resource": `);
<..>
                        generator.push(`if (${componentNamespace}__o) {`);
                        generator.indent();
                        generator.push(`${componentNamespace}__o.${variableName} = newI18N;`);
                        generator.deindent();
                        generator.push(`} else {`);
                        generator.indent();
                        generator.push(`${componentNamespace}${variableName} = newI18N;`);
                        generator.deindent();
                        generator.push(`}`);

Validations

[X] Read the Contributing Guidelines.
[X] Read the README
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.

P. S. I've set "Issue Package" to "vue-i18n-loader", but in fact I'm not sure.

Thank you for your reporting!
I tried to reproduce using your repo, but unfortunately, I could not reproduce it 😞
Could you give us the reproduction minimum repo that is forked the below link repo, please?  🙏
https://github.com/intlify/vue-next-webpack-preview

Sure! Please take a look: https://github.com/SniperJoe/vue-next-webpack-preview
Had to update few packages, including Webpack, as vue-class-component requires ts-loader@^9 and ts-loader@^9 requires Webpack 5.

Thank you for your reproduction code!
I've just tried to confirm this issue. and I could understand this issue.
Oh, I didn't know that in vue-class-component, options passing from the vue runtime are treated in a special way.
so, I had not implemented i18n custom blocks for vue-class-component.
This issue needs to be dealt with with the generator on the bundle-tools.
However, vue-class-component may not be used by everyone, in vue-i18n-loader we need to add useClassComponent options. This will allow us to control the code generation by the generator and use i18n custom blocks in vue-class-component as well.

@kazupon please take a look at the PR

@SniperJoe
Once again, Thank you for your contribution! ❤
I've just released vue-i18n-loader latest version. 🚀
https://github.com/intlify/bundle-tools/releases/tag/vue-i18n-loader%404.1.0
Please check it!
npm install --save-dev @intlify/vue-i18n-lader

@kazupon it works!
Thank you very much for collaboration :smiley:


Allow to run debug apk from appbundle like android studio

Use case

I have an install-time module from android platform channel. It works normally when I debug android project in android studio with apk from appbundle setting.It also works when I build release with flutter build appbundle
When I run debug from vscode or flutter run, it doesn't cognize this install-time module and I got asset not found error in platform channel code.

Proposal

Have an option to run debug with appbundle in flutter command or vscode is a great option.
I want to have this setting in flutter debug:

@ductranit
Are you looking to have the option in Android Studio ? or in flutter command flutter build appbundle by having an argument / flag to be able to run debug apk ?

@darshankawar I want to have this option (debug appbundle) on flutter run by have an argument or I can debug it on vscode (this option is existing in android studio).
The problem is that I can't debug the install-time assets (it needs to build appbundle) now

@darshankawar
I want to have this option (debug appbundle) on flutter run by have an argument or I can debug it on vscode (this option is existing in android studio).
The problem is that I can't debug the install-time assets (it needs to build appbundle) now

Any news on this? We also need to run our flutter app from app-bundle (debug) when developing as we have there fast_follow asset downloading implemented and so for our local development we need to run our app from app-bundle.

cc @christopherfujino It would be very useful to include bundletool capabilities in our flutter run to round out our android capabilities. It seems that the tool is incomplete if the tool is unable to run the primary android output.

I think 'flutter run' would need to first build a debug aab and then use bundletool to create an universal APK from it.
There is a related issue:
https://github.com/flutter/flutter/issues/117127


Select input responsive size

Github Issue or Trello Card
This PR addresses this issue: https://github.com/palmetto/palmetto-components/issues/437
Adding responsive sizes to SelectInput.
What type of change is this?
Please delete options that are not relevant.

[ ] Bug fix (non-breaking change which fixes an issue)
[X] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Updating documentation.
[ ] Updating deployment/build pipeline.

Checklist:

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

UI Checklist

[X] I have conducted visual UAT on my changes/features.
[X] My solution works well on desktop, tablet, and mobile browsers.

 Responsive size prop for SelectInput


fix: update engine and add .nvmrc

I have deployed this

Since the another pr with v2 api was deployed, I have merged and deployed this again.


[Feature] Add support for old.reddit.com

Title?

I have updated the script.
I don't use the old Reddit layout, so let me know if anything goes off.


Crash on Windows when generating tags on a large project

Steps to reproduce :

Load a large project
Click on "rebuild tags"
Close Atom
[Open it while you still see a ctags-win32.exe process in task manager
-> crashes even if I wait ctags to finish]
Atom crashes with "The editor has crashed", no message on the debug console

Note : If you open Atom again while you still see a ctags-win32.exe process in task manager
that will add .tags1 file in the root of the project.
Note2 : I'm building Chromium and the .tags file is above 1 GB
Related question, is it building tags while Atom is open?

Upon further testing, the crashes also appear with tags generated by other plugins (symbols-gen) so I'm closing the issue.
Sorry for opening it.


Multi nodes deletion should also delete the lg template

single nodes worked well but select multi nodes and click delete, the lg templates still exist in lg file.

addressed via https://github.com/microsoft/BotFramework-Composer/pull/2236