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.

Sign up for GitHub

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

fix: removed the configuration from the filesystem provider and used the one in the filesystem instead. #598

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: removed the configuration from the filesystem provider and used the one in the filesystem instead. #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 0 additions & 11 deletions src/main/java/software/amazon/nio/spi/s3/S3FileSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,6 @@ public class S3FileSystem extends FileSystem {
configuration = (config == null) ? new S3NioSpiConfiguration() : config;
bucketName = configuration.getBucketName();

// This is quite questionable and may be removed in future versions:
provider.setConfiguration(config);
// The configuration field in {@code S3FileSystemProvider} is used for certain tasks
// that are implemented there.
// But those tasks are in service of {@code S3FileSystem} instances.
// So if there are multiple ones with different configurations, the provider will use
// the one that has been set by the last created filesystem, overriding potentially
// different values in older {@code S3FileSystem} instances.
//
// See https://github.com/awslabs/aws-java-nio-spi-for-s3/issues/597

logger.debug("creating FileSystem for '{}://{}'", provider.getScheme(), bucketName);

clientProvider = new S3ClientProvider(configuration);
Expand Down
50 changes: 16 additions & 34 deletions src/main/java/software/amazon/nio/spi/s3/S3FileSystemProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,6 @@ public class S3FileSystemProvider extends FileSystemProvider {
static final String SCHEME = "s3";
private static final Map<String, S3FileSystem> FS_CACHE = new ConcurrentHashMap<>();

/**
* This variable holds the configuration for the S3 NIO Service Provider Interface (SPI).
* It is used to manage and handle the configuration details required for interaction
* with S3 NIO services.
*
* @deprecated This variable is deprecated and may be removed in future versions.
* Consider using updated configuration mechanisms if available.
*/
@Deprecated
protected S3NioSpiConfiguration configuration = new S3NioSpiConfiguration();

private final Logger logger = LoggerFactory.getLogger(this.getClass().getName());

/**
Expand Down Expand Up @@ -393,11 +382,12 @@ public void createDirectory(Path dir, FileAttribute<?>... attrs) throws IOExcept
directoryKey = directoryKey + PATH_SEPARATOR;
}

var timeOut = configuration.getTimeoutLow();
var s3FileSystem = s3Directory.getFileSystem();
var timeOut = s3FileSystem.getConfiguration().getTimeoutLow();
final var unit = MINUTES;

try {
s3Directory.getFileSystem().client().putObject(
s3FileSystem.client().putObject(
PutObjectRequest.builder()
.bucket(s3Directory.bucketName())
.key(directoryKey)
Expand Down Expand Up @@ -426,9 +416,10 @@ public void delete(Path path) throws IOException {
final var prefix = s3Path.toRealPath(NOFOLLOW_LINKS).getKey();
final var bucketName = s3Path.bucketName();

final var s3Client = s3Path.getFileSystem().client();
final var s3FileSystem = s3Path.getFileSystem();
final var s3Client = s3FileSystem.client();

var timeOut = configuration.getTimeoutLow();
var timeOut = s3FileSystem.getConfiguration().getTimeoutLow();
final var unit = MINUTES;
try {
var keys = s3Path.isDirectory() ?
Expand Down Expand Up @@ -479,10 +470,11 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep
var s3SourcePath = checkPath(source);
var s3TargetPath = checkPath(target);

final var s3Client = s3SourcePath.getFileSystem().client();
final var s3FileSystem = s3SourcePath.getFileSystem();
final var s3Client = s3FileSystem.client();
final var sourceBucket = s3SourcePath.bucketName();

final var timeOut = configuration.getTimeoutHigh();
final var timeOut = s3FileSystem.getConfiguration().getTimeoutHigh();
final var unit = MINUTES;

var fileExistsAndCannotReplace = cannotReplaceAndFileExistsCheck(options, s3Client);
Expand Down Expand Up @@ -654,7 +646,7 @@ public void checkAccess(Path path, AccessMode... modes) throws IOException {
final var s3Path = checkPath(path.toRealPath(NOFOLLOW_LINKS));
final var response = getCompletableFutureForHead(s3Path);

var timeOut = configuration.getTimeoutLow();
var timeOut = s3Path.getFileSystem().getConfiguration().getTimeoutLow();
var unit = MINUTES;

try {
Expand Down Expand Up @@ -758,8 +750,9 @@ public <A extends BasicFileAttributes> A readAttributes(Path path, Class<A> type
var s3Path = checkPath(path);

if (type.equals(BasicFileAttributes.class)) {
var timeoutLow = s3Path.getFileSystem().getConfiguration().getTimeoutLow();
@SuppressWarnings("unchecked")
var a = (A) S3BasicFileAttributes.get(s3Path, Duration.ofMinutes(configuration.getTimeoutLow()));
var a = (A) S3BasicFileAttributes.get(s3Path, Duration.ofMinutes(timeoutLow));
return a;
} else {
throw new UnsupportedOperationException("cannot read attributes of type: " + type);
Expand Down Expand Up @@ -795,8 +788,9 @@ public Map<String, Object> readAttributes(Path path, String attributes, LinkOpti
return Collections.emptyMap();
}

var timeoutLow = s3Path.getFileSystem().getConfiguration().getTimeoutLow();
var attributesFilter = attributesFilterFor(attributes);
return S3BasicFileAttributes.get(s3Path, Duration.ofMinutes(configuration.getTimeoutLow())).asMap(attributesFilter);
return S3BasicFileAttributes.get(s3Path, Duration.ofMinutes(timeoutLow)).asMap(attributesFilter);
}

/**
Expand All @@ -810,19 +804,6 @@ public void setAttribute(Path path, String attribute, Object value, LinkOption..
throw new UnsupportedOperationException("s3 file attributes cannot be modified by this class");
}

/**
* Set custom configuration. This configuration is referred to for API timeouts.
*
* @param configuration The new configuration containing the timeout info
*
* @deprecated This method is deprecated and may be removed in future versions.
*
*/
@Deprecated
public void setConfiguration(S3NioSpiConfiguration configuration) {
this.configuration = configuration;
}

/**
* @param path the path of the file to open or create
* @param options options specifying how the file is opened
Expand Down Expand Up @@ -894,8 +875,9 @@ private void closeFileSystemIfOpen(FileSystem fs) throws IOException {

boolean exists(S3AsyncClient s3Client, S3Path path) throws InterruptedException, TimeoutException {
try {
var timeoutLow = path.getFileSystem().getConfiguration().getTimeoutLow();
s3Client.headObject(HeadObjectRequest.builder().bucket(path.bucketName()).key(path.getKey()).build())
.get(configuration.getTimeoutLow(), MINUTES);
.get(timeoutLow, MINUTES);
return true;
} catch (ExecutionException | NoSuchKeyException e) {
logger.debug("Could not retrieve object head information", e);
Expand Down