[fix][client] Make the whole grabCnx() progress atomic

Motivation
In ConnectionHandler, there is a Connection interface whose methods will be called after the connection in grabCnx is established, the implementation of Connection might send some requests after the connection is established. For example, the consumer will send the CommandSubscribe request in connectionOpened. However, the whole process is not atomic, which leads to the message lost reported in https://github.com/apache/pulsar/pull/20591.
Modifications
Modify the Connection#connectionOpened interface to return a CompletableFuture.
The returned future should be completed once the implementation has done everything, e.g. for the consumer, the future should only be completed after receiving the response for CommandSubscribe.
In grabCnx, the ConnectionHandler could only connect to the broker once the whole process is completed.
Add ConnectionHandlerTest to verify the behavior.
Documentation

[ ] doc
[ ] doc-required
[x] doc-not-needed
[ ] doc-complete

Matching PR in forked repository
PR in forked repository: https://github.com/BewareMyPower/pulsar/pull/29

With this patch, we don't need any synchronization when implementing handleNewConnection (the combination of connectionOpened and connectionFailed) in future.

There are some failed tests, I will fix them ASAP.

/pulsarbot run-failure-checks

PTAL @codelipenghui @poorbarcode

I made new changes to avoid race condition described in https://github.com/apache/pulsar/pull/20595#discussion_r1245387433
The root cause is that for a grabCnx operation, duringConnect.set(false) could be called twice and reconnectLater could be called by the connection implementation. So I removed the reconnectLater calls in the Connection#connectionOpened implementations and just complete the future exceptionally so that reconnectLater will be called later.
            cnxFuture.thenCompose(cnx -> connection.connectionOpened(cnx))
                    .thenAccept(__ -> duringConnect.set(false))
                    .exceptionally(this::handleConnectionError);

All possible cases are:

cnxFuture completes with null
1.1  connectionOpened completes with null, thenAccept will set duringConnect with false.
1.2 connectionOpened completes exceptionally, exceptionally will call reconnectLater, which sets duringConnect with false.
cnxFuture completes exceptionally, exceptionally will call reconnectLater, which sets duringConnect with false.

Ideally, reconnectLater should be private, but it seems hard to remove the reconnectLater calls in TransactionMetaStoreHandler.
PTAL again. @poorbarcode @lifepuzzlefun @tisonkun @Technoboy-

When there are some reviewers you may try to do accumulative commits instead of force push; thus it's easy to review the changeset compared to the last review..

I know it so I didn't push it by force before. But since there were many new commits from the approved commit before, I think it might be better to review for a complete new commit.

The failed test testMsgRateExpired  will be fixed by https://github.com/apache/pulsar/pull/20629

Codecov Report

Merging #20595 (48540d0) into master (0e60340) will increase coverage by 0.53%.
The diff coverage is 77.85%.

@@             Coverage Diff              @@
##             master   #20595      +/-   ##
============================================
+ Coverage     72.60%   73.13%   +0.53%
- Complexity    32018    32076      +58
============================================
  Files          1855     1869      +14
  Lines        138569   138883     +314
  Branches      15250    15273      +23
============================================
+ Hits         100605   101578     +973
+ Misses        29945    29262     -683
- Partials       8019     8043      +24

Flag
Coverage Δ

inttests
24.11% <41.42%> (+<0.01%)
:arrow_up:

systests
25.07% <25.71%> (?)

unittests
72.41% <77.50%> (+0.05%)
:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

...ava/org/apache/pulsar/admin/cli/CmdNamespaces.java
76.17% <0.00%> (-0.08%)
:arrow_down:

...ulsar/client/impl/TransactionMetaStoreHandler.java
68.44% <44.44%> (-0.03%)
:arrow_down:

...rg/apache/pulsar/client/impl/TopicListWatcher.java
65.21% <46.15%> (+0.82%)
:arrow_up:

.../service/SystemTopicBasedTopicPoliciesService.java
76.48% <50.00%> (-1.19%)
:arrow_down:

...ar/common/naming/TopicBundleAssignmentFactory.java
66.66% <66.66%> (ø)

...n/java/org/apache/pulsar/admin/cli/CliCommand.java
57.95% <71.42%> (-1.60%)
:arrow_down:

...g/apache/pulsar/client/impl/ConnectionHandler.java
90.47% <76.19%> (+3.24%)
:arrow_up:

...va/org/apache/pulsar/client/impl/ProducerImpl.java
82.76% <80.00%> (+0.59%)
:arrow_up:

.../metadata/bookkeeper/PulsarRegistrationClient.java
80.85% <81.11%> (-4.11%)
:arrow_down:

...va/org/apache/pulsar/client/impl/ConsumerImpl.java
77.25% <84.61%> (+0.75%)
:arrow_up:

... and 10 more

... and 155 files with indirect coverage changes

@poorbarcode Thanks for your update.
Merging...
Then you can rebase your PR onto this one :D

@BewareMyPower, the TopicListWatcher.java is not included in the branch 2.10. So this PR is not need to cherry-pick to branch 2.10, right?

@BewareMyPower, the TopicListWatcher.java is not included in the branch 2.10. So this PR does not need to cherry-pick to branch 2.10, right?

Oh, my fault. We can cherry-pick this PR to branch 2.10. TopicListWatcher.java is not part of the core modification.


Feature/1 improve errors and tooltips

Resolve #1

Summary
Added tooltips for configuration of widgets. It was mostly reusable code from the github widgets.
However there was a problem with the "required" attribute in the 'SCM Markdown Content
'-widget. If this attribute was attached to the  the form could no longer be submitted. Therefore, the "required" attribute was only attached to the markdown-path field, but now the markdown-path is only displayed if a branch is available (per angular condition). Ergo, a repository has been selected.
The tooltip message was also extended a bit with more tips on how to fill the fields.


Maybe top panel of Figure 7 (age data) should have the same x-axis scale as bottom panel.

i.e. go from 1975 to 2016 (for 2017 assessment) so that you can more easily see whether the cohorts match up between data sources.
[I'm just putting together a talk on using state-space models to give advice to fisheries managers, and will show the hake age data as it's so clean! Hence why looking at hake assessment.]

Definitely an improvement. We used to have diagonal lines for the big cohorts on either this plot or the one for the Pearson residuals. I remember concern that adding the lines leads the reader to see cohorts and the bubbles should just speak for themselves, but I'm not sure if we actively decided to remove them on purpose or they just stopped working with some update to the plot code.

Yes, part of me thinks the data should speak for themselves, but on the other hand it does make it easier to understand. I'll try them at some point and we can easily take them off if necessary. May also put fishery age comps at the top to be consistent with Figures 22 (fit to age data) and 23 (Pearson residuals).

I may try faint lines every four years (so not picking out particular cohorts, but allows every cohort to be easily traced).

Note to self - I started (and stashed) doing the diagonal lines (every four years). Bit fiddly to finalise so leave as is for now and come back to if time.


cater for changed API in Django 1.11

as mentioned on https://docs.djangoproject.com/en/1.11/releases/1.11/#database-backend-api:
"The name keyword argument is added to the DatabaseWrapper.create_cursor(self, name=None) method to allow usage of server-side cursors on backends that support it."
(currently only postgresql)
also adjusted tox.ini and .travis.yml to consider the currently supported versions

Fixes #16

Coverage decreased (-4.9%) to 62.385% when pulling 8360856e53004764bf630fd818f542f9ebac5a8b on nschlemm:django-1.11-compatibility into 7215fcd3a6fba52ec38b94405949358d42bc2d4e on snowball-one:master.

Coverage decreased (-4.7%) to 62.609% when pulling dc65410bea1ab920a21f800fd427678401e090a8 on nschlemm:django-1.11-compatibility into 7215fcd3a6fba52ec38b94405949358d42bc2d4e on snowball-one:master.

Coverage decreased (-4.7%) to 62.609% when pulling dc65410bea1ab920a21f800fd427678401e090a8 on nschlemm:django-1.11-compatibility into 7215fcd3a6fba52ec38b94405949358d42bc2d4e on snowball-one:master.

Coverage decreased (-4.7%) to 62.609% when pulling dc65410bea1ab920a21f800fd427678401e090a8 on nschlemm:django-1.11-compatibility into 7215fcd3a6fba52ec38b94405949358d42bc2d4e on snowball-one:master.

Hi there.
First, thanks for the great software ! Nice to see there's already a port to Django 1.11.
Any idea if/when this will be merged and released ? We requires this PR to upgrade projects to Django 1.11

Hi @nschlemm , we (Polyconseil) have taken over the maintenance of django-cid (with the nice support of the original author). I have started to clean up pull requests and modernize the project.
Your pull request had changes which should be reflected in the following commits:

ac607f33f8c3f4ca9273a618117ed3d0b629f530 fixes the support of Django 1.11 for database backends;
4476392b4e6c0d9f9a38074359cb139f6612f77b fixes the generation of the cid when the middleware is not used.

I see that you also wanted to clean up MIDDLEWARE vs. MIDDLEWARE_CLASS. That's a good idea. I have decided to drop the support of versions <1.11, which will let us do that even more cleanly.
Feel free to contribute again if you feel like it. We use django-cid and are committed to maintain it.


cytometric bead array assays

This generalizes the definition for 'cytometric bead array assay' and adds two subclasses as discussed in issue #1399. It also imports 'antibody titer' from VO.

We will run this past @rvita

We're going to make a fresh PR for this.


Issue 116: Add functions for Rt and growth rates as outputs

This started as a PR for #165 (hence the branch name) but is just addressing (and closing #116) as #165 is turning out to be a little more complex than expected.
Here we add two new functions

expected_Rt Dispatching on a EpiData object or taking arguments directly.
expected_rt dispatching on a vector and taking the log differences.

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

