makePublic and makePrivate for buckets and files.

Breaking down #360 into this issue. We should add makePublic and makePrivate methods to buckets and files in the Storage API.

Can you remind me how the gcloud CLI does it?

gsutil doesn't really do it automatically. You have to specify certain flags, including -R for recursive to set all objects in the bucket to be publicly readable, and then once you have changed every object, you can then change the bucket itself to have a predefined acl of publicRead. This is an O(N) operation where N is the number of objects in your bucket, requires N + 1 API calls in the best case, must be capable of returning N + 1 possible errors in the worst case, and if we want to consider rolling back changes, the complexity will grow.

This issue / feature does not sit well with me. For one, it can have a bad effect if improperly documented and understood. Making an entire bucket's contents public is not something that is one-click away anywhere within the developer console. If the developer wants to do this, it should be possible to do with our API but not something that is suggested or promoted.
Now, making a single file public is, and that's a one-API-call thing away. We should support that.
Making a bucket's new content public by default is also one-API-call away. We should support that too.
Making a new, public bucket is one or two API calls, we can even support that.
But retroactively applying public to every file to a bucket introduces a lot of room for errors, it can be a slow operation which might confuse people and they might panic, kill it and then have half their bucket public and the other half private if they don't understand what it's doing behind-the-scenes. Just seems chaotic.
My proposal is to add the 2-3 methods suggested in paragraph 2, and then provide an example in the docs to make their entire bucket's contents public, assuming best case no-errors, no-pagination kinda situation.
/rant

+1
Does that leave us:

Make a single file public - file.acl.makePublic()
Make a bucket's new content public - bucket.acl.makePublic() + bucket.acl.default.makePublic()? What's the difference of these two, again?

We should have:

Make a single file public: file.acl.makePublic() (this api call)
Make a bucket's new content public: bucket.acl.default.makePublic() (this api call)
Make a bucket that is public by default createBucket('mybucket', { public : true }, cb) will just use predefinedDefaultObjectAcl: 'publicRead' on the createBucket metadata call. (I think this will work but if not, well we can scrap the idea as a result of the limitations of the API).

So no bucket.makePublic()?

Nope. I think it's more obvious to understand if it's under the default object. I'm not sure if that will be annoying to implement?

Haha, I think it will be, but oh well. We'll get this API perfect one way or another!

See https://github.com/GoogleCloudPlatform/gcloud-python/blob/master/gcloud/storage/bucket.py#L723 for reference.

I'll take a stab at this.

Implemented!


Peer reviewer areas of expertise

@ahmadmardeni1, kindly take a look at this PR.

Thank you @Ericgacoki


DOC: Documentation Update Needed for 'Return Source Documents' Functionality

Issue with current documentation:
Issue Description

Overview: The current documentation for the 'Return Source Documents' functionality seems to be outdated or incorrect. The provided code snippet results in errors when executed.

https://python.langchain.com/docs/integrations/providers/vectara/vectara_chat#conversationalretrievalchain-with-question-answering-with-sources

Details:

The current code:chat_history = []
query = "What did the president say about Ketanji Brown Jackson"
result = bot({"question": query, "chat_history": chat_history})

produces the following error:/python3.9/site-packages/langchain/memory/chat_memory.py", line 29, in _get_input_output
raise ValueError(f"One output key expected, got {outputs.keys()}")
ValueError: One output key expected, got dict_keys(['answer', 'source_documents'])

Examples: This issue occurs when trying to use the 'Return Source Documents' as outlined in the current documentation.

Additional Information

Related Issue: This documentation update is related to the issue raised in https://github.com/langchain-ai/langchain/issues/2256.

Idea or request for content:
Suggested Fix

Update the documentation with the correct code snippet:memory = ConversationBufferMemory(memory_key="chat_history", input_key='question', output_key='answer', return_messages=True)
bot = ConversationalRetrievalChain.from_llm(llm, retriever, memory=memory, return_source_documents=True)
result = bot({"question": query,})

This revision correctly handles the output and does not produce the aforementioned error.

#15106 This might help


skip testing type versions not in any deployed standard

Modifies _assert_extension_type_correctness to skip checking type versions that are not part of a non-development supported version of the asdf-standard.
Two other tests are added test_default_in_supported_versions and test_development_is_not_default to catch possible errors where the default is set to a non-supported (or development) version.
As this PR will skip testing completeness (that a type has a schema of the same version) of type versions that are only in development version, it's possible that updating the development version of the asdf-standard will cause test failures if the schema and type versions do not match. Put another way, a PR that advances the asdf-standard development version might reveal and then have to fix these type/schema version issues.
This should allow PR #1250 to pass CI without an asdf-standard release.

It might make more sense to add this to #1250 so we can verify it passes CI.
If the CI here passes we'll at least have confirmation that it didn't add any new failures to the master branch.

PR #1255 (the combination of this PR and #1250) passed CI which suggests that the changes included in this PR should be sufficient to allow #1250 CI to pass.

close/reopen to attempt to reset doc build


Non OpenAI models

It would be good to get some more documentation and examples of using models other than OpenAI. Currently the docs are really heavily skewed and in some areas such as conversation only offer an OpenAI option.
Thanks

@darth-veitcher I tried gpt4all but sometimes crashes my session,
https://python.langchain.com/en/latest/modules/models/llms/integrations/gpt4all.html


Tests are flakey

We have an example of a CI that was red, here:
https://github.com/aarani/NOnion/actions/runs/3320836041/jobs/5487692697

Another occurrence: https://github.com/parhamsaremi/NOnion/actions/runs/3327816501/jobs/5503078231

New occurrence: https://github.com/nblockchain/NOnion/actions/runs/5132132241/jobs/9233038055


HDDS-10165. Replace GenericTestUtils temp dirs with @TempDir in ozone unit tests

What changes were proposed in this pull request?
Replace usage of GenericTestUtils test/temp dir methods with @TempDir in hadoop-ozone except in integration tests.
(see parent task HDDS-9103 for details)

The change covered..
Introduced local @TempDir variable in some cases.
Introduced global @TempDir at one location because the directory was shared among directories.
Removed the deletion logic of manually created temp directories.
Removed setup methods if it has nothing else then initializing the temp dir.

What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10165
How was this patch tested?
Tested in CI-CI pipeline and a few manually.

Thanks @raju-balpande for working on this.
There are some conflicts, please merge from master.
Also, one unit test failure looks related:
Error:  Tests run: 6, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.059 s <<< FAILURE! - in org.apache.hadoop.ozone.security.TestOzoneTokenIdentifier
Error:  org.apache.hadoop.ozone.security.TestOzoneTokenIdentifier.testReadWriteInProtobuf(Path)  Time elapsed: 0.064 s  <<< ERROR!
java.io.FileNotFoundException: /tokenFile (Permission denied)
	at java.io.FileOutputStream.open0(Native Method)
	at java.io.FileOutputStream.open(FileOutputStream.java:270)
	at java.io.FileOutputStream.<init>(FileOutputStream.java:213)
	at java.io.FileOutputStream.<init>(FileOutputStream.java:162)
	at org.apache.hadoop.ozone.security.TestOzoneTokenIdentifier.testReadWriteInProtobuf(TestOzoneTokenIdentifier.java:255)

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
 210: 'testDir' hides a field.
 361: 'testDir' hides a field.
 452: 'testDir' hides a field.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java
 39: Unused import - org.apache.hadoop.security.authentication.client.AuthenticationException.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestParentAcl.java
 46: Unused import - org.apache.hadoop.security.authentication.client.AuthenticationException.

https://github.com/raju-balpande/apache_ozone/actions/runs/7741058283/job/21107319150

Thanks @raju-balpande for the patch.


Desktop - Error messages is shown in Spanish when the language was already changed to English

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Action Performed:

Go Preference
Set offline
Change the language to Spanish
Click on new chat with invalid number, example +15005550111
Send messages
Go back to Preference, change the language to English
Go to the same chat again and set online

Expected Result:
Verify that the error messages is shown in English when the language was already changed to English.
Actual Result:
The error messages is shown in Spanish when the language was already changed to English.
Workaround:
unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[X] MacOS / Desktop

Version Number: v1.3.18-2
Reproducible in staging?: Y
Reproducible in production?: N/A the toggle is not present in PROD app
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Found when executing PR https://github.com/Expensify/App/pull/19324
https://github.com/Expensify/App/assets/43995119/60b1a2d1-5839-48a7-97e5-164b2b2e4523
Expensify/Expensify Issue URL:
Issue reported by: Applause Internal Team
Slack conversation:
View all open jobs on GitHub

@mvtglobally I have checked your recorded video.
When user change the language preference, the following code will be executed.
https://github.com/Expensify/App/blob/0f1188de4d7b69ecb2b16485e9f03d92f54afe84/src/libs/actions/App.js#L85-L112
But, in the offline mode (or the user have already changed the force offline mode to online, but the app is still in the offline status, as you can see in your video), API.write only change the local Onyx data, and the request API that update the language preference does not send to the server.

After send the comment, the server will send the response error message and the Onyx data that is stored in the server to the app, and these data will contain the original language preference, as you can see in the below screenshot.

