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

CLDR-14399 move to OpenLiberty, add API server #936

Merged
merged 1 commit into from
Jan 26, 2021

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Jan 14, 2021

CLDR-14399 move to OpenLiberty, add API server

  • See CLDR-14213 - this doesn't remove Derby, but doesn't enable it in the server easily anymore.

@srl295 srl295 self-assigned this Jan 14, 2021
@srl295
Copy link
Member Author

srl295 commented Jan 14, 2021

maybe @yumaoka and @JCEmmons will want to review someday if it looks familliar 🤣

@@ -78,6 +78,8 @@ public static String getRelativeFileName(Class<?> class1, String filename) {
return resourceString.substring(5);
} else if (resourceString.startsWith("jar:file:")) {
return resourceString.substring(9);
} else if (resourceString.startsWith("wsjar:file:")) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yumaoka did i redo it right?

(context: ICU4J had the exact same bug and fix…)


@Path("/xpath")
public class XPathAPI {
@GET @Path("{hexId}")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yumaoka surveytool gets dragged into the 2010's … :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this API doesn't validate quite yet.

Comment on lines 25 to 32
TO SET UP:
Create a "server.env" file next to this xml
(that is, tools/cldr-apps/src/main/liberty/config/server.env)
with the following:

MYSQL_USER=surveytool
MYSQL_PASSWORD=yourpassword
MYSQL_DB=cldrdb
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interim setup instructions…

@srl295
Copy link
Member Author

srl295 commented Jan 15, 2021

@btangmu this is now working w/ the DB

@srl295
Copy link
Member Author

srl295 commented Jan 15, 2021

I'll clean this up and get it ready to be a real draft PR…

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/cldr-apps/README.md is different
  • tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/PropertiesResource.java is no longer changed in the branch
  • tools/cldr-apps/src/main/liberty/config/server.env.sample is now changed in the branch
  • tools/cldr-apps/src/main/resources/api/swagger.yaml is no longer changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@srl295 srl295 requested review from btangmu and yumaoka January 15, 2021 22:43
@srl295 srl295 marked this pull request as ready for review January 15, 2021 22:43
@srl295 srl295 changed the title CLDR-14399 scary API PR CLDR-14399 move to OpenLiberty, add API server Jan 15, 2021
@srl295
Copy link
Member Author

srl295 commented Jan 15, 2021

Still need to figure out the deployment mechanism (actions and such). so i guess this is still draft.

@srl295 srl295 marked this pull request as draft January 15, 2021 22:44
@srl295 srl295 requested a review from JCEmmons January 15, 2021 22:44
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/cldr-apps/pom.xml is different
  • tools/cldr-apps/src/main/liberty/config/server.xml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@btangmu
Copy link
Member

btangmu commented Jan 19, 2021

I've quickly looked through the changes and LGTM though I certainly don't understand it all. Seems worthwhile. How should we test?


@Path("/xpath")
public class XPathAPI {
@GET @Path("{hexId}")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this API doesn't validate quite yet.

@@ -0,0 +1,4 @@
# copy this to server.env and set the password properly
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@btangmu if you copy this file to server.env (with appropriate DB changes) then you can start the server with mvn -pl cldr-apps liberty:dev and it will fire up a dev server on port localhost:9080 until you hit control-C ..

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/scripts/ansible/openliberty-playbook.yml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@srl295
Copy link
Member Author

srl295 commented Jan 19, 2021

I've quickly looked through the changes and LGTM though I certainly don't understand it all. Seems worthwhile. How should we test?

#936 (comment) says how to do a local test. we should walk through that sometime.

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/scripts/ansible/openliberty-playbook.yml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .gitignore is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .github/workflows/maven.yml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .github/workflows/maven.yml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/scripts/ansible/cldr-apps-playbook.yml is different
  • tools/scripts/ansible/openliberty-playbook.yml is different
  • tools/scripts/ansible/server-playbook.yml is different
  • tools/scripts/ansible/templates/56-surveytool.j2 is now changed in the branch
  • tools/scripts/ansible/templates/deploy-sh.j2 is now changed in the branch
  • tools/scripts/ansible/vars/main.yml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@srl295 srl295 marked this pull request as ready for review January 20, 2021 20:33
@srl295
Copy link
Member Author

srl295 commented Jan 20, 2021

This is ready to review.

