[JENKINS-72030] Add checkbox to enable/disable Avatar retrieval

[Dohbedoh]

Description

Proposing a field configuration to enable / disable the Avatar action fetch that can be causing issues because of the anonymous call. Even with recent addition of a cache, this can still be a problem.
See JENKINS-72030 for further information.

I thought it would be a better to disable it by default after seeing recent discussions in #653 (comment).

Submitter checklist

• Link to JIRA ticket in description, if appropriate.
• Change is code complete and matches issue description
• Automated tests have been added to exercise the changes
• Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

• Run the changes and verify that the change matches the issue description
• Reviewed the code
• Verified that the appropriate tests have been written or valid explanation given

Documentation changes

• Link to jenkins.io PR, or an explanation for why no doc changes are needed

Users/aliases to notify

@jglick @bitwiseman @rsandell

[jtnord]

this will change behaviour for everyone - including those where it works today. Why opt in vs opt out where it causes issues?

The linked conversation does not seem relevant here as we will be backing off in the presence of a rate limit? otherwise the Jenkins ticket would not exist as the page would not hang - just failback or produce a broken icon..

[jglick]

Sorry, what does this have to do with #653? IIUC it does at least partly follow my suggestion in #700 (comment).

[Dohbedoh]

Allright, maybe I misunderstood the point in the discussion as anonymous calls were mentioned in the discussion. Will change the default to enable the fetch of the avatar.

[jglick]

change the default to enable the fetch of the avatar

To be clear, #738 (comment) meant switching the default to “do not fetch” for new org folders, and just retaining the fetch for existing folders unless the box is unchecked. #738 (review) is ambiguous

change behaviour for everyone - including those where it works today

[jglick]

Looks OK, but have not tried it.

[jtnord]

I think this should be isEnableAvatar ? (and then a NIT that the field should not be an action (enable x) but so isAvatarEnabled and avatarEnabled)

[jtnord]

do we actually even need this?
is there an issue that we are attempting to fetch the icon even when the setting has been set to not care?

I configured a GH org and toggled between the folder icon and metata icon - in the folder case I get a folder and in the other case I get my avatar (was using my user for testing) - so I do not beleive we should introduce another way to configure this but rather honour the original setting and not attempt to grab the avatar if it is not to be used. (if we already do this)

[Dohbedoh]

is there an issue that we are attempting to fetch the icon even when the setting has been set to not care?

@jtnord Yes this is something I mentioned in https://issues.jenkins.io/browse/JENKINS-72030?focusedId=441495&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-441495.
We don't have a reference to the folder itself to retrieve the icon setting. I guess it'd be possible by casting the SCMNavigatorOwner to an AbstractFolder ?

[jtnord]

is there an issue that we are attempting to fetch the icon even when the setting has been set to not care?

@jtnord Yes this is something I mentioned in https://issues.jenkins.io/browse/JENKINS-72030?focusedId=441495&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-441495. We don't have a reference to the folder itself to retrieve the icon setting. I guess it'd be possible by casting the SCMNavigatorOwner to an AbstractFolder ?

not sure on the specific base class you would need if there is a common one for this setting but yes that is the idea. There is the MultiBranchProject and OrganisationFolder according to the descriptor, not sure exactly where the property is stored.

[jglick]

Have not followed details, but my offhand opinion is that the value of the avatar on the org folder icon is low (just something implemented in the early days of multibranch as a neat demo feature) and if there are any field reports of performance problems as a result it should just be ripped out unless and until someone has the time to reimplement it better. Not worth spending years debating the best approach. It also seems inappropriate to expect a user to uncheck an obscure option to avoid known performance problems; if something is potentially expensive, they should have to opt into it with informed consent about the implications.

[Dohbedoh]

SCM API implementation actually don't have a requirement for a dependency on cloudbees-folders-plugin, only branch-api implementations have.. Does not look like a good idea to check on the actual Icon configuration here. Maybe the logic for fetching avatar should be within the AvatarMetadataAction itself, as mentioned in MetadataActionFolderIcon... Bitbucket Branch Source does this but with some issues as well: jenkinsci/bitbucket-branch-source-plugin#700.

It also seems inappropriate to expect a user to uncheck an obscure option to avoid known performance problems; if something is potentially expensive, they should have to opt into it with informed consent about the implications.

The proposed option is opted out by default and the help message mentions the implication.