So, after app receive the server response and merge the server's Onyx data, the language preference will be changed to the previous state, that is Spanish, and the error message is shown in Spanish.
That is, this issue is not a bug, or at least is not produced by this PR(https://github.com/Expensify/App/pull/19324).

The root cause of this issue is as follows.
When switched from offline mode to online mode, queued requests are sent. If user changes the locale from English to Spanish and again from Spanish to English, then 2 updatePreferredLocale APIs are called with en and then es.
This works fine, but the push notification arrives sometimes in reverse order and sometimes a bit later by my observation.

That's the main reason.
I think the push notification shouldn't be sent to the author(who sent the request). Or at least add user ID info not to update onyx store.
This should be handled in backend side

Not sure if it's related, but this bug seems similar to this other one: LHN Attachment taking some to to translate into Spanish

Proposal
Please re-state the problem that we are trying to solve in this issue.
The error messages are shown in Spanish when the language was already changed to English.
What is the root cause of that problem?
When the user changes from offline mode to online mode, the queued requests are sent to the server.
If user changes the language from English to Spanish, then one UpdatePreferredLocale API are sent to the server. In this case, the app's language is set to Spanish.
If user changes the language from English to Spanish and again from Spanish to English, then 2 UpdatePreferredLocale APIs are sent with en and then es language value. And the responses from the server are processed in reverse order, so the app's language is set to Spanish.
After that, though user changes the language, only 2 APIs are sent to the server, because the same request is removed by following code.
https://github.com/Expensify/App/blob/a6e6fe9252af0d3cc19966abaf550e5eba5a466c/src/libs/actions/PersistedRequests.js#L34-L37

That is, send multiple UpdatePreferredLocale API causes the app language is set different with the user set.
What changes do you think we should make in order to solve the problem?

We should send only the last UpdatePreferredLocale API to the server once.
For this, we can remove previous API requests and add the last UpdatePreferredLocale API request by changing the save function(https://github.com/Expensify/App/blob/a6e6fe9252af0d3cc19966abaf550e5eba5a466c/src/libs/actions/PersistedRequests.js) as below.
function save(requestsToPersist) {
    HttpUtils.cancelPendingReconnectAppRequest();
    persistedRequests = _.chain(persistedRequests)
        .concat(requestsToPersist)
        .partition((request) => request.command !== CONST.NETWORK.COMMAND.RECONNECT_APP)
        .flatten()
        .value();
    if (requestsToPersist[0].command === 'UpdatePreferredLocale') {
        persistedRequests =  _.filter(persistedRequests, (persistedRequest) => (persistedRequest.command !== 'UpdatePreferredLocale'));
        persistedRequests = _.chain(persistedRequests)
            .concat(requestsToPersist)
            .value();
    }

    Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests);
}

Now we can send the last one API to the server, so the app language is set correctly when changes from offline mode to online mode.
What alternative solutions did you explore? (Optional)
None.

@fedirjh  Thank you for pointing out.
@Christinadobrzyn Could you check this https://github.com/Expensify/App/issues/19651#issuecomment-1564929913
I am also not sure of the issue here, the error message is in sync with the app's language.
Are we thinking that,
"app language should directly change to the last choosen language, instead of switching through the previous selections?" If so I think that's a different issue and this issue's information has to be updated,
And the proposals here is also trying to fix the issue of "app language should directly change to the last choosen language, instead of switching through the previous selections"

@s-alves10 Not sure if this https://github.com/Expensify/App/issues/19651#issuecomment-1564544250 is a proposal or general rootcause comment. If it is a proposal, kindly update the proposal as per our guidelines. https://github.com/Expensify/App/blob/main/contributingGuides/PROPOSAL_TEMPLATE.md

Proposal
Please re-state the problem that we are trying to solve in this issue.
After forcing offline, changing language to Spanish, sending messages to an invalid number, then again changing language to English, then going online shows errors for the messages in Spanish.
What is the root cause of that problem?
Lines 39 to 71 of App/src/libs/API.js
function write(command, apiCommandParameters = {}, onyxData = {}) {
    ....

    // Write commands can be saved and retried, so push it to the SequentialQueue
    SequentialQueue.push(request);
}

API.js pushes requests to SequentialQueue
These requests are pushed to persistedRequests and they would be processed right away through flush if network is online. If NetworkStore.isOffline()), push returns and requests stay in persistedRequests.
Lines 105 to 121 of App/src/libs/Network/SequentialQueue.js
function push(request) {
    // Add request to Persisted Requests so that it can be retried if it fails
    PersistedRequests.save([request]);

    // If we are offline we don't need to trigger the queue to empty as it will happen when we come back online
    if (NetworkStore.isOffline()) {
        return;
    }
    ...
    flush();
}

Once network is online SequentialQueue all requests in persistedRequests are processed one by one.
Line 32 to 57 of App/src/libs/Network/SequentialQueue.js
function process() {
    const persistedRequests = PersistedRequests.getAll();
    ...
    currentRequest = Request.processWithMiddleware(requestToProcess, true)
        .then(() => {
            PersistedRequests.remove(requestToProcess);
            RequestThrottle.clear();
            return process();
        })
       ...
    return currentRequest;
}

Language change, and adding comments after forcing offline get added to persistentRequests and they are processed one by one after network is online and it appears slow.
In this case [UpdatePreferenceLocale, AddComment, AddComment, UpdatePreferenceLocale] are processed one after another. So, first language is changed to Spanish, then comments added and the errors would be displayed in Spanish, then language is changed to English and page changes to English.
What changes do you think we should make in order to solve the problem?
Remove the pushedUpdatePreferredLocale from persistentRequestswhen network is offline. When language is changed, the page changes to that language irrespective of the network connectivity. I think keeping it for running after getting online is unnecessary.
For this, the following change should be made.
Lines 105 to 107 of App/src/libs/Network/SequentialQueue.js
function push(request) {
    // Don't retry UpdatePreferredLocale requests so don't add to persistedRequests
     if (request.command !== 'UpdatePreferredLocale') {
        PersistedRequests.save(request);
    }
  ...
}

What alternative solutions did you explore? (Optional)
Only keep the latest UpdatePreferredLocale in peristedRequests and push it to the top of persistedRequests. So after coming to online only the last selected preference would be there and it will update first.
For this, retain the SequentialQueue.js as it is but following change is required in PersistedRequests.js
Line 21 to 29 of App/src/libs/action/PersistedRequests.js
function save(requestsToPersist) {
    HttpUtils.cancelPendingReconnectAppRequest();
   if (requestsToPersist[0].command === "UpdatePreferredLocale") {
       persistedRequests = _.filter(persistedRequests, persistedRequest => persistedRequest.command !== "UpdatePreferredLocale";
        persistedRequests = _.chain(requestsToPersist)
          .concat(persistedRequests)
         .partition((request) => request.command !== CONST.NETWORK.COMMAND.RECONNECT_APP)
          .flatten()
          .value();
  }
  else {
     persistedRequests = _.chain(persistedRequests)
          .concat(requestsToPersist);
         .partition((request) => request.command !== CONST.NETWORK.COMMAND.RECONNECT_APP)
          .flatten()
          .value();
   }

    Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests);
}

@abdulrahuman5196 - great question

"Are we thinking that "app language should directly change to the last choosen language, instead of switching through the previous selections?" If so I think that's a different issue and this issue's information has to be updated,
And the proposals here is also trying to fix the issue of "app language should directly change to the last choosen language, instead of switching through the previous selections"

The issue was that the error message was in Spanish after you changed the language to English (and enabled online). So "app language should directly change to the last choosen language, instead of switching through the previous selections" is what I would expect to happen here.
But I'm not able to reproduce this Spanish error anymore - going to reach out to QA to see if they can test again.

@Christinadobrzyn I'm able to reproduce this error. Please change the language an even number of times, and change to online mode.

Okay, apparently this is still happening and I can reproduce this. Updated the steps in the OP on how I was able to do this.
what is expected: When switching the language, all messages (included errors) should display in the selected language.

@abdulrahuman5196 can you please let us know if any of the submitted proposals will work? Otherwise, let us know if you'd like to see more proposals. Thanks!

Ah sorry, didn't mean to close this!

I couldn't review due to the churns in repro steps. Will try to repro from my end and review the proposals.
Meanwhile will also check if the above linked tracking issue would solve this.

Asking here - https://github.com/Expensify/App/issues/12775#issuecomment-1574385102

@Christinadobrzyn  I think it is not error message alone, but the whole app would have changed if we check in the preferences. Like the below video, correct me if not.
https://github.com/Expensify/App/assets/46707890/503e7454-054d-4110-bac3-54c89571c9bb
@cead22
If the above is confirmed by @Christinadobrzyn, I think the app is basically updating to last succeeded locale update API.
Not sure if we should consider this as an issue.
Even if we try to solve it by removing existing API's from persisted requests during offline and make sure only one API is send when user becomes online, it will not solve the total issue.
Like assume you have 2 devices in offline state and make some locale updates and then when both devices comes online, the account will still be set to the last succeeded locale update API response.

I'm not sure what would be best for your questions here @abdulrahuman5196 related to the last succeeded locale update API.
@cead22 - would you be able to help? Maybe this isn't something we should change at this time?

@cead22 Waiting on you for the above question - https://github.com/Expensify/App/issues/19651#issuecomment-1577224981

Sorry, I'm not 100% confident I understand this. I'll ping in slack to see if we can get some help!

@StevenKKC my bad, your proposal was the first I saw and I meant to comment on it, and forgot by the time I got to the end. I have a couple of questions

If user changes the language from English to Spanish and again from Spanish to English, then 2 UpdatePreferredLocale APIs are pushed to the queue and will be sent with en and then es language value. And the responses from the server are processed in reverse order, so finally the app process the first API's response, and language is set to Spanish.

Why are responses from the server processed in reverse order, and does this happen 100% of the time?

For this, in offline mode when the new API is pushed to the queue, we would remove previous API requests and push the last API request by changing the save function as below.

if (NetworkStore.isOffline() && requestsToPersist[0].command === 'UpdatePreferredLocale') {
persistedRequests = _.reject(persistedRequests, (persistedRequest) => (persistedRequest.command === 'UpdatePreferredLocale' && persistedRequest !== requestsToPersist[0]));
}

You say we should only send the last request, but this seems to be keeping the first one in requestsToPersist. Shouldn't we keep the last one .chain(persistedRequests).concat(requestsToPersist)?

@abdulrahuman5196 @c3024 @s-alves10 @fedirjh @StevenKKC is it possible that the actual issue is that we're storing the translated message in Onyx, eg, "Report no longer exists", instead of saving the translation key, and that's why we see a Spanish message "Error inesperado al..." along with "Report no longer exists"?

@cead22

Why are responses from the server processed in reverse order, and does this happen 100% of the time?

Please check this comment(https://github.com/Expensify/App/issues/19157#issuecomment-1555046815), and I tested several times closely, and responses are definitely processed in reverse order. But I think it's hard to say that this happens 100% of the time.
Even if responses are not processed in reverse order, there is no guarantee that they will be processed in the correct order. So I think send the last UpdatePreferredLocale request is the best solution.

You say we should only send the last request, but this seems to be keeping the first one in requestsToPersist. Shouldn't we keep the last one .chain(persistedRequests).concat(requestsToPersist)?

As you can see in the below code, first concatenate requests, and then remove all UpdatePreferredLocale request except the one in requestsToPersist, so will keep the last one.
function save(requestsToPersist) {
    HttpUtils.cancelPendingReconnectAppRequest();
    persistedRequests = _.chain(persistedRequests)
        .concat(requestsToPersist)
        .partition((request) => request.command !== CONST.NETWORK.COMMAND.RECONNECT_APP)
        .flatten()
        .value();

    if (NetworkStore.isOffline() && requestsToPersist[0].command === 'UpdatePreferredLocale') {
        persistedRequests = _.reject(persistedRequests, (persistedRequest) => (persistedRequest.command === 'UpdatePreferredLocale' && persistedRequest !== requestsToPersist[0]));
    }

    Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests);
}

