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

Move creating appeal table to a Laravel job #289

Merged
merged 13 commits into from
Feb 15, 2021
Merged

Move creating appeal table to a Laravel job #289

merged 13 commits into from
Feb 15, 2021

Conversation

supertassu
Copy link
Collaborator

closes #141

@supertassu supertassu added enhancement New feature or request Priority: Low labels Sep 11, 2020
@supertassu supertassu self-assigned this Sep 11, 2020
@pull-assistant
Copy link

Score: 1.00

Best reviewed: commit by commit

Optimal code review plan

     Move creating appeal table to a Laravel job

Powered by Pull Assistant. Last update 286f31a ... 286f31a. Read the comment docs.

@dqwiki dqwiki added the merge & testing issues When there have been changes merged that cause conflicts with the PR at hand label Oct 28, 2020
@supertassu supertassu removed the merge & testing issues When there have been changes merged that cause conflicts with the PR at hand label Oct 28, 2020
supertassu
supertassu commented Nov 18, 2020
View reviewed changes
app/Console/Kernel.php Outdated
// ->hourly();

// Wiki integration
$schedule->command('utrs-jobs:update-appeal-tables --wiki=enwiki')->everyThirtyMinutes();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure if this is ok or should it be more often

dqwiki
dqwiki requested changes Nov 18, 2020
View reviewed changes
Copy link
Member

@dqwiki dqwiki 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.

While I don't think I ever implemented cookie storage on the python task, we do need to be logging in as little as possible. The examples given for addwiki/mediawiki-api do not indicate that this is something that already exists.

https://github.com/addwiki/mediawiki-api#example-usage

See also:
https://phabricator.wikimedia.org/T256533
https://www.mediawiki.org/wiki/API:Login#Additional_notes
https://en.wikipedia.org/wiki/User_talk:AmandaNP#High_DeltaQuadBot_login_rate
Likely irrelevant python theory: mwclient/mwclient#256

@dqwiki
Copy link
Member

dqwiki commented Nov 18, 2020

I just reread #250 I'm assuming that solves the concern and has been merged?

@supertassu
Copy link
Collaborator Author

supertassu commented Nov 18, 2020 •
edited
Loading

I just reread #250 I'm assuming that solves the concern and has been merged?

@dqwiki If you're speaking about re-using sessions, nope, but I'm currently working on that. #250 does improve the situation so that UTRS will only log in when it needs to actually write something, otherwise it just uses the API anonymously

@supertassu
Copy link
Collaborator Author

see #347

dqwiki
dqwiki approved these changes Feb 15, 2021
View reviewed changes
Copy link
Member

@dqwiki dqwiki left a comment

Choose a reason for hiding this comment

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

Looks good from a code perspective.

@dqwiki dqwiki merged commit 27e9184 into master Feb 15, 2021
@dqwiki dqwiki deleted the laravel-job-appeallist branch February 15, 2021 03:31
@dqwiki dqwiki mentioned this pull request Feb 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dqwiki dqwiki dqwiki approved these changes

Assignees

@supertassu supertassu

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert appeal table updater to a Laravel Job
2 participants
@supertassu @dqwiki