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

Adding GitPod button to nf-core usage tutorial #1094

Closed
wants to merge 22 commits into from
Closed

Adding GitPod button to nf-core usage tutorial #1094

wants to merge 22 commits into from

Conversation

lescai
Copy link
Contributor

@lescai lescai commented Mar 17, 2022

This PR simply adds a link that spins a GitPod environment, which automatically opens up the nf-co.re tutorial page on top of a terminal.
The environment is using the nf-core/gitpod image, and is configure to run nf-core tools and nextflow.
The button works if the branch gitpod_config_training in nf-co.re exists, since it has to store the correct config files.

relates to #68

Copy link
Member

@ggabernet ggabernet left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @lescai !

@ggabernet ggabernet self-requested a review March 18, 2022 11:07
Copy link
Member

@ggabernet ggabernet left a comment

Choose a reason for hiding this comment

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

Actually testing this out, the GitPod space has on the left menu all files part of the nf-core website. Would it be possible to get a "clean" GitPod space for the training?

@lescai
Copy link
Contributor Author

lescai commented Mar 18, 2022

in order for that to be possible, we should create a new nf-core repository instead of a config branch.
the option was not favoured by the other people I discussed this initially, but I don't have any strong feelings about either solution, so I'm happy to implement what most people agree on :)
I think a training repo (we could call it gitpod_training) would also solve the problems @ewels highlighted about having sort of orphan branches, and would have the advantage of allowing multiple branches to be made to - for example - create configs that open different tutorial pages.
Naturally, the gitpod environment would still open that repo and never a completely empty one (this is the way gitpod is designed).
Any preference @mahesh-panchal ?

@mahesh-panchal
Copy link
Member

Thinking about it more, I like the idea of a nf-core training repository.
It would then be a bit more like https://training.galaxyproject.org/ perhaps then.
There would be a place for all the miscellaneous media like usage screenshots, exercise files, etc. ( and it doesn't clutter up the nf-core website repo either then).

One could probably start with a clean repo by running:

rm -r /workspace/*

but I think it would be nicer if we

mkdir -p /workspace/exercises
cd /workspace/exercises

and also open the training page in a preview panel (I don't think a learner should be looking at Markdown to start).
The explorer panel on the side though will always link to the repo. I wonder if there's a way to change that.
Hmm. But that also means we should use /workspace/nf-core-training/exercises rather than workspace/exercises so their files are viewable in the file explorer. And then open up Gitpod directly on a README in that folder with instructions. That way they can still see their files. If we want, we can also instruct how to use branches, so a learner could make a fork of the branch and have working solutions on a branch of their own fork.

@lescai
Copy link
Contributor Author

lescai commented Mar 18, 2022

got a reply from gitpod apparently it's possible to pass variables via URL
see here:
https://www.gitpod.io/docs/environment-variables#provide-env-vars-via-url
so I'll encode the page in the button URL and make the appropriate changes in the other PR.
then we'll still have to decide if we want to open a separate repo or not, but for the moment I'd suggest to go ahead with these 2 and then maybe give it a thought for the repo.

@lescai
Copy link
Contributor Author

lescai commented Mar 18, 2022

ok this is now working. since gitpod receives the URL of the repo, there's a conflict with passing environmental variables with that syntax containing url characters.
I had to come up with a substitution that I could then parse within the config commands.
this might not be the most elegant solution, but it works.
if there's any suggestion on different parsing or substitution, you're welcome to suggest in the other PR where the config is coded
#1093

@mahesh-panchal
Copy link
Member

Suggestion for working view.
bookmark: https://github.com/nf-core/nf-co.re/pull/1093/files#r830329267

@lescai
Copy link
Contributor Author

lescai commented Mar 19, 2022

Now that the config branch is merged, you can test the link here
https://github.com/lescai/nf-co.re/blob/master/markdown/usage/tutorials/nf_core_usage_tutorial.md
@ggabernet the config now checks out on the config branch, so that will remove most of the files and clean the left hand side as we discussed. this should address your request for change.
One thing that's not consistent is the welcome page: in my test the .vscode/settings.json would remove the welcome page and put the browser full width, but now it doesn't seem to do it anymore.
I'll have to address that on the config branch.
But technically this seems to work on my side now.

@lescai lescai requested a review from ggabernet March 19, 2022 06:58
@lescai
Copy link
Contributor Author

lescai commented Mar 19, 2022

and... the welcome page opens again because the checkout I introduced to address @ggabernet request.
this triggers a vscode reload, and that in turn reopens the welcome page despite the workspace settings I've introduced.
so I guess now it becomes a bit of a personal taste.
I don't mind having the full repo on the left hand side, but having the tutorial shown full width.
someone else would not mind closing the welcome page first, but having a cleaner repo on the left.

@ewels
Copy link
Member

ewels commented Mar 30, 2022

Just tried the link but I get an error in the Gitpod terminal:

$ gitpod /workspace/nf-co.re (gitpod_config_training) $  HISTFILE=/workspace/.gitpod/cmd-1 history -r; {
> parsed=`echo $url | tr -s "[" "/" | tr -s "+" "."`
> newurl="https://$parsed"
> gp preview $newurl
> 
> }
Unable to connect to VS Code server: Error in request.
Error: connect ECONNREFUSED 127.0.0.1:23000
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1146:16) {
  errno: -111,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '127.0.0.1',
  port: 23000
}

But basically I'm happy to merge this PR whenever as I guess the Gitpod image itself is kind of separate from the PR that adds the button for it.

@lescai
Copy link
Contributor Author

lescai commented Mar 30, 2022

I've noticed this happening in a few cases and it seems due to the "open" action being kicked off before the other actions have completed.
I have a solution in mind, just need to find an hour to test it :)
if this is not urgent, we can hold it for a few more days?

@ewels
Copy link
Member

ewels commented Mar 30, 2022

Sure 👍🏻

@ewels ewels added the WIP Work in progress label Mar 30, 2022
@lescai lescai mentioned this pull request Apr 4, 2022
@lescai
Copy link
Contributor Author

lescai commented Apr 4, 2022

@ewels I've tested a few things on the config (unfortunately another PR..)
the solution I had in mind didn't work, but I've added instead a sleep step which seems to allow the variable to be parsed before the browser is opened in preview.
I've run a few tests and it seems to be working on my side.
it is of course dependent on marging #1117

@ewels

This comment was marked as spam.

@ewels

This comment was marked as duplicate.

@ewels

This comment was marked as duplicate.

@ewels

This comment was marked as duplicate.

@ewels

This comment was marked as duplicate.

@edmundmiller

This comment was marked as duplicate.

@ewels
Copy link
Member

ewels commented Apr 13, 2022

@emiller88 the changes to the workflow need to be on master, not in this PR. The workflow in this PR doesn't run on new comments.

Comment on lines 16 to 17
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.ref }}
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't work, as the action runs on the issue comment being added and not the PR event. So there is no github.event.pull_request.

@ewels
Copy link
Member

ewels commented Apr 13, 2022

@emiller88 I removed your commits, hope that's ok 👍🏻

@ewels

This comment was marked as duplicate.

2 similar comments
@ewels

This comment was marked as duplicate.

@ewels
Copy link
Member

ewels commented Apr 13, 2022

@nf-core-bot fix linting

@ewels
Copy link
Member

ewels commented Apr 13, 2022

yes

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
No open projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

6 participants