@cead22
"Report no longer exists" is not translated and shown always in english regardless of the language. I think that should be a different issue. Correct me if wrong @Christinadobrzyn
Title and OP says a different issue
Desktop - Error messages is shown in Spanish when the language was already changed to English

**Expected Result:**
Error should be in English now that the account language is English
**Actual Result:**
The error messages is shown in Spanish when the language was already changed to English.

I am trying to repro the issue this way

Create invalid chat and send a message
Switch Language multiple times offline(5-10 UpdatePreferredLocale API in queue) and go online.
switch the language again while online(eg. english) and the language changes to spanish after a while(propably from the response of a UpdatePreferredLocale API from queue)
And all the errors change to spanish.

Didn't have much luck in reproduction, that's why questioned here - https://github.com/Expensify/App/issues/19651#issuecomment-1577224981 following this repro comment https://github.com/Expensify/App/issues/19651#issuecomment-1571113461. Kindly check back the question with the context I am testing out, it would provide clear information. If not do let me know I can explain(But even I haven't been able to repro the exact output issue and that's why I am checking with @Christinadobrzyn)
But if "Report no longer exists" is not translated, is the real issue then the issue title and OP is wrong.

Testing this again. I can no longer reproduce what is in the OP.
When I change the language to Spanish while offline > send a message > change the language and come back online > the error is in the correct language.
Asking QA to test again - https://expensify.slack.com/archives/C9YU7BX5M/p1686613044188359

@Christinadobrzyn I'm still able to reproduce this error. Please change the language an even number of times, and change to online mode.

Hey @abdulrahuman5196 can you review the above ^ to see what is happening and let me know if you still need me to change the title in the OP based on https://github.com/Expensify/App/issues/19651#issuecomment-1587807537

@kavimuru
Even in you video, not only the error message the whole app language and account language is changed to english.
If you go to the Settings -> Preferences, it would show english as selected language now.
It is not matching the Expected OP mentioned, correct me if wrong.

Error should be in English now that the account language is English

Sorry @Christinadobrzyn, It's like we are going in circles due to not clear OP. @mvtglobally  Could you kindly clear out the issue since you where the issue creator?

@mvtglobally  I have been testing with "Force Offline" in the app. Could you kindly check this comment and let us know?
https://github.com/Expensify/App/issues/19651#issuecomment-1589578362
cc: @kavimuru @Christinadobrzyn

As you can see in the below code, first concatenate requests, and then remove all UpdatePreferredLocale request except the one in requestsToPersist, so will keep the last one.

@StevenKKC we're keeping the first requests in requestsToPersist, ie requestsToPersist[0], but there could be more than one request to update the language right? Shouldn't we keep the last one from requestsToPersist?

@cead22 As you can see below, request is converted to an array. So requestsToPersist has only one request.
https://github.com/Expensify/App/blob/f15e6dc98d2c5853ed3456c5c6c648a25c66823f/src/libs/Network/SequentialQueue.js#L105-L107
There is no call to PersistedRequests.save anywhere else.

Hey folks, Could we kindly park the discussions on proposals for now? And confirm on the issue and expected output first? as requested here https://github.com/Expensify/App/issues/19651#issuecomment-1589578362
cc: @cead22 @Christinadobrzyn @mvtglobally Kindly confirm on the issue
@StevenKKC  if you are able to reproduce kindly post the video for the same.

I can't reproduce this anymore, i don't know if it's because I've tested too many times with<PHONE_NUMBER> and now it's not working (I've tried other numbers too but they don't work either)

I'm not sure what to do here
@abdulrahuman5196 and I can't reproduce this but others can. And I can't find other steps that will reproduce this.
The videos provided here match what the steps in the OP.

Maybe I'll assign another teammate to this GH to see if they can reproduce it.

My understanding of the issue is shown in the video in the issue description (the OP).

Language is set to English
Force offline is turned off
🐛 User sees Error inesperado... 🇪🇸  (this shouldn't happen, imo)
Error changes to English Unexpected Error... 🇺🇸

My understanding of the issue is shown in the video in the issue description (the OP).
* Language is set to English

* Force offline is turned off

* 🐛 User sees **Error inesperado...** 🇪🇸  (this shouldn't happen, imo)

* Error changes to English **Unexpected Error...** 🇺🇸

@cead22  Yes. Exactly, but the point missed I think by the reporter here is Its not just the errors but the whole app changes. But still I wasn't able to reproduce this from start, Only way i was able to reproduce is like switching the language multiple times and coming back online like this https://github.com/Expensify/App/issues/19651#issuecomment-1593541566. Kindly do let me know if i am doing something wrong.

I will just frame questions in a way it will be easy to get us out. Let me see if this works out before we go to external contributors.
Q1) The OP expectation here is The error messages is shown in Spanish when the language was already changed to English.
As per the OP videos provided by reporter, I don't think only the errors are changed, the language through out the app is changed similar to how its pointed out here - https://github.com/Expensify/App/issues/19651#issuecomment-1564929913 and here - https://github.com/Expensify/App/issues/19651#issuecomment-1589578362
Q2) Do reporter(@mvtglobally) or BZ (@Christinadobrzyn ) agree on the above answer?
option 1) If we agree, then the expected OP should change to App language is changed to Spanish even though the last selected language is english, we can proceed to next questions.
option 2) if we don't agree, then the OP video is wrong as per the OP expectation and we should try to repro a scenario where only the error message is in spanish whereas the account language is in english(header and LHN), and we should stop here and the reporter/BZ should come back with the repro video or updated steps.
Q3) (If option1 from Q2) How much is it reproducible if we only switch back to english once as per OP steps?
option 1) Not reproducible (For me never reproducible and contributor also did repro it only by switching back and forth multiple times)
option 2) Rarely
option 3) always

Kindly do let me know if i am doing something wrong.

I don't see anything wrong here. We know what the wrong behavior looks like, and we know what we want it to be

Q1: Agreed
Q2: option 1) I agree, @abdulrahuman5196 can you update?
Q3: I'm not sure, but it looks like we have steps to reliably reproduce, right? If so, why does this matter? just curious, I'm happy with us updating the repro steps to make them reliable, but I'd love to continue moving this forward

@cead22

Q3: I'm not sure, but it looks like we have steps to reliably reproduce, right? If so, why does this matter? just curious, I'm happy with us updating the repro steps to make them reliable, but I'd love to continue moving this forward

I was asking this question because to understand if this issue is worth solving separately in the path of Only one UpdatePreferredLocale call should be present in case of the user making multiple language switches during offline case. Or should we knock on some other tree,
Might be a big para, kindly bare with me
There are actually 2 sub-issue paths here
Issue 1) Reply effect
The steps in the video
During offline

User changes language from english to spanish(UpdatePreferredLocale API 1)
User changes the language from spanish to english(UpdatePreferredLocale API 2)
User has done 2 language switches during offline case.

During online

App language changes to spanish while successful UpdatePreferredLocale API 1
App language changes to english while successful UpdatePreferredLocale API 2

This is a proper reply effect bug same as here - https://github.com/Expensify/App/issues/12775
Initially @Christinadobrzyn  tried to see if this will be solved by the reply effect fixes, but the answer was no. But I doubt they would have also be confused with the issue description and thought this as a different issue. We can reach out again on clear context and take it in the path of reply effect fixes.
But as far as the issue 1 goes, it will ultimately return to the proper language after swaps if there were just 1 or 2 time switching of languages happened offline.
Issue 2) Multiple UpdatePreferredLocale API's in the queue
I agree this is a issue and we should solve. But I don't agree with the fact that we should just check if the command is UpdatePreferredLocale on the persistant requests and remove previous requests on the same.
We should put more thought on this,

