gergelyszabo94 / csgo-trader-extension

CSGO Trader Browser Extension to help with CS:GO item trading, marketing and much more
https://csgotrader.app
GNU General Public License v3.0
213 stars 42 forks source link

refactor: backend (priceScraper) #389

Closed hexiro closed 3 years ago

hexiro commented 3 years ago

how would one go about bug testing this? as of now, I haven't tested this, so there probably are a couple lurking bugs.

hexiro commented 3 years ago

oh and what python version is this running?

hexiro commented 3 years ago

Can I replace "null" with python None (null when converted to json)? To do this the extension would need to be worked on to not look for these "null" (most likely)

hexiro commented 3 years ago

based on the first conditional on this line, it looks like the extension checks for both "null" and null, so I should be able to proceed with using the more pythonic None over a string "null" which is more tedious to check against (a lot of != "null"s)

https://github.com/gergelyszabo94/csgo-trader-extension/blob/master/extension/src/utils/pricing.js#L462

hexiro commented 3 years ago

Let me know what ya think!

gergelyszabo94 commented 3 years ago

I looked through the code, that is a lot of work, thanks! It's unfortunately hard to test, it runs in an AWS Lambda function (Python 3.7). I tried running it but it failed during price extraction at a key error:

[ERROR] KeyError: 'starting_at' Traceback (most recent call last):   File "/var/task/priceScraper.py", line 65, in lambda_handler     ) = request_priceempire(stage)   File "/var/task/priceScraper.py", line 356, in request_priceempire     item_buff163_prices["starting_at"]["price"] = get_formatted_float_divided_by_100(item_buff163_price)

hexiro commented 3 years ago

alright ill get that sorted. It still needs so more work (mainly replacing string nulls), but im not sure if that will break the backend.

hexiro commented 3 years ago

do you want me to standardize single quotes or double quotes?

gergelyszabo94 commented 3 years ago

Either if fine by me, not sure what is custom for Python, as you can see I am not good at it :)

hexiro commented 3 years ago

you're good enough to calculate your own pricing. I'd say that's pretty impressive

hexiro commented 3 years ago

I'm updating some stuff based on my previous python experiences. If there is a reason why you did something that I don't know about, and I change it let me know and I'll revert it.

hexiro commented 3 years ago

looks like csgobackpack prices don't get pushed to s3. Is this intentional?

gergelyszabo94 commented 3 years ago

Yes, they are basically SCM prices, they are used as another source to check against because steamapis can return weird prices sometimes.

hexiro commented 3 years ago

Yes, they are basically SCM prices, they are used as another source to check against because steamapis can return weird prices sometimes.

makes sense. In my personal steam project I use csgobackpack's api and i've never had issues.

hexiro commented 3 years ago

Can you run it again and see if there are any exceptions?

gergelyszabo94 commented 3 years ago

It gets into a loop on lootfarm then times out after 5 minutes (max allowed to run). image (same happens after the last commit too)

hexiro commented 3 years ago

oops

hexiro commented 3 years ago

The push to s3 was happening inside of the loop instead of when it finishes, but I'm not sure how that would make it time out. Maybe pushing to s3 is a costly function and because it was happening so much it took a lot of time?

hexiro commented 3 years ago

any reason you don't convert csgo tm from string to float?

image

gergelyszabo94 commented 3 years ago

It takes about a second to push the biggest file (with all providers), a few hundred ms for the individual providers. I don't think the csgotm thing is intentional.

hexiro commented 3 years ago

It takes about a second to push the biggest file (with all providers), a few hundred ms for the individual providers. I don't think the csgotm thing is intentional.

yeah then this would likely take more than 5 minutes because of that bug.

bitskins prices are also represented as strings. would you like me to convert them to floats?

hexiro commented 3 years ago

Okay whenever you're free I think it's ready for another test!

gergelyszabo94 commented 3 years ago

This time it failed quick: [ERROR] AttributeError: 'str' object has no attribute 'get' Traceback (most recent call last): File "/var/task/priceScraper.py", line 61, in lambda_handler csmoney_prices = fetch_csmoney(stage) File "/var/task/priceScraper.py", line 473, in fetch_csmoney name = item.get('m').replace("/", '-')

hexiro commented 3 years ago

I believe that should do it

gergelyszabo94 commented 3 years ago

