search

kshitij10496 / hercules

The mighty hero helping you build projects on top of IIT Kharagpur's academic data
https://hercules-10496.herokuapp.com/api/v1/static/index.html
MIT License
34 stars 18 forks source link

ERP Login #2

Closed Ayushk4 closed 5 years ago

Ayushk4 commented 5 years ago

Refers #1

kshitij10496 commented 5 years ago

@Ayushk4 Can you use requests-html instead of using bs4?

Ayushk4 commented 5 years ago

@kshitij10496 I have updated the script. Now it requires neither of those. I will push the changes to remote soon.

Ayushk4 commented 5 years ago

@kshitij10496 Please look at the code now, I have made the necessary edits.

kshitij10496 commented 5 years ago

@Ayushk4 The ERP login feature is supposed to be used by the scrapping scripts, isn't it? If that is the case, it makes sense to write it as a package rather than a script.

Also, can you document each function using docstrings and define the global variables/constants (URLs) in a separate .py file for convenience?

Let me know if you face any question.

cc @icyflame @themousepotato

Ayushk4 commented 5 years ago

@kshitij10496 Sure, I will be working on it. :smile:

kshitij10496 commented 5 years ago

@Ayushk4 Any updates on this? 😄

themousepotato commented 5 years ago

Ping @Ayushk4

Ayushk4 commented 5 years ago

@themousepotato @kshitij10496 No updates yet :disappointed:. I will resume working on it, on Thursday.

kshitij10496 commented 5 years ago

Suggested Reading: https://docs.python-guide.org/writing/structure/#structuring-your-project

Ayushk4 commented 5 years ago

@kshitij10496 Can you tell the reason you prefer a package, its not even hundred lines of code? Also, can you explain the difference between script and package with respect to this case.

kshitij10496 commented 5 years ago

If a piece of code is generic enough to be used at multiple places, IMHO it should be structured as a package. The lines of code should not matter as long as the functionality is useful. Even if you implement a generic sum function, you should package it. This is just a case of programming aesthetics.

For our case, I believe you know that we are currently using 2 scrapping scripts for populating faculty information and course information. In the future, we would have to implement other scrapping scripts as well (e.g grade distribution for Kronos). Rather running two scripts sequentially, one for getting sessionID and one for the actual scrapping work, we should make the problem of login trivial once and for all. Also, I think we should not be writing a "login" function for every scrapping script we write. The code should be DRY and follow the Single Responsibility Principle. Thus, a utility package with all the niceties would be wonderful. Does this make sense?

Think of legos - plug and play!

Ayushk4 commented 5 years ago

ping @kshitij10496

Ayushk4 commented 5 years ago

Ping @kshitij10496 Can you review this? I believe that the docstrings may need some work, please suggest me regarding the same.

Ayushk4 commented 5 years ago

@kshitij10496 Can you give suggestions regarding docstrings? Can you review the error handling part?

Ayushk4 commented 5 years ago

Ping @kshitij10496 , all the requested changes have been made.

kshitij10496 commented 5 years ago

Looks good! Did you test it locally?

kshitij10496 commented 5 years ago

Were you able to get any data(such as student timetable) using the tokens generated he

Ayushk4 commented 5 years ago

@kshitij10496 Using the academicToken obtained, I was able to extract data for wimp.

kshitij10496 commented 5 years ago

Merging this in!