If we want to remove previous requests we should create a generic param in the network requests and remove previous requests if required. For ex, in API.write we can add a param removePreviousRequests and it should remove previous requests.
We should also consider how we should propagate successData and failureData in case of removing a request.(Currently UpdatePreferredLocale API doesn't add any success or failure data but good to investigate as a general implementation).

But still even if we solve this case and have only one API sent when the user goes online, this will not solve multiple device cases.
Like assume you have 2 devices in offline state and make some locale updates and then when both devices comes online, the account will still be set to the last succeeded locale update API response from one device.
@cead22 I am ok to solve this issue in the path of Only one UpdatePreferredLocale call should be present in case of the user making multiple language switches during offline case if we are fine with the thoughts on issue 2 mentioned here.
Bonus issue 3: 🤕 Different than current one.
While I was testing this I noticed. When you make a odd number of language switches offline. Like

english -> spanish
spanish -> english
english  -> spanish
For some reason the account language is set to english after sometime, not sure if this is related though, but anyways would be solved by this fix.

I agree this is a issue and we should solve. But I don't agree with the fact that we should just check if the command is UpdatePreferredLocale on the persistant requests and remove previous requests on the same.
We should put more thought on this,

Happy to discuss and brainstorm, but we don't need to build a general solution for all requests right now, we need to build a solution for this particular case, and if/when there's another case like this, we can generalize.

For ex, in API.write we can add a param removePreviousRequests and it should remove previous requests.

Specifically, I think we can avoid doing this until we have more than one case than can leverage it, and we can build a simpler solution for UpdatePreferredLocale

2. We should also consider how we should propagate successData and failureData in case of removing a request.(Currently UpdatePreferredLocale API doesn't add any success or failure data but good to investigate as a general implementation).

Again, let's solve the problem we have today and not problem we may have tomorrow

Like assume you have 2 devices in offline state and make some locale updates and then when both devices comes online, the account will still be set to the last succeeded locale update API response from one device.

This is interesting, but I think it's fine. Maybe a good solution for this would be to make the locale per-device, and not per-account, but I don't think we need to worry about this now.
@abdulrahuman5196 can we move forward with this approach?

On second thought, it looks like we may already have other places to apply this solution to, so the general solution might be the right way to go now, see https://github.com/Expensify/App/issues/19640

@cead22 Yes. I think even all other notification/settings kind of flow would have been affected by this kind of issue, where like multiple requests are queued.
we should go for a general solution. But the solution would become complex obviously.
tagging contributors who had already proposed @StevenKKC @c3024

Just following up - @abdulrahuman5196 @StevenKKC @c3024 any update on a proposal for this fix?

Currently we don't have a proposal satisfying of our generic requirement. If any contributors already proposed could update their solution it would be great. If not we have to look other options of increasing bounty or going internal

I started a discussion about how to move this forward here https://expensify.slack.com/archives/C01GTK53T8Q/p1687808610323569. We're leaning towards closing this issue

I'll check on that thread, not sure if we got to a conclusion.

I think we're moving forward with this, right @cead22?

I agree that my proposal is for a special case, not a general one.
Add removePreviousRequests to API.write function's apiCommandParameters parameter and change code as below maybe more acceptable.
    API.write(
        'UpdatePreferredLocale',
        {
            value: locale,
            removePreviousRequests: NetworkStore.isOffline(),
        },
        {optimisticData},
    );

function save(requestsToPersist) {
    HttpUtils.cancelPendingReconnectAppRequest();
    persistedRequests = _.chain(persistedRequests)
        .concat(requestsToPersist)
        .partition((request) => request.command !== CONST.NETWORK.COMMAND.RECONNECT_APP)
        .flatten()
        .value();
    if (requestsToPersist[0].data.removePreviousRequests && requestsToPersist[0].data.removePreviousRequests === true) {
        persistedRequests = _.reject(persistedRequests, (persistedRequest) => (persistedRequest.command === requestsToPersist[0].command && persistedRequest !== requestsToPersist[0]));
    }
    Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests);
}

For the general solution, there are not enough cases to consider, and I think the above solution is sufficient for some preference settings in offline mode.
It would be better if we discuss general solution when we have more cases in the future.

Not overdue.

I think we're moving forward with this, right @cead22?

I doubt that. Could you kindly provide us what is the path forward we are looking on this issue? @cead22

In this thread we identified the following two situations, and agreed that the first one isn't a bug, and that we'll use this issue to fix the second one, does that make sense?

If you switch languages multiple times while offline, when you come online, the language will switch multiple times as the requests queued while you were offline are dequeued and sent to the server
We have an error message that is always shown in English

Yes. @cead22 Fine by me

Correct me if I'm wrong but now that we're switching gears to solve only problem number 2 above, I don't think we have any proposals for that specifically right now.
If that's the case, @StevenKKC @c3024 @s-alves10 would you like to submit a proposal for this? Thanks

Still waiting for proposals

@cead22 / @Christinadobrzyn  The bug is still marked internal? Could we change it to external and update the bug title and description as per the new requirement from here - https://github.com/Expensify/App/issues/19651#issuecomment-1624166437

@abdulrahuman5196 should I raise the bounty to get more proposals?

That error is a back-end error, it cannot be fixed as we don't support handling translations for back-end errors yet. I would say that we can close this issue as we have done with many other instances when this was brought up.

@cead22 @abdulrahuman5196 should we close based on https://github.com/Expensify/App/issues/19651#issuecomment-1629867862?

Agree with Rocío, let's close


Trying to run Calculator fromm PowerToys Run causing an exception

Microsoft PowerToys version
0.72.0
Installation method
PowerToys auto-update
Running as admin
Yes
Area(s) with issue?
PowerToys Run
Steps to reproduce
alt shift to pop up the runner, enter calc, select calculator (it is the second option), enter, and then the error window is coming up:
Version: <IP_ADDRESS>
OS Version: Microsoft Windows NT 10.0.19044.0
IntPtr Length: 8
x64: True
Date: 8/23/2023 11:26:55 AM
Exception:
System.Runtime.InteropServices.COMException (0x80040154): Creating an instance of the COM component with CLSID {45BA127D-10A8-46EA-8AB7-56EA9078943C} from the IClassFactory failed due to the following error: 80040154 Class not registered (0x80040154 (REGDB_E_CLASSNOTREG)).
at Microsoft.Plugin.Program.Programs.UWPApplication.Launch(IPublicAPI api, String queryArguments)
at System.Threading.Tasks.Task.<>c.b__128_0(Object state)
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
✔ Expected Behavior
run the calculator
❌ Actual Behavior
does not run the calculator just pops up the error window
Other Software
No response

Not able to repro this, going to close. Let us know if you see this again!


Manually added payment processor fees not charged

Describe the bug
It seems that amounts put in the payment processor fees box when making a manual expense payout are not being applied.
A user reported this and I just tested it on staging.
I put $25 in the box and marked it as paid.

But on the expense and the transaction, it still shows $5000 not $5025.

Expected behavior
The system should add the processor fee amount to the total debited from the Collective balance for that expense. It should also show up transparently somewhere so you can see the fees that were applied.

@znarf @Betree can you take a look at this?

Looking at it

So the problem is not with the modal, things are properly recorded in our ledger. It's the value that we display in the transactions list that is incorrect. I'll make an update for that.
Showing the fees for paid expenses has been proposed in https://github.com/opencollective/opencollective/issues/3224, but it wouldn't hurt to also show that in the list - with the same "View details" pattern.


About the implementation of NpzDataset

I found that the NpzDataset in finetune_and_inference_tutorial.py is mostly implemented using numpy, which caused this code to run very slowly on my machine. I changed it to the following code implemented using tensor, and got a significant speed increase. At the same time The DSC on the sample data set MICCAI FLARE2022 is 0.9008, which is not lower than the result of the original code. I hope you can also try the following code.
class NpzDataset(Dataset):
    def __init__(self, data_root, image_size=256):
        self.data_root = data_root
        self.image_size = image_size
        self.npz_files = sorted(os.listdir(self.data_root))
        self.npz_data = [
            np.load(join(data_root, f
                         ), allow_pickle=True)
            for f in self.npz_files
        ]
        self.ori_gts = torch.vstack(
            [torch.from_numpy(d['gts']) for d in self.npz_data])
        self.img_embeddings = torch.vstack(
            [torch.from_numpy(d['img_embeddings']) for d in self.npz_data])
        print(self.ori_gts.shape, self.img_embeddings.shape)

    def __len__(self):
        return self.ori_gts.shape[0]

    def __getitem__(self, index):
        img_embed = self.img_embeddings[index]
        gt2D = self.ori_gts[index]
        y_indices, x_indices = torch.where(gt2D > 0)
        x_min, x_max = torch.min(x_indices), torch.max(x_indices)
        y_min, y_max = torch.min(y_indices), torch.max(y_indices)
        H, W = gt2D.shape
        x_min = max(0, x_min - torch.randint(0, 20, (1, )).item())
        x_max = min(W, x_max + torch.randint(0, 20, (1, )).item())
        y_min = max(0, y_min - torch.randint(0, 20, (1, )).item())
        y_max = min(H, y_max + torch.randint(0, 20, (1, )).item())
        bboxes = torch.tensor([x_min, y_min, x_max, y_max]).float()
        return img_embed.float(), gt2D[None, :, :].long(), bboxes

Hi @chenyangzhu1
Awesome! Thanks for sharing very much.
It's really a good idea to load the data as torch tensor directly! I will fix it this weekend.

Hi @chenyangzhu1
Awesome! Thanks for sharing very much.
It's really a good idea to load the data as torch tensor directly! I will fix it this weekend.

I am very glad that this code can be helpful. Learning the MedSAM repo is an assessment for my phd application. I want to prove to my target supervisor that I have made a meaningful contribution. If this code turns out to be effective, please keep me posted and I'll be very grateful!

Hi @chenyangzhu1
Thanks again for your support.
I just tested the two different ways of data loading (based on the 3D demo dataset) but it seems that their speed is quite similar. Please correct me if I'm wrong.
It would be great if you can figure out the 'magic' behind the speed improvements.
import time
data_root = './data/Npz_files/CT_Abd-Gallbladder/train'
npz_files = sorted(os.listdir(data_root))
npz_data = [np.load(join(data_root, f)) for f in npz_files]

start_time = time.time()
ori_gts = np.vstack([d['gts'] for d in npz_data])
ori_gts_tensor = torch.from_numpy(ori_gts)
end_time = time.time()
print(f'Loading {len(npz_files)} files takes {end_time - start_time:.2f} seconds.')

start_time = time.time()
ori_gts_tensor2 = torch.vstack([torch.from_numpy(d['gts']) for d in npz_data])
end_time = time.time()
print(f'Loading {len(npz_files)} files takes {end_time - start_time:.2f} seconds.')

You may also be interested in this discussion
https://github.com/bowang-lab/MedSAM/issues/32


fix: wrong autoReadDisabledRateLimiting status when limit by maxPendingSendRequests

Fixes #15149
(or if this PR is one task of a github issue, please add Master Issue: #<xyz> to link to the master issue.)
Master Issue: #15149
Motivation
Explain here the context, and why you're making that change. What is the problem you're trying to solve.
Modifications
Describe the modifications you've done.
set autoReadDisabledRateLimiting true
Verifying this change

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

(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)

Added integration tests for end-to-end deployment with large payloads (10MB)
Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:
If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (yes / no)
The public API: (yes / no)
The schema: (yes / no / don't know)
The default values of configurations: (yes / no)
The wire protocol: (yes / no)
The rest endpoints: (yes / no)
The admin cli options: (yes / no)
Anything that affects deployment: (yes / no / don't know)

Documentation
Check the box below or label this PR directly.
Need to update docs?

[ ] doc-required
(Your PR needs to update docs and you will update later)

[x] no-need-doc
(Please explain why)

[ ] doc
(Your PR contains doc changes)

[ ] doc-added
(Docs have been already added)

Hi @leizhiyuan I think ServerCnx.autoReadDisabledRateLimiting is for broker or Netty-Thread, not for serverCnx.  useful for that "When used to remove producer limiting, try to remove netty-thread limiting ( it will success when broker was not rate-limiting)". see:
https://github.com/apache/pulsar/blob/732049fc6ca1beb046deb43057be2b130736fbca/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L2712-L2718
So I think this will break the original design.
We can have a friendly discussion.


Deprecation warning for Dalli_store

There is a deprecation warning in the deployment process in production:
DEPRECATION: :dalli_store will be removed in Dalli 3.0.
Please use Rails' official :mem_cache_store instead.

The reason is l58 in config/environments/production.rb:
config.cache_store = :dalli_store, ENV["MEMCACHED_SERVER"]
where it should be :mem_cache_store instead.

Closed via #188.

Closed via #188.


 inside a  is not scaled correctly

Bug
When you nest a <Circle /> inside a <Mask /> and scale down the svg, circle will keep its initial size, and not scale down as expected. This seems to only happen for android.
For the following svg component we expect the masked circle component to be 30x30 pixels, but it keeps its size of 512x512:
<Svg height={30} width={30} viewBox="0 0 512 512">
	<Mask id="a">
	  <Circle cx={256} cy={256} r={256} fill="#fff" />
	</Mask>
	<G mask="url(#a)">
		<Path
			fill="#0052b4"
			d="M0 0h133.6l35.3 16.7L200.3 0H512v222.6l-22.6 31.7 22.6 35.1V512H200.3l-32-19.8-34.7 19.8H0V289.4l22.1-33.3L0 222.6z"
		/>
		<Path
			fill="#ffda44"
			d="M133.6 0v222.6H0v66.8h133.6V512h66.7V289.4H512v-66.8H200.3V0z"
		/>
	</G>
</Svg>

https://github.com/software-mansion/react-native-svg/assets/33149910/25f796d7-1186-4ca5-a926-3afac2e29d3c
I tried the same svg code on the web, and it works as expected:
<svg xmlns="http://www.w3.org/2000/svg" width="30" height="30" viewBox="0 0 512 512">
	<mask id="a">
		<circle cx="256" cy="256" r="256" fill="#fff"/>
	</mask>
	<g mask="url(#a)">
		<path fill="#0052b4" d="M0 0h133.6l35.3 16.7L200.3 0H512v222.6l-22.6 31.7 22.6 35.1V512H200.3l-32-19.8-34.7 19.8H0V289.4l22.1-33.3L0 222.6z"/>
		<path fill="#ffda44" d="M133.6 0v222.6H0v66.8h133.6V512h66.7V289.4H512v-66.8H200.3V0z"/>
	</g>
</svg>

Environment info
React native info output:
System:
  OS: macOS 14.1.1
  CPU: (12) arm64 Apple M2 Pro
  Memory: 162.22 MB / 32.00 GB
  Shell:
    version: 3.2.57
    path: /bin/bash
Binaries:
  Node:
    version: 18.18.2
    path: ~/.nvm/versions/node/v18.18.2/bin/node
  Yarn:
    version: 1.22.21
    path: ~/.nvm/versions/node/v18.18.2/bin/yarn
  npm:
    version: 9.8.1
    path: ~/.nvm/versions/node/v18.18.2/bin/npm
  Watchman: Not Found
Managers:
  CocoaPods:
    version: 1.14.3
    path: /opt/homebrew/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.0
      - iOS 17.0
      - macOS 14.0
      - tvOS 17.0
      - watchOS 10.0
  Android SDK: Not Found
IDEs:
  Android Studio: 2023.1 AI-231.9392.1.2311.11255304
  Xcode:
    version: 15.0.1/15A507
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.10
    path: /usr/bin/javac
  Ruby:
    version: 2.6.10
    path: /usr/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.2
    wanted: 0.73.2
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false

Library version: 14.1.0
Steps To Reproduce
Issues without reproduction steps or code are likely to stall.

git clone https://github.com/elmaxe/rnsvg-bug-repro
cd rnsvg-bug-repro
yarn
yarn android
Use the element inspector to see the sizes of the svg elements

Describe what you expected to happen:

Circle should be 30x30, not 512x512

Hey @sierraleonez,
Could you share a reproduction repository? The one provided by @elmaxe no longer works for me, and I couldn't reproduce the issue using the latest version in our example app.

Hi @jakex7, thank you for the response,
I'm able to reproduce the issue here.
How to repro:

Scroll down (10 items)
Press item with index 11, 12
Item index 10 will be pressed instead (should be item index 11 or 12)

After I tried to inspect it, the cause of this issue is due to circle element blocking with 512x512 size, and it only happened inside flashlist, It works well on normal flatlist.


provide test skeletons

it would be nice to already have some example tests to copy from

Testing has been greatly simplified, no test skeletons are longer needed.
Only missing some helpers for IT that should come from JVM SDK.


error in sample.py

thanks for your great work!
i got the error when running sample.py:
ValueError: zero-size array to reduction operation maximum which has no identity
the reason is query_tmp = [], after sampling.
https://github.com/luost26/3D-Generative-SBDD/blob/fabc98612d06713d5bab8e58bf5c62f229e718f2/sample.py#L282
do you know how to fix it? thanks

it also report "Ignoring duplicate molecule" during sampling.


Build failed when use react-router-dom and path-to-regexp

Describe the bug
use react-router-dom and path-to-regexp package in the same time comes a problem
import {
  BrowserRouter as Router,
  Switch,
  Route,
  useParams
} from "react-router-dom";
import { match } from 'path-to-regexp'         // change to 'path-to-regexp/dist.es2015' works
console.log(match)

Got error like this

I do prepare a codesandbox link, but it's works fine.
I can't make it right on my local environment.
Reproduction
try download this codesandbox demo and run npm run dev
System Info
Output of npx envinfo --system --npmPackages vite,@vitejs/plugin-vue --binaries --browsers:

  System:
    OS: macOS 10.15.6
    CPU: (8) x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
    Memory: 58.64 MB / 8.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 14.16.1 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.12 - /usr/local/bin/npm
  Browsers:
    Chrome: 89.0.4389.114
    Firefox: 86.0
    Safari: 13.1.2
    Safari Technology Preview: 14.0
  npmPackages:
    vite: 2.1.2 => 2.1.2

Used package manager:  npm / yarn
Logs

 > node_modules/react-router/esm/react-router.js:9:7: error: No matching export in "node_modules/path-to-regexp/dist.es2015/index.js" for import "default"
    9 │ import pathToRegexp from 'path-to-regexp';
      ╵        ~~~~~~~~~~~~

Before submitting the issue, please make sure you do the following

[x] Read the Contributing Guidelines.
[x] Read the docs.
[x] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[x] Provide a description in this issue that describes the bug.
[x] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/vue-next instead.
[x] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.

theprimone/vite-react#1

I had the same issue. For me it was caused by having multiple conflicting major versions of path-to-regexp. React Router depends on<EMAIL_ADDRESS>Make sure if any other dependency depends on path-to-regexp it uses the same version.
If you depend on path-to-regexp in your own codebase you can just set the version.
package.json
{
  "dependencies": {
    "path-to-regexp": "^1"
  }
}

Otherwise you might also try forcing the correct version via yarn resolutions.
package.json
{
  "resolutions": {
    "path-to-regexp": "1.8.0"
  }
}

Im getting
 > node_modules/react-router/esm/react-router.js:4:30: error: No matching export in "node_modules/history/index.js" for import "createLocation"
    4 │ import { createMemoryHistory, createLocation, locationsAreEqual, createPath } from 'history';
      ╵                               ~~~~~~~~~~~~~~

 > node_modules/react-router/esm/react-router.js:4:46: error: No matching export in "node_modules/history/index.js" for import "locationsAreEqual"
    4 │ import { createMemoryHistory, createLocation, locationsAreEqual, createPath } from 'history';
      ╵                                               ~~~~~~~~~~~~~~~~~

 > node_modules/react-router-dom/esm/react-router-dom.js:5:50: error: No matching export in "node_modules/history/index.js" for import "createLocation"
    5 │ import { createBrowserHistory, createHashHistory, createLocation } from 'history';
      ╵                                                   ~~~~~~~~~~~~~~

 > node_modules/react-virtualized/dist/es/WindowScroller/utils/onScroll.js:74:9: error: No matching export in "node_modules/react-virtualized/dist/es/WindowScroller/WindowScroller.js" for import "bpfrpt_proptype_WindowScroller"
    74 │ import { bpfrpt_proptype_WindowScroller } from "../WindowScroller.js";

I have a single version of path-to-regex 1.7.0 and I don't even depend on in in my code, its just react-router-dom that needs it

@seanaye did you find any solution to this?

@xeoneux no not yet

@seanaye I solved it by using "history": "^4.10.1" since react-router-dom uses this old version of history.
https://github.com/ReactTraining/react-router/blob/fc91700e08df8147bd2bb1be19a299cbb14dbcaa/packages/react-router-dom/package.json#L44
This issue is that vite cannot tell which version of history to use during the dev run and tries using the latest one (5.0.0).
Hope this helps

@seanaye I solved it by using "history": "^4.10.1" since react-router-dom uses this old version of history.
https://github.com/ReactTraining/react-router/blob/fc91700e08df8147bd2bb1be19a299cbb14dbcaa/packages/react-router-dom/package.json#L44
This issue is that vite cannot tell which version of history to use during the dev run and tries using the latest one (5.0.0). So remove/downgrade that in your package.json
Hope this helps

Caraca man.... save my day.... Thanks a lot....

What I did was copy the path-to-Regex library locally, and its code structure was very simple

i use vite-plugin-mock, it alse depens<EMAIL_ADDRESS>version. it changes node_modules structure，then it crashed.

I tested this in stackblitz with Vite 2.8.6 and all seems to be working fine. I'll close this as fixed, but feel free to send a repro if the issue still persists.


Add Razor views for ASP.NET

Hello! I see that you have a lot of integrations of your template for different frameworks (Django, Ruby on Rails, etc.). I think it would be great to create similar Razor views for ASP.NET MVC or ASP.NET Core. What do you think?

That would be great. Let me know if you are down for the task.


Ticket Tracking API

Since some cinemas (often smaller ones) still hand out paper-based tickets, the Backend needs to implement a endpoint which allows tracking the ticket numbers that have been issued.
This endpoint needs to be increment only and shall only be resettable if a transaction containing the tickets is cancelled afterwards.

Required Tasks

[ ] Extend Item Objects to allow setting items as ticket/containing a ticket
[ ] Create API Endpoint to receive the currently issued ticket number
[ ] Create API Endpoint to reverse a transaction


Add simple capture of exit codes of commands run.

This lets the user check $? after all implementations are run to see if there were any problems.
Without this change cim for... always returns successfully...

LGTM


fix(notebook): More specific error in query inspector

Pull Request
🤨 Rationale
Bug [NotebookPlugin] A generic error is exposed in UI
👩💻 Implementation
Added exception, errorCode on the execution
🧪 Testing

✅ Checklist

[x] This PR has a title that follows the commit message format.

:tada: This PR is included in version 3.14.1 :tada:
The release is available on:

v3.14.1
GitHub release

Your semantic-release bot :package::rocket:


Bytes translation

I think it would be interesting to have an entry in the translation definition to specify the "byte" symbol in other languages than en-US. For example in french it would be "o" for "octet".
I believe that a lot of french people are not even familiar with Mo/Go and what it represents, so i'd prefer not to use MB/GB. Maybe they are less silly than I might think :)

in numbro 2.3.0


command line not working with proxy

I am trying to run the command line. But got the below error. Any thought please?
$ /c/devtools/dependency-check/bin/dependency-check.bat -project web -s . -o web -proxyserver webproxy.zzz.com -proxyport 8080 -proxyuser zzz -proxypass zzz
[INFO] Checking for updates
[INFO] starting getUpdatesNeeded() ...
[WARN] Unable to download the NVD CVE data; the results may not include the most recent CPE/CVEs from the NVD.
[WARN] Unable to update Cached Web DataSource, using local data instead. Results may not include recent vulnerabilities.
[ERROR] No documents exist

Unable to continue dependency-check analysis.
[ERROR] One or more fatal errors occurred
[ERROR] Unable to download the NVD CVE data.
[ERROR] No documents exist

Please delete the data directory (you can run dependency-check.bat -purge). Then re-run dependency-check adding -log odc.log.  More details about the error will be in the log file and I can assist more if you can post the log file here.

I am getting below error
[ERROR] IO Exception: HEAD request returned a non-200 status code
[ERROR] IO Exception: HEAD request returned a non-200 status code
[ERROR] IO Exception: HEAD request returned a non-200 status code

@ashishWaghmare have you tried with v3.1.2? The NVD changed their URLs and we've released a new version in response.


[ASK] Save NCF Model

Description
I want to save the entire NCF model and use it without requiring the original model building code to run. How can I do that? The example in the deep dive notebook only shows how to save the model for further training which is not what I am looking for. Appreciate any reponses.
Other Comments

I have the same problem. I want to save model and then want to load it without retraining for prediction

I used
model.save(path)
to save the model, and got 4 files:
checkpoint
model.ckpt.data-00000-of-00001
model.ckpt.index
model.ckpt.meta
To load the model, I tried
model = NCF(parameters)
model.load(path)
it worked, but when I run
model.predict()
There's error:
AttributeError: 'NCF' object has no attribute 'user2id'
Have no idea how to fix this.

It would help if you fed it data parameters as shown in this notebook example
https://github.com/microsoft/recommenders/blob/main/examples/02_model_collaborative_filtering/ncf_deep_dive.ipynb
3.5.2 Load pre-trained GMF and MLP model for NeuMF

Does this mean that one cannot save NeuMF model, but have to train and save the GMF and MLP separately?

No, you can save the NeuMF model by providing the path
model.save('dir_path')
and then while loading depending on the type of your model in this case neumf pass it that dir parameter
model.load(neumf_dir='dir_path')
Note: you have to create your model before loading with the same parameters, as shown in the example.

I still get
'NCF' object has no attribute 'user2id'
What I did:
model.fit(data)
modl.save('dir_path')

then opened a new console
data = NCFDataset(train_file = train_file, test_file = test_file, seed = SEED) #as the saved model
model = NCF ( the same parameters as the saved model)
model.load(neumf_dir='dir_path')
model.predict(users, items, is_list = True)

After loading you will have to specifically assign these attributes to NCF class
  model.user2id = data.user2id     model.item2id = data.item2id     model.id2user = data.id2user     model.id2item = data.id2item

Thank you！Finally succeeded！


lapack/gonum: rewrite Dlassq and its test

Please take a look.
Updates #1623
I'm submitting as draft while I clarify the correct values of dssml and dsbig with the Reference Lapack.

Codecov Report

Merging #1632 (da78bc9) into master (1858083) will increase coverage by 1.74%.
The diff coverage is 82.91%.

@@            Coverage Diff             @@
##           master    #1632      +/-   ##
==========================================
+ Coverage   71.46%   73.21%   +1.74%
==========================================
  Files         489      503      +14
  Lines       54288    59714    +5426
==========================================
+ Hits        38797    43719    +4922
- Misses      13086    13516     +430
- Partials     2405     2479      +74

Impacted Files
Coverage Δ

blas/gonum/level2float32.go
0.00% <0.00%> (ø)

...sp/fourier/internal/fftpack/array_bounds_checks.go
51.11% <ø> (ø)

...fourier/internal/fftpack/array_no_bounds_checks.go
81.81% <ø> (ø)

dsp/window/window.go
100.00% <ø> (ø)

dsp/window/window_complex.go
100.00% <ø> (ø)

graph/formats/rdf/kind_string.go
0.00% <0.00%> (ø)

graph/formats/rdf/parse.go
91.78% <ø> (ø)

dsp/window/window_parametric.go
69.23% <20.00%> (-14.77%)
:arrow_down:

graph/formats/rdf/nodebug.go
25.00% <25.00%> (ø)

graph/encoding/digraph6/digraph6.go
56.86% <40.00%> (+3.52%)
:arrow_up:

... and 90 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 b4dd4cf...da78bc9. Read the comment docs.


There was a problem confirming the ssl certificate

Running on Windows 10 Pro 1703 b15063.786
Install fails as follows,
PS C:\Program Files (x86)\Python36-32> pip install mssql-cli
Collecting mssql-cli
  Could not fetch URL https://pypi.python.org/simple/mssql-cli/: There was a problem confirming the ssl certificate: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:777) - skipping
  Could not find a version that satisfies the requirement mssql-cli (from versions: )
No matching distribution found for mssql-cli
PS C:\Program Files (x86)\Python36-32>

Cheers
Phil

Can you check your pip version using pip --version.
Also try and update pip using 'pip install --upgrade pip' and then try the install.
Let me know if this problem persists even after this.
Thanks

Closing due to inactivity. @philcart  please reopen if you are still facing the issue.


Specific SMB3.0 Features for Hyper-V

What are the specific SMB3.0 features required for Hyper-V hosts to share virtual machines in a clustered environment?
Do the Hypoer-V hosts need at minimum just to negotiate with SMB3.0 with no special features? Or are there specific requirements like Multi Channel or Directory Leasing?

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 51695048-f8f8-c2c1-89ff-dc3614f9d3d8
Version Independent ID: 372b149c-9a75-7db5-f8fd-4820e7b05b35
Content: Plan for Hyper-V scalability in Windows Server
Content Source: WindowsServerDocs/virtualization/hyper-v/plan/plan-hyper-v-scalability-in-windows-server.md
Product: windows-server
Technology: hyper-v
GitHub Login: @BenjaminArmstrong
Microsoft Alias: benarm

We hugely appreciate your contribution, feedback, and support to our technical documentation.
We’ve phased out GitHub Issues on this repository as the feedback mechanism for content and have replaced it with a new feedback system. For more information see, Content User Feedback. As part of this rollout, we are migrating your issue where our team will use your comments to improve the documentation experience. Unfortunately, this does mean you won't be able to access this issue after the migration is complete.
All issues will be migrated by Thursday 29th August 2024.
Once again, thank you for your valuable feedback, we hope you’ll continue to contribute using our new feedback system.


[bug] rh-card public css property to modify background color

Currently the rh-card does not provide a public css property to modify background color --rh-card-background-color.  This feature would support to proposed changes in https://github.com/RedHat-UX/red-hat-design-system/pull/1188

duplicating comments from the PR, for design review:
Before we merge this, it's worth considering weather it would be wiser to advise users to change the surface color tokens for a given card or class of cards, rather than providing an overriding background variable.
My reasoning is as follows: if we expose the background color variable to users, then users who wish to implement for example a dark background will skip the use of our color context system all together. That is likely to lead to maintenance difficulties, feature creep, and design and accessibility violations.
On the other hand, if we advise users who wish to apply a custom background to explicitly change the surface token values for a given card or class of cards, then they will have to explicitly participate in the context system. Because the context surface tokens have the words "on light" and "on dark" in their names, the user will be subtly encouraged to provide values for both theme variants.
This would have the added advantage of inheriting the provided values to the card's context-consuming children e.g. accordions, which in most cases would be desirable, and could be explicitly opted out of with a child selector

Added docs to help developers customize background color through css tokens.  Can be seen at the bottom of the page here https://deploy-preview-1200--red-hat-design-system.netlify.app/patterns/card/


docs(structural-directives): fix broken links

Just a couple of broken links I found while reviewing the doc.

Thx!


nice to have: time limit and best list

It's not likely this will be implemented because right now, all state is saved on the client side. That way, you can play the game offline (can't imagine anyone would want to tho) and I don't have to implement a server. Sorry :(


Send a 502 to the client when there is an invalid Transfer-Encoding header value from the origin

Did a git mv on chunked.replay.yaml to chunked_diabled.replay.yaml
Added proxy verifier tests to test:
Transfer-Encoding header not equal to chunked from origin
Transfer-Encoding and Content-Length headers missing from origin
valid Transfer-Encoding: chunked response from the origin

[approve ci]

[approve ci]

[approve ci centos]

If chunking is disabled but origin server responds with invalid Transfer-Encoding header, will it send 502 (from bad response) or 200 (since it is not even considered)?

If any invalid Tranfer-Encoding header comes from the origin traffic server will respond with a 502, regardless if chunking is on or off.

[approve ci]


Support dynamically contributed commands for contributed trees context menus

Currently we have to know what items should appear on a TreeItem's context menu at the time that we render the tree. This means if we want to display a fully expanded tree at startup but add context menu items, we have to fetch them all immediately.
Would it be possible to have an async method that asks us for the items at the time of selection so we can lazily-fetch these items (I understand this will delay rendering of the context menu until the promise resolves).

@DanTup Not sure if I understand your scenario. Can you please explain a bit more

This means if we want to display a fully expanded tree at startup but add context menu items, we have to fetch them all immediately.

@sandy081
I'm rendering a tree where the items that should be on the context menu vary for each node and are (relatively) expensive to compute. Currently the API requires that I set the contextValue of a tree-node at creation time, and if I want the tree to be expanded immediately, that means I have to compute the context menus for every node all in one go.
I would like to be able to lazily provide the context menu/contextValue for a tree node at the point that the user clicks/right-clicks on it.
Hope that makes sense!

@DanTup I see. If it is expensive to compute the context menu, then delaying it until context menu appears will make context menu to be async (not instinctive). This might not be expected by the user when user does right click?

@sandy081 Computing a single one is hopefully not very slow (hopefully < 10ms), but computing them for every node of the tree in a single-threaded server would be. I understand the concerns with making the context menu async, but we hope it'd be fast enough to not affect the experience here.

@DanTup

but computing them for every node of the tree in a single-threaded server would be

But you will only compute when the node is shown (when getTreeItem is called) right, not for all nodes in the tree. Also it is synchronous only for sibling nodes.

But you will only compute when the node is shown (when getTreeItem is called) right, not for all nodes in the tree.

Yeah, but if we want the tree to default to expanded, that's basically everything in a very short space of time?

Also it is synchronous only for sibling nodes.

Yeah, it's more about the resource usage and total time for all the round trips we're trying to minimise. It seems wasteful for us to compute the context menus for 100 nodes if the user only ever invokes the context menu on one (or none).

@sandy081 Another possibility - what if we could provide an update the enabled/disabled status of the context menu asynchronously? That way the menu can render immediately, but items may become greyed later. It's up to the provider to ensure it's fast and handle the case where the item was still clicked but wasn't intended to be visible?

@DanTup Good suggestions.
@jrieken I think this is for asking prerequisite clause for menu items.

I think this is for asking prerequisite clause for menu items.

I'm not sure what exactly you have in mind, but as long as we can provide info at the time the menu is being shown (and not at the time of constructing TreeItem) then it should work for us.

I think this is for asking prerequisite clause for menu items.

There are ideas to have a precondition-clause on commands - that would manage the enablement-state, not visibility which is managed per menu.

can provide info at the time the menu is being shown

We don't plan for dynamic menus, the tree might want to accept vscode.Command objects somewhere.

I'm not sure I understand. I already have the ability to control the visibility using when (though what I did was a bit of a hack - using a regex so I can treat it like a bitmask to enable multiple different items per node). If preconditions still need declaring at the time of creating the TreeItem that won't help with this request. This request is specifically about allowing me to decide which items are valid (I don't care if it's disabled of hidden) at the time of showing the menu, so that I can avoid doing the computations for which actions are valid for each note all at the time of building the tree (since it starts expanded, all nodes are created up-front).

This request is specifically about allowing me to decide which items are valid (I don't care if it's disabled of hidden) at the time of showing the menu, so that I can avoid doing the

Like keybindings, menu-definitions are static and cannot be computed dynamically. We do that to eventually support user-custimizable menus (like user customisable keybindings) and we have no plans for dynamic menus.

Like keybindings, menu-definitions are static and cannot be computed dynamically. We do that to eventually support user-custimizable menus (like user customisable keybindings) and we have no plans for dynamic menus.

That's fine, the commands are already predefined in package.json. This is specifically about choosing which of those items are available for a given node, which I'd like to do when the menu is being shown rather than when the TreeItem is constructed.

The only thing you might be able to get is something special with the tree, e.g. a property that you need to provide with the TreeItem - that's up to @sandy081

@sandy081 Let me know your thoughts on this, as it may influence dev we do elsewhere. The simplest way would be for me to provide an async method on the tree provider that given a TreeItem is able to affect the visibility of the menu items (for ex. a resolveContextValue or something).

The only thing you might be able to get is something special with the tree, e.g. a property that you need to provide with the TreeItem - that's up to @sandy081

@jrieken What is the thinking behind exposing a property in TreeItem for dynamic custom menus? We already expose a property contextValue on TreeItem object. This is set when the item is shown in the view. But, I think what @DanTup requires is to contribute to context menu when the menu is requested to open. Ideally he requires following

Extension author contributes to context menu, when the user asks for context menu.

@DanTup Correct me if I am wrong.

requires is to contribute to context menu when the menu is requested to open

@sandy081 Blocking/delaying the right click with extension code won't happen therefore I propose middle ground. Instead of going through contextValues and package.json-contributions allow to have vscode.Command-objects. That's similar to code actions which don't require context keys and static commands

@sandy081 Your understanding is correct.
@jrieken I'm already able to provide the set of commands dynamically using a regex in the when clause. It's not beautiful but it works so I don't think you need to build anything new to support that.

Blocking/delaying the right click with extension code won't happen

How about letting the menu appear immediately, but an async call is able to disable (grey out) items? I'm sure we can easily respond in < 50ms so the user won't notice them update from black to grey and I'm happy to be responsible for handling the command graceful if the user is able to click one because I didn't respond fast enough.

Instead of going through contextValues and package.json-contributions allow to have vscode.Command-objects

This is like contributing commands when TreeItem is created. If I understand correctly what @DanTup asking is to contribute commands at the time of opening context menu which I think will not be supported Blocking/delaying the right click with extension code won't happen.

Btw, it seems like the explorer menu may already be doing this - eg. Paste is only available when you have a file in the clipboard. I can't figure out from the source how this works though - is it something we can piggy-back?

@DanTup This is done by using precondition which again uses context values.

Ah, gotcha :(

@DanTup Please provide your suggestion of having onDidSelectionChange event to have dynamic menus. Would be interested to know how this solves.
Thanks

@sandy081 My comment on the other issue was actually unrelated to dynamic context menus; but rather what's in that case. In this comment there's a screenshot suggesting we could contribute buttons to above the tree. If that's the case, then having onDidSelectionChange would allow these buttons to be enabled/disabled based on the selection.
That seems like a reasonable way for me to achieve what I wanted here, but is not possible (that is, provide some actions that are available/not available depending on the selected node).

Are you saying you can update the context (which a menu command depends on)  by listening to selection change event?

@sandy081 I might be missing something, but I was thinking that if we had an onSelectionChange for the tree, then we can set a context with executeCommand("setContext", "xxx", true); (and we could do this asynchronously, which has been the problem so far). Assuming the buttons on the toolbar shown on the screenshot I linked above are a) possible and b) update when contexts change, this would allow us to have buttons above a tree which enable/disable based on the selected tree node (without having to know all of the contexts for every node at the time it was rendered)?

Yes title bar buttons are

available already
can be made visible/invisible by changing the context.

@DanTup Yes here it is - https://github.com/Microsoft/vscode/issues/50662
Don't know what is the status of this item, may I know if current issue is still valid after implementing #50662? And if so what more is needed?

@sandy081 I think if #50662 gets implemented then I have another way to do what I need without requiring async-dynamic context menus (we'll just use the title bar buttons instead of context menu; which is actually what we originally wanted, but I didn't realise it existed :-))

Oh, one other question (slightly off-topic for this issue, but hey)... those title bar buttons - can we have them visible-but-disabled? So they don't jump around as the user clicks between nodes (just become disabled/enabled)?

@DanTup You can only show/hide them but not disable. I think there is a request for this in general.
Closing this.


Remove greenkeeper badge

Removes the greenkeeper badge instead of showing a missing image icon. I'm not sure if we intend on adding greenkeeper back or not.

Thanks for the pull request @lilleyse!

:heavy_check_mark: Signed CLA found.
:grey_question: CHANGES.md was not updated.

If this change updates the public API in any way, please add a bullet point to CHANGES.md.

:grey_question: Unit tests were not updated.

Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

[ ] Cesium Viewer works.
[ ] Works in 2D/CV.
[ ] Works (or fails gracefully) in IE11.

Thanks @lilleyse. LGTM.


Add weights to logrank_test

Problem
The logrank test expects one row per subject. This requires a lot of data when the groups are very large. For example, I'm analyzing hundreds of large A/B test with logrank tests. Downloading from my database one row per subject for all these logrank test is a bottle neck.
Potential solution
We could group multiple subject per rows and provide weights to the logrank_test function. This idea is similar to the weights parameter in the fit method of the KaplanMeiereFitter.
Alternative solution
If there is a better test (than logrank) that takes a weights argument that would also be a good option.
My hesitation with replacing logrank_test with univariate Cox regression is that fitting the regression sometimes doesn't converge and with hundreds of regressions to fit I won't have the time to manually fix the failing CoxPHFitter().fit() calls.

👋 Hey Tristan!
There's not yet an implemented solution to adding case-weights to the logrank. However, I took a look at what it would take, and all the pieces were there, so I want ahead and did it. Check out the branch weighted_logrank - likely this will be merged in the next release.

Thanks @CamDavidsonPilon :)
I'll give it a spin soon 😀


Wrap fedimint around an async rwlock

lgtm, I presume this is needed because before we couldn't read at the same time but with this we can

Correct, it was blocking the activity list while a fedimint payment was being made.

There's probably other places in the codebase we can do this too, would be a good followup


[$250] iOS Search - Filter icon is not responsive when navigating to Search via WS chat in WS settings

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 9.0.71-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers<EMAIL_ADDRESS>Issue reported by: Applause Internal Team
Action Performed:

Launch ND or hybrid app.
Go to workspace settings.
Tap on the workspace chat under "Submit expenses using your workspace chat below"
Tap on the search icon on the top right.
Type anything and search it.
On Search page, tap on the filter icon.

Expected Result:
Filter options will open.
Actual Result:
Filter icon is not responsive.
Workaround:
Unknown
Platforms:

[x] Android: Standalone
[x] Android: HybridApp
[x] Android: mWeb Chrome
[x] iOS: Standalone
[x] iOS: HybridApp
[x] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
https://github.com/user-attachments/assets/c5f591e9-e277-4a2c-86f2-ff0ad14ce824
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021864902077815200576
Upwork Job ID:<PHONE_NUMBER>815200576
Last Price Increase: 2024-12-06

Issue OwnerCurrent Issue Owner: @rayane-djouah

Proposal
Please re-state the problem that we are trying to solve in this issue.
The issue involves [specific problem, e.g., "inconsistent form behavior causing incorrect submissions"], affecting [impact, e.g., "data reliability and user experience"].
What is the root cause of that problem?
Triggered by [root cause, e.g., "a race condition in asynchronous state management"].
What changes do you think we should make in order to solve the problem?

Refactor state management to handle async operations correctly.
Add validation checks at client/server levels.
Enhance error handling for better feedback and logging.
Update outdated dependencies.

Suggested Fix for the Filter Icon Handler
function onFilterIconPress() {
    if (isExecuting) {
        return; // Prevent multiple executions.
    }

    singleExecution(() => {
        // Logic to show filter options
        Navigation.navigate(ROUTES.SEARCH_FILTER); // Adjust as per your route setup.
    })();
}

Add Accessibility and Clickability Checks
<TouchableOpacity
    onPress={onFilterIconPress}
    accessible={true}
    accessibilityLabel={translate('search.filter')}
    style={styles.filterIconContainer}
>
    <Expensicons.Filter />
</TouchableOpacity>

What alternative solutions did you explore? (Optional)

Rewrite module (time-intensive).
Adopt a new state library (steep learning curve).
Implementing targeted fixes and robust tests will resolve the issue and prevent recurrence.

Proposal
Please re-state the problem that we are trying to solve in this issue.
The filter button is "unresponsive"
What is the root cause of that problem?
In WorkspaceInitialPage, we check to see if the active route of the sub-page of the workspace settings page is accessible.
https://github.com/Expensify/App/blob/3baf965618ced48845ddd7534b0fad4d4110b5de/src/pages/workspace/WorkspaceInitialPage.tsx#L360-L376
If not, then we dismiss any RHP. For example, if the user opens a workspace as a member and tries to open the workspace categories setting, then the categories setting RHP will be dismissed and the not found page will show from the workspace categories list page.
In our case, we open the workspace's initial page. The active route becomes Workspace_Initial. However, the current canAccessRoute logic only checks for the sub-page route. So, when it compares the active route with itself, it fails because menuItems doesn't contain Workspace_Initial.
We actually have the logic to run it only once.
https://github.com/Expensify/App/blob/3baf965618ced48845ddd7534b0fad4d4110b5de/src/pages/workspace/WorkspaceInitialPage.tsx#L368-L371
But when we open the search page, all routes are "hidden".
https://github.com/Expensify/App/blob/3baf965618ced48845ddd7534b0fad4d4110b5de/src/libs/Navigation/AppNavigator/createResponsiveStackNavigator/useStateWithSearch.ts#L44-L57
So, the wasRendered logic in WorkspaceInitialPage won't work.
What changes do you think we should make in order to solve the problem?

We can update canAccessRoute to cover the case of the workspace's initial page itself.
const canAccessRoute = activeRoute && (menuItems.some((item) => item.routeName === activeRoute) || activeRoute === SCREENS.WORKSPACE.INITIAL);

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

Proposal
Please re-state the problem that we are trying to solve in this issue
Filter icon is not responsive when navigating to Search via WS chat in WS settings.
What is the root cause of that problem?

[!note]
This issue is only present on narrow layout devices (native / mWeb) where screens displayed in RHP act as a modal.

https://github.com/Expensify/App/blob/3baf965618ced48845ddd7534b0fad4d4110b5de/src/pages/workspace/WorkspaceInitialPage.tsx#L360-L376
When we open the workspace's initial page, the active route becomes Workspace_Initial. However, the current canAccessRoute logic only checks for the sub-page route - therefore when it compares the active route with itself, it fails because menuItems doesn't contain Workspace_Initial.
We actually have the logic to only run it once here, but when we open the search page, all routes are "hidden" as explained here. Because of this, the wasRendered logic in WorkspaceInitialPage won't work as expected -> when we try to open the filter page, the previously "hidden" routes are added back to the stack and canAccessRoute becomes false which will dismiss any RHP that we try to open (Filters).
Credits to bernhardoj for the RCA.
What changes do you think we should make in order to solve the problem?
We had a similar issue which I reviewed recently here where we navigated away from WorkspaceProfileSharePage to a report without dismissing the currently opened modal which caused a weird UI bug in the QR Code because the same modal would be opened twice.
A practical solution for this kind of issue which wouldn't break any other functionality is to simply replace the way we navigate away from WS to the report here:
https://github.com/Expensify/App/blob/b595f5d24141a403fcc3f44c73d00cd6951dca7d/src/pages/workspace/WorkspaceInitialPage.tsx#L457
to use Navigation.dismissModal instead:
onPress={() => Navigation.dismissModal(currentUserPolicyExpenseChat?.reportID ?? '-1')}

Why dismissing the modal first fixes our issue ?
Because as explained in RCA, on narrow layout devices (native / mWeb) screens displayed in RHP act as a modal and by dismissing it when navigating to the report, all the conditions which lead to the Filters RHP to being able to open when clicked on in our case, will now work as expected.
Result video (before / after)

iOS: Native

Before
After

[!important]
I proposed the same solution for this issue and because this solution would fix both issues, if my solution is selected on any of the two issues -> we should put the other one on HOLD.

cc @rayane-djouah

PR is ready
cc: @rayane-djouah

Payment summary:
@rayane-djouah due $250 for C+ work, paid via Upwork ✅
@bernhardoj due $250 for contributor work via NewDot manual request

Requested in ND.


Remove converImage function, using ImageConverter class instead

As title, It's related to PR #3.
I try removing the convertImage function and using the ImageConverter class directly.
The convertImage is unnecessary because we can use the ImageConverter class directly.
Once the PR #3 is approved, I can do this to update the example.

The convertImage is unnecessary because we can use the ImageConverter class directly.

Well, both yes and no.
Compare this...
convertImage('./image.png', './image.webp', 5);

...to this...
use Jens\ImageConverter\ImageConverter;
$from = __DIR__ . '/tests/fixtures/from/dices.png';
$to = __DIR__ . '/tests/fixtures/to/dices.webp';
$converter = new ImageConverter();
echo $converter->convert($from, $to, 5);

I added the helper function to keep it short and clean.
On the other hand, the function name can already be taken by something else. Also, a new class instance is created for every time the function runs. That's probably bad practice.
With other libraries I often make my own wrapper function to keep the code at a minimum. Maybe that's the best way in this case as well, to let the user figure out an own implementation that works best for him/her? (skipping the built in helper function)

Hi @jenstornell, thank you for your reply.
You're right. If we keep the helper function existed because it can let the code be clean.
And it also has the good practice in the code.
Maybe I can revert the helper function after the PR #3 is merged.
And the helper function should have the namespace to avoid collision function name happens :).

