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

first test to refactor action, will need likely updates #36

Merged
merged 3 commits into from
Mar 21, 2020

Conversation

vsoch
Copy link
Collaborator

@vsoch vsoch commented Mar 21, 2020

This will be a first shot at having the urlchecker-action use the urlchecker-python repository. I haven't removed the docs folder from here (but we probably should) as it's now with urlchecker-python. I'll likely need to update this PR as I test (and it fails) - note that I'm testing the action locally with GitHub actions like:

name: Main testing workflow
on: [pull_request]

jobs:
  run:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout Actions Repository
      uses: actions/checkout@v2
    - name: Test GitHub Action
      uses: ./
      with: 
        git_path: https://github.com/urlstechie/URLs-checker-test-repo
        branch: devel
        subfolder: docs
        file_types: .md,.py,.rst
        timeout: 5
        retry_count: 3
        white_listed_urls: https://github.com/SuperKogito/URLs-checker/issues/1,https://github.com/SuperKogito/URLs-checker/issues/2
        white_listed_patterns: https://github.com/SuperKogito/Voice-based-gender-recognition/issues
        white_listed_files: README.md,/github/workspace/_config.yml
        force_pass : true

Signed-off-by: vsoch vsochat@stanford.edu

@vsoch vsoch force-pushed the update/action-with-urlchecker-python branch from 7175090 to 6ac9c21 Compare March 21, 2020 21:09
@vsoch
Copy link
Collaborator Author

vsoch commented Mar 21, 2020

okay just hit a bug in the urlchecker-python - we need to enforce the retry count goes in as an int.

 URLs-checker-test-repo/test_files/sample_test_file.py 
 -----------------------------------------------------
0
Traceback (most recent call last):
  File "/usr/local/bin/urlchecker", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.8/site-packages/urlchecker/client/__init__.py", line 178, in main
    main(args=args, extra=extra)
  File "/usr/local/lib/python3.8/site-packages/urlchecker/client/check.py", line 72, in main
    check_results = run_urlchecker(path=path,
  File "/usr/local/lib/python3.8/site-packages/urlchecker/core/check.py", line 53, in run_urlchecker
    return check_files(
  File "/usr/local/lib/python3.8/site-packages/urlchecker/core/check.py", line 109, in check_files
    urlproc.check_urls(file_name, urls, check_results, retry_count, timeout)
  File "/usr/local/lib/python3.8/site-packages/urlchecker/core/urlproc.py", line 153, in check_urls
    while rcount > 0 and do_retry:
TypeError: '>' not supported between instances of 'str' and 'int'

Taking a look now.

@vsoch vsoch force-pushed the update/action-with-urlchecker-python branch from 84a28c1 to e87090b Compare March 21, 2020 21:44
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch vsoch force-pushed the update/action-with-urlchecker-python branch from e87090b to 3c58658 Compare March 21, 2020 21:47
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Collaborator Author

vsoch commented Mar 21, 2020

@SuperKogito we are good! What is basically happening here is that we use GitHub actions to run a test of the action (per the recipe I linked above). So - my suggestions for moving forward:

  • merge the PR into master, and do your local tests / perhaps another PR to add more cases for testing the GitHub action?
  • another PR to remove the docs from here (they are provided with the urlchecker-python, and maybe we need an automated build?)
  • is codacy/PR review necessary for this PR? For example, it's flagging pip install urlchecker as a bug (and wanting a version) but it would be more of a hassle to need to update the version here every time there is a release. If we remove the check, we can just do that.

What you can do is to copy paste additional sections like this in the main.yml if you want more tests. Do you want me to move forward with the automated container build for urlchecker-python? I can work on that next.

@vsoch
Copy link
Collaborator Author

vsoch commented Mar 21, 2020

Oh and I almost forgot - @SuperKogito after we merge here, I've updated the links in the README to be for the repository name urlchecker-action so we should change that accordingly. I know of two repos that are using the action, so I'll open PRs asap to update them. Do you know of any other users of the action? I can offer to do a PR to their repos to update as well - let me know!

@vsoch vsoch requested a review from SuperKogito March 21, 2020 22:00
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch vsoch merged commit c5b1887 into master Mar 21, 2020
@vsoch
Copy link
Collaborator Author

vsoch commented Mar 21, 2020

@SuperKogito I can't change the name, so it's all you! I suppose the same will be the case for urlchecker-test-repo.

@SuperKogito
Copy link
Member

I can change the action name on the repos that I am using as for the others they will have to refer to the GitHub urlstechie account and they should figure out the changes :) I don't see a better option at this point.

@SuperKogito
Copy link
Member

names changed 🚀

@vsoch
Copy link
Collaborator Author

vsoch commented Mar 21, 2020

okay sounds good - if you do think of any that aren't your own, let me know and I can PR. I've already updated usrse and have a buildtest PR underway to do the same.

@SuperKogito
Copy link
Member

I am not sure tbh, sadly GitHub still does not provide any stats or tracking of repos using a certain action. I know it tracks python packages but not sure if that will help us in this case :/

@vsoch
Copy link
Collaborator Author

vsoch commented Mar 21, 2020

Yeah I think it's okay too :) If someone's action breaks (which might not even be the case because GitHub handles redirects for a bit) they can come here to open an issue and oh! Look the name changed, problem solved! :)

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.

2 participants