rienafairefr / pynYNAB

a python client for the new YNAB
MIT License
138 stars 12 forks source link

Transactions are not added in new version #24

Closed tovolkmar closed 7 years ago

tovolkmar commented 7 years ago

Hi,

i have a problem with the new version of your api.

i use the following code to add transaction to my YNAB account

db = Database(sqliteConfig.path) ynabc = db.GetYnabLogin(account['UserID']) parser = configargparse.getArgumentParser('pynYNAB') args = parser.parse_args() args.email = ynabc['Email'] args.password = ynabc['Password'] args.budgetname = ynabc['Budget'] client = clientfromargs(args) client.sync() accounts = {x.account_name: x for x in client.budget.be_accounts} accountid = accounts[account['YnabAccountName']].id imported_date = datetime.now().date() transaction = Transaction( entities_account_id=accountid, amount=clearedTransaction['amount'], date=datetime.strptime(clearedTransaction['Date'], '%d.%m.%Y'), imported_date=imported_date, memo=clearedTransaction['Memo'], source="Imported", )

if not client.budget.be_transactions.containsduplicate(transaction): client.add_transaction(transaction) client.sync() return True

return False

It was working well , till i updated to version 3 of your api. Now, the transaction isn't added, but i don't get errors. Did i miss something, that was changed?

Thanks for your help and your work. Tobias

rienafairefr commented 7 years ago

Hi Tobias, Sorry about the regression. I've seen it as well. If version 0.2 works for you stay on it until I've sorted out the problem. SOmething caused changed_entities to be always empty, so nothing sent to nynab... Sorry, working on it.

tovolkmar commented 7 years ago

Hi, no problem. Take your time. I appreciated your work on the api.

gehan commented 7 years ago

I managed to add a new transaction using v0.3 - well a newer commit to that.

However something really weird has happened. A load - perhaps most - of my existing transactions have had their amounts changed. The amounts have been rounded to the nearest whole number:

e.g.

4.80 -> 5.00 5.80 -> 6.00 18.60 -> 19.00

This happened after I tried to add a transaction.

I'm not sure how long adding a transaction should take but it took a few minutes then had a connection error:

In [52]: ynab_client.add_transactions(transactions)
INFO:requests.packages.urllib3.connectionpool:Resetting dropped connection: app.youneedabudget.com
DEBUG:requests.packages.urllib3.connectionpool:"POST /api/v1/catalog HTTP/1.1" 200 None
---------------------------------------------------------------------------
ConnectionError                           Traceback (most recent call last)

some of the trace

/home/gehan_g/.virtualenvs/mondo-ynab/src/pynynab/pynYNAB/Client.py in sync_obj(self, obj, opname, knowledge, extra)
    186         request_data.update(extra)
    187 
--> 188         sync_data = self.connection.dorequest(request_data, opname)
    189         self.logger.debug('server_knowledge_of_device ' + str(sync_data['server_knowledge_of_device']))
    190         self.logger.debug('current_server_knowledge ' + str(sync_data['current_server_knowledge']))
/home/gehan_g/.virtualenvs/mondo-ynab/src/pynynab/pynYNAB/utils.pyc in rateLimitedFunction(*args, **kargs)
     12             if leftToWait > 0:
     13                 time.sleep(leftToWait)
---> 14             ret = func(*args, **kargs)
     15             lastTimeCalled[0] = time.clock()
     16             return ret
/home/gehan_g/.virtualenvs/mondo-ynab/src/pynynab/pynYNAB/connection.pyc in dorequest(self, request_dic, opname)
     62         params = {u'operation_name': opname, 'request_data': json_request_dict}
     63         self.logger.debug('%s  ... %s ' % (opname,params))
---> 64         r = self.session.post(self.urlCatalog, params, verify=False)
     65         self.lastrequest_elapsed=r.elapsed
     66         js = r.json()
