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

Upload results to results branch #152

Merged

Conversation

paulzierep
Copy link
Collaborator

@paulzierep paulzierep commented Jul 11, 2024

This is the initial CI set up to upload the results to a dedicated results branch, instead of the main branch.

  • No more merge conflicts
  • Clean separation between code and results
  • We can protect the main branch and use the CI

We need:
A good name for that branch -> results OK ?

All data generated by the CI must go into the results folder.

@nsoranzo for the filter and merge steps we need the functions from the main branch but also the content from results branch, do you have good idea to set that up in the CI ? Checkout main, then checkout only results folder?

@paulzierep paulzierep marked this pull request as ready for review July 11, 2024 14:09
Comment on lines 80 to 83
name: checkout results
- run: |
git checkout results -- results

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: checkout results
- run: |
git checkout results -- results
- name: Checkout results
uses: actions/checkout@v4
with:
path: results
ref: results

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that, but the output would be that the results folder is a subfolder of results as path specifies a Relative path under $GITHUB_WORKSPACE to place the repository. This below works, but is not so elegant I feel. But according to actions/checkout#1430 that seems to be the way atm.

    steps:
    - name: Checkout main
      uses: actions/checkout@v4
    - name: Checkout results
      uses: actions/checkout@v4
      with:
        ref: results
        sparse-checkout: results
        path: .tmp
    - name: Move results to root #https://github.com/actions/checkout/issues/1430
      run: |
          mv tmp/results results &&
          rm .tmp

Choose a reason for hiding this comment

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

Why do you want to use a sparse checkout in the first place?

I thought the result files would all be on the results branch. If you put the result files on that branch directly, and not under some sub-directory, then the result files should be checked out as /results, I'd expect. Am I missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nope I was missing something, I was assuming a subfolder, but it is indeed as you describe, so that should work, will adapt the code for it

@paulzierep
Copy link
Collaborator Author

I think this could work, but we need to:

  1. Add a results branch
  2. Remove the results folder from main, ones the CI works
  3. Fix remote paths that depend on the results

@paulzierep
Copy link
Collaborator Author

I did set up the branch structure as planned in my fork:
main: https://github.com/paulzierep/galaxy_tool_extractor/tree/main
results: https://github.com/paulzierep/galaxy_tool_extractor/tree/results

The test CI worked for the tools (tutorials fails due to plausible API key).

@kostrykin
Copy link

results: https://github.com/paulzierep/galaxy_tool_extractor/tree/results

I think you should set up the results branch as an orphan, so that it won't be cluttered with files from main branch (https://github.com/paulzierep/galaxy_tool_extractor/tree/results/.github/workflows and others) or previous commits.

@paulzierep
Copy link
Collaborator Author

results: https://github.com/paulzierep/galaxy_tool_extractor/tree/results

I think you should set up the results branch as an orphan, so that it won't be cluttered with files from main branch (https://github.com/paulzierep/galaxy_tool_extractor/tree/results/.github/workflows and others) or previous commits.

Ah nice, thanks for pointing that out !

@paulzierep
Copy link
Collaborator Author

I added some additional fixes such as:

  • more available_public_servers.csv to results (since the CI only pushed to this branch from now on !)
  • fixed the filter function for repositories where no tool with the category is found (this is the case for SPOC atm)
  • The CI is using only python 3.11 now (there was an issue with owlready2 and python 3.8)
  • Made sure the CI is always checking out the results branch

I am currently running the fetch tool and fetch tutorials CI on my fork and will make a PR once that succeeds.
Then we can merge the results and this one simultaneously.

@paulzierep
Copy link
Collaborator Author

@paulzierep
Copy link
Collaborator Author

Interesting, it always takes the test repo: https://github.com/paulzierep/galaxy_tool_extractor/actions/runs/9992041760/job/27616120191
Some more fixing required

@paulzierep
Copy link
Collaborator Author

Interesting, it always takes the test repo: https://github.com/paulzierep/galaxy_tool_extractor/actions/runs/9992041760/job/27616120191 Some more fixing required

Fixed with: 3d47a94

@paulzierep
Copy link
Collaborator Author

paulzierep commented Jul 25, 2024

With the fixes its working on my fork:
Example: https://github.com/paulzierep/galaxy_tool_extractor/tree/results

@paulzierep
Copy link
Collaborator Author

Added the up-to-date results here. I think we can merge.

@bebatut bebatut merged commit 671ce33 into galaxyproject:main Jul 29, 2024
2 checks passed
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.

4 participants