I re-added the helper function. I close this.


How can I do multiline input?

As soon as I press enter or shift+enter for a newline, it submits the query immediately. How can I put in a multiline input?
This is probably a terminal limitation.

Yeah we are using cmd which uses input which only accepts single lines.
I made a variant once where you had to hit enter twice to submit a question.  That allowed for basic multiline input.  But it was kind of obtrusive and I got rid of it.
There are a few different approaches we could take.

Make it so lines with a trailing backslash allow you to continue the same message on the next line.  In order to make this work cleanly I'd have to override some behavior of the cmd module..
Some kind of "begin" and "end" commands that start and end multi-line messages.  I'd have to add code to prevent the other commands such as nav and new from being used during multi-line input or the state would get too weird.
Make it so if the user adds some special string of characters, such as "\n" or something, that that string is replaced with an actual newline before we transmit it to ChatGPT.

None of them are perfect.. I think "begin" and "end" are probably the cleanest?


Add a message queue feature on the Waterfall UI

As a host, I'd like to be able to mark a collection of cards as 'preferred'.  These messages could be attached to the side of the screen so that its easy to locate those messages.

Components needed

[ ] Update the MessageDetails screen to be a modal on the moderation screen
[ ] Add 'Add to queue' to MessageDetails
[ ] 'Add to Queue' gesture implicitly approves message
[ ] Allow moderators to add 'speaker notes' about a message
[ ] Add a 'Queue' screen that shows the list of messages queued and the speaker notes for each message
[ ] Queue screen allows re-ordering of messages
[ ] Add a 'Drawer' user-interface that slides off the bottom-left of the waterfall ui and contains the list of current messages to review


