Skip to content

Commit

Permalink
Merge pull request #460 from exacaster/minors
Browse files Browse the repository at this point in the history
Log stactrace on submit failure, hide contacted_at when not set
  • Loading branch information
pdambrauskas authored Apr 26, 2023
2 parents 1f3f952 + 95ad5de commit bf21f82
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 21 deletions.
1 change: 1 addition & 0 deletions frontend/src/components/AppLogs.module.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.logs {
width: 100%;
white-space: pre-wrap;
max-width: 100%;
font-size: 12px;
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/components/AppTitle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ const AppTitle: React.FC<Props> = ({app, onDelete}) => {
<Text as="b">Created: </Text>
<DateTime>{app.createdAt}</DateTime>
{' | '}
<Text as="b">Last checked: </Text>
<DateTime>{app.contactedAt}</DateTime>
{app.contactedAt ? (
<>
<Text as="b">Last checked: </Text>
<DateTime>{app.contactedAt}</DateTime>
</>
) : null}
</Text>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.exacaster.lighter.storage.ApplicationStorage;
import java.time.LocalDateTime;
import jakarta.inject.Singleton;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;

@Singleton
Expand All @@ -26,8 +27,8 @@ public ApplicationStatusHandler(ApplicationStorage applicationStorage, Backend b
this.logService = logService;
}

public void processApplicationStarting(Application application) {
applicationStorage.saveApplication(ApplicationBuilder.builder(application)
public Application processApplicationStarting(Application application) {
return applicationStorage.saveApplication(ApplicationBuilder.builder(application)
.setState(ApplicationState.STARTING)
.setContactedAt(LocalDateTime.now())
.build());
Expand All @@ -51,7 +52,7 @@ public ApplicationState processApplicationRunning(Application app) {
}

public void processApplicationError(Application application, Throwable error) {
LOG.warn("Marking application {} failed because of error {}", application.getId(), error.getMessage());
LOG.warn("Marking application " + application.getId() + " failed because of error", error);
var appId = backend.getInfo(application).map(ApplicationInfo::getApplicationId)
.orElse(null);
applicationStorage.saveApplication(
Expand All @@ -63,7 +64,7 @@ public void processApplicationError(Application application, Throwable error) {

backend.getLogs(application).ifPresentOrElse(
logService::save,
() -> logService.save(new Log(application.getId(), error.toString()))
() -> logService.save(new Log(application.getId(), ExceptionUtils.getStackTrace(error)))
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.exacaster.lighter.storage.jdbc;

import static org.slf4j.LoggerFactory.getLogger;

import com.exacaster.lighter.application.Application;
import com.exacaster.lighter.application.ApplicationBuilder;
import com.exacaster.lighter.application.ApplicationState;
Expand All @@ -13,7 +15,6 @@
import jakarta.inject.Singleton;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Timestamp;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
Expand All @@ -22,10 +23,12 @@
import org.jdbi.v3.core.Jdbi;
import org.jdbi.v3.core.mapper.RowMapper;
import org.jdbi.v3.core.statement.StatementContext;
import org.slf4j.Logger;

@Singleton
@Requires(beans = DataSource.class)
public class JdbcApplicationStorage implements ApplicationStorage, RowMapper<Application> {
private static final Logger LOG = getLogger(JdbcApplicationStorage.class);

private final Jdbi jdbi;
private final ObjectMapper objectMapper;
Expand Down Expand Up @@ -72,14 +75,7 @@ public void deleteApplication(String internalApplicationId) {
@Override
@Transactional
public Application saveApplication(Application application) {

return jdbi.withHandle(handle -> {
String conf = null;
try {
conf = objectMapper.writeValueAsString(application.getSubmitParams());
} catch (JsonProcessingException e) {
// TODO
}
var updated = handle.createUpdate("UPDATE application SET "
+ "app_id=:app_id, "
+ "app_info=:app_info, "
Expand All @@ -88,22 +84,29 @@ public Application saveApplication(Application application) {
.bind("state", application.getState().name())
.bind("app_id", application.getAppId())
.bind("app_info", application.getAppInfo())
.bind("id", application.getId())
.bind("contacted_at", application.getContactedAt())
.bind("id", application.getId())
.execute();
// Not all SQL databases support ON CONFLICT syntax, so doing fallback if nothing updated
if (updated == 0) {
String conf = null;
try {
conf = objectMapper.writeValueAsString(application.getSubmitParams());
} catch (JsonProcessingException e) {
LOG.warn("Failed serializing submit params", e);
}
handle
.createCall(
"INSERT INTO application (id, type, state, app_id, app_info, submit_params, created_at) "
+ "VALUES (:id, :type, :state, :app_id, :app_info, :submit_params, :created_at)")
"INSERT INTO application (id, type, state, app_id, app_info, submit_params, created_at, contacted_at) "
+ "VALUES (:id, :type, :state, :app_id, :app_info, :submit_params, :created_at, :contacted_at)")
.bind("id", application.getId())
.bind("type", application.getType().name())
.bind("state", application.getState().name())
.bind("app_id", application.getAppId())
.bind("app_info", application.getAppInfo())
.bind("submit_params", conf)
.bind("created_at", Timestamp.valueOf(application.getCreatedAt()))
.bind("created_at", application.getCreatedAt())
.bind("contacted_at", application.getContactedAt())
.invoke();
}
return application;
Expand Down Expand Up @@ -133,11 +136,11 @@ public Application map(ResultSet rs, StatementContext ctx) throws SQLException {
try {
params = objectMapper.readValue(rs.getString("submit_params"), SubmitParams.class);
} catch (JsonProcessingException e) {
// TODO
LOG.warn("Failed deserializing submit params", e);
}

var contactedAt =
rs.getTimestamp("contacted_at") != null ? rs.getTimestamp("contacted_at").toLocalDateTime() : null;
var contactedAtTs = rs.getTimestamp("contacted_at");
var contactedAt = contactedAtTs != null ? contactedAtTs.toLocalDateTime() : null;
return ApplicationBuilder.builder()
.setId(rs.getString("id"))
.setType(ApplicationType.valueOf(rs.getString("type")))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class ApplicationStatusHandlerTest extends Specification {
then:
updated.state == ApplicationState.ERROR
updated.appId == "we"
1 * logService.save(_)
}

def "handles starting"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class JdbcApplicationStorageTest extends Specification {

then: "returns saved"
saved.id == app.id
saved.createdAt == app.createdAt

when: "saving updated"
saved = storage.saveApplication(ApplicationBuilder.builder(saved).setState(ApplicationState.ERROR).build())
Expand Down

0 comments on commit bf21f82

Please sign in to comment.