Skip to content

Commit

Permalink
Fix fetching single files from local clone
Browse files Browse the repository at this point in the history
  • Loading branch information
blacelle committed Oct 28, 2021
1 parent 11c7877 commit 1028492
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ protected AtomicLongMap<String> processFiles(ICodeProvider pr,
Thread.currentThread().interrupt();
throw new RuntimeException(e);
} catch (ExecutionException e) {
throw new RuntimeException("Arg", e);
throw new RuntimeException("Issue while one of the asynchronous tasks", e);
}
}

Expand Down Expand Up @@ -286,7 +286,9 @@ public String loadCodeOptMutated(ICodeProvider codeProvider,
String code = optAlreadyMutated.orElseGet(() -> {
try {
Optional<String> optContent = codeProvider.loadContentForPath(filePath);
return optContent.get();

return optContent
.orElseThrow(() -> new IllegalStateException("Issue fiding code for path=" + filePath));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,17 @@ public String deprecatedGetFilePath(Object file) {

@Override
public Optional<String> loadContentForPath(String path) throws IOException {
try {
return Optional.of(loadContent(repo, path, getSha1()));
} catch (GHFileNotFoundException e) {
LOGGER.trace("We miss: {}", path, e);
LOGGER.debug("We miss: {}", path);
return Optional.empty();
if (helper.localClone.get() != null) {
// We have a local clone: load the file from it
return helper.localClone.get().loadContentForPath(path);
} else {
try {
return Optional.of(loadContent(repo, path, getSha1()));
} catch (GHFileNotFoundException e) {
LOGGER.trace("We miss: {}", path, e);
LOGGER.debug("We miss: {}", path);
return Optional.empty();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public class GithubSha1CodeProviderHelper {
private static final Logger LOGGER = LoggerFactory.getLogger(GithubSha1CodeProviderHelper.class);

// To be compared with the limit of 5000 calls per hour per installation
private static final int MAX_FILE_BEFORE_CLONING = 512;

private static final boolean ZIP_ELSE_CLONE = true;
Expand All @@ -58,6 +59,10 @@ public void listFiles(Consumer<ICodeProviderFile> consumer) throws IOException {
localClone.get().listFiles(consumer);
}

/**
*
* @return true if we indeed clone locally. False if already cloned locally
*/
@SuppressWarnings("PMD.CloseResource")
protected boolean ensureLocalClone() {
// TODO Tests against multiple calls: the repo shall be cloned only once
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,16 @@ public void doExecuteWebhookEvent(ICodeCleanerFactory cleanerFactory, IWebhookEv
} else {
LOGGER.debug("The changes would have been committed directly in the head branch");
}

// This is useful to investigate unexpected rateLimitHit
{
try {
GHRateLimit rateLimit = github.getRateLimit();
LOGGER.info("After process, rateLimit={} for installationId={}", rateLimit, installationId);
} catch (IOException e) {
LOGGER.warn("Issue with RateLimit", e);
}
}
}

public Supplier<GHRef> prepareHeadSupplier(WebhookRelevancyResult relevancyResult,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package eu.solven.cleanthat.code_provider.github.refs;

import java.util.Objects;
import java.util.concurrent.atomic.AtomicReference;

import org.kohsuke.github.GHRef;
import org.kohsuke.github.GHRepository;

import eu.solven.cleanthat.code_provider.github.code_provider.AGithubSha1CodeProvider;
import eu.solven.cleanthat.codeprovider.ICodeProvider;
import eu.solven.cleanthat.jgit.JGitCodeProvider;

/**
* An {@link ICodeProvider} for Github pull-requests
Expand All @@ -18,8 +16,6 @@
public class GithubRefCodeProvider extends AGithubSha1CodeProvider {
final GHRef ref;

final AtomicReference<JGitCodeProvider> localClone = new AtomicReference<>();

public GithubRefCodeProvider(String token, GHRepository repo, GHRef ref) {
super(token, repo);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ public List<IClassTransformer> getTransformers() {

@Override
public void registerCodeStyleFixer(IStyleEnforcer codeStyleFixer) {
LOGGER.info("We register {} into {}", codeStyleFixer, this);
// TODO This could be in INFO, but it is called once per file (unexpectedly)
LOGGER.debug("We register {} into {}", codeStyleFixer, this);
this.optCodeStyleFixer = Optional.of(codeStyleFixer);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public abstract class AWebhooksLambdaFunction extends ACleanThatXxxFunction {
} else {
// This would happen on Lambda direct invocation
// But we always try to rely on events(SQS, DynamoDB, ...)
// It may also happens in localhot invocation (e.g. ITCleanEventLocallyInDynamoDb)
try {
LOGGER.warn("TODO Add unit-test for: {}", objectMapper.writeValueAsString(input));
} catch (JsonProcessingException ee) {
Expand Down Expand Up @@ -178,7 +179,7 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {

// SQS transfer the body 'as is'
try {
asMap = (Map<String, ?>) objectMapper.readValue(body, Map.class);
asMap = objectMapper.readValue(body, Map.class);
} catch (JsonProcessingException e) {
LOGGER.warn("Issue while parsing: {}", body);
LOGGER.warn("Issue while parsing body", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
import eu.solven.cleanthat.code_provider.github.refs.GithubRefCleaner;
import eu.solven.cleanthat.lambda.AWebhooksLambdaFunction;
import eu.solven.cleanthat.lambda.dynamodb.SaveToDynamoDb;
import eu.solven.cleanthat.lambda.step1_checkconfiguration.CheckConfigWebhooksLambdaFunction;
import eu.solven.cleanthat.lambda.step2_executeclean.ExecuteCleaningWebhooksLambdaFunction;

@RunWith(SpringRunner.class)
@ContextConfiguration(classes = { CheckConfigWebhooksLambdaFunction.class })
@ContextConfiguration(classes = { ExecuteCleaningWebhooksLambdaFunction.class })
public class ITCleanEventLocallyInDynamoDb {
GithubRefCleaner cleaner;

Expand All @@ -38,7 +38,9 @@ public class ITCleanEventLocallyInDynamoDb {
public void testInitWithDefaultConfiguration() throws IOException, JOSEException {
AmazonDynamoDB dynamoDbClient = SaveToDynamoDb.makeDynamoDbClient();

String key = "random-01be8d8f-fde0-4895-8689-70288ace3819";
// This is logged by: e.s.c.lambda.AWebhooksLambdaFunction|parseDynamoDbEvent
// You can search logs for this key, in order to process given event locally
String key = "random-6787961d-e2b6-4ec2-8df5-7a3db5722b82";
GetItemResult item = dynamoDbClient.getItem(new GetItemRequest().withTableName("cleanthat_accepted_events")
.withKey(Map.of("X-GitHub-Delivery", new AttributeValue().withS(key))));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,13 @@ public abstract class ACleanThatXxxFunction extends ACleanThatXxxApplication {
try {
return unsafeProcessOneEvent(input);
} catch (RuntimeException e) {
// if (input instanceof GithubWebhookEvent) {
Map<String, ?> body = input.getBody();

try {
LOGGER.warn("Issue with IWebhookEvent. body={}", new ObjectMapper().writeValueAsString(body));
} catch (JsonProcessingException e1) {
LOGGER.warn("Issue printing as json. body: {}", body);
}
// } else {
// LOGGER.warn("Issue with {}", input.getClass());
// }

RuntimeException wrapped = new RuntimeException(e);
Sentry.captureException(wrapped, "Lambda");
Expand Down

0 comments on commit 1028492

Please sign in to comment.