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

Remove Iptools and GeoIP dependencies #2896

Closed
hornc opened this issue Jan 21, 2020 · 5 comments · Fixed by #2955
Closed

Remove Iptools and GeoIP dependencies #2896

hornc opened this issue Jan 21, 2020 · 5 comments · Fixed by #2955
Assignees
Labels
Lead: @hornc Issues overseen by Charles (Staff: Data Engineering Lead) [managed] Priority: 2 Important, as time permits. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]

Comments

@hornc
Copy link
Collaborator

hornc commented Jan 21, 2020

These dependencies appear to only be used by the deprecated Libraries class and endpoint.

Rather than concern ourselves about modern version numbers and compatibility of modules we are not using, let's remove them and their references.

relates to #1694
should replace #2893
and #2866

Describe the problem that you'd like solved

Proposal & Constraints

Additional context

Stakeholders

@hornc hornc added Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] State: Backlogged labels Jan 21, 2020
@hornc
Copy link
Collaborator Author

hornc commented Jan 21, 2020

@mekarpeles are you able to confirm whether this method for getting loan stats is fully deprecated?

def get_item_status(self, ekey, iaid, collections, subjects):
if 'lendinglibrary' in collections:
if not 'Lending library' in subjects:
status = 'restricted'
else:
status = 'lendable'
elif 'inlibrary' in collections:
if not 'In library' in subjects:
status = 'restricted'
elif not self.get_inlibrary():
status = 'restricted'
if self.options.get('debug_items'):
status = 'restricted - not inlib'
elif self.options.get('show_inlibrary'):
status = 'lendable'
else:
status = 'lendable'
elif 'printdisabled' in collections:
status = 'restricted'
else:
status = 'full access'
if status == 'lendable':
loanstatus = web.ctx.site.store.get('ebooks/' + iaid, {'borrowed': 'false'})
if loanstatus['borrowed'] == 'true':
status = 'checked out'
return status

I beleive the current used method which uses the latest API (not fake subjects) is here:

def get_item_status(itemid, metadata, **server):
item_server = server.pop('item_server', None)
item_path = server.pop('item_path', None)
return ItemEdition.get_item_status(itemid, metadata, item_server=item_server,
item_path=item_path)

@hornc hornc added this to the Next Sprint (Proposed) milestone Jan 21, 2020
hornc added a commit that referenced this issue Jan 22, 2020
closes #2878

We know a large portion of our Python modules are out of date, and that some of them are not even used for current functionality.
Manual review and fixing of the versions that are causing active problems is required at this stage. Currently minor point version updates are more difficult and risky than they should be, if there is not an active reason to update a module, we shouldn't at this stage. 

Removing some out of date unused dependencies is preferable to keeping them current without understanding how we use them, e.g. 
#2896 

When we remove what is not needed and get to a stable point with Python 3, we can re-enable this tool.
hornc added a commit that referenced this issue Jan 22, 2020
closes #2878

We know a large portion of our Python modules are out of date, and that some of them are not even used for current functionality.
Manual review and fixing of the versions that are causing active problems is required at this stage. Currently minor point version updates are more difficult and risky than they should be, if there is not an active reason to update a module, we shouldn't at this stage.

Removing some out of date unused dependencies is preferable to keeping them current without understanding how we use them, e.g.  #2896

When we remove what is not needed and get to a stable point with Python 3, we can re-enable this tool.
@xayhewalo xayhewalo added Lead: @cclauss Issues overseen by Chris (Python3 & Dev-ops lead 2019-2021) [managed] Needs: Investigation This issue/PR needs a root-cause analysis to determine a solution. [managed] Priority: 2 Important, as time permits. [managed] and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] labels Jan 25, 2020
@tfmorris
Copy link
Contributor

I think @hornc said he was going to do this (any progress on this blocker?) so it should probably be assigned to him, not @cclauss.

@hornc
Copy link
Collaborator Author

hornc commented Feb 1, 2020

#2944 should remove iptool rather than update it.

@hornc
Copy link
Collaborator Author

hornc commented Feb 1, 2020

If someone wants to help with removing these modules please feel free to begin.

cclauss added a commit that referenced this issue Feb 1, 2020
@cclauss cclauss added Lead: @hornc Issues overseen by Charles (Staff: Data Engineering Lead) [managed] and removed Lead: @cclauss Issues overseen by Chris (Python3 & Dev-ops lead 2019-2021) [managed] labels Feb 1, 2020
@cclauss
Copy link
Collaborator

cclauss commented Feb 1, 2020

#2953 demonstrates that I will not be able to resolve this one. I still think that #2866 is a safe way for us to remove the roadblock.

@hornc hornc removed State: Backlogged Needs: Investigation This issue/PR needs a root-cause analysis to determine a solution. [managed] labels Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lead: @hornc Issues overseen by Charles (Staff: Data Engineering Lead) [managed] Priority: 2 Important, as time permits. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]
Projects
None yet
5 participants