  • should be deployable to cldr-smoke and production once ansible is re-run. (may need to iterate there)
  • for local command line test, see …/cldr-apps/README.md

@btangmu
Copy link
Member

btangmu commented Jan 22, 2021

Following …/cldr-apps/README.md, I get:

[ERROR] .../cldr/tools/cldr-apps/src/main/java/org/unicode/cldr/web/ErrorSubtypes.java:[24,43] cannot find symbol
  symbol:   method toHTML(java.lang.String)
  location: class org.unicode.cldr.util.CLDRURLS
...
...
[INFO] [ERROR   ] CWWKF0032E: The com.ibm.websphere.appserver.javaee-jdbc-4.3 feature requires a minimum Java runtime environment version of JavaSE 11.
...

Does the "CLDRURLS.toHTML" error ring any bells? Is JavaSE 11 really required?

@srl295
Copy link
Member Author

srl295 commented Jan 22, 2021

Following …/cldr-apps/README.md, I get:

[ERROR] .../cldr/tools/cldr-apps/src/main/java/org/unicode/cldr/web/ErrorSubtypes.java:[24,43] cannot find symbol
  symbol:   method toHTML(java.lang.String)
  location: class org.unicode.cldr.util.CLDRURLS
...
...
Does the "CLDRURLS.toHTML" error ring any bells? 

it sounds like possibly an out of date package. Can you Try running this first:

mvn --file=tools/pom.xml compile install package -DskipTests=true

The liberty:dev may only look at code within cldr-apps for rebuild.

[INFO] [ERROR ] CWWKF0032E: The com.ibm.websphere.appserver.javaee-jdbc-4.3 feature requires a minimum Java runtime environment version of JavaSE 11.
...


Is JavaSE 11 really required?

Hm, yes, I think it may be required. CLDR-14311 proposes a bump to 11 LTS.

easy way to add it to the command line or eclipse:

@btangmu
Copy link
Member

btangmu commented Jan 22, 2021

I switched to java version "11.0.2"
Now I still get:

[ERROR] .../cldr/tools/cldr-apps/src/main/java/org/unicode/cldr/web/ErrorSubtypes.java:[24,43] cannot find symbol
  symbol:   method toHTML(java.lang.String)
  location: class org.unicode.cldr.util.CLDRURLS

and then

...
[INFO] Listening for transport dt_socket at address: 7777
[INFO] Launching cldr (Open Liberty 20.0.0.12/wlp-1.0.47.cl201220201111-0736) on Java HotSpot(TM) 64-Bit Server VM, version 11.0.2+9-LTS (en_US)
[INFO] [AUDIT   ] CWWKE0001I: The server cldr has been launched.
[INFO] CWWKM2010I: Searching for CWWKF0011I: in /Users/tbishop/Documents/WenlinDocs/Organizations/Unicode/CLDR_job/cldr/tools/cldr-apps/target/liberty/wlp/usr/servers/cldr/logs/messages.log. This search will timeout after 90 seconds.
[INFO] [AUDIT   ] CWWKZ0058I: Monitoring dropins for applications.
[INFO] [AUDIT   ] CWWKT0016I: Web application available (default_host): http://192.168.0.120:9080/openapi/ui/
[INFO] [AUDIT   ] CWWKT0016I: Web application available (default_host): http://192.168.0.120:9080/openapi/
[INFO] [ERROR   ] CWWKO1650E: Validation of the OpenAPI document produced the following error(s): 
[INFO]                                                                                                                
[INFO]  - Message: Required "paths" field is missing or is set to an invalid value, Location: #
[INFO] 
[INFO] [AUDIT   ] CWWKT0016I: Web application available (default_host): http://192.168.0.120:9080/cldr-apps/
[INFO] [AUDIT   ] CWWKZ0001I: Application cldr-apps started in 10.497 seconds.
[INFO] [AUDIT   ] CWWKF0012I: The server installed the following features: [el-3.0, jaxrs-2.1, jaxrsClient-2.1, jdbc-4.3, jndi-1.0, jsonp-1.1, jsp-2.3, mpConfig-1.4, mpOpenAPI-1.1, servlet-4.0].
[INFO] [AUDIT   ] CWWKF0011I: The cldr server is ready to run a smarter planet. The cldr server started in 17.536 seconds.
[INFO] CWWKM2015I: Match number: 1 is [1/22/21, 9:49:58:673 EST] 00000022 com.ibm.ws.kernel.feature.internal.FeatureManager            A CWWKF0011I: The cldr server is ready to run a smarter planet. The cldr server started in 17.536 seconds..
[INFO] [ERROR   ] SRVE0319E: For the [SurveyMain] servlet, org.unicode.cldr.web.SurveyMain servlet class was found, but a resource injection failure has occurred. java.lang.reflect.InvocationTargetException
[INFO] [ERROR   ] SRVE0276E: Error while initializing Servlet [SurveyMain]: javax.servlet.UnavailableException: SRVE0319E: For the [SurveyMain] servlet, org.unicode.cldr.web.SurveyMain servlet class was found, but a resource injection failure has occurred. java.lang.reflect.InvocationTargetException
[INFO] 	at com.ibm.ws.webcontainer.servlet.ServletWrapper$1.run(ServletWrapper.java:1582)
[INFO] 	at [internal classes]
[INFO] 
[INFO] ************************************************************************
[INFO] *    Liberty is running in dev mode.
[INFO] *        To run tests on demand, press Enter.
[INFO] *        To restart the server, type 'r' and press Enter.
[INFO] *        To stop the server and quit dev mode, press Ctrl-C or type 'q' and press Enter.
[INFO] *        
[INFO] *    Liberty server port information:
[INFO] *        Liberty server HTTP port: [ 9080 ]
[INFO] *        Liberty debug port: [ 7777 ]
[INFO] ************************************************************************
[INFO] [AUDIT   ] CWWKT0017I: Web application removed (default_host): http://192.168.0.120:9080/cldr-apps/
[INFO] [AUDIT   ] CWWKZ0009I: The application cldr-apps has stopped successfully.
[INFO] [WARNING ] [ TargetsTableImpl@3e1d4b10 ] ANNO_TARGETS_CORRUPT_CLASS [ org.unicode.cldr.web.SurveyVettingParticipation ] (java.lang.ArrayIndexOutOfBoundsException: Index 6 out of bounds for length 0
[INFO] 	at org.objectweb.asm.ClassReader.readShort(ClassReader.java:3573)
[INFO] 	at [internal classes]
[INFO]  : Index 6 out of bounds for length 0)
[INFO] [WARNING ] Corrupt class
[INFO] Index 6 out of bounds for length 0
.../cldr/tools/cldr-apps/src/main/java/org/unicode/cldr/web/ErrorSubtypes.java:24: error: cannot find symbol
        r.put("CLDR_SUBTYPE_URL", CLDRURLS.toHTML(SubtypeToURLMap.getDefaultUrl()));
                                          ^
  symbol:   method toHTML(String)
  location: class CLDRURLS
[INFO] [ERROR   ] CWWKO1650E: Validation of the OpenAPI document produced the following error(s): 
[INFO]                                                                                                                
[INFO]  - Message: Required "paths" field is missing or is set to an invalid value, Location: #
...

@btangmu
Copy link
Member

btangmu commented Jan 22, 2021

Then I pasted into browser address bar: http://192.168.0.120:9080/cldr-apps/
Browser says:

HTTP Error Code:   500
--
Error Message:JSPG0049E: /index.jsp failed to compile : JSPG0091E: An error occurred at line: 60 in the file: /index.jspJSPG0093E: Generated servlet error from file: /index.jsp /Users/tbishop/Documents/WenlinDocs/Organizations/Unicode/CLDR_job/cldr/tools/cldr-apps/target/liberty/wlp/usr/servers/cldr/workarea/org.eclipse.osgi/211/data/temp/default_node/SMF_WebContainer/cldr-apps/cldr-apps/_index.java : 164 : StatisticsUtils cannot be resolved

@srl295
Copy link
Member Author

srl295 commented Jan 22, 2021

Then I pasted into browser address bar: http://192.168.0.120:9080/cldr-apps/
Browser says:

HTTP Error Code:   500
--
Error Message:JSPG0049E: /index.jsp failed to compile : JSPG0091E: An error occurred at line: 60 in the file: /index.jspJSPG0093E: Generated servlet error from file: /index.jsp /Users/tbishop/Documents/WenlinDocs/Organizations/Unicode/CLDR_job/cldr/tools/cldr-apps/target/liberty/wlp/usr/servers/cldr/workarea/org.eclipse.osgi/211/data/temp/default_node/SMF_WebContainer/cldr-apps/cldr-apps/_index.java : 164 : StatisticsUtils cannot be resolved

it sounds like it might just need a clean build… let's discuss on the call?

Copy link
Member

@btangmu btangmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to get it working in Eclipse. I think we need that before merging this to master.

@btangmu
Copy link
Member

btangmu commented Jan 22, 2021

I tried installing this: https://github.com/OpenLiberty/open-liberty-tools/files/5363408/openlibertytools-20.0.0.9.v2020-10-09_1337-with-fix.zip

Referenced from here: OpenLiberty/open-liberty-tools#407

I tried to create a new server by right-clicking in the Servers tab, but Open Liberty isn't one of the options.

More links:
OpenLiberty/blogs#791
OpenLiberty/open-liberty-tools#410

@srl295
Copy link
Member Author

srl295 commented Jan 22, 2021 via email

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .gitignore is different
  • tools/cldr-apps/.settings/org.eclipse.wst.common.project.facet.core.xml is different
  • tools/cldr-apps/pom.xml is different
  • tools/cldr-code/.settings/org.eclipse.wst.common.project.facet.core.xml is now changed in the branch
  • tools/cldr-rdf/.settings/org.eclipse.wst.common.project.facet.core.xml is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

- deployment is now different, but the github action has been updated
- for local dev: see cldr-apps/README.md
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/cldr-apps/src/main/liberty/config/server.xml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@btangmu btangmu self-requested a review January 26, 2021 02:04
Copy link
Member

@btangmu btangmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since your last change adding "localconnector-1.0" to server.xml (I guess that's what made the difference, though I'm not sure), the server finally appears to be running semi-normally for me in debug mode in Eclipse

The java console does have various errors like:

[WARNING ] table cldr_review_hide_40 did exist.
[ERROR   ] J2CA0081E: Method cleanup failed while trying to execute method cleanup on ManagedConnection WSRdbManagedConnectionImpl@7f7753d9 from resource jdbc/SurveyTool. Caught exception: com.ibm.ws.rsadapter.exceptions.DataStoreAdapterException: DSRA0080E: An exception was received by the Data Store Adapter. See original exception message: Cannot call 'cleanup' on a ManagedConnection while it is still in a transaction..
	at com.ibm.ws.rsadapter.impl.WSRdbManagedConnectionImpl.cleanupTransactions(WSRdbManagedConnectionImpl.java:3177)
	at [internal classes]
	at org.unicode.cldr.web.DBUtils.closeDBConnection(DBUtils.java:155)
	at org.unicode.cldr.web.DBUtils.close(DBUtils.java:950)
	at org.unicode.cldr.web.DBUtils.hasTable(DBUtils.java:479)
	at org.unicode.cldr.web.ReviewHide.createTable(ReviewHide.java:29)
	at org.unicode.cldr.web.UserRegistry.setupDB(UserRegistry.java:626)
	at org.unicode.cldr.web.UserRegistry.createRegistry(UserRegistry.java:586)

@srl295
Copy link
Member Author

srl295 commented Jan 26, 2021

@btangmu thanks for the thorough check, lets get this merged and then i'll work on the items mentioned above

@srl295 srl295 merged commit c1befe3 into unicode-org:master Jan 26, 2021
@srl295 srl295 deleted the cldr-14399-api branch January 26, 2021 14:58
@srl295
Copy link
Member Author

srl295 commented Jan 26, 2021

update-ansible-log.txt

here's the ansible log of updating smoketest…

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.

2 participants