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 CI / CD system to this repository #377

Open
aravindvnair99 opened this issue Feb 19, 2023 · 16 comments · Fixed by #381 or #384
Open

Add CI / CD system to this repository #377

aravindvnair99 opened this issue Feb 19, 2023 · 16 comments · Fixed by #381 or #384
Labels
Brainstorming Some New Feature to be added with prior proper discussion enhancement New feature or request help wanted Extra attention is needed

Comments

@aravindvnair99
Copy link
Collaborator

Objectives:

  • Spend less time reviewing PRs
  • Automatically show preview README with all options enabled based on changes for each PR
  • Having such a system can also help us queue PRs as necessary to merge one after the other
@aravindvnair99 aravindvnair99 added enhancement New feature or request help wanted Extra attention is needed Brainstorming Some New Feature to be added with prior proper discussion labels Feb 19, 2023
@pseusys
Copy link
Collaborator

pseusys commented Feb 19, 2023

Just to mention: manual reviewing changes shouldn't be a provlem now. All you have to do is:

  1. Copy .env.example to some other file, preferrably having .env extension so it will be ignored by git, let's say conf.env for example.
  2. Change INPUT_WAKATIME_API_KEY and INPUT_GH_TOKEN in your conf.env file to your WakaTime API token and GitHub API token respectively.
  3. Run make run-locally ENV=conf.env in the project root.
  4. Enjoy the changes directly in your GitHub profile.

@pseusys
Copy link
Collaborator

pseusys commented Feb 19, 2023

As this action requiers user GitHub API token, I think the only possible solution for testing would be be:
Put someone's GitHub API token as a repository secret and automatically run action for this user on every PR.
But this will be a great security hole: GitHub token for this action requires write permission, so anyone will be able to do literally anything with it.
I could've think of some dry run solution, but I'd suggest doing it after #371 - because it makes code navigation and understanding really easier. And also adds a codestyle to follow.

@aravindvnair99
Copy link
Collaborator Author

@pseusys We can create a dummy GitHub user and use that profile's secret. We don't have to use an actual individual's profile. If #371 is ready and tested, we can merge that and then come to the CI / CD system.

@pseusys
Copy link
Collaborator

pseusys commented Feb 19, 2023

We'd better add a kind of dry-run option that will be able to run without user password.
If you have some spare time, that would be great if you tested #371 locally once more.
I've tested it on my own, but I'd like to be sure.

@pseusys
Copy link
Collaborator

pseusys commented Feb 19, 2023

It would be also great if you could take a look at #375 as it appears to solve many issues and so it would be gread to merge it ASAP as well.

@anmol098
Copy link
Owner

anmol098 commented Feb 20, 2023

@aravindvnair99 @pseusys
This is the good feature which can be introduced to test the changes on PR.
Approach i can think of

We can introduce a new env variable flag for testing which will run the action with all options enabled and when our module is about to write readme on actual github repo with the env flag enabled the module will just print the output on console.
So this way we don't need github token with write permission only read permission can suffice
...
Other approach i can think of is we can mock all the external api calls and assert the output which we'll get at the end

Feel free to ask question
Thanks

@pseusys
Copy link
Collaborator

pseusys commented Feb 20, 2023

Yeah, I like the first solution. In addition we can make the output more verbose for this dry run.
I'll try to implement it, but I still suggest it after #371.

@pseusys
Copy link
Collaborator

pseusys commented Feb 22, 2023

@anmol098 so, we'll need a sample GitHub read token together with WakaTime read token in repository secrets.
I think, maybe we should use yours? Your profile seems to be very rich with data for the action testing!

@anmol098
Copy link
Owner

Ok perfect I'll add these 2 tokens to repository secret 👍

@pseusys
Copy link
Collaborator

pseusys commented Feb 22, 2023

@anmol098 great, keep me informed!

@anmol098
Copy link
Owner

@pseusys you can check my latest PR #384 feel free to make changes

@aravindvnair99 aravindvnair99 linked a pull request Feb 25, 2023 that will close this issue
5 tasks
@anmol098
Copy link
Owner

with last issue #398 for file not found exception. we need to make changes to the workflow how we test the action.
currently in ci.yml we just test the python code and send the output as a comment.

new approach could be build and publish the docker image with the tag as PR number then pull the docker image in that docker image run action with all flags enabled and pass the output as a comment.

@anmol098 anmol098 reopened this Feb 28, 2023
@pseusys
Copy link
Collaborator

pseusys commented Feb 28, 2023

Or better do both and compare the results because we shouldn't forget about local running possibility.

@pseusys
Copy link
Collaborator

pseusys commented Mar 1, 2023

I would also suggest publishing the Docker image for every pull request (named after the pull request, of course) for us and users to be able to test the action not on main branch only.
After the pull request is merge, the image can be deleted.

@aravindvnair99
Copy link
Collaborator Author

Considering we got #423 as well. We need to prioritise this. I agree with doing both, but primary should be GitHub action as that's what most people are using. I haven't come across a local user yet although we might have a few.

@pseusys
Copy link
Collaborator

pseusys commented Mar 13, 2023

I would suggest the following solution:

  1. Refactor build_image and ci workflows, combine them into build_and_test (or something).
  2. In this workflow create image building and publishing job.
  3. Create jobs for local and Docker (pulled from registry) debug running - then comparing outputs and publishing results in comments if they match.
  4. Optionally write some test jobs - and run them after debug running jobs.
  5. Optionally create another workflow, being triggered on branch deletion - for unpublishing Docker image associated with this branch from registry. This will never affect master as master never gets deleted.

Unfortunately, I won't be able to deal with it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Brainstorming Some New Feature to be added with prior proper discussion enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
3 participants