-
Notifications
You must be signed in to change notification settings - Fork 8
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
DACCESS 2025 Sprint 1 - Merge dev to main #2221
Conversation
…SS-291-Advanced-Search-Carryover
…SS-291-Advanced-Search-Carryover
…SS-291-Advanced-Search-Carryover
…SS-291-Advanced-Search-Carryover
Add the site code so the requests are routed to the correct database (kheel or rmc)
…n of the status app
LP-1237 - switch CORS to accept connections from the us-east-1 versio…
…SS-292-Advanced-Search-Carryover
Bump nokogiri to 1.16.8
…SS-291-Advanced-Search-Carryover
…is separate from the site code change
DACCESS-485 update the photoduplication form and enforce char limit for citation field
… page for an item
DACCESS-487: add click event for Articles & Full Text, update event f…
DACCESS-489: fix click events on non-catalog links in bento
…o test the navigation to the item detail page
DACCESS-472 - fix bookmarks item details page navigation
Hi everyone, Once you’ve reviewed and it looks good to you, lets get ready for deployment to Production. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel like my feedback is the strongest indicator since I am not a Ruby developer, but I looked through the changes and saw nothing worrying.
|
||
end | ||
def browse_search_field(browse_type, heading_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I shouldn't have merged this method into dev. It's not going to break anything, but it's not doing anything and nothing's calling it. I'll just create a branch for removing it and can merge it in in the next sprint, so it doesn't delay the deploy.
Update My Account and Edge library
## Parallel Testing | ||
Testing has been updated to allow for parallel testing with cucumber tests. | ||
### To run tests in parallel: | ||
* Update constant `NUM_PROCESSES=1` in `jenkins/cucumber-features.sh` to the number of processes you want to run in parallel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have missed discussion of this during the standups, but how does one determine the right number of processes to use? What are the disadvantages of just selecting the maximum number all the time? Presumably that might impact others who are trying to run Jenkins jobs on the same server for other projects, but how would one know that without sending out a general query to all the developers on Slack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Baroquem, As its setup now, It defaults to just using 1 processor, but you can manually set it to use more than one processor (with diminishing gains the more you add).
Currently there are only 4 processors available for Jenkins to use If there are more tests running than processors, then it increases the time it takes to finish those tests.
If you use 2 processors in Jenkins it cuts the time down to about 25 minutes.
Greg said there can be more processors added, but they are working on getting the docker container tests setup first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I’m more wondering about the impact on other people. The selfish thing to do is obviously to grab all the available processors for my own testing, to finish it as quickly as possible, but I wouldn’t do that if it negatively affects other people’s work. But then do I pick 2 processors … or 3…? I don’t have a sense of how to find the best compromise there.
And if nobody else happens to be using Jenkins at the time, then I could use all the processors without guilt — but I don’t think we have a good way of determining that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, everyone!
Summary of Changes
https://culibrary.atlassian.net/browse/DACCESS-208
ENV values for:
MY_ACCOUNT_ILLIAD_API_KEY
MY_ACCOUNT_ILLIAD_API_URL
can be found at
New env keys for ILLiad/My Account
In LastPass inShared Discovery and Access-Library Systems
Bug Fixes
Aeon Enhancements
Accessibility
Search Improvements
My Account Updates (v2.3.4)
cul-folio-edge Library (v3.2)
authenticate
method to support both new and old token systems.Advanced Search Carryover (PR #2196, DACCESS-291)
Test Parallelization (PR #2209, DACCESS-484)
NUM_PROCESSES
parameter for controlling test parallelization.NUM_PROCESSES
parameter to the pipeline configuration.