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.

Sign up for GitHub

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

Jump to bottom

feat: support agent tokens #2

Merged
merged 9 commits into from
Feb 25, 2021
Merged

feat: support agent tokens #2

merged 9 commits into from
Feb 25, 2021

Conversation

martinm82
Copy link
Contributor

First of all, thank you so much for this fantastic Python client for the QuickBuild REST API. The code is so clean and it was a pleasure to implement this feature.

About this PR: It is introducing support for the tokens endpoint which one can use to authorize/unauthorize build agents.

@martinm82
Copy link
Contributor Author

martinm82 commented Feb 9, 2021 •
edited
Loading

@pbelskiy the builds are currently failing for PRs coming from forks. This is because of some issue in coveralls and accessing GitHub secrets.
As far as I have seen there is no solution for coveralls. One workaround would be to move to codecov which supports tokenless. See codecov/codecov-action#29 (comment). Another one would be to skip coverage on PRs from forks but I don't believe this is the right way or check https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/. Seems that there might be a setting to make coveralls work with tweaking some settings.

pbelskiy
pbelskiy approved these changes Feb 9, 2021
View reviewed changes
Copy link
Owner

@pbelskiy pbelskiy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for PR! Looks good to me except few things I commented.

Also please add to docs/index.rst

Tokens
~~~~~~

.. autoclass:: quickbuild.endpoints.tokens.Tokens
    :members:

@pbelskiy
Copy link
Owner

pbelskiy commented Feb 10, 2021 •
edited
Loading

It's okay now, but your new tests fails on CI, you also can test it locally using tox, also please don't forget to rebase before submit, because yesterday I've added audits endpoints

@pbelskiy pbelskiy self-requested a review February 17, 2021 09:00
@pbelskiy pbelskiy merged commit 7428403 into pbelskiy:master Feb 25, 2021
@pbelskiy
Copy link
Owner

@martinm82 released under 0.4.0 version

@martinm82
Copy link
Contributor Author

@pbelskiy I think one of the tests was still failing. Sorry, I was recently fully occupied with work and could not check further on this PR

@pbelskiy
Copy link
Owner

@martinm82 Yes, but I've fixed it yesterday

@martinm82 martinm82 deleted the feature/tokens branch March 14, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pbelskiy pbelskiy pbelskiy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@martinm82 @pbelskiy