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

Reusable workflow #24

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Reusable workflow #24

merged 5 commits into from
Apr 16, 2024

Conversation

severinbratus
Copy link
Contributor

@severinbratus severinbratus commented Apr 14, 2024

  1. Make the core workflow reusable (callable)
    Introduce a caller workflow pointing to the callable workflow file in the TeachBooks template (at the main branch).
    This is the reason the pipeline is failing, since it is attempting to call a workflow that is not there.

  2. Log the "cleaned" name in the form:

feat/dummy -> feat-dummy
main -> main
second -> second

This happens in job get-branches, step set-branches. This may however be somewhat inconvenient for the users.
We could create another job, or list these in the 404 page. EDIT: this is to be resolved in another PR

Closes #20

@sheepmax
Copy link

So essentially we just have people reference the template instead of make a copy of it?

@Tom-van-Woudenberg
Copy link
Member

Tom-van-Woudenberg commented Apr 15, 2024

So essentially we just have people reference the template instead of make a copy of it?

Copy the template as a template and/or referencing the GitHub actions yml

Copy link
Member

@Tom-van-Woudenberg Tom-van-Woudenberg left a comment

Choose a reason for hiding this comment

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

I tested it in the sandbox (with adapted branch), but the deploy-books steps seems to fail: https://github.com/TeachBooks/Sandbox/actions/runs/8684188935/job/23811416749. @severinbratus could you look into that?

Two futher small things below, but you can merge it without those and treat those as issues seperately

Another major thing:
I think it's better to include this workflow in a repo on its own. The template repository is used not only for providing the github workflow, but also with some book-configurations ready. Or do you have other ideas about this @severinbratus? If you agree, you can delete the deploy-book-ghpages.yml here, add it in a seperate repo in TeachBooks and adapt the reference URL

About the logging, it's a bit hidden in the steps. Would it be possible to have it displayed somewhere on the main page of an action (a page like this; https://github.com/TeachBooks/Sandbox/actions/runs/8685977352) Might this be a solution: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary. That seems useful: https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/. You could even include a nice representation of the configuration variables so that it's perfectly clear how it is deployed. This can also be treated as a seperate issue later on

.github/workflows/call-deploy-book.yml Outdated Show resolved Hide resolved
.github/workflows/call-deploy-book.yml Outdated Show resolved Hide resolved
@severinbratus
Copy link
Contributor Author

I tested it in the sandbox (with adapted branch), but the deploy-books steps seems to fail: https://github.com/TeachBooks/Sandbox/actions/runs/8684188935/job/23811416749. @severinbratus could you look into that?

Will do.

Another major thing: I think it's better to include this workflow in a repo on its own. The template repository is used not only for providing the github workflow, but also with some book-configurations ready. Or do you have other ideas about this @severinbratus? If you agree, you can delete the deploy-book-ghpages.yml here, add it in a seperate repo in TeachBooks and adapt the reference URL

I have thought of this, and now I agree -- this would be less confusing.

And a final thing, it's not clear to me what 'logging' now means, where is it visible?

I meant the GitHub Actions workflow log, e.g. here under Set branches https://github.com/TeachBooks/Sandbox/actions/runs/8684188935/job/23811352889 I think the way of making a 404 listing available book versions is better than making the users search the names up in the logs, so what I have done is just a first step.

@Tom-van-Woudenberg
Copy link
Member

Tom-van-Woudenberg commented Apr 15, 2024

And a final thing, it's not clear to me what 'logging' now means, where is it visible?

I meant the GitHub Actions workflow log, e.g. here under Set branches https://github.com/TeachBooks/Sandbox/actions/runs/8684188935/job/23811352889 I think the way of making a 404 listing available book versions is better than making the users search the names up in the logs, so what I have done is just a first step.

@severinbratus , I've updated my response on the logging, please see the original message now after a refresh of this page ;)

@severinbratus severinbratus changed the title Reusable workflow Draft: Reusable workflow Apr 15, 2024
@severinbratus severinbratus marked this pull request as draft April 15, 2024 09:12
@severinbratus
Copy link
Contributor Author

I agree, summaries seem great for this. I have updated issue TeachBooks/deploy-book-workflow#1 to specify it

@severinbratus
Copy link
Contributor Author

severinbratus commented Apr 15, 2024

@Tom-van-Woudenberg @rlanzafame The pipeline is failing now on the upload-pages-artifact action, as is the pipeline at Sandbox. So I strongly suspect that this is because of our use of symlinks for aliasing. It turns out the upload-pages-artifact disallows symbolic links, even though https://github.blog/changelog/2023-02-21-github-pages-deprecating-symlinks-in-non-actions-builds/ implied it was OK.

What this means for us, is I believe we will have to drop the aliasing feature, since symlinks work unpredictably now, and the people at GitHub in fact do not want them to work for security reasons (source: actions/upload-pages-artifact#51 (comment))

@Tom-van-Woudenberg
Copy link
Member

Tom-van-Woudenberg commented Apr 15, 2024

@Tom-van-Woudenberg @rlanzafame The pipeline is failing now on the upload-pages-artifact action, as is the pipeline at Sandbox. So I strongly suspect that this is because of our use of symlinks for aliasing. It turns out the upload-pages-artifact disallows symbolic links, even though https://github.blog/changelog/2023-02-21-github-pages-deprecating-symlinks-in-non-actions-builds/ implied it was OK.

What this means for us, is I believe we will have to drop the aliasing feature, since symlinks work unpredictably now, and the people at GitHub in fact do not want them to work for security reasons (source: actions/upload-pages-artifact#51 (comment))

@severinbratus , but it worked before right? Why doesn't it work now? Here it works fine: https://github.com/TeachBooks/Sandbox_temp

@severinbratus
Copy link
Contributor Author

@severinbratus , but it worked before right? Why doesn't it work now? Here it works fine: https://github.com/TeachBooks/Sandbox_temp

For now not able to say what the reason for this inconsistency is, but I suppose it's bugs on GitHub's side. Will debug further later this week

@Tom-van-Woudenberg
Copy link
Member

@severinbratus , but it worked before right? Why doesn't it work now? Here it works fine: https://github.com/TeachBooks/Sandbox_temp

For now not able to say what the reason for this inconsistency is, but I suppose it's bugs on GitHub's side. Will debug further later this week

Could it be related to that the artifacts are not available because of the separation of those workflows? In sandbox_temp is didn't user the call workflow, just deploy. While in https://github.com/TeachBooks/Sandbox/actions/workflows/call-deploy-book.yml the call workflow never succeeded.

@severinbratus
Copy link
Contributor Author

severinbratus commented Apr 16, 2024

OK, I am sorry for the confusion - this was a bug in my code, fixed now. Beware however, that the upload-github-action README says (https://github.com/actions/upload-pages-artifact) no symlinks are allowed in the pages artifact, but somehow it works for us (not complaining). However if upload-github-action breaking becomes an issue down the road, we know where it is probably coming from

@severinbratus severinbratus marked this pull request as ready for review April 16, 2024 17:29
@severinbratus severinbratus changed the title Draft: Reusable workflow Reusable workflow Apr 16, 2024
@Tom-van-Woudenberg Tom-van-Woudenberg merged commit dba9e89 into main Apr 16, 2024
4 checks passed
@Tom-van-Woudenberg Tom-van-Woudenberg deleted the feat/17/reusable-workflow branch April 16, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Github actions as reusable workflow
3 participants