FALCON-2188 Rest api to register extension #304

PraveenAdlakha · 2016-11-19T06:52:56Z

$ bin/falcon extension -register -extensionName test -path hdfs://hostname:8020/user/dataqa/sampleextension -description "test ing"

Extension:testregistered succesfully.

Also we get file not found exception if jars or Readme is not present please look at the test cases.

pallavi-rao

One uber question I had regarding register, unregister etc. These calls must only go to Prism (in distributed mode) and the metadata store is only present on Prism. How/where are you handling in the code to ensure this?

pallavi-rao · 2016-11-21T03:33:45Z

cli/src/main/java/org/apache/falcon/cli/FalconExtensionCLI.java

@@ -158,6 +167,7 @@ public Options createExtensionOptions() {
        Option resume = new Option(FalconCLIConstants.RESUME_OPT, false, "Resume an extension job");
        Option delete = new Option(FalconCLIConstants.DELETE_OPT, false, "Delete an extension job");
        Option unregister = new Option(FalconCLIConstants.UREGISTER, false, "Delete metadata of extension job");
+        Option register = new Option(FalconCLIConstants.REGISTER, false, "Add metadata of extension job");


Description can be better. "Register an extension with Falcon. This will make the extension available for instantiation for all users.

pallavi-rao · 2016-11-21T03:38:02Z

prism/src/main/java/org/apache/falcon/resource/extensions/ExtensionManager.java

+        checkIfExtensionServiceIsEnabled();
+        validateExtensionName(extensionName);
+        try {
+            return ExtensionStore.get().registerExtensionMetadata(extensionName, path, description);


Add another validation to ensure description is not empty.

had a discussion with @sandeepSamudrala and decided it can be a optional field.Let me know if you think otherwise.

Fine. That is ok. Thanks for clarifying.

pallavi-rao · 2016-11-21T04:12:19Z

extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java

+                return file.getName().endsWith(".jar");
+            }
+        };
+        FileStatus[] fileStatusJar = fileSystem.listStatus(new Path(uri.getPath() + "/libs"), filter);


Believe we should check under libs/build.

Then we need to correct the doc too because in the doc there is no mention of libs/build or libs/runtime.

Which doc are you talking about? The design doc has it.

pallavi-rao · 2016-11-21T04:15:22Z

extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java

+        URI uri = new URI(path);
+        conf.set("fs.default.name", uri.getScheme() + "://" + uri.getAuthority());
+        FileSystem fileSystem =  HadoopClientFactory.get().createFalconFileSystem(uri);
+        FileStatus[] fileStatusReadMe = fileSystem.listStatus(new Path(uri.getPath() + "/README"));


Guess it is sufficient to check for the existence of README and check if it is a file.

pallavi-rao · 2016-11-21T04:16:06Z

extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java

+        };
+        FileStatus[] fileStatusJar = fileSystem.listStatus(new Path(uri.getPath() + "/libs"), filter);
+
+        if (fileStatusReadMe.length == 1 && fileStatusJar.length >0){


Additional check for existence at least 1 file under META is required.

pallavi-rao · 2016-11-21T04:20:14Z

extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java

+        if (fileStatusReadMe.length == 1 && fileStatusJar.length >0){
+            metaStore.storeExtensionMetadataBean(extensionName, path, ExtensionType.CUSTOM, description);
+        }else{
+            throw new ValidationException("Packaging structure is not currect please refer to documentation");


Error message : Packaging structure of extension ${extensionName} is not correct. Please refer the documentation for details

Also, it is preferable to do individual checks and print the appropriate error message, rather than asking user to refer docs.

PraveenAdlakha · 2016-11-21T07:09:23Z

@pallavi-rao ExtensionService will only be up at prism and that will take care of the requests. Please correct if I am missing some thing here.

pallavi-rao · 2016-11-21T07:23:08Z

@PraveenAdlakha ExtensionService may be up only for Prism. However, ExtensionManager is available for prism and falcon webapps. Hence the request will be accepted and may result in an improper error.

PraveenAdlakha · 2016-11-21T08:26:18Z

ohhkk got it..Then we can do what we are doing with EntityManager and InstanceManager for better error handling.Will take it up as a separate jira.

pallavi-rao · 2016-11-21T07:17:43Z

extensions/src/main/java/org/apache/falcon/extensions/store/ExtensionStore.java

+                return file.getName().endsWith(".jar");
+            }
+        };
+        FileStatus[] fileStatusJar = fileSystem.listStatus(new Path(uri.getPath() + "/libs"), filter);


Which doc are you talking about? The design doc has it.

pallavi-rao · 2016-11-21T07:18:20Z

prism/src/main/java/org/apache/falcon/resource/extensions/ExtensionManager.java

+        checkIfExtensionServiceIsEnabled();
+        validateExtensionName(extensionName);
+        try {
+            return ExtensionStore.get().registerExtensionMetadata(extensionName, path, description);


Fine. That is ok. Thanks for clarifying.

sandeepSamudrala · 2016-11-19T06:58:26Z

extensions/src/test/java/org/apache/falcon/extensions/store/ExtensionStoreTest.java

+
+    @BeforeMethod
+    public void clean() {
+        clear();


clean up the file system too

pallavi-rao · 2016-11-22T03:22:25Z

Will merge this. There is a test case that this PR breaks. I will address that with a separate PR as @PraveenAdlakha is out on vacation.

@sandeepSamudrala

$ bin/falcon extension -register -extensionName test -path hdfs://hostname:8020/user/dataqa/sampleextension -description "test ing" Extension:testregistered succesfully. Also we get file not found exception if jars or Readme is not present please look at the test cases. Author: Praveen Adlakha <adlakha.praveen@gmail.com> Reviewers: @sandeepSamudrala, @pallavi-rao Closes apache#304 from PraveenAdlakha/2188 and squashes the following commits: e4c21a0 [Praveen Adlakha] test case disabled 90e6ec9 [Praveen Adlakha] minor comments addressed 52f6ebc [Praveen Adlakha] comments addressed 05c9c5e [Praveen Adlakha] comments addressed and test cases updated 9a74a2c [Praveen Adlakha] FALCON-2188 Rest api to register extension

pallavi-rao suggested changes Nov 21, 2016

View reviewed changes

PraveenAdlakha added 4 commits November 21, 2016 17:59

FALCON-2188 Rest api to register extension

9a74a2c

comments addressed and test cases updated

05c9c5e

comments addressed

52f6ebc

minor comments addressed

90e6ec9

PraveenAdlakha force-pushed the 2188 branch from 420b779 to 90e6ec9 Compare November 21, 2016 12:40

test case disabled

e4c21a0

sandeepSamudrala reviewed Nov 21, 2016

View reviewed changes

asfgit closed this in fd384a7 Nov 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FALCON-2188 Rest api to register extension #304

FALCON-2188 Rest api to register extension #304

PraveenAdlakha commented Nov 19, 2016

pallavi-rao left a comment

pallavi-rao Nov 21, 2016

pallavi-rao Nov 21, 2016

PraveenAdlakha Nov 21, 2016

pallavi-rao Nov 21, 2016

pallavi-rao Nov 21, 2016

PraveenAdlakha Nov 21, 2016

pallavi-rao Nov 21, 2016

pallavi-rao Nov 21, 2016

pallavi-rao Nov 21, 2016

pallavi-rao Nov 21, 2016

pallavi-rao Nov 21, 2016

PraveenAdlakha commented Nov 21, 2016

pallavi-rao commented Nov 21, 2016

PraveenAdlakha commented Nov 21, 2016

pallavi-rao Nov 21, 2016

pallavi-rao Nov 21, 2016

sandeepSamudrala Nov 19, 2016

pallavi-rao commented Nov 22, 2016

FALCON-2188 Rest api to register extension #304

FALCON-2188 Rest api to register extension #304

Conversation

PraveenAdlakha commented Nov 19, 2016

pallavi-rao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PraveenAdlakha commented Nov 21, 2016

pallavi-rao commented Nov 21, 2016

PraveenAdlakha commented Nov 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pallavi-rao commented Nov 22, 2016