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

Removing the usage of wget in favor of ftplib #20

Merged
merged 5 commits into from
Jan 30, 2018

Conversation

lel-amri
Copy link
Contributor

@lel-amri lel-amri commented Nov 4, 2017

Dropping the usage of wget gives us portability

Le downloader n'est pas user-friendly. Aucune erreur n'est catch, mais en dehors du socket utilisé par ftplib, rien ne nécessite vraiment d'être "libéré" proprement.

L'implémentation proposée respecte le comportement de wget. En revanche, il y a un petit "souci" avec la date de modification des fichiers téléchargés. En effet, le serveur FTP semble retourner une date qui n'est pas en UTC. Du coup lors de la définition de la date de modification, le système ajuste la date à la timezone locale. Par conséquent certains fichiers apparaissent comme modifiés au matin du jour suiviant leur supposée date de modification réelle.

legi/download.py Outdated
time.time(), (
remote_files[filename]['mtim']
- datetime.datetime(1970, 1, 1
).total_seconds()
Copy link
Member

Choose a reason for hiding this comment

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

L'indentation n'est pas correcte ici et il manque une parenthèse fermante.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow. Je me demande comment mes tests sont passés à côté de la parenthèse manquante.

Pour ce qui est de l'indentation, quel style dois-je utiliser ?

@Changaco Changaco merged commit 7f82f6b into Legilibre:master Jan 30, 2018
@Changaco
Copy link
Member

Merci @lel-amri, ce n'est pas parfait mais ça fonctionne.

Dans le futur il faudrait modifier le code pour qu'il utilise une seule commande LIST pour récupérer les noms et tailles des fichiers en même temps, au lieu d'utiliser une NLST au début pour les noms puis une SIZE par fichier.

@lel-amri
Copy link
Contributor Author

lel-amri commented Jan 30, 2018

C'est loin d'être parfait en effet. C'est à peine propre en vérité.

Le problème de la commande LIST c'est que le formatage de la valeur de retour est implementation dependent. Rien dans la RFC959 n'impose de format particulier. Au contraire de NLST, qui retourne exactement l'information dont on a besoin. La RFC3659 introduit la commande MLST qui retourne une valeur formatée pour faciliter la lecture par un programme (Et retourne à la fois, le nom, la date de modification, la taille, etc...). Mais il semble que les serveurs FTP DILA ne la supportent pas.

On pourrait coder un truc DILA-specific ceci dit.

@Changaco
Copy link
Member

Oui ce n'est pas standard, mais parser la réponse est possible, d'ailleurs c'est ce que fait Firefox.

Seb35 pushed a commit to Seb35/legi.py that referenced this pull request Jan 8, 2020
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.

2 participants