Skip to content

Conversation

@waheedahmed
Copy link
Contributor

@symbolist @adampalay Send to grader django management command for effected students.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be if len(args) > 2, since you're using args[0], args[1], args[2]?

Copy link
Contributor

Choose a reason for hiding this comment

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

is it ever possible that self.task_states is None or []?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is done wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

How would we get this list of students?

Copy link
Contributor

Choose a reason for hiding this comment

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

@feanil what's the best way to do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess I was wondering what the criteria is for this list. Any query run to get a list of affected students externally should probably just be run here in code instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

the problem is is that we have to join over queries from LMS and the ORA dbs, and I don't know how to do that with the django orm

Copy link
Contributor

Choose a reason for hiding this comment

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

So that is what I was originally wondering about, what is an example query that would result in the list needed for this command? Is this query documented somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for removing the hashbang, but why remove the module comment? A docstring at the top would be great.

@adampalay adampalay closed this Dec 13, 2013
@adampalay
Copy link
Contributor

I'm going to close this PR for now since we found another way to fix the bug this addresses.

Copy link
Contributor

Choose a reason for hiding this comment

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

This function isn't properly named: it returns a list of the first column in the csv. Will this be used with csv files with more than one column?

jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 23, 2017
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.

7 participants