Log level does not change the output

Have you read the FAQ and checked for duplicate open issues?: Yes
What version of Shaka Player are you using?:  v2.5
Can you reproduce the issue with our latest release version?: Yes
Can you reproduce the issue with the latest code from master?: Yes
Are you using the demo app or your own custom app?: Demo
If custom app, can you reproduce the issue using our demo app?: --
What browser and OS are you using?: Chrome | Ubuntu 18.04
What are the manifest and license server URIs?: --

What did you do? Tried different log levels
What did you expect to happen? Different outputs
What actually happened? Exactly the same outputs

Can you please provide more details?
Changing the log level doesn't inherently change the logs that happen.  It changes which logs are shown.  So, for example, you may not see any difference between log level info and log level debug if your actions didn't trigger any debug logs.  Or if you reduce the log level, you'll never get more logs than you did with a higher level.
Can you tell us what actions you took in the demo, and what log levels you tried?

Thanks for the reply, but it did not solve the problem.


feat: Complete ListURLs Layout with Correct View and Data

Pull Request Summary:
In this PR, I am completing the ListURLs layout with the correct view and data.
Changes:

Added the correct view for the ListURLs layout.
Populated the layout with the appropriate data.

Checklist:

[ ] Optimized the List backend database query.
[ ] Added the ability to search by tags by prefixing the search string with a '#' symbol. This will be useful for filtering by tags in future.
[ ] To prevent overload attacks, consistently set a maximum query length, whether determined by AJAX or configured in files. Ensure that the posted length does not exceed a specified threshold, such as 150 requests.
[ ] Ensure that when listing URLs, they are restricted to those belonging to the user, except in cases where the user is a superadmin or has specific privileges to view other URLs.

Please review and merge as needed. Thank you!

I'd like to propose a change for the Controllers/Url.php controller. Instead of using queries directly within the controller, I suggest utilizing the getUrlsForUser() function and other functions I plan to create later. This approach will enhance code organization and maintainability.

I'd like to propose a change for the Controllers/Url.php controller. Instead of using queries directly within the controller, I suggest utilizing the getUrlsForUser() function and other functions I plan to create later. This approach will enhance code organization and maintainability.

done in commit 5ba50fc356e3a8246c4d1a4bcc9de36a41c8bd11


imgproxy health doesn't work when IMGPROXY_PATH_PREFIX is defined

How to test:
shadowlmd@home:~$ docker run --rm -e IMGPROXY_PATH_PREFIX=/i -d darthsim/imgproxy
ac067a0921d489332ece71caca58f09279907af3bafbdfbf0f99c5fcc5bb2f6e

shadowlmd@home:~$ docker container exec -it objective_dubinsky bash
root@ac067a0921d4:/# imgproxy health

root@ac067a0921d4:/# imgproxy health || echo fail

fail

The endpoint is available at /i/health in this case and can be queried using curl for example.

Sorry, I forgot to mention that this was fixed in v3.0.0

Thanks, it works now. :)


Add python 3.9 to CI.

Python 3.9 was released on October 5, 2020. Add 3.9 to CI and tox.
Actions support for Python 3.9 : https://github.com/actions/setup-python/issues/148

Will add this once https://github.com/actions/virtual-environments/issues/1740 is closed.