neothematrix / noip-renew

Auto renew (confirm) noip.com free hosts
https://hub.docker.com/r/moebiuss/noip-renew
Apache License 2.0
54 stars 20 forks source link

replace sleeps with proper wait events #10

Closed neothematrix closed 2 years ago

neothematrix commented 2 years ago

there are a few sleeps placed in the code that allows for screenshot to be properly taken on debug level 2, this also allows some slower connection to properly run the script. As a workaround #9 enables debug by default, but the correct solution would be to add some wait events to make sure the page is fully loaded.

Angel0ffDeath commented 2 years ago

I would suggest you to use explicit wait method of selenium. With this method you wait certain amount of time (let's say 5 or 10 sec) or until some given element appears. For login page the element could be login button and for confirm host page this could be the element with the expiration days. To do that you need to add the following code:

from selenium.webdriver.support.ui import WebDriverWait from selenium.webdriver.support import expected_conditions as EC

after driver.get add: try: elem = WebDriverWait(driver, 10).until( EC.presence_of_element_located((By.XPPATH, "xpath")) )

ADDED recently: After try, of course you can catch exception and to decide to terminate script (if page changed... probably with exit code) - connected with the Feedback issue I opened.

You should add this code after each page opening. In the above code timeout is 10 sec (you can lower it). If element appears faster than 10 seconds of driver will not wait 10 sec, but will proceed further immediately after element appears.

neothematrix commented 2 years ago

thanks @Angel0ffDeath this is what I thought, we just need to find the right element for every sleep to be replaced :-)

Angel0ffDeath commented 2 years ago

Selenium has 2 more wait methods - Implicit Wait (you wait certain amount of time) and Fluent Wait. Fluent Wait is more advanced from explicit wait and is mainly for dynamically loaded elements, so I think explicit wait is the most appropriate. Of course you can check selenium documentation for more info :)

Angel0ffDeath commented 2 years ago

BTW - I also would like to suggest you all regex used to find buttons and elements to be defined as constants in the beginning of the code. This way will be very easy to find them and change them after future updates of the site....

neothematrix commented 2 years ago

BTW - I also would like to suggest you all regex used to find buttons and elements to be defined as constants in the beginning of the code. This way will be very easy to find them and change them after future updates of the site....

I don't think it will be very useful anyways, all the time I had to fix the parsing I had to change the xpath, not the regular expressions, the structure of the page is usually what changes, meaning that the label you were looking for is now an attribute for example, or inside another nested table.

Angel0ffDeath commented 2 years ago

BTW - I also would like to suggest you all regex used to find buttons and elements to be defined as constants in the beginning of the code. This way will be very easy to find them and change them after future updates of the site....

I don't think it will be very useful anyways, all the time I had to fix the parsing I had to change the xpath, not the regular expressions, the structure of the page is usually what changes, meaning that the label you were looking for is now an attribute for example, or inside another nested table.

Will try to help with this... only not sure when will have enough time to do it

@neothematrix Actually instead xpath you can use combination of 2 find_element functions to locate desired child element - for instance:

ele_pwd = find_element(By.ID, "signup-wrap").find_element(By.NAME, "password")

and ID is unique

Also for more child elements: element = find_element(By.ID, "ID") a_ele = element.find_element(By.NAME, "name1") b_ele = element.find_element(By.NAME, "name2")

Will test this tonight, if I have some time

Angel0ffDeath commented 2 years ago

@neothematrix - Fully solved in my new PR... You can commit changes (after tests of course) and close this issue

Actually I don't like these implicit waits, but right now don't have time to analyze code of the web page - all elements we are searching for are dynamically loaded and in this case fluent wait should be used... Will do it when have time, but the team here is free to try :)

benyjr commented 2 years ago

I am on vacation on the 29th, and as a result will do some testing on this submission to verify that it works.

neothematrix commented 2 years ago

closed by #14