[GitHub] necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module
necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract
Permission Management from JDBC Authentication Module
URL: https://github.com/apache/guacamole-client/pull/359#discussion_r249314671
##
File path:
extensions/guacamole-auth-common-base/modules/guacamole-auth-common-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ObjectEntityModelInterface.java
##
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.jdbc.base;
+
+public interface ObjectEntityModelInterface extends ObjectModelInterface,
EntityModelInterface {
Review comment:
Does this class have a purpose?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module
necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module URL: https://github.com/apache/guacamole-client/pull/359#discussion_r249313520 ## File path: .gitignore ## @@ -1,2 +1,3 @@ *~ target/ +/.project Review comment: I don't think we want to add this. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module
necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract
Permission Management from JDBC Authentication Module
URL: https://github.com/apache/guacamole-client/pull/359#discussion_r249314760
##
File path:
extensions/guacamole-auth-common-base/modules/guacamole-auth-common-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionAttributeModelInterface.java
##
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.jdbc.connection;
+
+public interface ConnectionAttributeModelInterface {
Review comment:
An empty interface?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module
necouchman commented on a change in pull request #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module URL: https://github.com/apache/guacamole-client/pull/359#discussion_r249313585 ## File path: extensions/guacamole-auth-common-base/modules/guacamole-auth-common-base/pom.xml ## @@ -0,0 +1,104 @@ + + +http://maven.apache.org/POM/4.0.0; +xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; +xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 +http://maven.apache.org/maven-v4_0_0.xsd;> + +4.0.0 +org.apache.guacamole +guacamole-auth-common-base +jar +guacamole-auth-common-base +http://guacamole.apache.org/ + + +UTF-8 + + + +org.apache.guacamole +guacamole-auth-common +1.0.0 +../../ + + + + + + Review comment: This might be based on an older version of the Guacamole code, as, since 1.0.0 almost everything is written for 1.8. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] necouchman commented on issue #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module
necouchman commented on issue #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module URL: https://github.com/apache/guacamole-client/pull/359#issuecomment-455933119 Commits look much better, thanks! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
Re: [DISCUSS] Beyond 1.0.0
On Fri, Jan 18, 2019 at 12:52 PM Nick Couchman wrote: > On Fri, Jan 18, 2019 at 2:37 PM Mike Jumper wrote: > > > On Thu, Jan 17, 2019, 13:02 Nick Couchman > > > > On Fri, Jan 11, 2019 at 9:56 PM Mike Jumper > wrote: > > > > > > > > > > > Another option might be to provide some sort of API compatibility > layer > > > > within the webapp, allowing the extensions of prior releases to > > continue > > > to > > > > function. If compatibility doesn't break as a result of API changes, > > > > there's no need for a full major version bump, and downstream > migration > > > for > > > > future releases is much easier. > > > > > > > > > > > I'm open to this, as well. How much effort do we think would be > involved > > > in introducing something like this? > > > > > > > A compat layer would be pretty tricky. > > > > If we can somehow modify the API changes such that things are inherently > > compatible, then fairly easy. Maybe something can be done with default > > methods now that we're Java 8+? > > > > So, for a change like this (to the Connectable interface): > > https://github.com/apache/guacamole-client/commit/dfd43327618bd625bac7ce4fd35f9ccfe729ec6e#diff-1d2cb5f9d0009ea051d8a6211b20aaea > > something like this: > > https://github.com/necouchman/guacamole-client/commit/d53a6c3be576526ace6acf0432cab2480785a0ae > > ?? > > Something similar to that, yes. We would need default implementations for both old and new versions of connect(): https://github.com/mike-jumper/guacamole-client/commit/4a1527b1d4311bbf9d76468141dc68d02a90efa4 We would still run into trouble with the SimpleConnection class for any downstream uses which override the old connect(). Those overrides would suddenly cease to have any effect. Further, if such a downstream use also uses SimpleAuthenticationProvider, they might expect the provided GuacamoleConfiguration to already have tokens applied (the old behavior) rather than dynamically applied upon connect() (the new behavior). Maybe it's not possible without a compatibility layer... > On another note, I have started looking at the changes required to support > FreeRDP 2.0. I know you asked for help on this a while back, with no > takers. It might take me 10x as long as someone else to figure it out, but > it's clear from some recent traffic on the list that it's going to be > pretty important that we support FreeRDP 2.0 going forward. So, time to > tackle it :-). Unfortunately just an hour or so into it and I already want > to run my head through a brick wall. Speaking of API compatibility, these > changes are not insignificant... Yeah, that's pretty typical in my experience of FreeRDP API changes. I did some work recently trying to abstract away the VNC support from the underlying library, and I think we can do the same with the RDP - allowing FreeRDP 1.x and 2.x to be alternative backends fo the same support. It's not the easiest thing ever, but it wasn't too horrible. I'll give it a shot. - Mike
[GitHub] daniquir commented on issue #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module
daniquir commented on issue #359: GUACAMOLE-617: Extract Permission Management from JDBC Authentication Module URL: https://github.com/apache/guacamole-client/pull/359#issuecomment-455874524 Well, it seems that the cleaning of commits has gone well, please check it in case I have made an error. Thanks for the info. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