/home/gehan_g/.virtualenvs/mondo-ynab/lib/python2.7/site-packages/requests/sessions.pyc in post(self, url, data, json, **kwargs)
    509         """
    510 
--> 511         return self.request('POST', url, data=data, json=json, **kwargs)
    512 
    513     def put(self, url, data=None, **kwargs):
/home/gehan_g/.virtualenvs/mondo-ynab/lib/python2.7/site-packages/requests/sessions.pyc in request(self, method, url, params, data, headers, cookies, files, auth, timeout, allow_redirects, pro
xies, hooks, stream, verify, cert, json)
    466         }
    467         send_kwargs.update(settings)
--> 468         resp = self.send(prep, **send_kwargs)
    469 
    470         return resp
/home/gehan_g/.virtualenvs/mondo-ynab/lib/python2.7/site-packages/requests/sessions.pyc in send(self, request, **kwargs)
    574 
    575         # Send the request
--> 576         r = adapter.send(request, **kwargs)
    577 
    578         # Total elapsed time of the request (approximately)
/home/gehan_g/.virtualenvs/mondo-ynab/lib/python2.7/site-packages/requests/adapters.pyc in send(self, request, stream, timeout, verify, cert, proxies)
    424 
    425         except (ProtocolError, socket.error) as err:
--> 426             raise ConnectionError(err, request=request)
    427 
    428         except MaxRetryError as e:
ConnectionError: ('Connection aborted.', BadStatusLine("''",))

Any idea what happened!?! It seems really odd that this could even happen. I'm guessing that the sync method keeps the local and server copies of the budget in sync? Perhaps that is something to do with it?!

I can help test although I'll have to create a test budget that can be sacrificed. Probably should have done that rather than my actual one.

Does the server keep old versions like the desktop app or am I just gonna have to adjust the transactions myself? 😒

rienafairefr commented 7 years ago

Hi gehan, Ouch, so sorry that happened. I'm afraid the changes are final (in the end we dupe the server into thinking we are the web app, everything the lib does is the same as if it was done through the web app, and we completely bypass the undo that is afaik only client-side)

The rounding might be a problem wth the scaling. The server does not store amount = $ 1.23, but amount=123, so scaling by 100 - at least util now, I've seen that the server recently apparently changed to scale by 1000 instead of 100. Maybe the scaling is not active for every client (they have a system of gradually rolling out features to people), that's a mess. Any idea how we could decipher the api's convention for that scaling at runtime instead of hardcoding the 100 or 1000 ?

BadStatusLine might be a problem with the HTTPS connection, that might not have anything to do with the library, not sure, how much is it reproducible ?

gehan commented 7 years ago

I was running this on GAE - I'll try locally later to see if it's something to do with the connection.

Yeah it's annoying but I'll sort it out - I've borked many of my things over the years from hacking things about πŸ˜‰

I'm more interested in how the lib works to be honest! It seems a bit nuts that adding a transaction can cause existing transactions to update - however that's easy to say when I don't know how it works.

When the Client object is created it looks like the entire contents (or some?) of the budget is downloaded. So thats the account list, payee list... but also all transactions? Or last x days. From looking at server requests that seems to be what the webapp does.

So after a transaction has been added then the server returns some object of things to change, then the lib does another sync or something?

I'm wondering if it's possible to make the server the source of truth and therefore add safeguards to stop this happening to someone else - or me again!

rienafairefr commented 7 years ago

I'm tied to the way the web app works, and yep it fetches everything.

We can't really use the server as source of truth because it's completely feasible, and the library doesnt hold your hand, to modify everything or most everything. I can delete budgets, delete all the transactions, everything is possible. The library works the same way as the web app for modification of server data: fetches the data, takes of snapshot of it (in the web app, it's stored in a local-storage browser database, here it's stored in an in-memory sqlite database), and when you do client.sync() again, it calculates a diff between the current state and the snapshot, and sends that to the server (it's the changed_entities field in the request)

Maybe we could add another layer to the client (and that's what I've kind of started with the add_transaction methods on the client): limit the number of modified transaction to 1 and if the library wants to send modifications for N transactions, error out. I'll look into if we can refactor stuff to avoid modifying every transaction for a single transaction append :-)

gehan commented 7 years ago

Right that makes sense now.

Preloading data makes sense - for instance fetching payee list and account list. But fetching all transactions? That's insane to me! No wonder the webapp takes a minute or 2 to load for me. Anyway that's just how the person who made it decided to handle it.

I like the extra layer you're talking about! I think the client should only alter the server copy in specific circumstances otherwise bad things can happen - like incorrectly modifying 3-4,000 transactions when YNAB changes its API behaviour 😭

Is this what you envisage then? When the client is first instantiated it does the sync thing and creates the in-memory copy of the account. After we assume the server is correct. The only times we ever try to alter the the server copy is when we perform an operation explicitly - and then we only modify things to do with that operation. For instance:

For add transaction we just tell the server to add the transaction, nothing else. For edit/delete transaction we tell the server to edit/delete these specific ids.

Is that possible with how the nynab client works?!

Is it actually necessary to ever call sync except when first instantiating the client?

rienafairefr commented 7 years ago

I like your idea. For now the sync method is common to "first sync" and to the others, because there is no functional difference (merging created/modified/added entities, etc). But I will add a layer, hide the sync method, reserve it to first sync, and make the entry points "modification methods" seding the info "how many entities should be changed down the line and error out before sending a modification for thousands of entities ! Gimme a few days and that should work.

gehan commented 7 years ago

Nice one! Sounds very nice πŸ‘

gehan commented 7 years ago

I have just realised that I have nYNAB on my phone! Since the client appears to download the entire budget I guess this copy is correct!?

Do you think there is a way to modify the phone's data to be the latest version and therefore overwrite the cloud data when i refresh from the phone?

rienafairefr commented 7 years ago

no idea. You'd have to talk to nynab app developers ^^ If you can extract the data from the phone you could send it back to the server using pynynab, but that's a loooong stretch.

gehan commented 7 years ago

oh yeah good point. well - the server copy's borked anyway so nothing to lose!

On Tue, 24 Jan 2017 at 13:40 rienafairefr notifications@github.com wrote:

no idea. You'd have to talk to nynab app developers ^^ If you can extract the data from the phone you could send it back to the server using pynynab, but that's a loooong stretch.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rienafairefr/nYNABapi/issues/24#issuecomment-274804896, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOkRi7qk4TbVqK-nREizLzzlKob8lxHks5rVf82gaJpZM4K57s1 .

rienafairefr commented 7 years ago

gehan, I've pushed a syncchange branch that incorporates the ideas we floated here on safeguards and differentiation between sync (from server) and push (to server), check it out ! :-)

gehan commented 7 years ago

Nice work! Will check it out :)

On Fri, 27 Jan 2017, 16:02 rienafairefr, notifications@github.com wrote:

gehan, I've pushed a syncchange branch that incorporates the ideas we floated here on safeguards and differentiation between sync (from server) and push (to server), check it out

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rienafairefr/nYNABapi/issues/24#issuecomment-275685536, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOkRjOECfKfYINSd6_Jliy3UgV0RGrxks5rWgcZgaJpZM4K57s1 .