Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ID as fallback for developer name #1150

Merged
merged 1 commit into from
Apr 9, 2022

Conversation

zbynek
Copy link
Contributor

@zbynek zbynek commented Apr 9, 2022

Once again https://reports.jenkins.io/maintainers-info-report.json became corrupted, breaking the build of plugin website. This PR should make it more robust.

(The data seems correct again, still worth merging this I think)

@dduportal
Copy link
Contributor

@zbynek could it be related to jenkins-infra/helpdesk#2789 (thanks @lemeurherve for linking tasks!)?

We currently have the reports generated on both infra.ci (in the linked PR) and trusted.ci (actual "production") until we merge the linked PR + disable the job on trusted.

cc @daniel-beck for async. information

@zbynek
Copy link
Contributor Author

zbynek commented Apr 9, 2022

@dduportal the problem was that the JSON was prepended with one line that started with jq, unfortunately I didn't save the corrupted file. Though jq is the tool used for generating it, I'm not sure how could it print that line into the output file.

@dduportal
Copy link
Contributor

Thanks for the fix and the pointer! I guess I'm responsible for the error though: https://github.com/jenkins-infra/infra-reports/pull/37/files#diff-008848931d3412aba076fd9e8e5953e00a8b40486e87b0b4c6992e3035dd7a61R14

Note sure to catch "why did it print" the jq string though.

I would want to proceed on this migration and then iterate on fixing: let's see monday

@NotMyFault
Copy link
Member

NotMyFault commented Apr 9, 2022

Once the migration from trusted to infra is completed, are there plans to restore the original behavior to display the name of the maintainer instead of the ldap username again? This is a huge backstep in terms of recognisability.

@dduportal
Copy link
Contributor

I've closed jenkins-infra/infra-reports#37 temporarly to avoid messing up the generation of reports.

Expect things to be fixed in ~4 hours (time to have a full run i trusted.ci + RPU).

Sorry for the inconvenience, but the infra team have no knowledge on "what is inside this script / Json / report " and we need help to have this working as expected.

Proposal plan for monday:

  • Reopen the PR with the following changes:
    • If builds are not on the principal banch, then change the reports name to avoid overriding existing reports (wetehr they are on infra.ci or trusted)
    • rollback the "jq" change to keep using the jq 1.5 version downloaded on each actual run (instead of the image alpine jq)

I'll need help from you folks, to help me validate the infra.ci generated reports before merging PR and finish migration

@lemeurherve
Copy link
Member

We could generate the reports from infra (in another location) and on trusted simultaneously then compare their content.

@daniel-beck
Copy link
Contributor

Once the migration from trusted to infra is completed, are there plans to restore the original behavior to display the name of the maintainer instead of the ldap username again? This is a huge backstep in terms of recognisability.

It's already restored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants