IN 1194- merge employee appointment tables #203
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Why these changes are being introduced: A dependencies update was long overdue but luigui 3.6 brings a considerable amount of linting errors that will need addressing (or blanket ignoring). Currently, this project works well with luigi 3.5.1 so felt safe to pin this for the time being and allow other updates to proceed. How this addresses that need: * updates dependencies * pins luigi to 3.5.1 Side effects of this change: * luigi not automatically updated, but will get revisited during next maintenance Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-1194
jonavellecuerdo
approved these changes
Feb 26, 2025
Why these changes are being introduced: For reasons beyond the scope of this commit, it has been determined that having a table like "LibHR Employee Appointments" (LIBHR) distinct from the more widely used "Employee Appointments" (EA) is more problematic than it is helpful. As such, it has been decided that data formerly found in the LIBHR table will now be directly added and managed in the EA table. How this addresses that need: * Completely removes any tasks associated with the LIBHR table * Removes any copying of data to the EA table from the LIBHR table Side effects of this change: * LIBHR table is no longer a concern for this middleware Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-1194
ghukill
force-pushed
the
IN-1194-merge-libhr-tables
branch
from
eb50224 to
5498f0a
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose and background context
This PR makes two updates to the HRQBClient that runs daily to load data into Quickbase.
1- Commit which pins
luigi == 3.5.1It appears that
luigi>= 3.6 introduces some type changes that result in hundreds of linting errors in this project. However, we needed to udpatetornado(a dependency ofluigi) that has a security vulnerability. Pinningluigiallows us to avoid the type errors at this time while still updatingtornado.This should also allow for updating the python version of this project from 3.11 to 3.12 at our next convenience, which was paused for the same reasons of
luigi.I can see a couple of paths here:
luigipinned for the foreseeable future, given the somewhat limited fashion in which we use it.luigiand work to address all type errros, if if that means some targeted, blanket ignoresI would like to follow option 2 soon, but not now.
2- Commit to remove the
LibHR Employee Appointmentstable from this data loaderThe more substantative change is removing any mention or management of the table
LibHR Employee Appointments. It has been determined that this table is no longer needed, allowing for some dramatic simplification of code in this client.How can a reviewer manually see the effects of these changes?
Not readily possible.
Includes new or updated dependencies?
YES: all dependencies updates except
luigiwhich is pinniedChanges expectations for external applications?
YES
What are the relevant tickets?
Developer
Code Reviewer(s)