[ERROR] TypeError: unsupported operand type(s) for /: 'NoneType' and 'int' Traceback (most recent call last):   File "/var/task/priceScraper.py", line 68, in lambda_handler     ) = fetch_priceempire(stage)   File "/var/task/priceScraper.py", line 384, in fetch_priceempire     "Emerald": get_formatted_float_divided_by_100(pricempire_prices.get("buff_emerald", {}).get("price")),   File "/var/task/priceScraper.py", line 802, in get_formatted_float_divided_by_100     return get_formatted_float(price / 100)

hexiro commented 3 years ago

I believe that should do it

hexiro commented 3 years ago

Do you think that instead of pushing all these different dictionary types to the s3, you could instead push like steam_last24h, bitskins_instant_sale, etc and make each response just name: price. I believe this would make it easier to work with on the extension side too, because you don't have to worry about all these different keys.

hexiro commented 3 years ago

so instead of bitskins.json you have bitskins.json and bitskins_instant_price.json and all of them are just marketHashName: price image

hexiro commented 3 years ago

that wouldn't be that difficult to do on the backend and it would work on the extension much easier imo

gergelyszabo94 commented 3 years ago

True, although I would not want to change a working thing now, people might be relying on it working the way it is. This is the latest error: { "errorMessage": "'Music Kit | Damjan Mravunac, The Talos Principal'", "errorType": "KeyError", "stackTrace": [ " File \"/var/task/priceScraper.py\", line 73, in lambda_handler\n csgotrader_prices = create_csgotrader_prices(buff163_prices, csgobackpack_prices, csmoney_prices, own_prices, stage, steam_prices)\n", " File \"/var/task/priceScraper.py\", line 192, in create_csgotrader_prices\n steam_aggregate = get_steam_price(item, steam_prices, week_to_day, month_to_week)\n", " File \"/var/task/priceScraper.py\", line 720, in get_steam_price\n item_prices = steam_prices[item]\n" ] }

hexiro commented 3 years ago

alright that once should be good now too. Sorry about all these pesky little errors, but I don't really have a way of bug testing this on my local machine.

hexiro commented 3 years ago

on my machine, the .idea/ folder wasn't getting ignored, but if this .gitignore has issues on your machine you can always revert the changes.

gergelyszabo94 commented 3 years ago

It ran! image You can access these at: https://prices.csgotrader.app/test/${provider}.json. You can also test it inside the extension this way by just changing "latest" to "test" on line 381 in pricing,js

hexiro commented 3 years ago

as of now, the only thing that was changed for the price providers, was changing string nulls "null" to None (null in json), But I could also parse the strings to floats.

hexiro commented 3 years ago

Hopefully nothing breaks on the extension side

hexiro commented 3 years ago

From my testing, It looks like everything works great on the extension side. Not sure if you found any issues.

hexiro commented 3 years ago

I actually did this on accident and didn't realize. So I can revert this if it makes a difference. image

gergelyszabo94 commented 3 years ago

I left for vacation, I can test it on Tuesday.

hexiro commented 3 years ago

yeah no pressure! have fun on your vacation!

gergelyszabo94 commented 3 years ago

I tested and merged it, thanks for the work once more!

hexiro commented 3 years ago

no problem! I'm glad it works! I hope I didn't break any of the logic for calculating prices, but if you notice something off let me know!

gergelyszabo94 commented 3 years ago

An issue has been reported (about missing buff buy order prices on some doppler phases). There was previously a logic that made the phase 3 price the buyorde price if there was none for the specific phase. I think this might not have survived the refactor. I will check it properly tomorrow.

hexiro commented 3 years ago

oops! I'll look into this too and create a pull request if i figure out the issue

hexiro commented 3 years ago

can you refer me to where this logic was?

hexiro commented 3 years ago

oh, and does the buy order price mean the starting at price?

gergelyszabo94 commented 3 years ago

No, starting at is the current lowest listing, highest order the the current highest buy order.

hexiro commented 3 years ago

ohh gotchaaa

gergelyszabo94 commented 3 years ago

Actually, that logic I mentioned is in the extension code in the GetPrice function, it does not work at the moment since it expect 'null' string but you changed that. I tihnk I am going to fix it in that function and revert back to the null string in the backend until a new extension version is released with the fix.

hexiro commented 3 years ago

Sounds good to me. I'm making good progress on the typescript refactor if you're still interested in that.

gergelyszabo94 commented 3 years ago

I pushed the fix, I hope it does not interfere with your refactor efforts too much.

hexiro commented 3 years ago

nahh it'll be fine, I think i'll just have to resolve that one conflict. I think using typescript its going to make it much easier to add features, fix bugs, etc, because it tells you exactly data you're going to receive from local storage / steam api which helps for sure.