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

PR1576 - Updated - Add Captcha to ps and status #1597

Closed
wants to merge 10 commits into from

Conversation

gabaod
Copy link
Contributor

@gabaod gabaod commented Nov 29, 2016

I have merged pr 1578 login delay and added captcha count by worker to status and ps outputs

Description

Sorry I have no clue how to rebase etc when develop was updated, so I recreated it from the latest develop branch

Motivation and Context

helps the user see captcha use

How Has This Been Tested?

personal

Screenshots (if appropriate):

captchasdrf

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x ] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Copy link
Member

@FrostTheFox FrostTheFox left a comment

Choose a reason for hiding this comment

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

Well, I was looking to merge this, but I don't like that some file permissions have been changed unnecessarily, if you could fix that that'd be nice :3

@gabaod
Copy link
Contributor Author

gabaod commented Nov 30, 2016

yea i had issues trying to get sourcetree access to that shared drive. Ill see if i can figure out which ones all changed. gotta be a simple diff cmd to figure that out ;x also do note if this is merged, might have issues with some of the other prs, 1501,1530 etc with the db_schema going up in dev.

@gabaod
Copy link
Contributor Author

gabaod commented Nov 30, 2016

ok I think file perms and all should be fixed.

@gabaod
Copy link
Contributor Author

gabaod commented Dec 1, 2016

is that correct now? think it finally went through on git.

@rndx539
Copy link
Contributor

rndx539 commented Dec 1, 2016

file changes 626 seems something wrong?
and why the diff replace the whole file and not just some code?

@gabaod
Copy link
Contributor Author

gabaod commented Dec 1, 2016

I had my dir structure at chmod644, and the dev branch is 755 so I had to go through and update all files for the file permissions only in the last commit. If that was not the right method to fix his requested changes than I need FrostTheFox to inform me.

@fihzy
Copy link

fihzy commented Dec 1, 2016

I noticed one file had ^M characters at the end, windows to unix conversion issue

@st0nec0ld
Copy link

It is possible to add a link for solve captcha, similar to PokeAlert app?

@Thyllz
Copy link

Thyllz commented Dec 2, 2016

I just pulled this to try it out, and I got error message:

"C:...\PokemonGoMap\PokemonGo-Map>runserver.py -os -fl -l "lon, lat" -nsc
Traceback (most recent call last):
File "C:...\PokemonGoMap\PokemonGo-Map\runserver.py", line 25, in
from pogom.search import search_overseer_thread
File "C:...\PokemonGoMap\PokemonGo-Map\pogom\search.py", line 543
<<<<<<< HEAD
^
IndentationError: unexpected unindent"

This error might be obvious to some people but I don't know what is causing it. Any ideas?

@simonbuehler
Copy link
Contributor

simonbuehler commented Dec 2, 2016

@Thyllz not specific to this PR, you have unresolved merge conflicts, resolve them using e.g. gitextensions or commandline

works good for me, don't forget to run

npm install
npm run build

edit: @gabaod better save your changes away and apply them on a clean checkout so no 600 files do get changed, then rebase

@Thyllz
Copy link

Thyllz commented Dec 2, 2016

@simonbuehler Thanks for the info. I started over with a clean clone to try out a few PR's. Seems to be working fine now.

@Alderon86
Copy link
Contributor

Could u please fix the compatibility to 1501? i just cant get it to work, too many conflicts

@gabaod
Copy link
Contributor Author

gabaod commented Dec 3, 2016

@lastrolo what file was that?

@Alderon86 https://github.com/gabaod/PokemonGo-Map/tree/test1501 first backup the original files then replace the pogom/models.py and pogom/search.py and static/js/status.js files and run this query, alter table workerstatus add captchas int default 0; dont forget to npm run build after replacing files ;)

@Alderon86
Copy link
Contributor

Alderon86 commented Dec 3, 2016

nvm gave up on it

@divnotded
Copy link

@gabaod Thanks for the integration to 1501 instructions followed your advice works like a charm. Running 1501 with 1580, 1591 and 1597 Dramatic decrease captchas since the holiday event spike I saw. Needed a better way to monitor thank you for this

@tomballgithub
Copy link
Contributor

This check-in is a mess with 626 changed files. However, the PR does work great with manual changes made to models.py, search.py, and status.js. It is very convenient to see the statistics both individually per worked and at the bottom in aggregate.

@gabaod
Copy link
Contributor Author

gabaod commented Dec 4, 2016

Yea I agree. Youre welcome to redo it for me.. It was ready to be merged but I guess I did a chmod +x at somepoint and pushed it to my github which changed some perms and fox told me to fix it so I just removed the s bit through git on the full dir structure and was never reinformed if that was improper or not. For the time being most folks are using 1501, so Ive been just keeping this updated https://github.com/PokemonGoMap/PokemonGo-Map/pull/1597#issuecomment-264610982 instead, obviously not the proper way to issue someone to do a pr, but it works as prs arent really meant to be for other prs. By all means go ahead and make a new pr with how this was supposed to be for dev, but hopefulyl 1501 is optimized first and merged which can easily include this as its redesigning the status output and changing mysql already.

@onilton onilton mentioned this pull request Dec 5, 2016
5 tasks
@onilton
Copy link
Contributor

onilton commented Dec 5, 2016

Hi I really liked the PR so I have cleaned it up a little bit and opened another pull request: #1604 . Also I have removed #1576 to keep the PR to a single thing.

@gabaod
Copy link
Contributor Author

gabaod commented Dec 5, 2016

Closing this due to above comment. plus messy file structure.

@gabaod gabaod closed this Dec 5, 2016
@tomballgithub
Copy link
Contributor

@gabaod Thank you for creating this PR. It's fantastic...

@springjools
Copy link
Contributor

I don't like this at all. It just messes with the output.

@gabaod gabaod deleted the pr1576 branch December 17, 2016 23:30
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.

None yet