Skip to content

Conversation

@rullzer
Copy link
Member

@rullzer rullzer commented Jan 17, 2018

  • Add sleep
  • Add log message

Signed-off-by: Roeland Jago Douma [email protected]

* Add sleep
* Add log message

Signed-off-by: Roeland Jago Douma <[email protected]>
@rullzer rullzer added 3. to review Waiting for reviews technical debt labels Jan 17, 2018
@rullzer rullzer added this to the Nextcloud 14 milestone Jan 17, 2018
@rullzer rullzer requested a review from MorrisJobke January 17, 2018 14:25

OC::$server->getRouter()->match('/ocs'.\OC::$server->getRequest()->getRawPathInfo());

sleep(1);
Copy link
Member

Choose a reason for hiding this comment

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

Ah ... I guess we should then not do this (even if I proposed this), because the Android app for example uses it. On the other side: a good reason to migrate to v2 ;)

@tobiasKaminsky @marinofaggiana @mario @AndyScherzinger @camilasan @juliushaertl Opinions? cc'ing you for the clients

Copy link
Member Author

Choose a reason for hiding this comment

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

Also happens on v2.php ;). Basically this only happens if the old route matches. The match throws an exception if it can't be found etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

So it only does the sleep if the request is one that matches an old OCP\API::register route

Copy link
Member

@tobiasKaminsky tobiasKaminsky Jan 17, 2018

Choose a reason for hiding this comment

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

We should migrate, yes. But please give us some time :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Like i said. This in no way enforces you to use v2. (that is a different PR maybe :P)

Copy link
Member

Choose a reason for hiding this comment

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

Exactly. I checked also all apps in the appstore and this is the only usage:

\OCP\API::register('get',
'/apps/files_external/api/v1/mounts',
array('\OCA\Files_External\Lib\Api', 'getUserMounts'),
'files_external');

Copy link
Member

Choose a reason for hiding this comment

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

Sorry was wrong - survey_client, survey_server and ocdownloader also use it:

bildschirmfoto 2018-01-17 um 16 35 35

Copy link
Member

Choose a reason for hiding this comment

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

But anyways - fine by me.

Copy link
Member

Choose a reason for hiding this comment

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

So what are the ocs path that are old?
Then I can check them on android.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tobiasKaminsky nothing for you to do here. This is only the implentation that needs to be adjusted. The endpoints should stay the same

@codecov
Copy link

codecov bot commented Jan 17, 2018

Codecov Report

Merging #7917 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #7917      +/-   ##
============================================
- Coverage     51.23%   51.23%   -0.01%     
  Complexity    24922    24922              
============================================
  Files          1604     1604              
  Lines         94982    94984       +2     
  Branches       1376     1376              
============================================
  Hits          48667    48667              
- Misses        46315    46317       +2
Impacted Files Coverage Δ Complexity Δ
ocs/v1.php 0% <0%> (ø) 0 <0> (ø) ⬇️

@rullzer
Copy link
Member Author

rullzer commented Jan 17, 2018

Files external fix is already in #7923

@rullzer
Copy link
Member Author

rullzer commented Jan 17, 2018

Survery client in nextcloud/survey_client#72

@rullzer
Copy link
Member Author

rullzer commented Jan 17, 2018

I don't think I can fix ocdownloader as it also works on oC

@MorrisJobke MorrisJobke merged commit f39f95e into master Jan 24, 2018
@MorrisJobke MorrisJobke deleted the warn_old_ocs_route_cons branch January 24, 2018 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants