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

Service to download and serve a PR artifact #1754

Merged
merged 5 commits into from
Jun 22, 2020
Merged

Conversation

andrevmatos
Copy link
Contributor

@andrevmatos andrevmatos commented Jun 19, 2020

Fixes #1746 , #1749 and #1511

Short description
Provides a service to serve PR builds locally.

To test:
Create a .env file on the serve_pr folder, containing CIRCLE_TOKEN=<your_token>.

Also, create a serve_pr/app folder and be sure it has write and execution permissions for user:group nobody, as nginx's process will try to run script and write to this folder as such.

Then, after running docker-compose up, one should be able to go to https://localhost:8080/?branch=pull/1754 or any other PR/branch and get it downloaded and served on https://localhost:8080/pull/1754 . Notice the branch used on the first request needs to be the upstream branch: for PRs opened from external repo, it'll be pull/<#PR>; for PRs opened from some upstream branch, it'll be the actual branch.
To "refresh" the served files (e.g. after a new successful build), access the first (root+param) URL again.

One can also use the script directly, like:

./ci_dapp.sh branch=pull/1754 -s

The -s param will cause the build to be served on https://localhost:8080/ with a python -m http.server.

A README was added as per @kelsos request.

Definition of Done

  • Steps to manually test the change have been documented
  • Acceptance criteria are met
  • Change has been manually tested by the reviewer (dApp)

Steps to manually test the change (dApp)

@andrevmatos andrevmatos added dApp 📱 infrastructure 🚧 Tests, CI, and general project infrastructure labels Jun 19, 2020
@andrevmatos andrevmatos self-assigned this Jun 19, 2020
@andrevmatos andrevmatos force-pushed the feature/test_pr branch 3 times, most recently from 5734a09 to 89084bd Compare June 19, 2020 18:16
@codecov
Copy link

codecov bot commented Jun 19, 2020

Codecov Report

Merging #1754 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1754   +/-   ##
=======================================
  Coverage   95.30%   95.30%           
=======================================
  Files         148      148           
  Lines        5328     5328           
  Branches      964     1019   +55     
=======================================
  Hits         5078     5078           
  Misses        203      203           
  Partials       47       47           
Flag Coverage Δ
#dapp 91.48% <ø> (ø)
#sdk 96.85% <ø> (ø)
Impacted Files Coverage Δ
raiden-dapp/src/router/index.ts 0.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5208c9f...ba71d9c. Read the comment docs.

@github-actions
Copy link

You modified raiden-dapp/src,
Please remember to add a change log entry at raiden-dapp/CHANGELOG.md if necessary.

@kelsos
Copy link
Contributor

kelsos commented Jun 22, 2020

Is anything special needed in regard to the personal token to make it work?

For some reason I am getting no response from the API

curl -H Circle-Token:*** 'https://circleci.com/api/v2/project/github/raiden-network/light-client/pipeline?branch=pull/1754'
{
  "next_page_token" : null,
  "items" : [ ]
}%        

@kelsos
Copy link
Contributor

kelsos commented Jun 22, 2020

ok basically pull/1754 won't work since it is on the upstream repo and not on a fork

@kelsos
Copy link
Contributor

kelsos commented Jun 22, 2020

Also it seems that when running docker-compose up the ci_dapp.sh copied to the app folder is empty.

@kelsos
Copy link
Contributor

kelsos commented Jun 22, 2020

I tried quite a few times but things don't seem to work properly for me. Out of all the tries fetching worked once, most of the time it fails with Script error: closed. I have no idea why, in some cases, the proper folder is generated in the app folder but other than that nothing seems to happen.

In the one case where it finally worked, I could see no redirect to root. Could you describe how to replicate the redirect step by step? or provide a video of it happening if that is easier?

@andrevmatos
Copy link
Contributor Author

andrevmatos commented Jun 22, 2020

Is anything special needed in regard to the personal token to make it work?

For some reason I am getting no response from the API

curl -H Circle-Token:*** 'https://circleci.com/api/v2/project/github/raiden-network/light-client/pipeline?branch=pull/1754'
{
  "next_page_token" : null,
  "items" : [ ]
}%        

This doesn't work because there's no such branch pull/1754 (it was just an example). Usually, there is when you PR from an external repo. As one of the paragraphs I wrote on the PR, this branch name is for PRs from outer branches, while on own branches (as is the case with my PR), the branch name is the actual source one. On this case, it'd be branch=feature/test_pr, it should work as well.

Also it seems that when running docker-compose up the ci_dapp.sh copied to the app folder is empty.

No, it's not. If you look into the app folder from the host, you'll see an empty file holding the place for the mounted file on the container, but if you look into the container, you'll see the full script mounted there.

Out of all the tries fetching worked once, most of the time it fails with Script error: closed.

This is a script error. Could be permissions or no artifact on given branch/PR. You can check the last script logs by running in parallel with the running compose (another terminal, same folder) something like: docker-compose exec dapp cat /tmp/log.txt

@andrevmatos
Copy link
Contributor Author

In the one case where it finally worked, I could see no redirect to root. Could you describe how to replicate the redirect step by step? or provide a video of it happening if that is easier?

I fixed the redirect thing already on this PR, by moving router to hash mode when on development. Hash mode doesn't meddle with root as history mode, and works fine for relatively served dapp, and it actually doesn't look bad IMO.

@kelsos
Copy link
Contributor

kelsos commented Jun 22, 2020

I am gonna approve the PR but I have a last request. Can we also add a basic readme
It would be nice to have:

  • A small description of the steps you need to take (e.g. .env creation/ folder creation and permissions)
  • Linking to the documentation about getting the CIRCLE_TOKEN

@andrevmatos andrevmatos marked this pull request as ready for review June 22, 2020 17:57
@andrevmatos andrevmatos merged commit 300a654 into master Jun 22, 2020
@andrevmatos andrevmatos deleted the feature/test_pr branch June 22, 2020 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dApp 📱 infrastructure 🚧 Tests, CI, and general project infrastructure
Projects
None yet
2 participants