Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220397187
@rhtyd thanks, just wanted to make sure we extracted anything that was
useful from @anshul1886's experience. Thanks for working this out guys...
---
If your
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220389940
@swill the problem was environment dependent where an unclean environment
was used to build/run management server, it is recommended to git clean -fdx
one's local
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220385286
Thanks guys for getting that sorted out. Is the problems that @anshul1886
ran into something that others could potentially run into? Would it be useful
to document
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220243535
@rhtyd It looks fine now. It was present in some tomcat folder.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220241843
@swill I had a GTM with Anshul to figure out the issue, his environment is
not clean and I've asked him to setup a new environment (effectively a new dev
box). I've
Github user abhinandanprateek commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220239213
@rhtyd @anshul1886 This week I rebased the ospf changes with current master
and did not have any issues. OSPF adds 2 new APIs and they are using the new
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220238585
@anshul1886 if you need help join this GTM now:
https://app.gotomeeting.com/?meetingId=981540549 (or use the meeting id for a
desktop client). I've personally tested
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220238146
dev machine which I am using for last 2-3 years
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220237753
@anshul1886 is this environment installed using rpms/deb or it's a
developer machine with Ubuntu 12.04? Would you prefer a GTM to get to the
bottom of the issue,
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220237502
@rhtyd Default value for dynamic.apichecker.enabled is true. role_id for
default account is 1.
---
If your project is set up for it, you can reply to this
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220236230
https://cloud.githubusercontent.com/assets/1712815/15384187/2e8d4aa2-1db6-11e6-9358-659ff6f6df3f.png;>
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220230192
@anshul1886 I'm yet to see chrome dev-tool screenshots, actual errors
reported in the UI and see answers for questions I asked; again -- the role_id
in accounts
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220227876
@rhtyd Its a clean setup. It is not even complete because of one other bug
introduced by PR #816.
Summary of the steps:
1. mvn -P systemvm
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220094101
@rhtyd thank you sir. I am trying to get everything ready to freeze. Damn
this is a lot of work. :(
---
If your project is set up for it, you can reply to this
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220093571
@swill yes sir. I'll personally make sure to perform another test round
tomorrow both with UI, marvin and APIs/cloudmonkey.
---
If your project is set up for it,
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220090972
I agree. But I have not tested the UI at all. Has your team been testing
through the UI as well as through the API?
---
If your project is set up for it, you can
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220090191
@swill Travis and your CI are passing with this feature during which a data
center is deployed and all sorts of APIs are called so it's likely an env issue
specific
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220037863
If we have to revert this, it will be a mess. Lets try to get to the
bottom of this ASAP as the freeze is basically here...
---
If your project is set up for it,
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-220005496
@anshul1886 okay, let's debug;
- After you mvn re-built cleanly, did you run mvn deploydb?
- If yes, what is the output of: select * from configuration where
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-219990167
@rhtyd I have rebuilt my branch which was rebased yesterday. I am creating
new setup. There is no new API introduced in my branch. I am talking about
existing
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-219988321
@anshul1886 with dynamic-roles feature, we no longer want to use or promote
use of commands.properties file which is why this has been removed in master.
We no
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-219987354
@rhtyd Is commands.properties exist in your setup or not? If it exists then
things are working fine.
---
If your project is set up for it, you can reply to
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-219986204
@anshul1886 I'm unable to reproduce the shared issues nor they were caught
by CI/Jenkins/Travis runs. The role specific rules in role_permissions table
have been
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-219980293
@rhtyd There are many APIs. I have not prepared any list. But creating
setup from scratch should reproduce it. Some operations in which it can be
observed
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-219953009
@anshul1886 can you share the details, which APIs. Also, are you facing
this on master or on your personal/feature branch?
---
If your project is set up for it, you
Github user anshul1886 commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-219944062
@swill @rhtyd Some APIs are giving permission exceptions. They can be seen
by just logging into UI. I have added the commands.properties to overcome these
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-218370503
Thanks you @swill finally :smile:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1489
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-218369488
@swill all green now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-218359137
Thank you. I am really trying to avoid that, but lets see what happens. :)
---
If your project is set up for it, you can reply to this email and have your
reply
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-218358868
@swill done, though if it fails again we may ignore that as long as travis
is green
---
If your project is set up for it, you can reply to this email and have your
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-218358102
thank you sir. can you repush right away again. jenkins already failed
and I like to make sure everything is green before i merge stuff. thx...
---
If your
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-218357625
@swill I've fixed the issues, here are the changes:
[changes.diff.txt](https://github.com/apache/cloudstack/files/258551/changes.diff.txt)
The conflicts
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-218356331
Sorry to do this to you @rhtyd, but I just merged like 10 PRs and we now
have merge conflicts. Can you resolve the merge conflicts and re-push. When
you resolve
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217855833
We have the required code reviews and CI results. I will add this to merge
queue. Thx...
---
If your project is set up for it, you can reply to this email and
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217774436
@swill thanks, let's merge this
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217606327
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 2
Errors: 0
Duration: 6h 11m 18s
```
**Summary of the
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217500553
Thanks @swill as soon as we can merge this one, I can rebase and use some
of the annotations work and ListUtils from this PR into out-of-band management
PR /cc
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217456452
I will get this CI'ed. I have 4 new CI boxes in play now, but I am trying
to figure out how to stabilize master, so I will run this as soon as I get
master sorted
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217450750
@rhtyd @swill LGTM based on code review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217413190
@swill this PR is ready for CI test run and merge, thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217343619
@swill rebased against latest master and pushed
@borisstoyanov can you help test this one last time, I think all major
issues including ordering related issues
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-217264750
@rhtyd we have merge conflicts. can you fix?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216851115
We performed end-to-end testing of this PR by building packages today [1]
and found an issue with maven (not related to this PR) where users get sent to
error.jsp
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216749200
@jburwell The API is transactional and you're right if more than one admin
decide to change the order, client side final order checking will be needed.
This is also
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216734107
@rhtyd I am trying to understand how reordering should work. User creates
a role with rules ordered as follows:
1. Rule A
1. Rule B
1. Rule C
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216603249
@swill I'm fine with all changes, I've ran a final set of tests as well.
@jburwell please share any outstanding issue that should be fixed,
@borisstoyanov and I are
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216559351
Great, thanks for the additional details @borisstoyanov. ð Once Rohit
and John are in agreement on the final details, I think we are ready to merge
this one.
Github user borisstoyanov commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216550819
Thanks @swill as Rohit mentioned we did migration testing of 4.8.1 to 4.9
without enabling the feature, to confirm users are able to continue using CS
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216488843
@jburwell I did not get the question, is it for oobm? dynamic-checker has
no update counter in any of the api or schema. If this was for oobm, the update
counter is
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216487485
@swill I think you mentioned the wrong Boris :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216486309
@rhtyd for optimistic locking, is the update counter returned on the get
APIs and required on the update APIs?
---
If your project is set up for it, you can
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216441837
Thanks @rhtyd, this makes me feel a little more comfortable. @borisroman
if you can give us a bit of an outline of what you have tested, I think it will
help
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216435191
@swill yes indeed we've performed upgrade tests with 4.8.1 to
4.9.0-SNAPSHOT (not using the feature by default) and then using
migrate-dyanamicroles.py script; the
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216231216
I will make sure this gets into 4.9 because I think it is really needed.
We do need one more LGTM on the code. Also, since this PR does have some
implications
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-216228334
The PR is ready for merge, any testing/feedback welcome /cc @swill
tag:mergeready
---
If your project is set up for it, you can reply to this email and
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215889804
@swill fixed the issue of orderable permissions, PR is ready for
testing/merge
---
If your project is set up for it, you can reply to this email and have your
reply
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215501683
@swill existing users won't be automatically migrated to use this feature,
see the admin-doc PR that documents the upgrade/migrate procedure; though new
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215479369
In reading this, is it the case that if someone upgrades from a previous
version to 4.9 (with this code included) they will not be able to take
advantage of this
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215389142
+1 on the feature. Based on testing done on the simulator with root admin
and normal user for a few APIs
+0 on the immediate removal of
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215352448
@DaanHoogland @wido @mlsorensen @jburwell @agneya2001 @koushik-das
@terbolous @resmo @K0zka review and LGTM please?
@swill I think we've resolved all
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215291402
The CI results show this PR to be in pretty good shape. We have one LGTM
and lots of commentary. Can we get a summary of any concerns and get our code
reviews in?
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215290621
### CI RESULTS
```
Tests Run: 100
Skipped: 1
Failed: 0
Errors: 0
```
**Associated Uploads**
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215197356
Rebased against latest master, please consider this for merge
Performed final rounds of testing:
- Build/maven, unit and integration tests
- Upgrade
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215171740
@koushik-das I don't understand how adoption of this feature differs from
any other feature. A user deploys the release into a test/pilot environment
and
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215165382
@mlsorensen
- presently, setting bitmask to 0 in commands.properties does not disable
the API but that no roles would have that API; though if an API through its
Github user mlsorensen commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215147096
@koushik-das -- I'd say yes, that testing is good enough. If a test can
determine that the default roles allow the expected access to the APIs then
we're good.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-215008749
>>That is to say that the API permissions on a fresh install before and
after this is merged should behave the same out of the box. If that's the case
then I
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214960750
@mlsorensen +1 -- well said. command.properties presents a number of
significant problems. First, in a clustered environment, it is possible (and
quite easy)
Github user mlsorensen commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214843061
It seems complicated to try to actively promote a choice of multiple ways
to check for API authorization (dynamic vs static). I am for deprecating
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214728007
@rhtyd I think you still didn't get my point. Read the comment again. I
would like to see the option for using command.properties available for both
new and
Github user borisstoyanov commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214725108
LGTM. :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user borisstoyanov commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214724962
LGTM. :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user borisstoyanov commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214724784
LGTM. :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214723830
> @rhtyd The current way you have implemented it using global config is
confusing. Especially when you say that simply disabling the flag won't fall
back to the
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214707925
>>Consider this an admin creates a read-only admin role and accounts with
that (such users can only do list* calls), now if they go back to
static-checker all
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60944411
--- Diff: test/integration/smoke/test_dynamicroles.py ---
@@ -0,0 +1,478 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214430311
> @rhtyd My comment regarding the test was more in the context of perf.
test. In the DB for regular user I saw ~250 permissions got created. So this
means iterating
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60938011
--- Diff:
plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
---
@@ -0,0 +1,167 @@
+// Licensed to
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60937993
--- Diff:
plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
---
@@ -0,0 +1,167 @@
+// Licensed to
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214296336
@rhtyd My comment regarding the test was more in the context of perf. test.
In the DB for regular user I saw ~250 permissions got created. So this means
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214281121
@koushik-das can you share what you think are the advantages of static
role-base api checker?
---
If your project is set up for it, you can reply to this email and
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214276787
@koushik-das Yes, all tests run as a regular user too. See the integration
test, we're using user api clients (search self.getUserApiClient) to perform
tests -- i.e.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214269005
@rhtyd Have you ran the tests using a regular user? As per dynamic checker
code, for root admin all checks are bypassed. I think a good comparison will be
to
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214240215
I've fixed all outstanding issues, please comment if you see any
outstanding issue.
LGTMs welcome, thanks.
---
If your project is set up for it, you can reply
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60882026
--- Diff:
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
---
@@ -119,6 +131,9 @@ private Long
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60881906
--- Diff:
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java
---
@@ -70,10 +72,12 @@
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60881499
--- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java ---
@@ -53,6 +62,139 @@ public boolean supportsRollingUpgrade() {
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60881018
--- Diff: server/src/org/apache/cloudstack/acl/RoleManagerImpl.java ---
@@ -0,0 +1,273 @@
+// Licensed to the Apache Software Foundation (ASF) under
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60880846
--- Diff: test/integration/smoke/test_dynamicroles.py ---
@@ -0,0 +1,474 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60880204
--- Diff: test/integration/smoke/test_dynamicroles.py ---
@@ -0,0 +1,474 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60880231
--- Diff: test/integration/smoke/test_dynamicroles.py ---
@@ -0,0 +1,474 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60880213
--- Diff: test/integration/smoke/test_dynamicroles.py ---
@@ -0,0 +1,474 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60880195
--- Diff: test/integration/smoke/test_dynamicroles.py ---
@@ -0,0 +1,474 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60880160
--- Diff: test/integration/smoke/test_dynamicroles.py ---
@@ -0,0 +1,474 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60879938
--- Diff: utils/src/main/java/com/cloud/utils/PropertiesUtil.java ---
@@ -34,6 +34,10 @@
public class PropertiesUtil {
private static final
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-214212648
@koushik-das this is part of the feature to be able to check access based
on rules in DB and be consistent across all mgmt servers. In my local
environment with
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1489#issuecomment-213693730
I did some basic tests yesterday. Some comments based on that. Will do some
more testing next week.
- I see that commands.properties is removed altogether
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60802098
--- Diff: utils/src/main/java/com/cloud/utils/PropertiesUtil.java ---
@@ -34,6 +34,10 @@
public class PropertiesUtil {
private static
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60801905
--- Diff:
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
---
@@ -119,6 +131,9 @@ private Long
1 - 100 of 240 matches
Mail list logo
