-
Notifications
You must be signed in to change notification settings - Fork 12
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
621: use default connection #1134
base: main
Are you sure you want to change the base?
Conversation
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 must be doing something wrong.
I tried this but got two test failures; related to the "Depends on part 1 of 2 from NASA-PDS/registry-common@121" I assume.
$ mvn package
…
[ERROR] Tests run: 251, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 346.4 s <<< FAILURE! -- in cucumber.CucumberTest
[ERROR] < 3.6.NASA-PDS/validate#690-1 -- Time elapsed: 19.37 s <<< FAILURE!
org.opentest4j.AssertionFailedError: summary:totalWarnings ==> expected: <888> but was: <900>
at cucumber.StepDefs.compare_to_the(StepDefs.java:182)
at ✽.compare to the expected outcome "summary:totalWarnings=888,summary:messageTypes:warning.table.field_value_out_of_special_constant_min_max_range=888".(file:///Users/kelly/Documents/Clients/JPL/PDS/Development/nasa-pds/validate/src/test/resources/features/pre.3.6.x.feature:5)
[ERROR] < 3.6.NASA-PDS/validate#427-1 -- Time elapsed: 17.79 s <<< FAILURE!
org.opentest4j.AssertionFailedError: summary:totalWarnings ==> expected: <2> but was: <14>
at cucumber.StepDefs.compare_to_the(StepDefs.java:182)
at ✽.compare to the expected outcome "summary:totalWarnings=2,summary:messageTypes:warning.table.field_value_out_of_special_constant_min_max_range=2".(file:///Users/kelly/Documents/Clients/JPL/PDS/Development/nasa-pds/validate/src/test/resources/features/pre.3.6.x.feature:5)
[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] summary:totalWarnings ==> expected: <2> but was: <14>
[ERROR] summary:totalWarnings ==> expected: <888> but was: <900>
[INFO]
[ERROR] Tests run: 274, Failures: 2, Errors: 0, Skipped: 23
…
So then I tried building the distribution and skipping the tests:
$ mvn -DskipTests=true package
…
$ tar xzf target/validate-3.7.0-SNAPSHOT-bin.tar.gz
$ validate-3.7.0-SNAPSHOT/bin/validate --help | egrep -e -r
-r,--report-file <file name> Specify the report file name. Default
-R,--rule <validation rule name> Specifies the validation rules to
-s,--report-style <full|json|xml> Specify the level of detail for the
--skip-context-reference-check Disable context reference check. The
to include in the human-readable
and I don't see the new -r
option.
I think I better just let someone more experienced review this! 😁
No. This is because the JAXA website vanishes for long periods of time and we cannot get the data we need for testing. There is a ticket to download said data and put it into validate test data to remove this problem. Oh, this is for validate-refs not validate. Tool does not belong here software logically. However it is here because those that run validate will want to run validate-refs and cannot(?) download registry-mgr. I suspect it is keep the bar low (low effort to adopt). |
@al-niessner thanks for the explanation! |
* added new encoding * comment out jaxa --------- Co-authored-by: Al Niessner <Al.Niessner@xxx.xxx>
|
Okay. Using my computer at JPL on JPLNet this is what I see with changes on this branch and the index of en-registry (showing it overrides what I told it):
Hopefully that is what you expected. If you review #1146 I will merge those changes to here and make it so the unit tests pass. Or I guess you can do it. |
Just doing clean up of issue and saw that it worked for you with en-registry but not registry. What are all of the non-alias registry names? Just one other valid one will do. I tried en-registry-fred and it got the timeout error you did. It seems something is afoot but I need another valid index name to start the chase. |
There is something else amiss beyond our code. I went back to en-registry and it fails too with a timeout. The timeout is big like minutes. I am going to let it idle for a bit then try again to see what happens if I do the same thing twice in a row. |
@al-niessner your account will only work with en-registry (I think). Each user gets access to 1 index. We will have to get @sjoshi-jpl @viviant100 or @tloubrieu-jpl to help us add you to other indexes if we need to test that. They should all work the same. |
@al-niessner I think I mentioned this before, but as an extra sanity check for your access, the registry-client is super simple, and really helpful IMO. https://nasa-pds.github.io/registry-client/ Once I configured my login and the appropriate info (same as my validate-refs config), I get this when trying to ping the
|
validate-refs is Java SDK2. pds-registry-client is python and presumably some version of boto. The failure is in the Java SDK2, which claims they seamlessly handle aliases, not in validate-refs our code. Are you trying to tell me that you want me to rewrite validate-refs in python (I am not opposed) because we know that the python SDK works and is better supported? Are you saying that pds-registry-client already figured out the alias issues and I should be looking at their code? side note: I have isolate the problem to be the alias and not any of the real nodes |
Asking for additional help/guidance |
@al-niessner I was not asking to rewrite this in Python, I was just saying it may be good to confirm you have sufficient access to the alias outside of the SDK to check that off as a possible reason for the timeout. If you are confident that is not the case, then that works for me. |
🗒️ Summary
Use the
app://connections/cognito/registry.xml
as the default index since it is an alias that connects all of the indices to one name.⚙️ Test Data and/or Report
NA - user interface change not a procedural change
♻️ Related Issues
Closes #621
Closes #1053