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 github action of transifex upload #3958

Merged
merged 2 commits into from
Apr 24, 2020

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Apr 22, 2020

Migrate transifex integration from travis.ci to github action. @csadorf

I am not sure whether TRANSIFEX_USER and TRANSIFEX_PASSWORD set previously still available. You may need to set them if not.

@codecov
Copy link

codecov bot commented Apr 22, 2020

Codecov Report

Merging #3958 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3958   +/-   ##
========================================
  Coverage    78.31%   78.31%           
========================================
  Files          461      461           
  Lines        34076    34076           
========================================
  Hits         26687    26687           
  Misses        7389     7389           
Flag Coverage Δ
#django 70.36% <ø> (ø)
#sqlalchemy 71.21% <ø> (ø)

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 e8e7e46...341327e. Read the comment docs.

@csadorf csadorf self-requested a review April 23, 2020 07:20
Copy link
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

This looks fine to me. I just have two requests to add some comments to provide a little bit more context.

.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
@unkcpz
Copy link
Member Author

unkcpz commented Apr 23, 2020

There seems no way to manually trigger the action. So have to wait until next release to make sure this workflow is working properly. 🤞

I trigger it in a hack way and it failed, maybe since the secret keys are not set.

@unkcpz unkcpz force-pushed the transifex-action branch from 29dc84b to 8cdc32a Compare April 23, 2020 08:37
@unkcpz unkcpz marked this pull request as draft April 23, 2020 08:43
@unkcpz unkcpz force-pushed the transifex-action branch 2 times, most recently from a07ac06 to 11eacfd Compare April 23, 2020 09:03
@unkcpz
Copy link
Member Author

unkcpz commented Apr 23, 2020

I recall that the TRANSIFEX_USER shall be set as api and TRANSIFEX_PASSWORD is the token generated from transifex. fyi, https://docs.transifex.com/api/introduction#authentication

And API token: https://www.transifex.com/user/settings/api/

As a reference, the context of my transifexrc file is:

[https://www.transifex.com]
api_hostname = https://api.transifex.com
hostname = https://www.transifex.com
password = **** [redacted]
username = api

Edit: I redacted the password. csa

@unkcpz unkcpz marked this pull request as ready for review April 23, 2020 09:10
@unkcpz unkcpz requested a review from csadorf April 23, 2020 09:10
@csadorf
Copy link
Contributor

csadorf commented Apr 23, 2020

I recall that the TRANSIFEX_USER shall be set as api and TRANSIFEX_PASSWORD is the token generated from transifex. fyi, https://docs.transifex.com/api/introduction#authentication

And API token: https://www.transifex.com/user/settings/api/

As a reference, the context of my transifexrc file is:

[https://www.transifex.com]
api_hostname = https://api.transifex.com
hostname = https://www.transifex.com
password = ****
username = api

@unkcpz Did you post the actual credentials? In this case, please reset this password immediately and send the new password to my personal email. Thank you.

Edit: In either case, please send the actual credentials to my personal email, thank you.

@unkcpz
Copy link
Member Author

unkcpz commented Apr 23, 2020

@csadorf no is just a example, I don't have the privilege to log in aiida account in transifex. @ltalirz has the actual credentials.

@csadorf
Copy link
Contributor

csadorf commented Apr 23, 2020

I trigger it in a hack way and it failed, maybe since the secret keys are not set.

@unkcpz It might be a good idea for us to debug this workflow before we merge it by simply running it temporarily on push. It would only be executed for this branch in this case. We would then remove that trigger prior to merge after we have verified that it works. What do you think?

@ltalirz Can you verify that the secrets have been added to the repository?

@ltalirz
Copy link
Member

ltalirz commented Apr 23, 2020

Sorry guys, I've added them now (user: api, password: the api token).
Please try again

Copy link
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

Please try to run the workflow on push and then re-request review. Thx!

@unkcpz unkcpz force-pushed the transifex-action branch 4 times, most recently from ab527f1 to ee241ba Compare April 23, 2020 16:29
@unkcpz
Copy link
Member Author

unkcpz commented Apr 23, 2020

It is strange. I can successfully upload the pot to my account check here (and the workflow file with only project name different), but with all similar processes, still failed when setting and uploading to aiida account.

I got: tx ERROR: EOF when reading a line . This is most likely caused by incorrectly setting configuration file.

@ltalirz Can you reconfirm you add and set secrets:

TRANSIFEX_PASSWORD -> token from transifex
TRANSIFEX_USER -> api

Thanks!

@ltalirz
Copy link
Member

ltalirz commented Apr 23, 2020

Hi @unkcpz , sorry about this, I believe the token I had stored was missing a few characters.
I've generated a new one and updated the github secrets.

I've also changed your status in the aiida docs to project maintainer in case it is useful at any point.

@unkcpz unkcpz force-pushed the transifex-action branch from ee241ba to 1a2285c Compare April 23, 2020 17:14
@unkcpz
Copy link
Member Author

unkcpz commented Apr 23, 2020

emmm... unfortunately, same error here.

Maybe, it is not allowed to using secrets keys in pull request actions? It cause anyone can access and use the secret token by just making a pull request, which is unsafe. In this case, could you help to make a push for test?

I've also changed your status in the aiida docs to project maintainer in case it is useful at any point.

Thanks! 😄

@unkcpz unkcpz force-pushed the transifex-action branch from 1a2285c to 0aaa72d Compare April 23, 2020 17:39
@csadorf
Copy link
Contributor

csadorf commented Apr 23, 2020

Maybe, it is not allowed to using secrets keys in pull request actions? It cause anyone can access and use the secret token by just making a pull request, which is unsafe. In this case, could you help to make a push for test?

Ah sorry that I missed that. The secret is indeed not available for pull requests. We will need to push directly to the repository. I'll do that.

@csadorf
Copy link
Contributor

csadorf commented Apr 23, 2020

@unkcpz Just pushed to ci-test-transifex-action branch, action running here: https://github.com/aiidateam/aiida-core/actions/runs/86005424

@ltalirz
Copy link
Member

ltalirz commented Apr 23, 2020

Seems to have worked, right?

@unkcpz unkcpz force-pushed the transifex-action branch from 0aaa72d to b2af3f3 Compare April 24, 2020 02:11
@unkcpz
Copy link
Member Author

unkcpz commented Apr 24, 2020

It works now. Cheers! And change trigger event back to release, ready to be merged.

Thank you guys! 😃

@unkcpz unkcpz requested a review from csadorf April 24, 2020 02:14
@unkcpz
Copy link
Member Author

unkcpz commented Apr 24, 2020

However, I concerned if release event can successfully run the action as push? I didn't find information about which event is able to use secrets. @csadorf

@csadorf
Copy link
Contributor

csadorf commented Apr 24, 2020

However, I concerned if release event can successfully run the action as push? I didn't find information about which event is able to use secrets. @csadorf

The release event is going to trigger the workflow with the appropriate permissions. So don't worry about that.

@csadorf csadorf merged commit 62e2972 into aiidateam:develop Apr 24, 2020
@unkcpz unkcpz deleted the transifex-action branch April 24, 2020 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants