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

Use common time comparison logic to enable better logging #5782

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

nicksinger
Copy link
Member

check() was previously using the current time. _key_auth() however used the time provided in a header (which is attached right as the transaction starts on the server side). This change makes use of the already existing subroutine _is_timestamp_valid for both places and uses the current time instead of one attached in some hook earlier. It also adds a debug log output to print both plain timestamps in case of mismatches.

Related ticket: https://progress.opensuse.org/issues/162038

@nicksinger nicksinger force-pushed the auth_timing branch 4 times, most recently from 40aa000 to 3653b43 Compare July 22, 2024 09:33
@kalikiana
Copy link
Member

kalikiana commented Jul 22, 2024

[09:36:29] t/full-stack.t ......... Prototype mismatch: sub main::any: none vs (&@) at /usr/lib/perl5/5.26.1/Exporter.pm line 66.
 at t/full-stack.t line 25.

I guess this was run before the last change, but somehow not updated afterwards. Hence re-triggering.

@perlpunk
Copy link
Contributor

https://app.circleci.com/pipelines/github/os-autoinst/openQA/14067/workflows/d939f2da-77c9-4edc-a2ec-b9d545fefea7/jobs/132486?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link&utm_content=summary

[09:36:29] t/full-stack.t ......... 36/? executeScript: unknown error: session deleted because of page crash at /home/squamata/project/t/lib/OpenQA/SeleniumTest.pm:78 at /home/squamata/project/t/lib/OpenQA/SeleniumTest.pm line 81.
        OpenQA::SeleniumTest::__ANON__(Test::Selenium::Chrome=HASH(0x561f4ca50838), "Error while executing command: executeScript: unknown error: "..., HASH(0x561f4d1d0fc8), HASH(0x561f4d1d7e48)) called at /usr/lib/perl5/vendor_perl/5.26.1/Selenium/Remote/Driver.pm line 356

There is a ticket: https://progress.opensuse.org/issues/164239 - we can force merge

@perlpunk
Copy link
Contributor

The Prototype mismatch is fixed meanwhile, just needs to be rebased

@nicksinger
Copy link
Member Author

rebased now, thanks. Wasn't there some fancy bot to rebase on my behalf? :)

@perlpunk
Copy link
Contributor

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Jul 22, 2024

rebase

✅ Branch has been successfully rebased

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.49%. Comparing base (09ba4d8) to head (ff7be61).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5782   +/-   ##
=======================================
  Coverage   98.49%   98.49%           
=======================================
  Files         394      394           
  Lines       38668    38671    +3     
=======================================
+ Hits        38085    38088    +3     
  Misses        583      583           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@okurz
Copy link
Member

okurz commented Jul 22, 2024

s/comparision/comparison/

@okurz okurz changed the title Use common time comparision logic to enable better logging Use common time comparison logic to enable better logging Jul 22, 2024
@okurz
Copy link
Member

okurz commented Jul 22, 2024

All checks had been green except for docker-compose failing due to zypper repo refresh errors. nicksinger only fixed the typo in the git commit message subject line. Merging manually.

@okurz okurz merged commit bf55007 into os-autoinst:master Jul 22, 2024
12 of 21 checks passed
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.

4 participants