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

Add artifacts with EDM4hep files #353

Merged
merged 7 commits into from
Sep 2, 2024
Merged

Add artifacts with EDM4hep files #353

merged 7 commits into from
Sep 2, 2024

Conversation

jmcarcell
Copy link
Contributor

@jmcarcell jmcarcell commented Aug 27, 2024

BEGINRELEASENOTES

The artifacts have been added in this file https://github.com/key4hep/key4hep-actions/blob/main/key4hep-build/action.yml that will have to be changed once the new names here are used. With the last runs there are already some files (TTree being uploaded): https://github.com/key4hep/EDM4hep/actions/runs/10575395035. Since the Key4hep build workflow runs every day, this means new files every day with the latest nightlies (this is why I say latest scheduled is preferred since pull requests will also generate these files).

Tagging @peremato

Comment on lines +145 to +148
Example EDM4hep files can be obtained from the Continuous Integration (CI)
workflows. From the EDM4hep github page, go to Actions -> Key4hep build, click
one of the runs (the latest scheduled is preferred) and they will appear at the
bottom, under Artifacts.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these publicly visible in the end?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even without github account
screenshot_2024-08-28_1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice :) Thanks for checking.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. Thanks!

@m-fila
Copy link
Contributor

m-fila commented Aug 28, 2024

Do I understand correctly that with matrix of systems and build types each job will upload an artifact and overwrite the artifact uploaded by other jobs in that batch? So the artifact that'll be available at the end would come from the job that finished last?

Also what is the retention policy for artifacts?

@tmadlener
Copy link
Contributor

tmadlener commented Aug 28, 2024

Do I understand correctly that with matrix of systems and build types each job will upload an artifact and overwrite the artifact uploaded by other jobs in that batch? So the artifact that'll be available at the end would come from the job that finished last?

Not entirely sure. I seem to remember that some workflows also fail if they try to overwrite artifacts from another matrix job. But that might be configurable.

The retention policy can be configured as well. I think the default is 90 days.
edit: anything between 1 and 90 days seems possible

@jmcarcell
Copy link
Contributor Author

Do I understand correctly that with matrix of systems and build types each job will upload an artifact and overwrite the artifact uploaded by other jobs in that batch? So the artifact that'll be available at the end would come from the job that finished last?

Yes, that is what would happen. This could be changed, but since we don't expect differences for different OSes or compilers if we wanted to check if they are different this should be done somewhere else.

@peremato
Copy link

peremato commented Sep 2, 2024

To be more complete. It would be nice to create files with both TTree and RNTuple.

@jmcarcell
Copy link
Contributor Author

jmcarcell commented Sep 2, 2024

To be more complete. It would be nice to create files with both TTree and RNTuple.

Yes, this is what will happen once this PR is merged. If there aren't any more comments I'll merge this today.

@tmadlener tmadlener enabled auto-merge (squash) September 2, 2024 09:45
@tmadlener tmadlener merged commit c5a7462 into main Sep 2, 2024
13 of 15 checks passed
@tmadlener tmadlener deleted the artifacts branch September 2, 2024 09:50
@jmcarcell
Copy link
Contributor Author

See https://github.com/key4hep/EDM4hep/actions/runs/10664951162 as an example, there will be a zip with a TTree file and a RNTuple file.

@peremato
Copy link

peremato commented Sep 2, 2024

Sorry @jmcarcell. Another problem.

        state.d0 = next(counter)
        state.z0 = next(counter)

These two statements have no effect. See the TBrowser output:
Screenshot 2024-09-02 at 15 37 52

@m-fila
Copy link
Contributor

m-fila commented Sep 2, 2024

Looks like another capitalization typo, should be state.D0, state.Z0

@jmcarcell
Copy link
Contributor Author

Yeah, unfortunately using EDM4hep from Python has several caveats like it's possible to set any attribute even if it doesn't actually exist.

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