Project coverage is 92.45%. Comparing base (6a6a1d4) to head (c3dba88).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #184      +/-   ##
==========================================
- Coverage   99.31%   92.45%   -6.87%
==========================================
  Files          33       38       +5
  Lines         293      371      +78
==========================================
+ Hits          291      343      +52
- Misses          2       28      +26

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

Closing as we. are now planing on doing this within the inf submodel

Okay looking at this again I vote to review and merge as a partial PR towards #116 leaving the expected growth rate to work out

Worth noting that this will give the effective  as opposed to unadjusted

I think we should put this in the paper as a discussion point

Worth noting that this will give the effective  as opposed to unadjusted

I think we should put this in the paper as a discussion point

One docstring small fix.

Do we have anything in CI etc that tests docstring examples at least run / docstrings are valid? It seems like this is quite important so it would be odd if the julia community didn't have tools for this

https://documenter.juliadocs.org/stable/man/doctests/#Doctesting-as-Part-of-Testing
IIRC we have talked about this but not done it yet.


Midi Training Error

System. ArgumentOutOfRangeException: The value of '7' is not valid for 'Value'. 'Value' should be between 'minimum' and 'maximum'.
Parameter name: Value
In System.Windows.Forms.ProgressBar.set_ Value(Int32 value)
In System. Windows. Forms. Control. InvokeMarshalledCallbackHelper (Object obj)
In System. Threading. ExecutionContext. RunInternal (ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
In System. Threading. ExecutionContext. Run (ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
In System. Threading. ExecutionContext. Run (ExecutionContext executionContext, ContextCallback callback, Object state)
In System. Windows. Forms. Control. InvokeMarshalledCallback (ThreadMethodEntry tme)
In System. Windows. Forms. Control. InvokeMarshalledCallbacks()
This value is random from 5 to 9. "Training can be executed and saved, but it cannot take effect in a game. I also made this error while trying on another computer. Strangely, I was able to use and take effect this feature correctly two months ago.".

"I noticed that this issue may be in the progress bar below, but I don't know how to fix this error."o(╥﹏╥)o


.editorconfig not working correctly for comments with indent_style = tab

Type: Bug
Issue Description
When using .editorconfig with indent_style: tab, IDE0055 warnings are displayed around comments.
Steps to Reproduce

Create an editor config with the following settings

# This file is the top-most EditorConfig file
root = true

# All Files
[*]
tab_size = 4
charset = utf-8
indent_style = tab
indent_size = 4
insert_final_newline = true
trim_trailing_whitespace = true

##########################################
# Default .NET Code Style Severities
##########################################

[*.{cs,csx,cake,vb,vbx}]
# Default Severity for all .NET Code Style rules below
dotnet_analyzer_diagnostic.severity = warning

Add comments to your code

Expected Behavior
Comments appear without any warnings

Actual Behavior
Comments have warnings before and after

Logs
N/A
C# log
N/A
C# LSP Trace Logs
C# LSP.log
Environment information
VSCode version: 1.85.1
C# Extension: 2.15.30
Using OmniSharp: false
Dotnet Information
.NET SDK:
 Version:           8.0.101
 Commit:            6eceda187b
 Workload version:  8.0.100-manifests.825c015e
Runtime Environment:
OS Name:     Windows
OS Version:  10.0.22621
OS Platform: Windows
RID:         win-x64
Base Path:   C:\Program Files\dotnet\sdk\8.0.101\
.NET workloads installed:
Workload version: 8.0.100-manifests.825c015e
There are no installed workloads to display.

If that's not installed, VS Code itself doesn't really know about .editorconfig files; I can imagine you might be in a case where the .editorconfig says it should be a tab, but VS Code thinks it should still be a space, and we might be giving preference to VS Code's opinion in that case (not realizing it doesn't know better.)

No, I already have that installed

@AfroSpartan Strange -- the information filed with your bug doesn't list it which should have been filed in automatically.

(although if you also have it configured for tabs it's moot anyways)


Update mender artifact docs

The main change is the second commit:
Improve instructions for mender-artifact build: version and deps
We want the instructions to point to the latest released version. This
is important now that for the 3.0.0b release the code is not merged into
master, so without the checkout we cannot build mender-artifact suitable
for Update Modules
Also, document de LZMA depdendency with specific instructions for Mac OS
users, as we do not provice a native binary for them (yet).

@mzedel Please beta test the new instructions on Mac OS
@eysteins Please have a look. I will port this to 2.0.x docs release once it is approved.

:+1:
At some point we should split out all the build & tool download instructions into a separate page / area of the docs. But that is out of scope here I think. Looks good!

Thank you @mzedel for the beta testing.
I merged it before your feedback, so I needed to open a new PR for the last change, see #634 Sorry for the inconvenience.


Uses Zipkin's Stackdriver sender

The HttpClientParser and HttpServerParser portions of the
auto-configuration were left intact, since they currently enrich the
traces in a way that Zipkin/Brave can't (as far as I'm aware of).
This change depends on a new version of zipkin-translation-stackdriver
due to a bug.
Fixes #485

@marcingrzejszczak @adriancole FYI, this change depends on https://github.com/openzipkin/zipkin-gcp/pull/78 (and zipkin-gcp's subsequent version).

well I tried this a few weeks ago and didn't need to override the
asyncreporter..
https://github.com/openzipkin/sleuth-webmvc-example/blob/use-stackdriver-sender/src/main/java/sleuth/webmvc/StackdriverConfig.java
On Tue, May 1, 2018 at 10:29 PM, João Martins<EMAIL_ADDRESS>wrote:

@joaoandremartins commented on this pull request.
In spring-cloud-gcp-autoconfigure/src/main/java/
org/springframework/cloud/gcp/autoconfigure/trace/
StackdriverTraceAutoConfiguration.java
https://github.com/spring-cloud/spring-cloud-gcp/pull/648#discussion_r185325146
:

@@ -112,25 +96,24 @@ public SleuthProperties stackdriverSleuthProperties(SleuthProperties sleuthPrope

@Bean
@ConditionalOnMissingBean

public Reporter<zipkin2.Span> reporter(

	FlushableTraceConsumer traceConsumer,

	SpanTranslator spanTranslator) {

return new StackdriverTraceReporter(

		this.finalProjectIdProvider.getProjectId(),

		traceConsumer,

		spanTranslator);

public Reporter reporter(Sender sender, BytesEncoder encoder) {

Interesting. I tried to leave it out and the spans weren't being sent.
Maybe I messed up, or maybe I missed any (auto-configuration) dependency?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/spring-cloud/spring-cloud-gcp/pull/648#discussion_r185325146,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD611O068Ku3j8hHC30gTuADcVO86vgks5tuMWUgaJpZM4TuVuA
.

sorry few days ago

I just tried it again and it doesn't work.
By the way, I only added zipkin-sender-stackdriver to my dependencies. Perhaps I should have added another dependency that auto-configures the reporter based on the sender and encoder?

the sample is using spring-cloud-sleuth.. you are using this? also, maybe
run the example itself for sanity check?
On Tue, 1 May 2018, 22:49 João Martins<EMAIL_ADDRESS>wrote:

I just tried it again and it doesn't work.
By the way, I only added zipkin-sender-stackdriver to my dependencies.
Perhaps I should have added another dependency that auto-configures the
reporter based on the sender and encoder?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/spring-cloud/spring-cloud-gcp/pull/648#issuecomment-385785603,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD617U0n4esVc3CZ0as1UGQ4dfjfrSEks5tuMpDgaJpZM4TuVuA
.

Yep, the example works.
Also, we're tying with Sleuth's properties and using TraceHttpAutoConfiguration which is in Sleuth land.

Version now set to 0.4.1.
This is now ready for another pass. Please let me know if I missed any earlier requirement. I'd be interested in how we can not have to specify the reporter, but I'm afraid we're missing some auto-configuration...

rename the sender bean to just "sender" and see if you can remove your own
reporter config. the only thing wrong with this change is the redundantly
declared reporter.
I am going on holiday so sorry I can't comment more for a couple weeks.

@adriancole I made that test and it doesn't work.
I suggest we merge it this way and get back to this later, then.
@meltsufin can you please review?

@adriancole https://github.com/adriancole I made that test and it
doesn't work.
I suggest we merge it this way and get back to this later, then.

I'm on holiday, but I disagree on punting this part (about the extra
declaration of the AsyncReporter). Folks who own this repo can feel free to
merge eventhough I disagree, but I'd recommend them to not release it until
it is fixed.
On that note, seems someone else could actually try this (not just look at
it): While redundant declaration looks small but in practice it hints
something isn't working, or should be understood. Maybe someone from google
or sleuth can help. Try on sleuth's gitter? I will be back online 21 May,
in worst case.

i thought if we simply rely on sleuth zipkin starter (not just sleuth) that also has all the asyncreporter constructs; from there, all we need to do is to provide the Sender bean, right?

We needed to depend on ZipkinAutoConfiguration, which is where the reporter gets auto-configured.
@balopat can you please review this here? @meltsufin and @adriancole are away, but I think all of their concerns are addressed.

@ChengyuanZhao can you please review this PR next week whenever you have some time?


Ability to submit solutions from API

How about to adding possibility of submitting solutions from API? It will be g8 to solve katas from your favorite text editor
like https://api.codewars.com/<method>?<options> or something like that
┆Issue is synchronized with this Clickup  by Unito

opened door for more cheaters, imo...

How? This is the same thing as in browser
you only send your code to server
you can get only description of kata and test;
you will get test status and stderr
that all

not sure, but... One more way. Maybe easier than the current one.

When designed and implemented properly, API and web UI would meet in the same implementation on the backend, so submitting a solution via API should be no different than submitting it via web editor. Ideally, end result visible on the site would be identical. What I am concerned with is whether current implementation of API would allow such proper implementation. There are already many cases when API calls ignore privilege checks and authorization requirements, and I think it can be difficult to implement it appropriately with current state of the code :(
I was thinking about such API feature too, when I was wondering how difficult would it be to create a series of plugins for VS to solve and submit solutions.

I wrote a Codewars Intellij plugin that allows submission from Intellij. I was able to reverse-engineer the submission REST call simply by analyzing the network traffic in chrome dev tools. My Intellij Codewars plugin is open-source so if you are relly interested you could acctually get the details from there. I hope the codewars dev team does not change the API.


[$1000]  Chat - Bad performance when opening, closing or zooming big image

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: 1.4.51-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4418462
Email or phone of affected tester (no customers<EMAIL_ADDRESS>Issue reported by: Applause - Internal Team
Action Performed:
Pre-requisite: the user must be logged in

Go to any chat
Send the attached GIF
Send the attached image
Click on the image preview to open it
Verify the opening process is not smooth
Click on the image to zoom in
Verify is not instantly zoomed in, the behavior is slowly
Try to move the zoom view, verify is not smooth
Close the image
Verify the closing process is not smooth

Expected Result:
There should be no performance issues when opening, zooming or closing big images
Actual Result:
The behavior of the app gets slower when opening, zooming or closing the big image
Workaround:
Unknown
Platforms:

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

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

Screenshots/Videos
Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/bd11fa5f-47c6-4ba4-b4be-4c83e3fd4701

View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0184b3ab8ca5055856
Upwork Job ID:<PHONE_NUMBER>768452608
Last Price Increase: 2024-06-24

@mallenexpensify  FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

We think that this bug might be related to #vip-vsp
CC @quinthar

Check on in vip-vsb
https://expensify.slack.com/archives/C066HJM2CAZ/p1710294208664919

Adding to #vip-vsb since it's a bug that doesn't involve money.

Bumping to weekly while we await a status update

I think/hope this can be External

@mallenexpensify I do agree. I think this can be handled externally.

@sobitneupane , can you review the above proposal plz?

Thanks for the proposal @ShridharGoel
We would want solution that solves the issue for big image as the one attached here. Thank you for your efforts

@ShridharGoel Can you please add more details to RCA.
In which file does the issue lie? Where are you proposing to make the change? What will be the affect on quality of image after the change? Can you also attach what the output would look like?

@ShridharGoel I am fine with slight delay when zooming big image rather than decreasing quality as it only happens first time I zoom the image after opening the modal. I believe we can improve the performance issue when opening and closing the modal without decreasing the quality.
@mallenexpensify What's your take?
If we are okay with decreasing the image resolution, rather than compressing it to half, I think we should calculate the aspect ratio to make either height or width equal to max allowed value.

@ShridharGoel We can do that.  But won't it degrade the time required to upload file. I mean once I chose a heavy file,how long will it take to open upload modal?

What's your take?

Awaiting reply from @ShridharGoel on your comment above @sobitneupane .  If there's no degradation, that sounds like the way to go.   If there is, and we feel we might have to compromise, can you start a discussion in #expensify-open-source to get more 👀?  I know nothing about the coding behind the feature and fix so I don't feel comfortable making a guess or assumption.

Proposal
Please re-state the problem that we are trying to solve in this issue.
Chat - Bad performance when opening, closing or zooming big image.
What is the root cause of that problem?
Opening big image:
The provided image is png which takes over 10 seconds to loads. The front end or backend should consider implementing support for webp which takes under 2 seconds to load.
Closing big image:
In AttachmentModal.tsx, the HeaderWithBackButton component has onCloseButtonPress={closeModal}, the onModalClose does not have implementation, the onModalHide can be used instead which calls Navigation.dismissModal() of ReportAttachments.tsx.
https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/components/AttachmentModal.tsx#L505
Zooming big image:
Zooming for web needs to be reviewed and improvised. The current implementation in useEffect is calling scrollableRef.current.scrollLeft = initialScrollLeft + moveX, which takes several milliseconds.
https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/components/ImageView/index.tsx#L165
What changes do you think we should make in order to solve the problem?
For closing big image:
In AttachmentModal, onModalClose() has no effect due to lack of implementation, instead call onModalHide() which calls Navigation.dismissModal.
https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/components/AttachmentModal.tsx#L505
For opening big image:
Front end is capable of providing webp support by converting the image to webp using canvas api i.e canvas.toDataURL('image/webp'), backend can also convert provided image files (png, jpegs) into webp. Webp are much smaller in size than pngs and loads much faster than pngs and also do not decrease the image quality. Need to discuss.
Zooming big image:
Need to improve zooming using GestureDetector from 'react-native-gesture-handler'.
What alternative solutions did you explore? (Optional)

@jsdev2547 Similar to closing image, can we improve the opening image experience (without converting)? It is okay to show loading indicator by opening the attachment modal. But we have some lag while opening the image. What is the root cause for it?

@sobitneupane I have revised proposal. I am finding the lag and UI becoming non-responsive when opening the big image is specific only to pngs. To test need to check for non-responsive behavior in UI, i.e. the attachment modal should be able to get closed when the image is loading. When testing, I found the delay in opening and closing the image is considerably more noticeable in pngs than in other formats.
Also the zoom issue is also resolved when using backend provided 1024px jpg images for pngs. The advantage for using the 1024px jpg images is the conversion is already done by the backend.
Please do help test and confirm the findings if the delay is noticeable only in pngs then the provided 1024px jpg images could provide solution for the issue.

Also the zoom issue is also resolved when using backend provided 1024px jpg images for pngs.

@jsdev2547 I don't think your above statement is true. I can reproduce the issue even with 1024px jpg image (I am quite surprised tbh). I thought the issue was due to the dimension but the issue is being reproduced even with 1024px image.
https://github.com/Expensify/App/assets/25876548/694d53e1-d400-46d8-b632-cdf6ca01cdd3

I can reproduce the issue even with 1024px jpg image

@sobitneupane When I reset the closing modal portion ie. remove setShouldLoadAttachment(false); from closeModal callback, then the closing lag occurs even with 1024px jpg image. To resolve the lag issue need to include setShouldLoadAttachment(false) in closeModal. I have prepared branch for testing, try testing zooming on 1024px jpg.
poc.webm

Thanks for the update @jsdev2547. Your proposal will improve the experience but still the screen still freezes while opening the modal. The issue with zoom is still reproducible.
https://github.com/Expensify/App/assets/25876548/19cb5d4a-6eb2-40f1-bee9-6b2e62fbefd7

@sobitneupane I wanted to confirm if the screen freeze and the zoom freeze are related to the StyleUtils.getZoomSizingStyle
https://github.com/Expensify/App/blob/64d22534b3a06fb516684198167ab2e263d99a0e/src/components/ImageView/index.tsx#L225
During testing, when I comment out the PressableWithoutFeedback component and leave only the Image component, the screen freeze disappears but so does the zoom. Perhaps the zoom functionality needs to be revised and use gesture handler technique i.e zoom using double tap gesture.
Please test the branch, to confirm if the screen freeze still exists. In the test branch, the image loading for png is using 1024x jpg, while the jpg are loading original size. The jpg take few seconds to load but the screen does freeze.

@jsdev2547 My bad. I cannot reproduce the issue I mentioned here with this changes (initial commit)
I have three major questions about your proposal though.

I couldn't understand how does changing the format solve the issue? By making the change, are we referencing to image returned by backend instead of local image? In that case, why don't we use backend returned image instead of local image in the first place as we wait for the image to be uploaded before displaying it?

const [imageUrl, setImageUrl] = useState(() => {
    const { fileExtension } = FileUtils.splitExtensionFromFileName(url ?? '');
    const isvalid = (fileExtension === "png") ? true : false;
    const u = isvalid ? url + ".1024.jpg" : url;
    return u;
})

Will the solution work for user uploading an image?
There are the cases where we cannot change the url extension. For example: if we are referring to image that is not from Expensify. ![https://hatrabbits.com/wp-content/uploads/2017/01/random.jpg](https://hatrabbits.com/wp-content/uploads/2017/01/random.jpg)

Issue not reproducible during KI retests. (First week)

Bump @jsdev2547
https://github.com/Expensify/App/issues/38180#issuecomment-2084724901

@sobitneupane

By making the change, are we referencing to image returned by backend instead of local image? In that case, why don't we use backend returned image instead of local image in the first place as we wait for the image to be uploaded before displaying it?
The backend returns the original uploaded image along with the 1024px jpeg versions for when the image is already uploaded.
When uploading attachment, currently the image preview is using the local image, the url is in blob format and the backend has yet to process the image and return the jpeg version.

There are the cases where we cannot change the url extension. For example: if we are referring to image that is not from Expensify.

If the image is hosted externally, then the loading occurs without delay and even the big image loads without delay. So using the backend provided jpeg version has no impact.

Will the solution work for user uploading an image?
The only problem remains for when uploading the attachment which uses the local image (blob url). The backend provided versions are only available after upload. I tried the manipulateAsync helper utility used by cropOrRotateImage which can convert images to jpeg, but the loading is much slower. The other approach, which I am still testing is using canvas api.

@sobitneupane

By making the change, are we referencing to image returned by backend instead of local image? In that case, why don't we use backend returned image instead of local image in the first place as we wait for the image to be uploaded before displaying it?

The backend returns the original uploaded image along with the 1024px jpeg versions for when the image is already uploaded.
When uploading attachment, currently the image preview is using the local image, the url is in blob format and the backend has yet to process the image and return the jpeg version.

There are the cases where we cannot change the url extension. For example: if we are referring to image that is not from Expensify.

If the image is hosted externally, then the loading occurs without delay and even the big image loads without delay. So using the backend provided jpeg version has no impact.

Will the solution work for user uploading an image?

The only problem remains for when uploading the attachment which uses the local image (blob url). The backend provided versions are only available after upload. I tried the manipulateAsync helper utility used by cropOrRotateImage which can convert images to jpeg, but the loading is much slower. The other approach, which I am still testing is using canvas api.

@sobitneupane I am still trying to find viable solution for when the user uploads the image attachment.

@sobitneupane I am still trying to find reliable solution for when the user uploads the image attachment.

@sobitneupane , what do you propose are the next best steps here?

We are still waiting for the better proposals.
@jsdev2547 Any update?

Waiting on proposals

Thanks @sobitneupane , bumped to $500

Waiting on Proposal

Proposal
Please re-state the problem that we are trying to solve in this issue.
Chat - Bad performance when opening, closing or zooming big image
What is the root cause of that problem?
Image resolution too high to handle, particularly when interacting with the image through zoom or pan functions. By checking the performance tab in Chrome DevTools, we can see that the lag primarily occurs during the painting phase rather than the rendering phase.

What changes do you think we should make in order to solve the problem?
Simply don't show the image preview if it has to big resolution. This is the behavior of slack, they aren't shows the image if it's more than 8000px. But they provide download link.

We can add check in src/components/Attachments/AttachmentView/index.tsx

    const isFileHaveDimension = (file: FileObject): file is ImagePickerResponse => {
        return 'width' in file && 'height' in file;
    };
    const isResolutionValid = file && isFileHaveDimension(file) && (file?.height ?? 0) <= 8000 && (file?.width ?? 0) <= 8000;

    if ((isImage || (file?.name && Str.isImage(file.name))) && isResolutionValid) {
        if (imageError && (typeof fallbackSource === 'number' || typeof fallbackSource === 'function')) {

If we need the high resolution image directly download the image, then in ImageRendered.tsx change onPress to:
                <PressableWithoutFocus
                    style={[styles.noOutline]}
                    onPress={() => {
                        if ((imageWidth ?? 0) > 8000 || (imageHeight ?? 0) > 8000) {
                            const sourceURLWithAuth = addEncryptedAuthTokenToURL(source);
                            fileDownload(sourceURLWithAuth, imageName);
                            return;
                        }

Branch to test.
What alternative solutions did you explore? (Optional)
We have similar performance issue with avatar image picker. We decided to validate the resolution.
If we want this approach, we can adopt it by modifying isValidFile and validateAndDisplayFileToUpload function in src/components/AttachmentModal.tsx
       const isValidFile = useCallback((fileObject: FileObject) => {
            return new Promise<void>((resolve, reject) => {
                if (fileObject.size && fileObject.size > CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE) {
                    setIsAttachmentInvalid(true);
                    setAttachmentInvalidReasonTitle('attachmentPicker.attachmentTooLarge');
                    setAttachmentInvalidReason('attachmentPicker.sizeExceeded');
                    reject();
                }

                if (fileObject.size && fileObject.size < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) {
                    setIsAttachmentInvalid(true);
                    setAttachmentInvalidReasonTitle('attachmentPicker.attachmentTooSmall');
                    setAttachmentInvalidReason('attachmentPicker.sizeNotMet');
                    reject();
                }

                type AttachmentPickerType = typeof CONST.ATTACHMENT_PICKER_TYPE[keyof typeof CONST.ATTACHMENT_PICKER_TYPE];
                let fileType: AttachmentPickerType = CONST.ATTACHMENT_PICKER_TYPE.FILE;

                if (fileObject.type && fileObject.type.startsWith('image/')) {
                    fileType = CONST.ATTACHMENT_PICKER_TYPE.IMAGE;
                }

                if (fileType === CONST.ATTACHMENT_FILE_TYPE.IMAGE) {
                    isValidResolution(fileObject)
                        .then((isValid) => {
                            if (!isValid) {
                                setIsAttachmentInvalid(true);
                                setAttachmentInvalidReasonTitle('attachmentPicker.attachmentResolutionTooLarge');
                                setAttachmentInvalidReason('attachmentPicker.resolutionSizeExceeded');
                                reject();
                            } else {
                                resolve();
                            }
                        })
                        .catch(reject);
                } else {
                    resolve();
                }
            });
        }, []);

Branch to test

I will review the proposal by EOD.

Thanks for the proposal @wildan-m
@wildan-m Backend returns the image in 1024px in jpg format which doesn't have any lag. I believe we can make use of it after the server returns the image in 1024 px. The issue only lies when user is uploading. What do you think of this approach?

@sobitneupane I believe that using 1024px for a thumbnail may not result in good quality when zooming in, is that for thumbnail purpose? also convert it to jpg will remove transparent layer of png image. If we want to use convert approach then to avoid lag during upload commit, we should consider having an API that can convert the image before it is previewed and committed.

Thanks for the update @wildan-m

I believe that using 1024px for preview may not result in good quality when zooming in, is that for thumbnail purpose? also convert it to jpg will remove transparent layer of png image.

It's a good point.
@wildan-m Can you please attach screen recording(for your proposal) of the flow from uploading to previewing in chat report?

@sobitneupane proposal updated.
Add video demo and fix resolution check when uploading.

Is there any specific reason for choosing 8000px as the cut off point? I think we should test it for lower resolution images as well to make sure we don't have any issue with lower resolution image.

I am simply replicating Slack behavior by conducting a quick test with various sample images. The lower the resolution, the quicker the process (approximate value). 8000px might be too high; we can decrease it if necessary.

@wildan-m Thanks for the update.
We will consider your solution as an option, but my preference is to display the image if possible. I will wait for proposals over the weekend and start discussing your suggested approach early next week.

Checking on internally.

Being discussed internally.

Started discussion in slack:
https://expensify.slack.com/archives/C066HJM2CAZ/p1718003479823739

@wildan-m Backend returns the image in 1024px in jpg format which doesn't have any lag. I believe we can make use of it after the server returns the image in 1024 px. The issue only lies when user is uploading. What do you think of this approach?

Yeah this is correct. One complication on the backend was that very wide or tall images led to a very blurry preview image, so I recently started cropping these images. We should be careful to test an image like this one:
1024x10573:

Not overdue.

Lets double bounty

Waiting for proposals

@sobitneupane How is the discussion going?

@wildan-m Here is the input from @Julesssss

@sobitneupane It appears that @Julesssss has commented on your feedback. My suggestion was not to optimize performance, but rather to modify the behavior when uploading high-resolution images.
Even if we manage to enhance performance successfully, there should still be a limit on the resolution that the image preview feature can handle. Without constraints, it may result in additional lag or unexpected errors.

Even if we manage to enhance performance successfully, there should still be a limit on the resolution that the image preview feature can handle

To clarify, I think we might be able to use the 1024x1024 thumbnail for the preview.
But to be honest, I'm a bit lost about the proposals' state and which are still valid. If you have a proposal could you please share a sentence about the plan, and a link to the original proposal? Thanks

Proposal
Please re-state the problem that we are trying to solve in this issue.
Bad performance when opening, closing, or zooming a big image
What is the root cause of that problem?
The image resolution pixel is too big 9000x9000 pixels.
When I test with simple HTML and local image (9000x9000) it's still rendering slowly when I zoom in and zoom out
Test Result:
https://github.com/Expensify/App/assets/76243292/a44d5c16-67d8-42de-b22d-7da6aa0ed572
What changes do you think we should make in order to solve the problem?
We can detect if the pixels of the image are equal or more than 3000, we can resize the image using Sharp or if we don't want to use Sharp for react native, we can use another package.  But the main point is we need to resize the image to a smaller size for instance 1000 pixels or less
I've tested it with a local HTML file using Sharp and it renders the image very fast when I zoom in and zoom out in the browser
Test Result:
https://github.com/Expensify/App/assets/76243292/8519678f-79ff-4e47-a7aa-e42197d25b35
What alternative solutions did you explore? (Optional)
We can set a limited number of pixels of the image when we're going to upload an attachment

@Julesssss this is the link to the original proposal.

Proposal
Updated

Okay thanks all. I still like @wildan-m's proposal because it allows us to make further backend improvements later on while still fixing the issue.
@ShridharGoel This works, but I would prefer to do compression on the backend, using our existing logic so that we don't have different resizing logic throughout the product. However, we don't have time for the backend work currently.
@NJ-2020 again, I'd prefer not to use a client library because this splits our image resizing logic into different solutions and it becomes messy.

@sobitneupane do you agree?
One thing we'll need to improve is how the image is downloaded. @wildan-m in your example we immediately download the image when selected. But let's see if the design team can help us with how this should work.

For comparison, Slack will display a placeholder image/icon if the resolution is over ~7000px.

I'm not totally sure I'm following... but am I understanding correctly that we can't display a smaller preview image in the attachment modal because we are uploading the local file and therefore can't create a smaller preview for it yet?
If that's the case... I guess we just update the "empty" state of the attachment modal?
Some random options:

Big placeholder with file info

Slight tweak on our existing "file" display

Basically do nothing
@Expensify/design any thoughts/ideas/preferences on this one?

but am I understanding correctly that we can't display a smaller preview image in the attachment modal because we are uploading the local file and therefore can't create a smaller preview for it yet?

Ah not quite. I shouldn't have added that screenshot as it confuses things.
We will always generate a 1024x1024 thumbnail, which gets displayed in the item row, and could be shown. We just cant render the full-resolution image.

Hmm if we have the 1024x1024 preview image, I would love to use that. Though I kind of think we might need to consider doubling that in size so the image generally looks good on bigger screens, as 1024 will look blurry on most modern laptops I think?

@shawnborton I agree that a higher-resolution preview would be a better solution, but that would require a new/updated API on the backend, and there's no way we can prioritise that work currently.

Oh hmm, then why wouldn't we just render the image that we can render, and then the download button in the attachment modal would download the full resolution image? I feel like I'm not understanding something.

why wouldn't we just render the image that we can render, and then the download button in the attachment modal would download the full resolution image?

Nah you understand correctly, ^ that sounds great. So we'd just re-use the existing Image preview modal?

Yeah I think so! @shawnborton do you have anything else to add?

Nope, sounds like the idea is to use the generated thumbnail we have in the view modal if the image is too big, right?

Nope, sounds like the idea is to use the generated thumbnail we have in the view modal if the image is too big, right?

@Julesssss @dannymcclain @shawnborton Is the solution scalable? Please also consider this case, especially with very long height.
Imagine we upload an image with a greater height than that scenario, and we need to make a smaller version of it.
My questions are:

Should we maintain the aspect ratio? If we resize the height to, let's say, 2048px, the width may become too small.
If we create a cropped version of the thumbnail and display it in the viewer, it won't accurately represent the actual image.

In my opinion, Slack's approach is excellent. There's no need to show them in the viewer if it's too large; allow the user's OS image viewer to manage it after downloading.

After reviewing the current behaviour of being able to pan and zoom, I do agree the proposed solution is quite a downgrade.
If we show the preview after a certain height/width is met, I think we'd need to make sure that (A) the height/width cutoff is reasonably high 4000-5000? and (B) that we make it clear to the users that the rendered image is just a preview of the full downloadable image.

There are two cases that need to be addressed:

Preview before the attachment is uploaded (we won't have the compressed/cropped image from the server at this point).
Preview after the attachment is uploaded.

Probable solutions so far:

Crop the high-dimension image before displaying it
Display the compressed image (this only works for the 2nd case)
Show a placeholder instead of the image (suggested here)

I would vote for 3rd option with 2nd/3rd design

WhatsApp uses the second approach with additional converting to JPEG, as displayed in their image viewer and after the image is re-downloaded:

It is evident that the image is unusable.

I think our backend logic is fine, a 1024* preview and full size download is good. We just need to figure out how to handle these two cases in the client.

I suggest that while the preview is loading we show the loading spinner (as we currently do within the chat)
Then when clicked, we show the model with the full-resolution image

UNLESS the image is above a specific width/height, in which case we replace the preview with the below component within the model:

Then when clicked, we show the model with the full-resolution image
UNLESS the image is above a specific width/height, in which case we replace the preview with the below component within the model:

Personally I don't see any value in opening a modal just to display a little file component/lockup. If we're not going to show them a preview of the image, we might as well treat it like any other non-previewable file and only allow them to download (not open in the modal at all). IMO it would be better to show a low-res preview with a little message that says something like "This image has been resized for previewing. Download for full resolution." I just think it would be better to give the user something to see, even if it's downsized, rather than nothing. I think the most common use case for uploading photos to a chat app is to be able to see them in the chat app. So we should do our best to make that happen (even if the image preview is less than ideal). But that's just my 2 cents! @shawnborton @dubielzyk-expensify do y'all have any thoughts to add?

That sounds great to me Danny, I agree with you.

"This image has been resized for previewing. Download for full resolution."

Cool, that sounds great. As (I believe) this is new UI, would you mind creating a mock we can pass onto @wildan-m for implementation?

I was thinking something like this, where we just reuse our little banner from archived chats:

We could do something more minimal/out of the way if we wanted. Maybe something like this:

@Expensify/design what do you think?

I like it, I think I like the second version so it takes up less space? We could consider linking the word Download too to the download action?

I like it, I think I like the second version so it takes up less space? We could consider linking the word Download too to the download action?

Sounds good to me. This then yeah? I don't think it's totally necessary, but I don't really mind it either. Would probably be helpful for some.

Cool, yeah that was what I was thinking. I can see keeping things simple and not doing the link, since there is an obvious icon  in the top right.

@sobitneupane @Julesssss Check out my updated proposal here.
Change log:

Moved my previous main solution to alternative 2.
Added a new solution based on recent requirement changes (including demo and branch link).

In my view, the suggested solution is similar to my previous proposed solution, with some minor differences.
Previous solution: Suggested not showing a preview or using a placeholder icon, and providing the original link.
Current requirement change: Suggests showing a downsized image preview and providing the original link.
Since this may be considered a new feature, I believe further minor adjustments (refactoring, adding an icon, styling) should be done in the PR review phase.

@shawnborton @dannymcclain please review the draft outcome in this video.

Pretty close, but some notes:

We decided not to use the blue download link
There seems to be a good bit of space below the warning text and the green button below
The text should use our textSupporting color

But all of this can be sorted out in the PR review process.

Also @mallenexpensify why is the bounty for this one so high? Can we knock it back down to $250 like all other bugs?

@wildan-m
We won't have the compressed image available for the 1st case. How are you handling the 1st case? I don't think compressing it in client side is good idea as it might take some time to compress the image.

Preview before the attachment is uploaded (we won't have the compressed/cropped image from the server at this point).

We will be displaying 1024 px compressed image (used by thumbnail) for the 2nd case. Is it available in AttachmentView? If not, how will we pass it?

@sobitneupane @shawnborton @dannymcclain Can the preview image for high-resolution image be zoomable? I believe we can utilize a local blob image for the case before uploading, with a non-zoomable preview. This would eliminate the need to wait for the image to be compressed from the server.

I would think our preview for this should just work the same as our other image previews when uploading.

@sobitneupane @Julesssss Is there an image compression API that can be used before sending the attachment?
If so, the flow for the first case would be as follows:

User uploads the file
The file is sent to the compressImage API
The compressed image is used for image preview
The user sends the attachment, which will send the original image file.

@wildan-m I think it would be overkill. For the case before the image is uploaded, I think we should go with this approach.

Also @mallenexpensify why is the bounty for this one so high? Can we knock it back down to $250 like all other bugs?

@shawnborton This issue is 3 months old, so I believe the current bounty is fair.
Here is my perspective on reducing the bounty.

I think this is a super edge case bug that has impacted nearly 0 paying customers, and therefore I think we should place it at the smallest starting point possible.

Alternatively, we could just close this issue out and wait for an actual paying customer to report it first before making any adjustments here.

why is the bounty for this one so high? Can we knock it back down to $250 like all other bugs?

We didn't have any proposals for a long time, so I followed the process and doubled the bounty. It seems a bit unfair to reduce this because detailed proposals were made based on the current bounty at that time. I agree the solution ended up being less difficult than a detailed performance improvement might be, but we did get the desired outcome.
As for whether we drop these changes, I want to move forward as this issue was created to improve app performance -- a major problem. If we just wanted to improve image uploading I would agree with you that this wasn't essential 👍

Can you elaborate more on what you mean by major problem? From my point of view, it's hard to call something a major problem if we haven't had a single customer report this issue. Even in our internal usage of the product, I don't know that anyone has encountered this?

I would think our preview for this should just work the same as our other image previews when uploading.

I agree with this. And @sobitneupane's comment here. With these I think we are very close to a working proposal now.

can you elaborate more on what you mean by major problem? From my point of view, it's hard to call something a major problem if we haven't had a single customer report this issue.

Performance, in general, is the major problem. We have had many experts working to improve app performance and are seeking even minor gains. Most of our performance gains haven't been individually raised as problematic by an internal or external user, but have been detected and handled by 3rd party teams under the 'new dot-quality' project (previously new dot-performance).
In this case we are close to a proposal that has been approved by the design team and noticeably reduces UI lag when viewing an uploaded image, so my vote is to implement it. But I'd happily defer to the project team.

reduces UI lag when viewing an uploaded image

My understanding is that the proposals only address viewing a super large image, not just any image. Is that your understanding too?
But you haven't really addressed my comments at all. I'm curious if you think this is a major problem impacting our users or even our internal usage. Thoughts?

Yeah, I share the understanding. The improvement scales with the dimensions, which is noticable only as the images get really large.
Also no, I don't think there is an argument for this improvement being a major problem, nor impacting users. Where I disagree, is that in the case of app performance, I believe it is better to make the pre-optimisation (just for performance, not generally).

Alright. Sorry to reduce this bounty, but as we're on the fence about this issue I'm reducing the bounty to $500. That gives us a bit more of a reason to implement the solution.

The reason the bounty was high was because the issue has been open for months and it's been a difficult bug to fix (over 100 comments here and multiple proposals).  I addressed in #contributor-plus and Sobit addressed in #vip-vsb to discuss options.

@sobitneupane @shawnborton @Julesssss my proposal updated
Change log:

Ensure high resolution preview image is zoomable
Resolve the 1st case (ref)
Update video result

@wildan-m Can you please address following comments.

We will be displaying 1024 px compressed image (used by thumbnail) for the 2nd case. Is it available in AttachmentView? If not, how will we pass it? (ref)

We decided not to use the blue download link (ref)

We have decided to go with 2nd design from this comment.

To distinguish between uploaded and non-uploaded images, we can use the isUsedInAttachmentModal prop.

I'm a bit confused about isUsedInAttachmentModal. This is an existing prop, right? What use case is it currently serving? How does it distinguish between uploaded and non-uploaded images?

@sobitneupane Proposal Updated
Change log:
change isUsedInAttachmentModal to isUploaded

I'm a bit confused about isUsedInAttachmentModal. This is an existing prop, right? What use case is it currently serving? How does it distinguish between uploaded and non-uploaded images?
@sobitneupane isUsedInAttachmentModal is not accurate, it will also true for uploaded receipt image, so I change it with new prop called isUploaded

We decided not to use the blue download link (https://github.com/Expensify/App/issues/38180#issuecomment-2193905335)
Does that mean the 'Download' text is clickable but with the same grey color, or do we only display unclickable text?

We have decided to go with 2nd design from https://github.com/Expensify/App/issues/38180#issuecomment-2192167500.
What is the name of that (i) icon? can someone point me to similar existing text in a screen? @shawnborton @dannymcclain

or do we only display unclickable text?

No need to display any kind of link here, we have the download icon in the top right

What is the name of that (i) icon? can someone point me to similar existing text in a screen?

The icon is here. I don't know that we have the icon + text as a component but it should be easy to make with our style helpers (flex, gap2, fontLabelSupporting, etc)

@sobitneupane @shawnborton Proposal Updated
Change Log:

Remove download link
Add Info icon
Explain where to extract thumbnail image

Thanks for the update @wildan-m
Overall your proposal looks good to me. We can work on the minor details in the PR.
🎀 👀 🎀 C+ Reviewed

@sobitneupane I need more time to re-test the solution and prevent regressions, as there are a few glitches on native that I am working to fix.

@sobitneupane The PR is ready for your review. The translation in slack is not confirmed yet.
https://expensify.slack.com/archives/C01GTK53T8Q/p1719953316322399

PR still lin review, I'm bumping it today

PR merged, awaiting deployment 🎉

@sobitneupane plz complete the BZ checklist above.

Contributor: @wildan-m paid $500 via Upwork
Contributor+: @sobitneupane owed $500 via NewDot
@sobitneupane plz complete the BZ checklist above.

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

Rather than a bug, it is a performance issue.

Regression Test Proposal

Go to any chat and upload an image with dimension less than 7000*7000 px.
Verify that the original image is displayed and can be zoomed.
Select Send button.
Open the newly uploaded image and verify that the original image is displayed and can be zoomed.
Go to any chat and upload an image with dimension more than 7000*7000 px.
Verify that the attachment modal opens up with filename and info text at the bottom of the modal.
Select Send button.
Verify that the image is displayed and can be zoomed and info text is displayed at the bottom of the image.

Do we agree 👍 or 👎

@sobitneupane I think that is good, but lets add a case for downloading the image.

@Julesssss Updated.

$500 approved for @sobitneupane

Thanks @sobitneupane, test case created

https://github.com/Expensify/Expensify/issues/416544


ALARM1

I just had a very weird episode with ioSender.
I've updated settings to see if hard limits are the way to go.
After the change I was getting an "ALARM1".
And the error description doesn't really make sense to in the current context.
There wasn't much I can do. Unlock didn't work. And restart also seemed to have issues.
So I went back into the settings to change it back. But the settings were just a white screen.
After a lot of restarting and poking I somehow got access to the settings again and changed it back.
It seems to be working again now.
I don't quite understand if this was a firmware problem, or whether it was ioSender having a bad day.
I'll try to gather more details with the latest build - but if you already got some input I am all ears.

I don't quite understand if this was a firmware problem

It is a firmware problem carried over from legacy Grbl: alarm 1 is an event/state that block all commands until a reset and/or the condition is cleared (depends on a setting). FYI I recently updated grblHAL to allow $-commands when in a "blocking" state - this, and later, versions will behave better in such circumstances.

I upgraded to ioSender edge and switching to hard limits and returning from ALARM1 seems to work fine now.
Thanks for the help!


Clangd not finding include for external library headers

I use conan to manage projects. For example I have three projects: A that depends only on system libs, B that depends on A, and C that depends on B.
For A, it builds fine and clangd works fine. Conan would place the built project header under like $HOME/.conan/data/A/include/A.h;
For B, conan can provide FindA.cmake, so it builds fine and clangd works fine (as long as cmake is requested to generate compile_commands.json). The code check and insight works both in project B's files, and when Ctrl+Click on a symbol from A and it jumps to header files of A in $HOME/.conan/data/A/include. Conan would place the built project header under $HOME/.conan/data/B/include/B.h;
The problem is for C. Conan provide FindA.cmake and FindB.cmake, and the project also builds fine. Clangd also work for C's files. However,  when Ctrl+Click on a symbol from B and it jumps to header file $HOME/.conan/data/B/include/B.h, clangd no longer works and reports it cannot find header A.h which is included in B.h (pp_file_not_found).
System information
Output of clangd --version:  (I built it from llvm/llvm-project@1343632)
clangd version 15.0.0
Features: linux
Platform: x86_64-unknown-linux-gnu

Editor/LSP plugin: vscode-clangd
Operating system: Oricale Linux Server 7.9

Please have a look at https://clangd.llvm.org/faq#how-do-i-fix-errors-i-get-when-opening-headers-outside-of-my-project-directory


Fixed #20393 -- Stopped QuerySet repr from executing database queries

Discussion on the mailing list: https://groups.google.com/forum/#!topic/django-developers/WzRZ0IfBipc

Closing per ticket. Ticket is not accepted, and as you can see on the mailing list there are votes against this change.


Upgrade: opengl es for Android compatible (videocall)

Hello,
I like baresip and use it a lot on android. I would like to add videocall on android version, but module opengl es on source is mainly for MAC or iOS. Is there any possibility of porting the code to android and add interaction with baresip?
Regards,

William Anderson writes:

I like baresip and use it a lot on android. I would like to add
videocall on android version, but module opengl es on source is mainly
for MAC or iOS. Is there any possibility of porting the code to
android and add interaction with baresip?

If I remember correctly, opengles module compiles using NDK toolchain,
but I don't know what needs to be changed to make it usable for an
Android app.

it is probably better to create a new module for video output for Android.
I think there are better ways to display a video frame in a Window, using
Android specific APIs.
Have a look at the SDK/NDK documentation and check if there are other methods.

note: https://github.com/alfredh/baresip/wiki/Guidelines-for-reporting-Issues


[doc] Grafana deployment with a Persistent Volume

What's the problem?
While trying the example for using Persistent Volume in grafana-operator, which is documented here:
https://grafana.github.io/grafana-operator/docs/examples/persistent_volume/readme/
I encountered an error:
GF_PATHS_DATA='/var/lib/grafana' is not writable. You may have issues with file permissions, more information here: http://docs.grafana.org/installation/docker/#migrate-to-v51-or-later mkdir: can't create directory '/var/lib/grafana/plugins': Permission denied

This happens because the new config in Deployment, meant to override the original config, was actually causing permission issues as fixed in this issue: https://github.com/grafana/grafana-operator/issues/300
  deployment:
    securityContext:
      fsGroup: 472

However, I managed to resolve it by setting securityContext - fsGroup: 472 again as shown below:
apiVersion: grafana.integreatly.org/v1beta1
kind: Grafana
metadata:
  name: grafana
  labels:
    dashboards: "grafafna"
spec:
  deployment:
    spec:
      template:
        spec:
+         securityContext:
+           fsGroup: 472
          containers:
            - name: grafana
              image: grafana/grafana:9.4.3

This issue is related to https://github.com/grafana/grafana-operator/issues/1418#issuecomment-1953609940
What I want
Could we update the documentation example to include the securityContext - fsGroup ?

Seems like you're confusing two different scenarios here.
Persistent volumes
When we're talking about Persistent Volumes, they come empty and they are writable. If you deploy the example you referred to as is, a new volume becomes provisioned through a PVC, the volume gets mounted to /var/lib/grafana, and it'll all works just fine:

The snippet (with fsGroup) you shared is contradicting, because it looks like you're not using persistent volumes.
Ephemeral volumes
When you deploy a basic example (quoted below), any changes in Grafana are not persistent (they're gone once the respective pod is gone). To make sure Grafana has enough permissions to store its data, an emptyDir volume is automatically mounted to /var/lib/grafana. Again, everything works just fine.
apiVersion: grafana.integreatly.org/v1beta1
kind: Grafana
metadata:
  name: grafana
  labels:
    dashboards: "grafana"
spec:
  config:
    log:
      mode: "console"
    auth:
      disable_login_form: "false"
    security:
      admin_user: root
      admin_password: secret

What I think is happening
uid 472 is used in the default grafana image:

But grafana-operator automatically adds runAsNonRoot: true to a pod securityContext, which changes the default uid to something else, the exact id is likely to be different in your case:

Assuming you're using persistent volumes (unlike in the snippet you shared), I guess you deployed grafana with modified securityContext (runAsNonRoot: false or something else; directly or through mutating webhooks), grafana created some files, then you redeployed it with another set of settings, and now grafana fails to write data to the pre-existing files, because they had been created with different permissions.
When you specify a custom fsGroup, Kubernetes changes ownership and permissions for files upon pod start (docs).
I don't think we need to update the example. If you redeploy grafana with a brand new volume using an up-to-date version of the operator, it should all just work.

I'm also seeing what @harrythecode reported, this time I made sure that I have no PV/PVCs prior to deploying the Grafana object, so a volume with stale files with wrong permissions is unlikely to be the cause.
Currently running v5.6.3 of the operator.

@caguiclajmg @harrythecode It'd be helpful if you could share more information around your environment:

Kubernetes / OpenShift?
Version;
Details on underlying storage that you're using (e.g. local storage, a managed disk from a cloud provider, etc);
Full Grafana manifest;
Full Deployment manifest;
Full Pod manifest;
Full PersistentVolumeClaim spec.

Also, if it's something that can be reproduced in local (kind, microk8s, ...) or cloud provider environment, then full instructions would be helpful.

Hi,
I ran into the same issue just today, when I tried to use a persistent volume claim for Grafana in a Kubernetes Cluster (Amazon EKS). With the options from the example yaml I also experienced the same error "missing file permissions".
(I also started from scratch, no existing pvcs and so on)
I played around a bit and also found this Github issue, which helped me in debugging.
When I add:
spec.deployment.spec.template.spec.securityContext.fsGroup: 10001
It seems to work.  Maybe this helps in digging into the root cause of this issue. :)
I removed some individual stuff from the YAML I use but with the following code it seems to run at the moment:
`apiVersion: grafana.integreatly.org/v1beta1
kind: Grafana
metadata:
name: {{ .Values.metadata.app_name }}
namespace: {{ .Values.metadata.namespace }}
labels:
dashboards: "grafana"
spec:
persistentVolumeClaim:
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 10Gi
config:
log:
mode: "console"
[...]
deployment:
spec:
template:
spec:
serviceAccountName: secrets-csi-sa-monitoring
securityContext:
fsGroup: 10001
containers:
- name: grafana
securityContext:
allowPrivilegeEscalation: true
readOnlyRootFilesystem: false
readinessProbe:
failureThreshold: 3
[...]
image: grafana/grafana:10.4.0
[...]
volumes:
- name: grafana-data
persistentVolumeClaim:
claimName: grafana-pvc
[...]

@bavarian-ng Thanks for reporting this, though, unfortunately, it's not enough to share only Grafana CR here as the end pod spec can be influenced by various webhooks. Please, take a look at my comment above, which describes which of the resources can give us a better understanding of the things you experience in your cluster.

@weisdd , sorry here the requested info:

Kubernetes / OpenShift?: Kubernetes
Version: v1.27
Details on underlying storage that you're using: AWS GP2

# kubectl describe storageclass gp2
Name:            gp2
IsDefaultClass:  Yes
Annotations:     kubectl.kubernetes.io/last-applied-configuration={"apiVersion":"storage.k8s.io/v1","kind":"StorageClass","metadata":{"annotations":{"storageclass.kubernetes.io/is-default-class":"true"},"name":"gp2"},"parameters":{"fsType":"ext4","type":"gp2"},"provisioner":"kubernetes.io/aws-ebs","volumeBindingMode":"WaitForFirstConsumer"}
,storageclass.kubernetes.io/is-default-class=true
Provisioner:           kubernetes.io/aws-ebs
Parameters:            fsType=ext4,type=gp2
AllowVolumeExpansion:  <unset>
MountOptions:          <none>
ReclaimPolicy:         Delete
VolumeBindingMode:     WaitForFirstConsumer
Events:                <none>

Full Grafana manifest;

apiVersion: grafana.integreatly.org/v1beta1
kind: Grafana
metadata:
  name: {{ .Values.metadata.app_name }}
  namespace: {{ .Values.metadata.namespace }}
  labels:
    dashboards: "grafana"
spec:
  persistentVolumeClaim:
    spec:
      accessModes:
        - ReadWriteOnce
      resources:
        requests:
          storage: 10Gi
  config:
    log:
      mode: "console"
    auth:
      disable_login_form: "false"
    auth.google:
      enabled: "true"
      scopes: https://www.googleapis.com/auth/userinfo.profile https://www.googleapis.com/auth/userinfo.email
      auth_url: https://accounts.google.com/o/oauth2/auth
      token_url: https://oauth2.googleapis.com/token
      allowed_domains: <REDACTED OUR DOMAIN>
      allow_sign_up: "true"
    server:
      root_url: https://{{ .Values.ingress.servicename }}.{{ .Values.metadata.stage }}.{{ .Values.ingress.dns_zone }}
      serve_from_sub_path: "true"
    users:
      auto_assign_org_role: "Editor"
  deployment:
    spec:
      template:
        spec:
          serviceAccountName: secrets-csi-sa-monitoring
          securityContext:
            fsGroup: 10001
          containers:
            - name: grafana
              securityContext:
                allowPrivilegeEscalation: true
                readOnlyRootFilesystem: false
              readinessProbe:
                failureThreshold: 3
              env:
                - name: GF_AUTH_GOOGLE_CLIENT_ID
                  valueFrom:
                    secretKeyRef:
                      name: grafana-google-sso
                      key: client_id
                - name: GF_AUTH_GOOGLE_CLIENT_SECRET
                  valueFrom:
                    secretKeyRef:
                      name: grafana-google-sso
                      key: client_secret
                - name: GF_INSTALL_PLUGINS
                  value: grafana-oncall-app
                - name: GF_SECURITY_ADMIN_USER
                  valueFrom:
                    secretKeyRef:
                      name: grafana-admin-creds
                      key: adminuser
                - name: GF_SECURITY_ADMIN_PASSWORD
                  valueFrom:
                    secretKeyRef:
                      name: grafana-admin-creds
                      key: adminpassword
              image: grafana/grafana:10.4.0
              volumeMounts:
              - name: secret-store-grafana
                mountPath: "/mnt/secrets"
              - name: secret-store-grafana-sso
                mountPath: "/mnt/sso-secrets/"
              - name: plugin-config
                mountPath: "/etc/grafana/provisioning/plugins/"
              resources:
                limits:
                  memory: {{ .Values.deployment.containers.grafana.resources.limits.memory }}
                requests:
                  cpu: {{ .Values.deployment.containers.grafana.resources.requests.cpu }}
                  memory: {{ .Values.deployment.containers.grafana.resources.requests.memory }}
          volumes:
            - name: grafana-data
              persistentVolumeClaim:
                claimName: grafana-pvc
            - name: plugin-config
              configMap:
                name: grafana-oncall-plugin-config
            - name: plugin-folder
              hostPath:
                path: /etc/grafana/provisioning/plugins
                type: DirectoryOrCreate
            - name: secret-store-grafana
              csi:
                driver: secrets-store.csi.k8s.io
                readOnly: true
                volumeAttributes:
                  secretProviderClass: "spc-grafana"
            - name: secret-store-grafana-sso
              csi:
                driver: secrets-store.csi.k8s.io
                readOnly: true
                volumeAttributes:
                  secretProviderClass: "spc-grafana-sso"

  service:
    spec:
      type: NodePort
      ports:
        - protocol: TCP
          port: 3000
  ingress:
    metadata:
      annotations:
        kubernetes.io/ingress.class: alb
        alb.ingress.kubernetes.io/scheme: internet-facing
        alb.ingress.kubernetes.io/target-type: instance
        alb.ingress.kubernetes.io/load-balancer-name: central-loadbalancer-{{ .Values.metadata.stage }}
        alb.ingress.kubernetes.io/backend-protocol: HTTP
        alb.ingress.kubernetes.io/listen-ports: '[{"HTTP":80}, {"HTTPS":443}]'
        alb.ingress.kubernetes.io/ssl-redirect: '443'
        alb.ingress.kubernetes.io/ssl-policy: ELBSecurityPolicy-TLS13-1-2-2021-06
        alb.ingress.kubernetes.io/certificate-arn: {{ .Values.ingress.certificate_arn }}
        alb.ingress.kubernetes.io/subnets: 'steelmntn-{{ .Values.metadata.stage  }}-vpc-public-eu-central-1a,steelmntn-{{ .Values.metadata.stage  }}-vpc-public-eu-central-1b,steelmntn-{{ .Values.metadata.stage  }}-vpc-public-eu-central-1c'
        alb.ingress.kubernetes.io/group.name: steelmountain-ingress-group
        external-dns.alpha.kubernetes.io/hostname: {{ .Values.ingress.servicename }}.{{ .Values.metadata.stage }}.{{ .Values.ingress.dns_zone }}
        external-dns.alpha.kubernetes.io/ttl: "60"
        external-dns.alpha.kubernetes.io/ingress-hostname-source: annotation-only
    spec:
      ingressClassName: alb
      rules:
        - host: {{ .Values.ingress.servicename }}.{{ .Values.metadata.stage }}.{{ .Values.ingress.dns_zone }}
          http:
            paths:
              - backend:
                  service:
                    name: {{ .Values.service.name }}
                    port:
                      number: 3000
                path: /
                pathType: Prefix
      tls:
        - hosts:
            - {{ .Values.ingress.servicename }}.{{ .Values.metadata.stage }}.{{ .Values.ingress.dns_zone }}

Full Deployment manifest; (resulting deployment created by operator:)

apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    deployment.kubernetes.io/revision: '1'
  creationTimestamp: '2024-03-11T14:41:22Z'
  generation: 1
  name: grafana-deployment
  namespace: monitoring
  ownerReferences:
    - apiVersion: grafana.integreatly.org/v1beta1
      kind: Grafana
      name: grafana
      uid: 67785905-eb4c-4922-a68a-fe4821aebfb0
  resourceVersion: '90847627'
  uid: 855a9b34-532c-4440-ace1-88aadf728f6a
spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app: grafana
  strategy:
    rollingUpdate:
      maxSurge: 25%
      maxUnavailable: 25%
    type: RollingUpdate
  template:
    metadata:
      creationTimestamp: null
      labels:
        app: grafana
      name: grafana-deployment
    spec:
      containers:
        - args:
            - '-config=/etc/grafana/grafana.ini'
          env:
            - name: GF_AUTH_GOOGLE_CLIENT_ID
              valueFrom:
                secretKeyRef:
                  key: client_id
                  name: grafana-google-sso
            - name: GF_AUTH_GOOGLE_CLIENT_SECRET
              valueFrom:
                secretKeyRef:
                  key: client_secret
                  name: grafana-google-sso
            - name: PLUGINS_HASH
              valueFrom:
                configMapKeyRef:
                  key: PLUGINS_HASH
                  name: grafana-plugins
                  optional: true
            - name: CONFIG_HASH
              value: a5f231ba1d21e0391bebb602b1d9f4066be7b5a625ffc17c5a9396e121888feb
            - name: GF_INSTALL_PLUGINS
              value: grafana-oncall-app
            - name: TMPDIR
              value: /var/lib/grafana
            - name: GF_SECURITY_ADMIN_USER
              valueFrom:
                secretKeyRef:
                  key: adminuser
                  name: grafana-admin-creds
            - name: GF_SECURITY_ADMIN_PASSWORD
              valueFrom:
                secretKeyRef:
                  key: adminpassword
                  name: grafana-admin-creds
          image: 'grafana/grafana:10.4.0'
          imagePullPolicy: IfNotPresent
          name: grafana
          ports:
            - containerPort: 3000
              name: grafana-http
              protocol: TCP
          readinessProbe:
            failureThreshold: 3
            httpGet:
              path: /api/health
              port: 3000
              scheme: HTTP
            initialDelaySeconds: 5
            periodSeconds: 10
            successThreshold: 1
            timeoutSeconds: 3
          resources:
            limits:
              memory: 256Mi
            requests:
              cpu: 64m
              memory: 256Mi
          securityContext:
            allowPrivilegeEscalation: true
            capabilities:
              drop:
                - ALL
            privileged: false
            readOnlyRootFilesystem: false
            runAsGroup: 10001
            runAsNonRoot: true
            runAsUser: 10001
          terminationMessagePath: /dev/termination-log
          terminationMessagePolicy: File
          volumeMounts:
            - mountPath: /mnt/secrets
              name: secret-store-grafana
            - mountPath: /mnt/sso-secrets/
              name: secret-store-grafana-sso
            - mountPath: /etc/grafana/provisioning/plugins/
              name: plugin-config
            - mountPath: /etc/grafana/
              name: grafana-ini
            - mountPath: /var/lib/grafana
              name: grafana-data
            - mountPath: /var/log/grafana
              name: grafana-logs
      dnsPolicy: ClusterFirst
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext:
        fsGroup: 10001
        seccompProfile:
          type: RuntimeDefault
      serviceAccount: secrets-csi-sa-monitoring
      serviceAccountName: secrets-csi-sa-monitoring
      terminationGracePeriodSeconds: 30
      volumes:
        - configMap:
            defaultMode: 420
            name: grafana-ini
          name: grafana-ini
        - emptyDir: {}
          name: grafana-logs
        - name: grafana-data
          persistentVolumeClaim:
            claimName: grafana-pvc
        - configMap:
            defaultMode: 420
            name: grafana-oncall-plugin-config
          name: plugin-config
        - hostPath:
            path: /etc/grafana/provisioning/plugins
            type: DirectoryOrCreate
          name: plugin-folder
        - csi:
            driver: secrets-store.csi.k8s.io
            readOnly: true
            volumeAttributes:
              secretProviderClass: spc-grafana
          name: secret-store-grafana
        - csi:
            driver: secrets-store.csi.k8s.io
            readOnly: true
            volumeAttributes:
              secretProviderClass: spc-grafana-sso
          name: secret-store-grafana-sso

Full Pod manifest; (resulting manifest deployed by Operator)

apiVersion: v1
kind: Pod
metadata:
  creationTimestamp: '2024-03-11T14:41:22Z'
  generateName: grafana-deployment-7bcdb6d464-
  labels:
    app: grafana
    pod-template-hash: 7bcdb6d464
  name: grafana-deployment-7bcdb6d464-pdgdt
  namespace: monitoring
  ownerReferences:
    - apiVersion: apps/v1
      blockOwnerDeletion: true
      controller: true
      kind: ReplicaSet
      name: grafana-deployment-7bcdb6d464
      uid: cd7e44e8-2337-4b70-93c7-310ce84f5490
  resourceVersion: '90847622'
  uid: 4564c1bf-a682-466c-9cb9-2767ca0c7dad
spec:
  containers:
    - args:
        - '-config=/etc/grafana/grafana.ini'
      env:
        - name: GF_AUTH_GOOGLE_CLIENT_ID
          valueFrom:
            secretKeyRef:
              key: client_id
              name: grafana-google-sso
        - name: GF_AUTH_GOOGLE_CLIENT_SECRET
          valueFrom:
            secretKeyRef:
              key: client_secret
              name: grafana-google-sso
        - name: PLUGINS_HASH
          valueFrom:
            configMapKeyRef:
              key: PLUGINS_HASH
              name: grafana-plugins
              optional: true
        - name: CONFIG_HASH
          value: a5f231ba1d21e0391bebb602b1d9f4066be7b5a625ffc17c5a9396e121888feb
        - name: GF_INSTALL_PLUGINS
          value: grafana-oncall-app
        - name: TMPDIR
          value: /var/lib/grafana
        - name: GF_SECURITY_ADMIN_USER
          valueFrom:
            secretKeyRef:
              key: adminuser
              name: grafana-admin-creds
        - name: GF_SECURITY_ADMIN_PASSWORD
          valueFrom:
            secretKeyRef:
              key: adminpassword
              name: grafana-admin-creds
        - name: AWS_STS_REGIONAL_ENDPOINTS
          value: regional
        - name: AWS_DEFAULT_REGION
          value: eu-central-1
        - name: AWS_REGION
          value: eu-central-1
        - name: AWS_ROLE_ARN
          value: >-
           <REDACTED ROLE ARN>
        - name: AWS_WEB_IDENTITY_TOKEN_FILE
          value: /var/run/secrets/eks.amazonaws.com/serviceaccount/token
      image: 'grafana/grafana:10.4.0'
      imagePullPolicy: IfNotPresent
      name: grafana
      ports:
        - containerPort: 3000
          name: grafana-http
          protocol: TCP
      readinessProbe:
        failureThreshold: 3
        httpGet:
          path: /api/health
          port: 3000
          scheme: HTTP
        initialDelaySeconds: 5
        periodSeconds: 10
        successThreshold: 1
        timeoutSeconds: 3
      resources:
        limits:
          memory: 256Mi
        requests:
          cpu: 64m
          memory: 256Mi
      securityContext:
        allowPrivilegeEscalation: true
        capabilities:
          drop:
            - ALL
        privileged: false
        readOnlyRootFilesystem: false
        runAsGroup: 10001
        runAsNonRoot: true
        runAsUser: 10001
      terminationMessagePath: /dev/termination-log
      terminationMessagePolicy: File
      volumeMounts:
        - mountPath: /mnt/secrets
          name: secret-store-grafana
        - mountPath: /mnt/sso-secrets/
          name: secret-store-grafana-sso
        - mountPath: /etc/grafana/provisioning/plugins/
          name: plugin-config
        - mountPath: /etc/grafana/
          name: grafana-ini
        - mountPath: /var/lib/grafana
          name: grafana-data
        - mountPath: /var/log/grafana
          name: grafana-logs
        - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
          name: kube-api-access-szvnd
          readOnly: true
        - mountPath: /var/run/secrets/eks.amazonaws.com/serviceaccount
          name: aws-iam-token
          readOnly: true
  dnsPolicy: ClusterFirst
  enableServiceLinks: true
  nodeName: <REDACTED NODE NAME>
  preemptionPolicy: PreemptLowerPriority
  priority: 0
  restartPolicy: Always
  schedulerName: default-scheduler
  securityContext:
    fsGroup: 10001
    seccompProfile:
      type: RuntimeDefault
  serviceAccount: secrets-csi-sa-monitoring
  serviceAccountName: secrets-csi-sa-monitoring
  terminationGracePeriodSeconds: 30
  tolerations:
    - effect: NoExecute
      key: node.kubernetes.io/not-ready
      operator: Exists
      tolerationSeconds: 300
    - effect: NoExecute
      key: node.kubernetes.io/unreachable
      operator: Exists
      tolerationSeconds: 300
  volumes:
    - name: aws-iam-token
      projected:
        defaultMode: 420
        sources:
          - serviceAccountToken:
              audience: sts.amazonaws.com
              expirationSeconds: 86400
              path: token
    - configMap:
        defaultMode: 420
        name: grafana-ini
      name: grafana-ini
    - emptyDir: {}
      name: grafana-logs
    - name: grafana-data
      persistentVolumeClaim:
        claimName: grafana-pvc
    - configMap:
        defaultMode: 420
        name: grafana-oncall-plugin-config
      name: plugin-config
    - hostPath:
        path: /etc/grafana/provisioning/plugins
        type: DirectoryOrCreate
      name: plugin-folder
    - csi:
        driver: secrets-store.csi.k8s.io
        readOnly: true
        volumeAttributes:
          secretProviderClass: spc-grafana
      name: secret-store-grafana
    - csi:
        driver: secrets-store.csi.k8s.io
        readOnly: true
        volumeAttributes:
          secretProviderClass: spc-grafana-sso
      name: secret-store-grafana-sso
    - name: kube-api-access-szvnd
      projected:
        defaultMode: 420
        sources:
          - serviceAccountToken:
              expirationSeconds: 3607
              path: token
          - configMap:
              items:
                - key: ca.crt
                  path: ca.crt
              name: kube-root-ca.crt
          - downwardAPI:
              items:
                - fieldRef:
                    apiVersion: v1
                    fieldPath: metadata.namespace
                  path: namespace

Full PersistentVolumeClaim manifest.

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  annotations:
    pv.kubernetes.io/bind-completed: 'yes'
    pv.kubernetes.io/bound-by-controller: 'yes'
    volume.beta.kubernetes.io/storage-provisioner: ebs.csi.aws.com
    volume.kubernetes.io/selected-node: <NODE NAME REDACTED>
    volume.kubernetes.io/storage-provisioner: ebs.csi.aws.com
  creationTimestamp: '2024-03-11T14:41:22Z'
  finalizers:
    - kubernetes.io/pvc-protection
  name: grafana-pvc
  namespace: monitoring
  ownerReferences:
    - apiVersion: grafana.integreatly.org/v1beta1
      kind: Grafana
      name: grafana
      uid: 67785905-eb4c-4922-a68a-fe4821aebfb0
  resourceVersion: '90847469'
  uid: 0a718f39-5eb2-4a23-9d29-b630be8539f6
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 10Gi
  storageClassName: gp2
  volumeMode: Filesystem
  volumeName: pvc-0a718f39-5eb2-4a23-9d29-b630be8539f6
status:
  accessModes:
    - ReadWriteOnce
  capacity:
    storage: 10Gi
  phase: Bound

any update on this one?

Was there any answer provided after relevant informations were posted?
I'm also faced with the same issue on a brand new Grafana deployment on GKE.

The operator does not do anything special with regard to persistent volumes. You need to ensure the PVC has the correct permissions for the Grafana container writing to it. The default Grafana container runs as uid=472(grafana) gid=0(root) groups=0(root) so make sure the PVC is utilizing the same UIDs/GIDs.