gci_no_issue_tasks.py: Add Django command - #218
Open
Man-Jain wants to merge 1 commit into
Open
Conversation
gitmate-bot
reviewed
Oct 30, 2018
gitmate-bot
reviewed
Oct 30, 2018
gitmate-bot
reviewed
Oct 30, 2018
gitmate-bot
reviewed
Oct 30, 2018
gitmate-bot
reviewed
Oct 30, 2018
gitmate-bot
reviewed
Oct 30, 2018
gitmate-bot
reviewed
Oct 30, 2018
Travis tests have failedHey @Man-Jain, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: a971b920-dc35-11e8-8e0f-4de0fcdedaae |
Man-Jain
force-pushed
the
importing-yaml-way
branch
from
October 30, 2018 11:51
eceb208 to
fb94da0
Compare
gitmate-bot
reviewed
Oct 30, 2018
gitmate-bot
reviewed
Oct 30, 2018
Travis tests have failedHey @Man-Jain, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: 289f4d70-dc3b-11e8-8e0f-4de0fcdedaae |
jayvdb
requested changes
Oct 30, 2018
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = 'Fetch GCI data' |
|
|
||
| if [[ -n "$GCI_TOKEN" ]]; then | ||
| python manage.py fetch_gci_task_data private | ||
| python manage.py gci_no_issue_tasks private _site |
Member
There was a problem hiding this comment.
this should be run after the cleanse, otherwise it risks publishing private information.
| tasks = yaml.load(f) | ||
|
|
||
| for task in tasks: | ||
| if not tasks[task]['external_url']: |
Member
Member
Author
There was a problem hiding this comment.
Omg! I misinterpreted the whole issue. So sorry 😞
Man-Jain
commented
Oct 30, 2018
Man-Jain
force-pushed
the
importing-yaml-way
branch
from
October 31, 2018 04:37
fb94da0 to
a1dc082
Compare
li-boxuan
requested changes
Oct 31, 2018
| from ruamel.yaml import YAML | ||
| from gci.gitorg import get_issue | ||
|
|
||
| from django.core.management.base import BaseCommand |
Member
There was a problem hiding this comment.
Can you rearrange the import a bit? Put django import into the second group and gci import to the third group.
Standard library imports
Third-party imports
Application-specific imports
| def handle(self, *args, **options): | ||
| output_dir = options.get('output_dir') | ||
|
|
||
| tasks_issues = {} |
Member
There was a problem hiding this comment.
The name here is a bit confusing.
| if [[ -n "$GCI_TOKEN" ]]; then | ||
| python manage.py fetch_gci_task_data private | ||
| python manage.py cleanse_gci_task_data private _site | ||
| python manage.py gci_no_issue_tasks private _site |
Member
There was a problem hiding this comment.
Maybe better name (add a verb)? extract_gci_non_issue_tasks
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = 'Filter GCI tasks without issues' |
Member
There was a problem hiding this comment.
Can be more clear. e.g. Extract non-issue tasks from GCI tasks.yaml and dump into non_issue_tasks.yaml
Member
There was a problem hiding this comment.
Maybe the file name is not worth mentioning here, but at least elaborate a bit more.
| if not issue: | ||
| tasks_issues[tasks[task]] = tasks[task] | ||
|
|
||
| with open(os.path.join(output_dir, 'tasks_no_issue.yaml'), 'w') as nf: |
Member
There was a problem hiding this comment.
I personally feel non_issue_tasks might be better
Member
There was a problem hiding this comment.
Maybe also change the file name ^^
Member
|
Hmmm, https://deploy-preview-218--coala-community.netlify.com/static/tasks_no_issue.yaml does not exist |
Member
Author
That's what I am trying to figure out since I pushed last time. Can you tell why it doesn't exits? |
Member
|
@Man-Jain Debug locally 😉 You can print it out locally and see if that's correct. |
Man-Jain
force-pushed
the
importing-yaml-way
branch
from
November 1, 2018 07:51
cab0248 to
9e94902
Compare
gitmate-bot
reviewed
Nov 1, 2018
| tasks = yaml.load(f) | ||
|
|
||
| for task in tasks: | ||
| external_url=tasks[task]['external_url'] |
Collaborator
There was a problem hiding this comment.
The code does not comply to PEP8.
Origin: PEP8Bear, Section: all.python.default.
The issue can be fixed by applying the following patch:
--- a/tmp/tmp5joox2ay/gci/management/commands/filter_gci_non_issue_tasks.py
+++ b/tmp/tmp5joox2ay/gci/management/commands/filter_gci_non_issue_tasks.py
@@ -25,7 +25,7 @@
tasks = yaml.load(f)
for task in tasks:
- external_url=tasks[task]['external_url']
+ external_url = tasks[task]['external_url']
print('external issue for task is {}'.format(external_url))
try:
issue = get_issue(external_url)
gitmate-bot
reviewed
Nov 1, 2018
| tasks = yaml.load(f) | ||
|
|
||
| for task in tasks: | ||
| external_url=tasks[task]['external_url'] |
Collaborator
There was a problem hiding this comment.
E225 missing whitespace around operator
Origin: PycodestyleBear (E225), Section: all.python.default.
Travis tests have failedHey @Man-Jain, 1st Buildcoala --non-interactive -VTravisBuddy Request Identifier: 26b41690-ddac-11e8-9e24-43be9eb2178e |
Member
|
all CI is red. pycodestyle errors. |
Member
Author
|
I ran coala for like 3 times before pushing. It gave no errors. I'll have to do it in some other system maybe. |
Man-Jain
force-pushed
the
importing-yaml-way
branch
from
November 5, 2018 07:01
9e94902 to
39aa320
Compare
Travis tests have failedHey @Man-Jain, 1st BuildpytestTravisBuddy Request Identifier: fcea76c0-e0ca-11e8-90db-29214a2b47d8 |
jayvdb
requested changes
Nov 6, 2018
| print('task {} has a valid issue link'.format(task)) | ||
|
|
||
| except Exception: | ||
| print('tasks.yaml file not found') |
Member
There was a problem hiding this comment.
So if the file exists, but isnt valid yaml, you are going to emit error message "tasks.yaml file not found" on stdout?
We have a function which loads the tasks.yaml already
Man-Jain
force-pushed
the
importing-yaml-way
branch
from
November 12, 2018 10:20
39aa320 to
a516de7
Compare
Travis tests have failedHey @Man-Jain, 1st Build./.ci/build.shTravisBuddy Request Identifier: c5fbd0a0-e665-11e8-9705-413b2685d188 |
Man-Jain
force-pushed
the
importing-yaml-way
branch
from
November 12, 2018 12:51
a516de7 to
aa2275f
Compare
Adds a new command to the gci app which filters out tasks which don't have an issue associated with them. Closes coala#212
Man-Jain
force-pushed
the
importing-yaml-way
branch
from
November 12, 2018 12:57
aa2275f to
4d641e4
Compare
Travis tests have failedHey @Man-Jain, 1st Build./.ci/build.shTravisBuddy Request Identifier: a7b53770-e67a-11e8-9705-413b2685d188 |
Travis tests have failedHey @Man-Jain, 1st Build./.ci/build.shTravisBuddy Request Identifier: 81b7c370-e67b-11e8-9705-413b2685d188 |
li-boxuan
requested changes
Nov 20, 2018
li-boxuan
left a comment
li-boxuan
left a comment
Member
There was a problem hiding this comment.
I've tested it locally and seems it works. Let's improve it a bit 👍
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = 'Extract the tasks which dont have an issue associated with them' |
|
|
||
| for task in tasks: | ||
| external_url = tasks[task]['external_url'] | ||
| print('external issue for task is {}'.format(external_url)) |
Member
There was a problem hiding this comment.
external issue -> external url.
The word issue here is confusing, or even wrong.
Member
There was a problem hiding this comment.
Currently, link is logged first before task info is logged, e.g.
external issue for task is https://github.com/coala/gci-leaders/issues/30
task 6567626035691520 has a valid issue link
I think user experience would be better if log looks like this:
task 6567626035691520 has a valid issue link, external url is https://github.com/coala/gci-leaders/issues/30
i.e. put link and task info on the same line, and put task info before link url.
| else: | ||
| print('task {} has a valid issue link'.format(task)) | ||
| except Exception: | ||
| print('task {} has no external_url'.format(task)) |
Member
There was a problem hiding this comment.
From your log, seems that this exception is for the situation that a task has no external url. Then the try block is at a wrong position. external_url = tasks[task]['external_url'] would cause an error already.
Unless this exception is trying to catch something else, probably with issue = get_issue(external_url)
Contributor
|
@Man-Jain Also, please re-base your branch 😇 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new command to the gci app which filters out tasks which don't
have an issue associated with them.
Closes #212