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

use suro file's lastModify/dateCreated time for build upload path #147

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import com.fasterxml.jackson.annotation.JsonTypeInfo;

import java.io.File;

/**
* When uploading files to the remote file system such as S3 or HDFS, the file
* path should be specified. This interface is describing which file path would
Expand All @@ -27,5 +29,5 @@
*/
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type")
public interface RemotePrefixFormatter {
public String get();
public String get(File file);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;

import java.io.File;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.rmi.server.UID;
Expand Down Expand Up @@ -57,4 +58,9 @@ public static String get(String dir) {
.append(new UID().toString());
return sb.toString().replaceAll("[-:]", "");
}

public static DateTime getFileCreateTime(File file){
String fileName = file.getName();
return fmt.parseDateTime(fileName.substring(0,"P20141006T213521".length()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is P20141006T213521 hardcoded? This might need more explanation or comments.

I guess because of private static DateTimeFormatter fmt = DateTimeFormat.forPattern("'P'yyyyMMdd'T'HHmmss"); could you add comments?

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public void run() {
}

private String makeUploadPath(File file) {
return prefixFormatter.get() + file.getName();
return prefixFormatter.get(file) + file.getName();
}

@Monitor(name = "uploadedFileSize", type = DataSourceType.COUNTER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;

import java.io.File;

/**
* It would be useful to append region and stack information to the file path
* when we upload files to AWS S3. region and stack can be injected through
Expand All @@ -49,7 +51,7 @@ public DateRegionStackFormatter(
}

@Override
public String get() {
public String get(File file) {
StringBuilder sb = new StringBuilder();
sb.append(format.print(new DateTime())).append('/')
.append(region).append('/')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.netflix.suro.sink.remotefile.RemotePrefixFormatter;

import java.io.File;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -22,7 +23,7 @@ public DynamicRemotePrefixFormatter(@JsonProperty("format") String formatString)
}

@Override
public String get() {
public String get(File file) {
StringBuilder sb = new StringBuilder();

for (PrefixFormatter formatter : formatterList) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.netflix.suro.sink.remotefile.formatter;
Copy link
Contributor

Choose a reason for hiding this comment

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

I need a copyright

/*

  • Copyright 2014 Netflix, Inc.
    *
  • Licensed 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.
    */


import com.fasterxml.jackson.annotation.JsonProperty;
import com.netflix.suro.sink.localfile.FileNameFormatter;
import com.netflix.suro.sink.remotefile.RemotePrefixFormatter;
import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;

import java.io.File;

/**
* Created by [email protected] on 10/6/14.
*/
public class FileDatePrefixFormatter implements RemotePrefixFormatter{
Copy link
Contributor

Choose a reason for hiding this comment

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

We need the unit test for this class even though this class's logic is simple.

public static final String TYPE = "FileDate";
private final DateTimeFormatter formatter;
//lastModify or dateCreated
private String dateType;

public FileDatePrefixFormatter(@JsonProperty("format") String formatString,
@JsonProperty("dateType") String dateType){
formatter = DateTimeFormat.forPattern(formatString);
this.dateType = dateType;

}

@Override
public String get(File file) {
String prefix = formatter.print(getDateCreated(file));//use dateCreated as default.
if("lastModify".equalsIgnoreCase(dateType)){
Copy link
Contributor

Choose a reason for hiding this comment

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

lastModified would be better

prefix = formatter.print(file.lastModified());
}
if(!prefix.endsWith("/")) prefix += "/";
return prefix;
}

private DateTime getDateCreated(File file){
return FileNameFormatter.getFileCreateTime(file);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.netflix.suro.sink.notice.QueueNotice;
import com.netflix.suro.sink.remotefile.formatter.DateRegionStackFormatter;
import com.netflix.suro.sink.remotefile.formatter.DynamicRemotePrefixFormatter;
import com.netflix.suro.sink.remotefile.formatter.FileDatePrefixFormatter;

public class SuroSinkPlugin extends SuroPlugin {
@Override
Expand All @@ -17,6 +18,7 @@ protected void configure() {
this.addSinkType(HdfsFileSink.TYPE, HdfsFileSink.class);
this.addRemotePrefixFormatterType(DateRegionStackFormatter.TYPE, DateRegionStackFormatter.class);
this.addRemotePrefixFormatterType(DynamicRemotePrefixFormatter.TYPE, DynamicRemotePrefixFormatter.class);
this.addRemotePrefixFormatterType(FileDatePrefixFormatter.TYPE,FileDatePrefixFormatter.class);

this.addSinkType(SuroSink.TYPE, SuroSink.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void testDynamicStatic() throws IOException {

ObjectMapper mapper = injector.getInstance(ObjectMapper.class);
RemotePrefixFormatter formatter = mapper.readValue(spec, new TypeReference<RemotePrefixFormatter>(){});
assertEquals(formatter.get(), "prefix/");
assertEquals(formatter.get(null), "prefix/");
}

@Test
Expand All @@ -69,7 +69,7 @@ public void testDynamicDate() throws IOException {
RemotePrefixFormatter formatter = mapper.readValue(spec, new TypeReference<RemotePrefixFormatter>(){});

DateTimeFormatter format = DateTimeFormat.forPattern("YYYYMMDD");
assertEquals(formatter.get(), format.print(new DateTime()) + "/");
assertEquals(formatter.get(null), format.print(new DateTime()) + "/");
}

@Test
Expand All @@ -84,7 +84,7 @@ public void testDynamicProperty() throws IOException {
ObjectMapper mapper = injector.getInstance(ObjectMapper.class);
RemotePrefixFormatter formatter = mapper.readValue(spec, new TypeReference<RemotePrefixFormatter>(){});

assertEquals(formatter.get(), "prop1/");
assertEquals(formatter.get(null), "prop1/");
}

@Test
Expand All @@ -101,7 +101,7 @@ public void testDynamicCombination() throws IOException {
ObjectMapper mapper = injector.getInstance(ObjectMapper.class);
RemotePrefixFormatter formatter = mapper.readValue(spec, new TypeReference<RemotePrefixFormatter>(){});

assertEquals(formatter.get(), "routing_key/" + format.print(new DateTime()) + "/propvalue1/");
assertEquals(formatter.get(null), "routing_key/" + format.print(new DateTime()) + "/propvalue1/");
}


Expand All @@ -116,7 +116,7 @@ public void testInjectedDateRegionStack() throws IOException {
RemotePrefixFormatter formatter = mapper.readValue(spec, new TypeReference<RemotePrefixFormatter>() {});
DateTimeFormatter format = DateTimeFormat.forPattern("YYYYMMDD");
String answer = String.format("%s/eu-west-1/gps/", format.print(new DateTime()));
assertEquals(formatter.get(), answer);
assertEquals(formatter.get(null), answer);
}

@Test
Expand All @@ -132,6 +132,6 @@ public void testDateRegionStack() throws IOException {
RemotePrefixFormatter formatter = mapper.readValue(spec, new TypeReference<RemotePrefixFormatter>() {});
DateTimeFormatter format = DateTimeFormat.forPattern("YYYYMMDD");
String answer = String.format("%s/us-east-1/normal/", format.print(new DateTime()));
assertEquals(formatter.get(), answer);
assertEquals(formatter.get(null), answer);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.netflix.suro.sink.remotefile.S3FileSink;
import com.netflix.suro.sink.remotefile.formatter.DateRegionStackFormatter;
import com.netflix.suro.sink.remotefile.formatter.DynamicRemotePrefixFormatter;
import com.netflix.suro.sink.remotefile.formatter.FileDatePrefixFormatter;

/**
*
Expand All @@ -35,6 +36,7 @@ protected void configure() {
this.addSinkType(HdfsFileSink.TYPE, HdfsFileSink.class);
this.addRemotePrefixFormatterType(DateRegionStackFormatter.TYPE, DateRegionStackFormatter.class);
this.addRemotePrefixFormatterType(DynamicRemotePrefixFormatter.TYPE, DynamicRemotePrefixFormatter.class);
this.addRemotePrefixFormatterType(FileDatePrefixFormatter.TYPE,FileDatePrefixFormatter.class);

this.addSinkType(SuroSink.TYPE, SuroSink.class);

Expand Down